feat: mfa handshake infrastructure by Thomas-Shephard · Pull Request #17 · Thomas-Shephard/ashlar

Thomas-Shephard · 2026-01-18T22:03:00Z

No description provided.

sonarqubecloud · 2026-01-18T22:04:29Z

Quality Gate passed

Issues
10 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

Copilot

Pull request overview

This pull request implements the foundational infrastructure for multi-factor authentication (MFA) handshakes in the Ashlar identity library. It introduces a session ticket mechanism that tracks verified authentication factors during a multi-step authentication flow, enabling support for requiring multiple authentication factors before granting access.

Changes:

Adds MFA handshake abstractions and models (IAuthenticationHandshake, SessionTicket, AuthenticationHandshake) for tracking authentication state across multiple factors
Extends IAuthenticationProvider with MFA-related properties (IsPrimary, IsSecondary, BypassesMfa) to classify authentication factors
Implements SessionTicketSerializer for secure serialization and deserialization of MFA session state with configurable expiry
Refactors IdentityService.LoginAsync to support both email-based and session-ticket-based authentication flows, with automatic MFA requirement detection
Adds GetCredentialsForUserAsync to IIdentityRepository to check if secondary factors are configured for a user

Reviewed changes

Copilot reviewed 16 out of 16 changed files in this pull request and generated 17 comments.

Show a summary per file

File	Description
src/Ashlar/Identity/Abstractions/IAuthenticationHandshake.cs	New interface defining MFA handshake state contract
src/Ashlar/Identity/Abstractions/ISessionTicketSerializer.cs	New interface for session ticket serialization/deserialization
src/Ashlar/Identity/Abstractions/IAuthenticationProvider.cs	Extended with MFA classification properties and bypass method
src/Ashlar/Identity/Abstractions/IIdentityRepository.cs	Added method to retrieve all user credentials
src/Ashlar/Identity/Abstractions/IIdentityService.cs	Added session ticket LoginAsync overload and MfaRequired status
src/Ashlar/Identity/AuthenticationHandshake.cs	Concrete implementation of IAuthenticationHandshake
src/Ashlar/Identity/SessionTicketSerializer.cs	Implements encrypted session ticket serialization with expiry
src/Ashlar/Identity/IdentityService.cs	Refactored to support MFA flows with internal authentication method
src/Ashlar/Identity/Models/SessionTicket.cs	Value object wrapping encrypted session ticket with redacted ToString
src/Ashlar/Identity/Models/ProviderType.cs	Added JSON converter for serialization support
src/Ashlar/Identity/Models/IdentityServiceOptions.cs	Added HandshakeExpiry configuration option
tests/Ashlar.Tests/Identity/SessionTicketSerializerTests.cs	Unit tests for session ticket serializer
tests/Ashlar.Tests/Identity/MfaHandshakeTests.cs	Integration tests for MFA handshake flows
tests/Ashlar.Tests/Identity/IdentityServiceTests.cs	Updated tests for refactored IdentityService constructor and new MFA scenarios
tests/Ashlar.Tests/Identity/ModelTests.cs	Added test for SessionTicket model
tests/Ashlar.Tests/Identity/AuthenticationProviderTests.cs	Added test for GetProviderName with various types

tests/Ashlar.Tests/Identity/MfaHandshakeTests.cs

Copilot · 2026-01-18T22:08:29Z

tests/Ashlar.Tests/Identity/IdentityServiceTests.cs

        var providerMock = new Mock<IAuthenticationProvider>();
+        providerMock.Setup(p => p.IsPrimary).Returns(true);
        providerMock.Setup(p => p.SupportedType).Returns((ProviderType)"MOCK");
+        providerMock.Setup(p => p.IsPrimary).Returns(true);


Duplicate setup call detected. Line 659 sets IsPrimary to true, but this is immediately repeated on line 659. The second setup on line 659 is redundant and should be removed.

Suggested change

providerMock.Setup(p => p.IsPrimary).Returns(true);

Copilot · 2026-01-18T22:08:29Z

src/Ashlar/Identity/AuthenticationHandshake.cs

+using Ashlar.Identity.Models;
+
+namespace Ashlar.Identity;
+


Missing XML documentation for the AuthenticationHandshake class. This is a public class implementing IAuthenticationHandshake and should have a summary comment explaining its purpose, particularly noting that it's the concrete implementation used for MFA handshake state.

Suggested change

/// <summary>

/// Represents the state of an authentication handshake, including the user and verified

/// authentication factors. This is the concrete implementation of <see cref="IAuthenticationHandshake"/>

/// used to track multi-factor authentication (MFA) handshake state.

/// </summary>

Copilot · 2026-01-18T22:08:29Z

tests/Ashlar.Tests/Identity/MfaHandshakeTests.cs

