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

fix: Disable password confirmation for SSO #668

Merged
merged 1 commit into from
Aug 29, 2023
Merged

fix: Disable password confirmation for SSO #668

merged 1 commit into from
Aug 29, 2023

Conversation

juliusknorr
Copy link
Member

@juliusknorr juliusknorr commented Aug 17, 2023

Similar behaviour as the user_saml app to avoid asking for password confirmation for SSO where we don't have a password to validate against.

Fixes #468

Signed-off-by: Julius Härtl <jus@bitgrid.net>
@juliusknorr juliusknorr marked this pull request as ready for review August 17, 2023 12:42
@juliusknorr juliusknorr added bug Something isn't working 3. to review labels Aug 17, 2023
@juliusknorr juliusknorr merged commit 024ef1a into main Aug 29, 2023
46 checks passed
@juliusknorr juliusknorr deleted the bugfix/468 branch August 29, 2023 05:51
@julien-nc julien-nc mentioned this pull request Nov 14, 2023
@thlehmann-ionos
Copy link

I don't think this is sufficient and completely thought through.

TL;DR: The password confirmation library also checks for backendAllowsPasswordConfirmation, which should be false, since a user authenticated via SSO can not confirm their password via Nextcloud's user database. The variable however will only be false if the user backend implements IPasswordConfirmationBackend.

More details

This can only be the case if the user could be looked up in the user_oidc backend by their ID. See user Manager->get().

This, in turn, means the user has to be provisioned in the user_oidc backend by the same ID as in the Nextcloud backend, since otherwise, the backend lookup would yield the Nextcloud user backend, which does not implement the above mentioned interface. See how the user is looked up in the code auth phase and later associated with the user's session.

The error would be reproducible if the user was auto-provisioned and the (--unique-id) was not false (0) (true is the default) since user_oidc would generate a new ID in the process. See the code.

Note that a auto-provisioned user with unique user ID could not be looked up as the backend implementing IPasswordConfirmationBackend as it would by looked up by the ID in the sub claim. This is a separate issue #898.

Proposed solutions

  1. Hard (=non-auto) de-provisioning should be implemented to allow an explicitly created user_oidc backed user entity to be removed, since it otherwise breaks de-provisioning via Nextcloud too or
  2. The hard provisioning should provision the user in the Nextcloud backend
  3. The ID-uniqueness feature should be dropped or changed in a way that the user_oidc backend allows lookup by the hashed user ID too

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
3. to review bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Cannot create app passwords due to password prompt
3 participants