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

SSO error handling #18111

Open
wants to merge 2 commits into
base: implementation/61178-update-remoteidentity-after-token-exchange
Choose a base branch
from

Conversation

NobodysNightmare
Copy link
Contributor

Ticket

What are you trying to accomplish?

Fixing errors related to showing the connection status of SSO users to their storage.

In the files tab, the following cases should now be indicated correctly:

  • A "classic" (OAuth) user is not yet linked to the storage (existed before)
  • A "classic" (OAuth) user could not successfully authenticate at the storage (new: indicated as an error)
  • An SSO user could not successfully authenticate at the storage (indicated as an error)

In the project storages member overview (e.g. /projects/:name/settings/project_storages/:id/members):

  • SSO members are now indicated as connected, once they obtained a remote identity
  • SSO members have a specific not-connected message, when there is no remote identity yet

Which approach did you choose and why?

It occured to me, that the question of which authentication method will really be used for a user at a storage has to be asked in many different places. Thus I extracted the relevant code into a helper called AuthenticationMethodSelector.

Merge checklist

  • Added/updated tests
  • Tested major browsers (Chrome, Firefox, Edge, ...)

@NobodysNightmare NobodysNightmare changed the title Sso error handling SSO error handling Feb 28, 2025
@NobodysNightmare NobodysNightmare force-pushed the sso-error-handling branch 2 times, most recently from 9f35efa to fb3bde7 Compare February 28, 2025 13:29
Introducing an explicit difference between users that were never linked
and those that have a link (i.e. a remote identity) but receive an authentication
error anyways. There is also differentiation between SSO users and non-SSO users.
When authenticating to the storage via SSO, it wouldn't make sense to show a login
button to users.
The display would not work at all for SSO-only storages
and would fail to recognize connections that didn't happen through
a storage's oauth client.

Now properly recognizing all remote identities and showing a special
warning message when a non-connected user would connect through SSO.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

1 participant