+    [Test]
+    public void ProviderTypeJsonConverterShouldHandleNullOrWhitespace()
+    {
+        var jsonNull = "null";
+        var jsonEmpty = "\"\"";
+        var jsonSpace = "\"  \"";
+
+        var resultNull = System.Text.Json.JsonSerializer.Deserialize<ProviderType>(jsonNull);
+        var resultEmpty = System.Text.Json.JsonSerializer.Deserialize<ProviderType>(jsonEmpty);
+        var resultSpace = System.Text.Json.JsonSerializer.Deserialize<ProviderType>(jsonSpace);
+
+        using (Assert.EnterMultipleScope())
+        {
+            Assert.That(resultNull.Value, Is.EqualTo(string.Empty));
+            Assert.That(resultEmpty.Value, Is.EqualTo(string.Empty));
+            Assert.That(resultSpace.Value, Is.EqualTo(string.Empty));
+        }
+    }


Missing test coverage for the ProviderType JSON converter's Write method. The test on line 307 only tests deserialization (Read method), but doesn't verify that serialization (Write method) produces the expected JSON string output.

Copilot · 2026-01-18T22:08:30Z

src/Ashlar/Identity/Abstractions/IIdentityService.cs


    Task<AuthenticationResponse> LoginAsync(string email, IAuthenticationAssertion assertion, Guid? tenantId = null, CancellationToken cancellationToken = default);


Missing XML documentation for the new LoginAsync overload. This method should have a summary explaining its purpose for continuing an MFA authentication flow using a session ticket, along with parameter descriptions and return value documentation.

Suggested change

Task<AuthenticationResponse> LoginAsync(string email, IAuthenticationAssertion assertion, Guid? tenantId = null, CancellationToken cancellationToken = default);

/// <summary>

/// Initiates an authentication flow for a user identified by email using the provided authentication assertion.

/// </summary>

/// <param name="email">The email address used to identify the user.</param>

/// <param name="assertion">The authentication assertion (for example, a password or external provider assertion) to validate.</param>

/// <param name="tenantId">An optional tenant identifier for multi-tenant scenarios.</param>

/// <param name="cancellationToken">A token that can be used to cancel the asynchronous operation.</param>

/// <returns>

/// An <see cref="AuthenticationResponse"/> describing the outcome of the authentication attempt, which may include

/// the authenticated user, status, claims, and an optional session ticket when additional factors are required.

/// </returns>

Task<AuthenticationResponse> LoginAsync(string email, IAuthenticationAssertion assertion, Guid? tenantId = null, CancellationToken cancellationToken = default);

/// <summary>

/// Continues a multi-factor authentication flow using an existing <see cref="SessionTicket"/> and a follow-up authentication assertion.

/// </summary>

/// <param name="sessionTicket">The session ticket issued by a previous authentication step, typically when MFA is required.</param>

/// <param name="assertion">The follow-up authentication assertion (such as an MFA code or verification) to validate.</param>

/// <param name="cancellationToken">A token that can be used to cancel the asynchronous operation.</param>

/// <returns>

/// An <see cref="AuthenticationResponse"/> describing the outcome of the continued authentication flow, which may include

/// the authenticated user, status, claims, and an updated session ticket.

/// </returns>

Copilot · 2026-01-18T22:08:32Z

tests/Ashlar.Tests/Identity/IdentityServiceTests.cs

+
+        var result = await service.LoginAsync(sessionTicket, assertion);
+        Assert.That(result.Status, Is.EqualTo(AuthenticationStatus.Failed));
+    }


Missing test coverage for ArgumentNullException when calling LoginAsync with a null SessionTicket parameter. The LoginAsync method on line 64 of IdentityService.cs validates the sessionTicket parameter with ArgumentNullException.ThrowIfNull, but there is no test verifying this behavior.

Copilot · 2026-01-18T22:08:32Z

tests/Ashlar.Tests/Identity/IdentityServiceTests.cs

+
+        var result = await service.LoginAsync(sessionTicket, assertion);
+        Assert.That(result.Status, Is.EqualTo(AuthenticationStatus.Failed));
+    }


Missing test coverage for ArgumentNullException when calling LoginAsync(SessionTicket, IAuthenticationAssertion) with a null assertion parameter. The LoginAsync method on line 63 of IdentityService.cs validates the assertion parameter with ArgumentNullException.ThrowIfNull, but there is no test verifying this behavior for the SessionTicket overload.

Copilot · 2026-01-18T22:08:32Z

tests/Ashlar.Tests/Identity/AuthenticationProviderTests.cs

+
+        mockProvider.Setup(p => p.SupportedType).Returns(ProviderType.OAuth);
+        Assert.That(mockProvider.Object.GetProviderName(assertion.Object), Is.EqualTo(ProviderType.OAuth.Value));
+    }


Missing test coverage for the new default interface members IsPrimary, IsSecondary, and BypassesMfa on IAuthenticationProvider. These default implementations should be tested to verify they return the expected values (true, false, and false respectively).

Copilot · 2026-01-18T22:08:32Z

tests/Ashlar.Tests/Identity/MfaHandshakeTests.cs

