Remove the IsComplete property from AuthenticationState (#725)

dotnet-authserver/src/TeacherIdentity.AuthServer/AuthenticationState.cs

@@ -189,23 +189,16 @@ public bool TryGetOAuthState([NotNullWhen(true)] out OAuthAuthorizationState? oA
 
     public IEnumerable<Claim> GetInternalClaims()
     {
-        if (!IsComplete)
+        if (!UserId.HasValue)
         {
-            throw new InvalidOperationException("Cannot retrieve claims until authentication is complete.");
+            throw new InvalidOperationException("User is not signed in.");
         }
 
         return UserClaimHelper.GetInternalClaims(this);
     }
 
     public UserType[] GetPermittedUserTypes() => UserRequirements.GetPermittedUserTypes();
 
-    public bool IsComplete =>
-        EmailAddressVerified &&
-        (!UserRequirements.HasFlag(UserRequirements.TrnHolder) ||
-        HasTrnToken ||
-        TrnLookup == TrnLookupState.Complete) &&
-        UserId.HasValue;
-
     public bool HasExpired(DateTime utcNow) => (StartedAt + JourneyLifetime) <= utcNow;
 
     public void Reset(DateTime utcNow)
@@ -655,11 +648,6 @@ public void OnTrnLookupCompleted(FindTeachersResponseResult? findTeachersResult,
 
     public async Task<ClaimsPrincipal> SignIn(HttpContext httpContext)
     {
-        if (!IsComplete)
-        {
-            throw new InvalidOperationException("Journey is not complete.");
-        }
-
         var claims = GetInternalClaims();
         return await httpContext.SignInCookies(claims, resetIssued: true, AuthCookieLifetime);
     }

dotnet-authserver/src/TeacherIdentity.AuthServer/Controllers/AuthorizationController.cs

@@ -181,7 +181,9 @@ public async Task<IActionResult> Authorize()
             authenticationState.Reset(_clock.UtcNow);
         }
 
-        if (!authenticationState.IsComplete)
+        var signInJourney = _signInJourneyProvider.GetSignInJourney(authenticationState, HttpContext);
+
+        if (!signInJourney.IsCompleted())
         {
             // If the client application requested promptless authentication,
             // return an error indicating that the user is not logged in.
@@ -196,7 +198,6 @@ public async Task<IActionResult> Authorize()
                     }));
             }
 
-            var signInJourney = _signInJourneyProvider.GetSignInJourney(authenticationState, HttpContext);
             return Redirect(signInJourney.GetStartStepUrl());
         }
 
@@ -207,7 +208,7 @@ public async Task<IActionResult> Authorize()
             await _trnTokenHelper.ApplyTrnTokenToUser(authenticationState.UserId, trnToken.TrnToken);
         }
 
-        Debug.Assert(authenticationState.IsComplete);
+        Debug.Assert(signInJourney.IsCompleted());
 
         // It's possible that the user doesn't have a 'signed in' cookie but the sign in journey is completed
         // (if, say, the user retried the page that sent the response cookie and they never got it).

dotnet-authserver/src/TeacherIdentity.AuthServer/Infrastructure/Filters/RequireAuthenticationStateFilter.cs

@@ -1,32 +1,25 @@
 using Microsoft.AspNetCore.Http.Extensions;
 using Microsoft.AspNetCore.Mvc;
 using Microsoft.AspNetCore.Mvc.Filters;
+using TeacherIdentity.AuthServer.Journeys;
 using TeacherIdentity.AuthServer.State;
 
 namespace TeacherIdentity.AuthServer.Infrastructure.Filters;
 
