Refactoring Radius 2.0 by KotikovAlexander · Pull Request #129 · MultifactorLab/multifactor-radius-adapter

KotikovAlexander · 2026-02-05T08:22:34Z

What's new:

Внесены архитектурные и значительные изменения в кодовой базе
Исправлены ошибки соединения с AD
Добавлены новые атрибуты ideco
Добавлены новые атрибуты вендора Cisco
Реализована возможность перечисления LDAP групп при недоступности API в блоке ldapServer
Удален неактуальный функционал
Актуализирована работа с конфигурацией

apashkov-ext · 2026-02-05T08:54:19Z

src/Multifactor.Radius.Adapter.v2/Program.cs

-    var errorMessage = FlattenException(ex);
-    StartupLogger.Error(ex, "Unable to start: {Message:l}", errorMessage);
+    if(ex is InvalidConfigurationException)
+        StartupLogger.Error(null, "Unable to start: {Message:l}", ex.Message);


Почему не прокинута переменная ex?

apashkov-ext · 2026-02-05T08:55:24Z

src/Multifactor.Radius.Adapter.v2.Application/Extensions/ApplicationExtensions.cs

+        services.AddSingleton(appVars);
+    }
+
+    private static void AddLdapBindNameFormation(IServiceCollection services)


Предлагаю эти вещи раскидать по модулям, чтобы сделать честными фичами и избавиться от DI hell

apashkov-ext · 2026-02-05T08:56:32Z

src/Multifactor.Radius.Adapter.v2/Server/AdapterServer.cs

-                    _logger.LogWarning(ex, "Error while processing packet from '{client:l}'", udpPacket.RemoteEndPoint.Address);
-                }
-            }
+        _udpClient = udpClient ?? throw new ArgumentNullException(nameof(udpClient));


Проверки на null излишни, если аллокацией занимается Service Provider

apashkov-ext · 2026-02-05T08:57:09Z

src/Multifactor.Radius.Adapter.v2/Server/AdapterServer.cs


 namespace Multifactor.Radius.Adapter.v2.Server;

-public class AdapterServer : IDisposable
+public class AdapterServer : IAsyncDisposable


internal sealed

apashkov-ext · 2026-02-05T08:57:17Z

src/Multifactor.Radius.Adapter.v2/Server/ServerHost.cs

@@ -1,47 +1,63 @@
 using Microsoft.Extensions.Hosting;
 using Microsoft.Extensions.Logging;
-using Multifactor.Core.Ldap.LangFeatures;

 namespace Multifactor.Radius.Adapter.v2.Server;

 public class ServerHost : IHostedService


internal sealed

apashkov-ext · 2026-02-05T09:12:23Z

src/Multifactor.Radius.Adapter.v2.Application/Features/Ldap/Ports/ILdapAdapter.cs

+
+namespace Multifactor.Radius.Adapter.v2.Application.Features.Ldap.Ports;
+
+public interface ILdapAdapter


Это не порт, а толстый неудобный интерфейс. А еще в папке Features/Ldap не нашел собственно фичи. Есть только этот интерфейс имодели

apashkov-ext · 2026-02-05T10:03:53Z

src/Multifactor.Radius.Adapter.v2.Infrastructure/Configurations/Models/ConfigurationFile.cs

+
+namespace Multifactor.Radius.Adapter.v2.Infrastructure.Configurations.Models;
+
+public class ConfigurationFile


Только не файл, потому что можно читать ото всюду

apashkov-ext · 2026-02-05T10:04:24Z

src/Multifactor.Radius.Adapter.v2.Infrastructure/Configurations/Reader/ConfigurationReader.cs

