Merge pull request #47 from MultifactorLab/feature/dev-135

Always check membership on behalf of the process user
MultifactorLab · Jul 1, 2024 · 555983c · 555983c
2 parents ef45d6f + 29d7faf
commit 555983c
Show file tree

Hide file tree

Showing 2 changed files with 41 additions and 22 deletions.
diff --git a/MultiFactor.Radius.Adapter/Server/RadiusRouter.cs b/MultiFactor.Radius.Adapter/Server/RadiusRouter.cs
@@ -215,6 +215,7 @@ public async Task HandleRequest(PendingRequest request)
                 if (request.AuthenticationState.SecondFactor == AuthenticationCode.Awaiting)
                 {
                     var code = await ProcessSecondAuthenticationFactor(request);
+
                     if (code == PacketCode.AccessChallenge) 
                     {
                         request.ResponseCode = request.AuthenticationState.GetResultPacketCode();
@@ -223,7 +224,17 @@ public async Task HandleRequest(PendingRequest request)
                         return;
                     }
 
-                    if (code != PacketCode.AccessAccept)
+                    if (code == PacketCode.AccessAccept)
+                    {
+                        _logger.Information("Second factor accepted for user '{user:l}' from {host:l}:{port}",
+                            request.UserName, request.RemoteEndpoint.Address, request.RemoteEndpoint.Port);
+                        request.AuthenticationState.SetSecondFactor(AuthenticationCode.Accept);
+                        request.ResponseCode = request.AuthenticationState.GetResultPacketCode();
+                        CreateAndSendRadiusResponse(request);
+                        return;
+                    }
+
+                    if (code == PacketCode.AccessReject)
                     {
                         _logger.Information("Second factor rejected for user '{user:l}' from {host:l}:{port}",
                             request.UserName, request.RemoteEndpoint.Address, request.RemoteEndpoint.Port);
@@ -232,11 +243,6 @@ public async Task HandleRequest(PendingRequest request)
                         CreateAndSendRadiusResponse(request);
                         return;
                     }
-
-                    request.AuthenticationState.SetSecondFactor(AuthenticationCode.Accept);
-                    request.ResponseCode = request.AuthenticationState.GetResultPacketCode();
-                    CreateAndSendRadiusResponse(request);
-                    return;     
                 }
 
                 request.ResponseCode = request.AuthenticationState.GetResultPacketCode();

diff --git a/MultiFactor.Radius.Adapter/Services/ActiveDirectory/ActiveDirectoryService.cs b/MultiFactor.Radius.Adapter/Services/ActiveDirectory/ActiveDirectoryService.cs
@@ -84,13 +84,8 @@ public bool VerifyCredentialAndMembership(PendingRequest request)
 
             try
             {
-                _logger.Debug("Verifying user '{User:l}' credential and status at {Domain:l}", user, _domain);
-
-                using (var connection = _connectionFactory.Create(_domain, user.Name, request.Passphrase.Password))
-                {
-                    _logger.Information("User '{User:l}' credential and status verified successfully in {Domain:l}", user, _domain);
-                    return VerifyMembership(request.Configuration, connection, _domain, user, request);
-                }
+                VerifyCredential(user, request);
+                return VerifyMembership(request.Configuration, user, request);
             }
             catch (LdapException lex)
             {
@@ -193,17 +188,25 @@ public bool ChangePassword(PendingRequest request, string currentPassword, out b
             return false;
         }
 
-        private bool VerifyMembership(ClientConfiguration clientConfig, LdapConnection connection, string userDomain, LdapIdentity user, PendingRequest request)
+        private bool VerifyMembership(ClientConfiguration clientConfig, LdapIdentity user, PendingRequest request)
         {
-            var domain = LdapIdentity.FqdnToDn(userDomain);
-            var schema = _forestMetadataCache.Get(
-                clientConfig.Name,
-                domain,
-                () => new ForestSchemaLoader(clientConfig, connection, _logger).Load(domain));
-            var profile = new ProfileLoader(schema, _logger).LoadProfile(clientConfig, connection, domain, user);
-            if (profile == null)
+            var domain = LdapIdentity.FqdnToDn(_domain);
+
+            LdapProfile profile;
+
+            using (var connection = _connectionFactory.CreateAsCurrentProcessUser(_domain))
             {
-                return false;
+                var forestSchema = _forestMetadataCache.Get(
+                    clientConfig.Name,
+                    domain,
+                    () => new ForestSchemaLoader(clientConfig, connection, _logger).Load(domain));
+
+                profile = new ProfileLoader(forestSchema, _logger).LoadProfile(clientConfig, connection, domain, user);
+
+                if (profile == null)
+                {
+                    return false;
+                }
             }
 
             //user must be member of security group
@@ -264,6 +267,16 @@ private bool VerifyMembership(ClientConfiguration clientConfig, LdapConnection c
             return true;
         }
 
+        private void VerifyCredential(LdapIdentity user, PendingRequest request)
+        {
+            _logger.Debug("Verifying user '{User:l}' credential and status at {Domain:l}", user, _domain);
+
+            using (_ = _connectionFactory.Create(_domain, user.Name, request.Passphrase.Password))
+            {
+                _logger.Information("User '{User:l}' credential and status verified successfully in {Domain:l}", user, _domain);
+            }
+        }
+
         private bool IsMemberOf(LdapProfile profile, string group)
         {
             return profile.MemberOf?.Any(g => g.ToLower() == group.ToLower().Trim()) ?? false;