fix: make session AAL satisfaction check resilient against a nil iden…

…tity in the session Also fix tracing.
ory · Feb 7, 2023 · 5ab1a56 · 5ab1a56
1 parent 71d35dd
commit 5ab1a56
Show file tree

Hide file tree

Showing 2 changed files with 33 additions and 9 deletions.
diff --git a/session/manager_http.go b/session/manager_http.go
@@ -257,7 +257,7 @@ func (s *ManagerHTTP) PurgeFromRequest(ctx context.Context, w http.ResponseWrite
 }
 
 func (s *ManagerHTTP) DoesSessionSatisfy(r *http.Request, sess *Session, requestedAAL string) (err error) {
-	_, span := s.r.Tracer(r.Context()).Tracer().Start(r.Context(), "sessions.ManagerHTTP.DoesSessionSatisfy")
+	ctx, span := s.r.Tracer(r.Context()).Tracer().Start(r.Context(), "sessions.ManagerHTTP.DoesSessionSatisfy")
 	defer otelx.End(span, &err)
 
 	sess.SetAuthenticatorAssuranceLevel()
@@ -267,23 +267,28 @@ func (s *ManagerHTTP) DoesSessionSatisfy(r *http.Request, sess *Session, request
 			return nil
 		}
 	case config.HighestAvailableAAL:
-		i := *sess.Identity
-
-		// If credentials are not expanded, we load them here.
-		if len(i.Credentials) == 0 {
-			if err := s.r.PrivilegedIdentityPool().HydrateIdentityAssociations(r.Context(), &i, identity.ExpandCredentials); err != nil {
+		i := sess.Identity
+		if i == nil {
+			i, err = s.r.IdentityPool().GetIdentity(ctx, sess.IdentityID, identity.ExpandCredentials)
+			if err != nil {
+				return err
+			}
+			sess.Identity = i
+		} else if len(i.Credentials) == 0 {
+			// If credentials are not expanded, we load them here.
+			if err := s.r.PrivilegedIdentityPool().HydrateIdentityAssociations(ctx, i, identity.ExpandCredentials); err != nil {
 				return err
 			}
 		}
 
 		available := identity.NoAuthenticatorAssuranceLevel
-		if firstCount, err := s.r.IdentityManager().CountActiveFirstFactorCredentials(r.Context(), &i); err != nil {
+		if firstCount, err := s.r.IdentityManager().CountActiveFirstFactorCredentials(ctx, i); err != nil {
 			return err
 		} else if firstCount > 0 {
 			available = identity.AuthenticatorAssuranceLevel1
 		}
 
-		if secondCount, err := s.r.IdentityManager().CountActiveMultiFactorCredentials(r.Context(), &i); err != nil {
+		if secondCount, err := s.r.IdentityManager().CountActiveMultiFactorCredentials(ctx, i); err != nil {
 			return err
 		} else if secondCount > 0 {
 			available = identity.AuthenticatorAssuranceLevel2
@@ -294,7 +299,7 @@ func (s *ManagerHTTP) DoesSessionSatisfy(r *http.Request, sess *Session, request
 		}
 
 		return NewErrAALNotSatisfied(
-			urlx.CopyWithQuery(urlx.AppendPaths(s.r.Config().SelfPublicURL(r.Context()), "/self-service/login/browser"), url.Values{"aal": {"aal2"}}).String())
+			urlx.CopyWithQuery(urlx.AppendPaths(s.r.Config().SelfPublicURL(ctx), "/self-service/login/browser"), url.Values{"aal": {"aal2"}}).String())
 	}
 
 	return errors.Errorf("requested unknown aal: %s", requestedAAL)

diff --git a/session/manager_http_test.go b/session/manager_http_test.go
@@ -549,6 +549,25 @@ func TestDoesSessionSatisfy(t *testing.T) {
 			} else {
 				require.NoError(t, err)
 			}
+
+			// This should still work even if the session does not have identity data attached yet...
+			s.Identity = nil
+			err = reg.SessionManager().DoesSessionSatisfy((&http.Request{}).WithContext(context.Background()), s, string(tc.requested))
+			if tc.err != nil {
+				require.ErrorAs(t, err, &tc.err)
+			} else {
+				require.NoError(t, err)
+			}
+
+			// ..or no credentials attached.
+			s.Identity = id
+			s.Identity.Credentials = nil
+			err = reg.SessionManager().DoesSessionSatisfy((&http.Request{}).WithContext(context.Background()), s, string(tc.requested))
+			if tc.err != nil {
+				require.ErrorAs(t, err, &tc.err)
+			} else {
+				require.NoError(t, err)
+			}
 		})
 	}
 }