chore: limit aal1 sessions correctly#2452
Conversation
![depthfirst-app[bot]](https://avatars.githubusercontent.com/in/1021002?s=60&v=4){kind=small}
| // if user has mfa enabled and the session has not yet been upgraded | ||
| // and Limit duration of AAL1 sessions is enabled | ||
| // expiresAt should be set to the maximum duration for low aal sessions | ||
| expiresAt = session.CreatedAt.UTC().Add(*config.Sessions.AllowLowAAL) |
There was a problem hiding this comment.
🟡 Severity: MEDIUM
No upper-bound against JWT.Exp is applied. If AllowLowAAL > JWT.Exp (e.g., AllowLowAAL=1h, JWT.Exp=300s), AAL1 sessions (unauthenticated with MFA) receive longer-lived JWTs than fully-authenticated AAL2 sessions, inverting the intended security constraint. Config validation only enforces AllowLowAAL > 0, not AllowLowAAL ≤ JWT.Exp.
Helpful? Add 👍 / 👎
💡 Fix Suggestion
Suggestion: Cap the low-AAL expiry against the standard JWT expiry (JWT.Exp) so that AAL1 sessions can never receive a longer-lived token than a fully-authenticated AAL2 session. Replace the single assignment at line 679 with a comparison that picks the earlier of the two expiry times.
⚠️ Experimental Feature: This code suggestion is automatically generated. Please review carefully.
| expiresAt = session.CreatedAt.UTC().Add(*config.Sessions.AllowLowAAL) | |
| standardExp := issuedAt.Add(time.Second * time.Duration(config.JWT.Exp)) | |
| lowAALExp := session.CreatedAt.UTC().Add(*config.Sessions.AllowLowAAL) | |
| if lowAALExp.Before(standardExp) { | |
| expiresAt = lowAALExp | |
| } else { | |
| expiresAt = standardExp | |
| } |
1 participant
What kind of change does this PR introduce?
Bug fix
What is the current behavior?
When
MFA_ALLOW_LOW_AALis false, AAL1 sessions JWTs should be limited to 15 minutes. Currently the JWT is created with an expiry set to the standard session timeout. And the LOW_AAL timeout is only checked when the refresh_token is issued.This means AAL1 sessions can be valid beyond the 15 minute window.
What is the new behavior?
AAL1 JWTs are limited to 15minutes.