Skip to content

chore: limit aal1 sessions correctly#2452

Open
staaldraad wants to merge 4 commits into
masterfrom
etienne/prodsec-76
Open

chore: limit aal1 sessions correctly#2452
staaldraad wants to merge 4 commits into
masterfrom
etienne/prodsec-76

Conversation

@staaldraad
Copy link
Copy Markdown
Member

What kind of change does this PR introduce?

Bug fix

What is the current behavior?

When MFA_ALLOW_LOW_AAL is 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.

@staaldraad staaldraad requested a review from a team as a code owner March 27, 2026 16:48
Comment thread internal/tokens/service.go Outdated
// 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)
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 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.

Suggested change
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
}

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant