Skip to content
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.

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

Do not rely on claims to figure out user access + prepare for claims removal #16552

Merged

Conversation

kjac
Copy link
Contributor

@kjac kjac commented Jun 4, 2024

Prerequisites

  • I have added steps to test this contribution in the description below

Description

We're currently challenged by our claims and how they're tied to the tokens. Specifically, whenever we change something on a user or a user group, we need to revoke all issued tokens. But we won't revoke admin tokens, because we don't want to risk logging the current admin out.

This PR addresses these challenges by obsoleting the claims for user start nodes and "allowed apps" (sections). These are now resolved directly from the user instance, not from the token bound claims.

Eventually these claims will be removed. There will be a separate announcement for this, as it is potentially quite breaking for some.

What about the role claims?

We could in principle also remove the role claims (ClaimsIdentity.DefaultRoleClaimType). At this point I'm a little weary about doing that, though.

In effect this means an access token will carry over roles for the duration of its lifetime, even if a user is removed from one role and added to another.

Testing this PR

The backoffice should function as per usual - auth and endpoint protection based on "allowed apps" (sections) should still work.

Changes in user start nodes or "allowed apps" should be immediately visible for all users.

@bergmania bergmania merged commit 1f52d01 into v14/dev Jun 19, 2024
13 of 15 checks passed
@bergmania bergmania deleted the v14/feature/do-not-use-start-node-and-allowed-apps-claims branch June 19, 2024 13:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants