Skip to content

fix: add pending_password_set claim for invite/recovery sessions #2539

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
deepshekhardas wants to merge 1 commit into
base: master
Choose a base branch
from
Open

fix: add pending_password_set claim for invite/recovery sessions #2539

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 7 additions & 0 deletions internal/api/user.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -92,6 +92,13 @@ func (a *API) UserUpdate(w http.ResponseWriter, r *http.Request) error {

user := getUser(ctx)
session := getSession(ctx)
claims := getClaims(ctx)

if claims != nil && claims.PendingPasswordSet {
if params.Email != "" || params.Phone != "" || params.Data != nil || params.AppData != nil {
return apierrors.NewForbiddenError(apierrors.ErrorCodeUserNotAllowed, "Session requires password to be set before updating other fields")
}
}

if err := a.validateUserUpdateParams(ctx, params); err != nil {
return err
Expand Down
6 changes: 6 additions & 0 deletions internal/tokens/service.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -81,6 +81,7 @@ type AccessTokenClaims struct {
AuthenticationMethodReference AMRClaim `json:"amr,omitempty"`
SessionId string `json:"session_id,omitempty"`
IsAnonymous bool `json:"is_anonymous"`
PendingPasswordSet bool `json:"pending_password_set,omitempty"`
ClientID string `json:"client_id,omitempty"`
Scope string `json:"scope,omitempty"`
}
Expand Down Expand Up @@ -694,6 +695,10 @@ func (s *Service) GenerateAccessToken(r *http.Request, tx *storage.Connection, p
scopes = *session.Scopes
}

userHasPassword := params.User.HasPassword()
isPendingPasswordSet := session.IsRecovery() ||

@depthfirst-app depthfirst-app Bot May 25, 2026

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

session.IsRecovery() is based on immutable AMR claims and will remain true for the lifetime of a recovery session, regardless of whether the user has since set their password. This means PendingPasswordSet is permanently true for any recovery session, preventing legitimate users from ever updating email/phone/data within that session after completing the password reset — contradicting the stated intent of "before the password is set, this session should be restricted."
Helpful? Add 👍 / 👎

💡 Fix Suggestion

Suggestion: The root cause is that session.IsRecovery() is evaluated based solely on immutable AMR claims in the session, which never get cleared even after the user successfully sets a password. The fix is to also require !userHasPassword when checking the recovery session, so that isPendingPasswordSet becomes false as soon as the user has a password. Change line 699 to gate session.IsRecovery() with the !userHasPassword condition, making the intent consistent: the restriction applies only before a password exists, regardless of how the session was created.

⚠️ Experimental Feature: This code suggestion is automatically generated. Please review carefully.

Suggested change
isPendingPasswordSet := session.IsRecovery() ||
isPendingPasswordSet := (!userHasPassword && session.IsRecovery()) ||

(!userHasPassword && params.User.InvitedAt != nil)

claims := &v0hooks.AccessTokenClaims{
RegisteredClaims: jwt.RegisteredClaims{
Subject: params.User.ID.String(),
Expand All @@ -711,6 +716,7 @@ func (s *Service) GenerateAccessToken(r *http.Request, tx *storage.Connection, p
AuthenticatorAssuranceLevel: aal.String(),
AuthenticationMethodReference: amr,
IsAnonymous: params.User.IsAnonymous,
PendingPasswordSet: isPendingPasswordSet,
ClientID: clientID,
Scope: scopes,
}
Expand Down