+    public void SessionTicketSerializerShouldReturnNullOnInvalidFormat()
+    {
+        var result = _ticketSerializer.Deserialize("not-a-valid-ticket");
+        Assert.That(result, Is.Null);
+    }


Duplicate test detected. This test is identical to the test on line 39 of SessionTicketSerializerTests.cs. Tests for SessionTicketSerializer should be consolidated in SessionTicketSerializerTests.cs rather than duplicated in MfaHandshakeTests.cs.

Copilot · 2026-01-18T22:08:32Z

src/Ashlar/Identity/IdentityService.cs

+        if (_providers.TryGetValue(assertion.ProviderType, out var currentProvider))
        {
-            if (result.IsCredentialConsumed)
+            if (currentProvider.BypassesMfa(assertion))
            {
-                // Fail authentication if we cannot guarantee the credential was consumed (prevent replay/race conditions).
-                return new AuthenticationResponse(false, user);
+                return false;
            }
        }


These 'if' statements can be combined.

sonarqubecloud · 2026-02-18T15:11:39Z

Quality Gate passed

Issues
10 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

feat: mfa handshake infrastructure

f8c027d

Copilot AI review requested due to automatic review settings January 18, 2026 22:03

Copilot started reviewing on behalf of Thomas-Shephard January 18, 2026 22:03 View session

Copilot AI reviewed Jan 18, 2026

View reviewed changes

Merge branch 'main' into feature/mfa-handshake-infrastructure

aa6d102

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: mfa handshake infrastructure#17

feat: mfa handshake infrastructure#17
Thomas-Shephard wants to merge 2 commits intomainfrom
feature/mfa-handshake-infrastructure

Thomas-Shephard commented Jan 18, 2026

Uh oh!

sonarqubecloud bot commented Jan 18, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

Copilot AI Jan 18, 2026

Uh oh!

Copilot AI Jan 18, 2026

Uh oh!

Copilot AI Jan 18, 2026

Uh oh!

Copilot AI Jan 18, 2026

Uh oh!

Copilot AI Jan 18, 2026

Uh oh!

Copilot AI Jan 18, 2026

Uh oh!

Copilot AI Jan 18, 2026

Uh oh!

Copilot AI Jan 18, 2026

Uh oh!

Copilot AI Jan 18, 2026

Uh oh!

sonarqubecloud bot commented Feb 18, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

+/// <summary>
+/// Represents the state of an authentication handshake, including the user and verified
+/// authentication factors. This is the concrete implementation of <see cref="IAuthenticationHandshake"/>
+/// used to track multi-factor authentication (MFA) handshake state.
+/// </summary>


		Task<AuthenticationResponse> LoginAsync(string email, IAuthenticationAssertion assertion, Guid? tenantId = null, CancellationToken cancellationToken = default);

-    Task<AuthenticationResponse> LoginAsync(string email, IAuthenticationAssertion assertion, Guid? tenantId = null, CancellationToken cancellationToken = default);
+    /// <summary>
+    /// Initiates an authentication flow for a user identified by email using the provided authentication assertion.
+    /// </summary>
+    /// <param name="email">The email address used to identify the user.</param>
+    /// <param name="assertion">The authentication assertion (for example, a password or external provider assertion) to validate.</param>
+    /// <param name="tenantId">An optional tenant identifier for multi-tenant scenarios.</param>
+    /// <param name="cancellationToken">A token that can be used to cancel the asynchronous operation.</param>
+    /// <returns>
+    /// An <see cref="AuthenticationResponse"/> describing the outcome of the authentication attempt, which may include
+    /// the authenticated user, status, claims, and an optional session ticket when additional factors are required.
+    /// </returns>
+    Task<AuthenticationResponse> LoginAsync(string email, IAuthenticationAssertion assertion, Guid? tenantId = null, CancellationToken cancellationToken = default);
+    /// <summary>
+    /// Continues a multi-factor authentication flow using an existing <see cref="SessionTicket"/> and a follow-up authentication assertion.
+    /// </summary>
+    /// <param name="sessionTicket">The session ticket issued by a previous authentication step, typically when MFA is required.</param>
+    /// <param name="assertion">The follow-up authentication assertion (such as an MFA code or verification) to validate.</param>
+    /// <param name="cancellationToken">A token that can be used to cancel the asynchronous operation.</param>
+    /// <returns>
+    /// An <see cref="AuthenticationResponse"/> describing the outcome of the continued authentication flow, which may include
+    /// the authenticated user, status, claims, and an updated session ticket.
+    /// </returns>

Conversation

Thomas-Shephard commented Jan 18, 2026

Uh oh!

sonarqubecloud bot commented Jan 18, 2026

Quality Gate passed

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Copilot AI Jan 18, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 18, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 18, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 18, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 18, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 18, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 18, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 18, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 18, 2026

Choose a reason for hiding this comment

Uh oh!

sonarqubecloud bot commented Feb 18, 2026

Quality Gate passed

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants