Skip to content

Fix JWT subject when issuing tokens via the for field#9207

Open
begelundmuller wants to merge 5 commits intomainfrom
begelundmuller/fix-for-token-subject
Open

Fix JWT subject when issuing tokens via the for field#9207
begelundmuller wants to merge 5 commits intomainfrom
begelundmuller/fix-for-token-subject

Conversation

@begelundmuller
Copy link
Copy Markdown
Contributor

@begelundmuller begelundmuller commented Apr 9, 2026

Previously, GetDeployment, GetDeploymentCredentials, and GetIFrame always used claims.OwnerID() as the JWT subject, even when for was set to issue a token on behalf of another identity. Now the subject is derived from the for field: the user ID when provided, a SHA-256 hash of the email for virtual users, or the id attribute from custom attributes.

Checklist:

  • Covered by tests
  • Ran it and it works as intended
  • Reviewed the diff before requesting a review
  • Checked for unhandled edge cases
  • Linked the issues it closes
  • Checked if the docs need to be updated. If so, create a separate Linear DOCS issue
  • Intend to cherry-pick into the release branch
  • I'm proud of this work!

@begelundmuller begelundmuller self-assigned this Apr 9, 2026
@begelundmuller begelundmuller requested a review from pjain1 April 9, 2026 16:34
}

case *adminv1.GetDeploymentCredentialsRequest_UserEmail:
subject = safeHash(forVal.UserEmail)
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

If the email is a Rill user then s.getAttributesForUser return the actual userId, can that be returned here from s.getAttributesAndResourceRestrictionsForUser and used?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Heh, yeah I also did that first, but realized that this could cause the resolved ID to change later if the user initially is not a Rill user, but then later signs up for Rill (maybe for a different reason/account entirely). So thought it was safer just to hash so it stays stable in any circumstance.

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.

2 participants