-
-
Notifications
You must be signed in to change notification settings - Fork 4.7k
Allow SSO authentication to provide a user secret #24837
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
Conversation
Allow Authentication\IApacheBackend to return a per-user secret. This secret is used in lieu of a passwort to initialize the session. This allows an SSO backend to support per-user encrypted files. Signed-off-by: Peter Meier <[email protected]>
MorrisJobke
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Makes sense to pass it here. 👍
|
moving to 23 since we are in feature freeze. CI is unhappy also. |
|
@immerda Can you rebase and try to make CI happy ? |
Implementing PR #24837 from immerda Signed-off-by: MichaIng <[email protected]>
Implementing PR #24837 from immerda Signed-off-by: MichaIng <[email protected]>
| * Optionally returns a stable per-user secret. This secret is for | ||
| * instance used to secure file encryption keys. | ||
| * @return string|null | ||
| * @since 21.0.0 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| * @since 21.0.0 | |
| * @since 23.0.0 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actually we could move this to a separate interface like IProvideUserSecretBackend to avoid breaking existing implementations on new Nextcloud releases. With that we could also avoid the null return value and check in OC_User and do a check there if the interface is implemented by the backend.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
To avoid confusion, the PR has been redone here as immerda seems to be not maintaining it here. The version string has been adjusted already, I'll do the same with pw => password. About the new interface, you need to tell me more in detail how to do that, over there, I guess this means a new script?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah indeed, I already wondered, because I though I already reviewed something similar but was not seeing my comments on this one 👍
| $userSession->createSessionToken($request, $uid, $uid); | ||
| $secret = $backend->getCurrentUserSecret(); | ||
| $userSession->createSessionToken($request, $uid, $uid, $secret); | ||
| $pw = $secret === null ? '' : $secret; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| $pw = $secret === null ? '' : $secret; | |
| $password = $secret === null ? '' : $secret; |
| [ | ||
| 'uid' => $uid, | ||
| 'password' => '', | ||
| 'password' => $pw, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| 'password' => $pw, | |
| 'password' => $password, |
Implementing PR #24837 from immerda Signed-off-by: MichaIng <[email protected]>
|
Thanks @juliushaertl for the review and @MichaIng for pushing this further. The train is moving, so let's all move with it to #27929 |
Implementing PR #24837 from immerda Signed-off-by: MichaIng <[email protected]>
Implementing PR #24837 from immerda Signed-off-by: MichaIng <[email protected]>
Implementing PR #24837 from immerda Signed-off-by: MichaIng <[email protected]>
Implementing PR #24837 from immerda Signed-off-by: MichaIng <[email protected]>
Implementing PR #24837 from immerda Signed-off-by: MichaIng <[email protected]>
Implementing PR #24837 from immerda Signed-off-by: MichaIng <[email protected]>
Implementing PR #24837 from immerda Signed-off-by: MichaIng <[email protected]>
Implementing PR #24837 from immerda Signed-off-by: MichaIng <[email protected]>
Implementing PR #24837 from immerda Signed-off-by: MichaIng <[email protected]>
Implementing PR #24837 from immerda Signed-off-by: MichaIng <[email protected]>
Implementing PR #24837 from immerda Signed-off-by: MichaIng <[email protected]>
Implementing PR #24837 from immerda Signed-off-by: MichaIng <[email protected]>
Implementing PR #24837 from immerda Signed-off-by: MichaIng <[email protected]>
Implementing PR #24837 from immerda Signed-off-by: MichaIng <[email protected]>
Implementing PR #24837 from immerda Signed-off-by: MichaIng <[email protected]>
Implementing PR #24837 from immerda Signed-off-by: MichaIng <[email protected]>
Implementing PR #24837 from immerda Signed-off-by: MichaIng <[email protected]>
Implementing PR #24837 from immerda Signed-off-by: MichaIng <[email protected]>
Allow Authentication\IApacheBackend to return a per-user secret. This
secret is used in lieu of a passwort to initialize the session.
This allows an SSO backend to support per-user encrypted files.