+        }
+        catch (Exception ex)
+        {
+            Console.WriteLine($"\n=== Ошибка при Get<ConfigurationFile>(): {ex.Message} ===");


логгер

apashkov-ext · 2026-02-05T10:05:16Z

src/Multifactor.Radius.Adapter.v2.Infrastructure/Configurations/Reader/ConfigurationReader.cs

+        try
+        {
+            var config = builder.Get<ConfigurationFile>();
+            config.FileName =  Path.GetFileNameWithoutExtension(filePath);


нет проверки на null, добавить

apashkov-ext · 2026-02-05T10:07:20Z

...tor.Radius.Adapter.v2.Infrastructure/Configurations/Reader/ConfigurationBuilderExtensions.cs

+
+public static class ConfigurationBuilderExtensions
+{
+    public static IConfigurationBuilder AddLegacyXmlConfig(


не легаси

AleksandrovVN · 2026-02-05T09:54:44Z

src/Multifactor.Radius.Adapter.v2.Infrastructure/Adapters/Multifactor/Http/ActivityContext.cs

@@ -0,0 +1,45 @@
+namespace Multifactor.Radius.Adapter.v2.Infrastructure.Adapters.Multifactor.Http;
+
+public class ActivityContext


по хорошему стоит от него отказаться в пользу OpenTelemtry

AleksandrovVN · 2026-02-05T12:38:52Z

src/Multifactor.Radius.Adapter.v2.Infrastructure/Logging/SerilogLoggerFactory.cs

-                RadiusAdapterConfigurationFile.ConfigName);
-        }
+        var level = rootConfiguration.LoggingLevel;
+        var skip = string.IsNullOrWhiteSpace(level);


зачем тут skip?

AleksandrovVN · 2026-02-05T12:39:25Z

src/Multifactor.Radius.Adapter.v2.Infrastructure/Logging/SerilogLoggerFactory.cs

+        // {
+        //  throw new InvalidConfigurationException(
+        //      string.Concat("'{prop}' element not found. Config name: '{0}'", "rootConfiguration.ConfigurationName"));
+        // }


комменты можно дропнуть

AleksandrovVN · 2026-02-05T12:40:18Z

src/Multifactor.Radius.Adapter.v2.Infrastructure/Logging/SerilogLoggerFactory.cs

-                    RadiusAdapterConfigurationDescription.Property(x => x.AppSettings.LoggingFormat),
-                    RadiusAdapterConfigurationDescription.Property(x => x.AppSettings.FileLogOutputTemplate));
+                // Log.Logger.Warning(
+                //     "The {LoggingFormat:l} parameter cannot be used together with the template. The {FileLogOutputTemplate:l} parameter will be ignored.",


комменты можно дропнуть

AleksandrovVN · 2026-02-05T12:41:14Z

src/Multifactor.Radius.Adapter.v2.Infrastructure/Logging/StartupLogger.cs

+        // if (!Directory.Exists(dir))
+        // {
+        //     Directory.CreateDirectory(dir);
+        // }


комменты можно дропнуть

AleksandrovVN · 2026-02-05T12:43:36Z

src/Multifactor.Radius.Adapter.v2.Infrastructure/Radius/Builders/RadiusAttributeSerializer.cs

+            {
+                contentBytes = RadiusPasswordProtector.Encrypt(sharedSecret, authenticator, contentBytes);
+            }
+            else if (attributeDefinition.Code == 80) // Message-Authenticator


то же самое

AleksandrovVN · 2026-02-05T12:46:30Z

src/Multifactor.Radius.Adapter.v2.Infrastructure/Radius/Builders/RadiusPacketBuilder.cs

+    {
+        _radiusDictionary = radiusDictionary ?? throw new ArgumentNullException(nameof(radiusDictionary));
+        _cryptoProvider = cryptoProvider ?? throw new ArgumentNullException(nameof(cryptoProvider));
+        // _attributeSerializer = attributeSerializer ?? throw new ArgumentNullException(nameof(attributeSerializer));


комменты можно дропнуть

AleksandrovVN · 2026-02-05T12:51:10Z

src/Multifactor.Radius.Adapter.v2.Infrastructure/Radius/Parsers/RadiusAttributeParser.cs

+    private readonly IRadiusDictionary _radiusDictionary;
+    private readonly ILogger<RadiusAttributeParser> _logger;
+    const int VendorSpecific = 26;
+    const int MessageAuthenticator = 80;


снова те же константы

AleksandrovVN · 2026-02-05T12:53:07Z

src/Multifactor.Radius.Adapter.v2.Infrastructure/Radius/Parsers/RadiusAttributeParser.cs

+        byte vendorType = contentBytes[4];
+        byte vendorLength = contentBytes[5];
+
+        if (vendorLength < 2 || 2 + vendorLength - 2 > contentBytes.Length)


???? 2 + vendorLength - 2 ???? = просто vendorLength > contentBytes.Length

AleksandrovVN · 2026-02-05T12:53:38Z

src/Multifactor.Radius.Adapter.v2.Infrastructure/Radius/Parsers/RadiusAttributeParser.cs

+        var content = ParseContentBytes(
+            vendorContentBytes,
+            vendorAttribute.Type,
+            26,


magic number

Aleksandr added 8 commits January 14, 2026 13:46

First refactoring iteration

d898a5c

Second iteration

467842f

Second iteration

a42e35b

End of refactoring

b487323

another fix

689237d

end refactoring

3fa2cb7

return radius v1

3ec4959

remove radius V1

f1401ca

KotikovAlexander marked this pull request as ready for review February 5, 2026 09:03

apashkov-ext requested changes Feb 5, 2026

View reviewed changes

AleksandrovVN reviewed Feb 5, 2026

View reviewed changes


		namespace Multifactor.Radius.Adapter.v2.Application.Features.Ldap.Ports;

		public interface ILdapAdapter


		namespace Multifactor.Radius.Adapter.v2.Infrastructure.Configurations.Models;

		public class ConfigurationFile

		@@ -0,0 +1,45 @@
		namespace Multifactor.Radius.Adapter.v2.Infrastructure.Adapters.Multifactor.Http;

		public class ActivityContext

Conversation

KotikovAlexander commented Feb 5, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

KotikovAlexander commented Feb 5, 2026 •

edited

Loading