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 password confirmation #1061

Open
wants to merge 3 commits into
base: main
Choose a base branch
from
Open

Fix password confirmation #1061

wants to merge 3 commits into from

Conversation

julien-nc
Copy link
Member

refs nextcloud/server#43612

  1. Replicate what's done in user_saml's user backend with the session: Set last-password-confirm in the session everytime the user ID is obtained in OCA\UserOIDC\User\Backend::getCurrentUserId()
  2. Adjust auth token scope in login controller. Inspired by https://github.com/nextcloud/server/pull/43942/files#diff-b30a8c4cef5da5cede4d9038c16ede43e1746f85270e4249598fd305b0fa77deR173-R176
  3. Bump min NC version to 28 to make sure we have OCP\Authentication\Token\IToken
  4. Only do 2. if IToken::SCOPE_SKIP_PASSWORD_VALIDATION is defined (it was introduced in 30)

Maybe 1. is not necessary. Wdyt?

…whenever the user ID is obtained from the user backend

Signed-off-by: Julien Veyssier <julien-nc@posteo.net>
…TION to true to prevent password validation

Signed-off-by: Julien Veyssier <julien-nc@posteo.net>
@waza-ari
Copy link

Just for confirmation, SCOPE_SKIP_PASSWORD_VALIDATION is just a constant, nothing that needs to be changed from a user perspective, correct?

@julien-nc
Copy link
Member Author

@waza-ari It is internal to the server. Nothing that the users or the admins should be aware of.

@julien-nc julien-nc force-pushed the fix/noid/password-confirm branch from 34615e6 to b0b5588 Compare February 18, 2025 12:15
run cs:fix

Signed-off-by: Julien Veyssier <julien-nc@posteo.net>
@julien-nc julien-nc force-pushed the fix/noid/password-confirm branch from b0b5588 to f1f7992 Compare February 18, 2025 12:19
Copy link
Member

@blizzz blizzz left a comment

Choose a reason for hiding this comment

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

One thing to double check

// prevent password confirmation
if (defined(IToken::class . '::SCOPE_SKIP_PASSWORD_VALIDATION')) {
$token = $this->authTokenProvider->getToken($this->session->getId());
$token->setScope([IToken::SCOPE_SKIP_PASSWORD_VALIDATION => true]);
Copy link
Member

Choose a reason for hiding this comment

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

IIRC, if another scope was set, it needs to be taken over, this would replace it.

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.

3 participants