-public class RequireAuthenticationStateFilterFactory : IFilterFactory
-{
-    public bool IsReusable => false;  // RequireAuthenticationStateFilter needs an IClock which is transient
-
-    public IFilterMetadata CreateInstance(IServiceProvider serviceProvider)
-    {
-        var filter = serviceProvider.GetRequiredService<RequireAuthenticationStateFilter>();
-        return filter;
-    }
-}
-
 public class RequireAuthenticationStateFilter : IAuthorizationFilter
 {
+    private readonly SignInJourneyProvider _signInJourneyProvider;
     private readonly IdentityLinkGenerator _linkGenerator;
     private readonly ILogger<RequireAuthenticationStateFilter> _logger;
     private readonly IClock _clock;
 
     public RequireAuthenticationStateFilter(
+        SignInJourneyProvider signInJourneyProvider,
         IdentityLinkGenerator linkGenerator,
         ILogger<RequireAuthenticationStateFilter> logger,
         IClock clock)
     {
+        _signInJourneyProvider = signInJourneyProvider;
         _linkGenerator = linkGenerator;
         _logger = logger;
         _clock = clock;
@@ -44,10 +37,11 @@ public void OnAuthorization(AuthorizationFilterContext context)
         }
 
         var authenticationState = authenticationStateFeature.AuthenticationState;
+        var signInJourney = _signInJourneyProvider.GetSignInJourney(authenticationState, context.HttpContext);
 
         // If the journey has been completed then forward to the completion page
         // (prevents going 'back' to amend submitted details)
-        if (authenticationState.IsComplete)
+        if (signInJourney.IsCompleted())
         {
             if (context.HttpContext.GetEndpoint()?.Metadata.Contains(AllowCompletedAuthenticationJourneyMarker.Instance) != true)
             {

dotnet-authserver/src/TeacherIdentity.AuthServer/Journeys/CoreSignInJourney.cs

@@ -37,11 +37,11 @@ public override bool CanAccessStep(string step)
             Steps.EmailConfirmation => AuthenticationState is { EmailAddressSet: true, EmailAddressVerified: false },
             Steps.ResendEmailConfirmation => AuthenticationState is { EmailAddressSet: true, EmailAddressVerified: false },
             Steps.InstitutionEmail => AuthenticationState is { EmailAddressSet: true, EmailAddressVerified: true, IsInstitutionEmail: true },
-            Steps.EmailExists => AuthenticationState.IsComplete,
+            Steps.EmailExists => AuthenticationState.UserId is not null,
             Steps.Phone => AuthenticationState.EmailAddressVerified,
             Steps.PhoneConfirmation => AuthenticationState is { MobileNumberSet: true, MobileNumberVerified: false, EmailAddressVerified: true },
             Steps.ResendPhoneConfirmation => AuthenticationState is { MobileNumberSet: true, MobileNumberVerified: false },
-            Steps.PhoneExists => AuthenticationState.IsComplete,
+            Steps.PhoneExists => AuthenticationState.UserId is not null,
             Steps.Name => AuthenticationState.ContactDetailsVerified,
             Steps.PreferredName => AuthenticationState is { NameSet: true, ContactDetailsVerified: true },
             Steps.DateOfBirth => AuthenticationState is { PreferredNameSet: true, ContactDetailsVerified: true },
@@ -74,20 +74,20 @@ public override string GetLastAccessibleStepUrl(string? requestedStep)
         return (currentStep, AuthenticationState) switch
         {
             (SignInJourney.Steps.Email, _) => SignInJourney.Steps.EmailConfirmation,
-            (SignInJourney.Steps.EmailConfirmation, { IsComplete: true }) => Steps.EmailExists,
-            (SignInJourney.Steps.EmailConfirmation, { IsComplete: false }) => shouldCheckAnswers ? Steps.CheckAnswers : Steps.NoAccount,
+            (SignInJourney.Steps.EmailConfirmation, { UserId: not null }) => Steps.EmailExists,
+            (SignInJourney.Steps.EmailConfirmation, _) => shouldCheckAnswers ? Steps.CheckAnswers : Steps.NoAccount,
             (Steps.NoAccount, _) => Steps.Phone,
             (Steps.Landing, _) => Steps.Email,
             (Steps.Email, _) => Steps.EmailConfirmation,
-            (Steps.EmailConfirmation, { IsComplete: true }) => Steps.EmailExists,
-            (Steps.EmailConfirmation, { IsComplete: false, IsInstitutionEmail: true }) => shouldCheckAnswers ? Steps.CheckAnswers : Steps.InstitutionEmail,
-            (Steps.EmailConfirmation, { IsComplete: false }) => shouldCheckAnswers ? Steps.CheckAnswers : Steps.Phone,
+            (Steps.EmailConfirmation, { UserId: not null }) => Steps.EmailExists,
+            (Steps.EmailConfirmation, { IsInstitutionEmail: true, UserId: null }) => shouldCheckAnswers ? Steps.CheckAnswers : Steps.InstitutionEmail,
+            (Steps.EmailConfirmation, _) => shouldCheckAnswers ? Steps.CheckAnswers : Steps.Phone,
             (Steps.ResendEmailConfirmation, _) => Steps.EmailConfirmation,
             (Steps.InstitutionEmail, { EmailAddressVerified: false }) => Steps.EmailConfirmation,
             (Steps.InstitutionEmail, { EmailAddressVerified: true }) => shouldCheckAnswers ? Steps.CheckAnswers : Steps.Phone,
             (Steps.Phone, _) => Steps.PhoneConfirmation,
-            (Steps.PhoneConfirmation, { IsComplete: true }) => Steps.PhoneExists,
-            (Steps.PhoneConfirmation, { IsComplete: false }) => shouldCheckAnswers ? Steps.CheckAnswers : Steps.Name,
+            (Steps.PhoneConfirmation, { UserId: not null }) => Steps.PhoneExists,
+            (Steps.PhoneConfirmation, _) => shouldCheckAnswers ? Steps.CheckAnswers : Steps.Name,
             (Steps.ResendPhoneConfirmation, _) => Steps.PhoneConfirmation,
             (Steps.Name, { ExistingAccountFound: true }) => Steps.AccountExists,
             (Steps.Name, { ExistingAccountFound: false }) => shouldCheckAnswers ? Steps.CheckAnswers : Steps.PreferredName,
@@ -139,7 +139,7 @@ public override string GetLastAccessibleStepUrl(string? requestedStep)
 
     protected override string GetStartStep() => Steps.Landing;
 
-    protected override bool IsFinished() => AuthenticationState.IsComplete;
+    protected override bool IsFinished() => AuthenticationState.UserId.HasValue;
 
     protected override string GetStepUrl(string step) => step switch
     {

dotnet-authserver/src/TeacherIdentity.AuthServer/Journeys/TrnTokenSignInJourney.cs

@@ -70,7 +70,7 @@ public override bool CanAccessStep(string step)
             SignInJourney.Steps.EmailConfirmation => AuthenticationState is { EmailAddressSet: true, EmailAddressVerified: false },
             CoreSignInJourney.Steps.Phone => AuthenticationState.EmailAddressVerified,
             CoreSignInJourney.Steps.PhoneConfirmation => AuthenticationState is { MobileNumberSet: true, MobileNumberVerified: false, EmailAddressVerified: true },
-            CoreSignInJourney.Steps.PhoneExists => AuthenticationState.IsComplete,
+            CoreSignInJourney.Steps.PhoneExists => AuthenticationState.UserId is not null,
             CoreSignInJourney.Steps.ResendPhoneConfirmation => AuthenticationState is { MobileNumberSet: true, MobileNumberVerified: false },
             CoreSignInJourney.Steps.Email => AuthenticationState.MobileNumberVerified,
             CoreSignInJourney.Steps.EmailConfirmation => AuthenticationState is { EmailAddressSet: true, EmailAddressVerified: false, MobileNumberVerified: true },
@@ -97,11 +97,11 @@ public override bool CanAccessStep(string step)
             (Steps.Landing, { ExistingAccountFound: true }) => CoreSignInJourney.Steps.AccountExists,
             (Steps.Landing, { ExistingAccountFound: false }) => CoreSignInJourney.Steps.Phone,
             (SignInJourney.Steps.Email, _) => SignInJourney.Steps.EmailConfirmation,
-            (SignInJourney.Steps.EmailConfirmation, { IsComplete: true }) => CoreSignInJourney.Steps.EmailExists,
-            (SignInJourney.Steps.EmailConfirmation, { IsComplete: false }) => shouldCheckAnswers ? Steps.CheckAnswers : CoreSignInJourney.Steps.Phone,
+            (SignInJourney.Steps.EmailConfirmation, { UserId: not null }) => CoreSignInJourney.Steps.EmailExists,
+            (SignInJourney.Steps.EmailConfirmation, _) => shouldCheckAnswers ? Steps.CheckAnswers : CoreSignInJourney.Steps.Phone,
             (CoreSignInJourney.Steps.Phone, _) => CoreSignInJourney.Steps.PhoneConfirmation,
-            (CoreSignInJourney.Steps.PhoneConfirmation, { IsComplete: true }) => CoreSignInJourney.Steps.PhoneExists,
-            (CoreSignInJourney.Steps.PhoneConfirmation, { IsComplete: false }) => shouldCheckAnswers ? Steps.CheckAnswers : CoreSignInJourney.Steps.PreferredName,
+            (CoreSignInJourney.Steps.PhoneConfirmation, { UserId: not null }) => CoreSignInJourney.Steps.PhoneExists,
+            (CoreSignInJourney.Steps.PhoneConfirmation, _) => shouldCheckAnswers ? Steps.CheckAnswers : CoreSignInJourney.Steps.PreferredName,
             (CoreSignInJourney.Steps.Email, _) => CoreSignInJourney.Steps.EmailConfirmation,
             (CoreSignInJourney.Steps.EmailConfirmation, { IsInstitutionEmail: false }) => Steps.CheckAnswers,
             (CoreSignInJourney.Steps.EmailConfirmation, { IsInstitutionEmail: true }) => CoreSignInJourney.Steps.InstitutionEmail,
@@ -170,7 +170,9 @@ AuthenticationState is
 
     protected override string GetStartStep() => Steps.Landing;
 
-    protected override bool IsFinished() => AuthenticationState.IsComplete;
+    protected override bool IsFinished() =>
+        AuthenticationState.UserId.HasValue &&
+        AuthenticationState.TrnLookupStatus == TrnLookupStatus.Found;
 
     protected override string GetStepUrl(string step) => step switch
     {

dotnet-authserver/src/TeacherIdentity.AuthServer/Program.cs

@@ -293,7 +293,7 @@ public static async Task Main(string[] args)
                 "/SignIn",
                 model =>
                 {
-                    model.Filters.Add(new RequireAuthenticationStateFilterFactory());
+                    model.Filters.Add(new ServiceFilterAttribute(typeof(RequireAuthenticationStateFilter)));
                     model.Filters.Add(new NoCachePageFilter());
                 });
 

dotnet-authserver/src/TeacherIdentity.AuthServer/UserClaimHelper.cs

@@ -21,9 +21,9 @@ public UserClaimHelper(TeacherIdentityServerDbContext dbContext, IDqtApiClient d
 
     public static IReadOnlyCollection<Claim> GetInternalClaims(AuthenticationState authenticationState)
     {
-        if (!authenticationState.IsComplete)
+        if (!authenticationState.UserId.HasValue)
         {
-            throw new InvalidOperationException("Cannot retrieve claims until authentication is complete.");
+            throw new InvalidOperationException("User is not signed in.");
         }
 
         return GetInternalClaims(
@@ -37,9 +37,8 @@ public static IReadOnlyCollection<Claim> GetInternalClaims(AuthenticationState a
             authenticationState.StaffRoles);
     }
 
-    public static IReadOnlyCollection<Claim> GetInternalClaims(User user)
-    {
-        return GetInternalClaims(
+    public static IReadOnlyCollection<Claim> GetInternalClaims(User user) =>
+        GetInternalClaims(
             user.UserId,
             user.EmailAddress,
             user.FirstName,
@@ -48,7 +47,6 @@ public static IReadOnlyCollection<Claim> GetInternalClaims(User user)
             user.Trn,
             user.UserType,
             user.StaffRoles);
-    }
 
     public static string MapUserTypeToClaimValue(UserType userType) => userType.ToString();