Skip to content

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.

Sign up for GitHub

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

Closed
immerda wants to merge 1 commit into from
Closed

Allow SSO authentication to provide a user secret #24837

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 4 additions & 2 deletions lib/private/legacy/OC_User.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -170,7 +170,9 @@ public static function loginWithApache(\OCP\Authentication\IApacheBackend $backe
$userSession = \OC::$server->getUserSession();
$userSession->setLoginName($uid);
$request = OC::$server->getRequest();
$userSession->createSessionToken($request, $uid, $uid);
$secret = $backend->getCurrentUserSecret();
$userSession->createSessionToken($request, $uid, $uid, $secret);
$pw = $secret === null ? '' : $secret;
Copy link
Member

@juliusknorr juliusknorr Jul 21, 2021

Choose a reason for hiding this comment

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

Suggested change
$pw = $secret === null ? '' : $secret;
$password = $secret === null ? '' : $secret;

// setup the filesystem
OC_Util::setupFS($uid);
// first call the post_login hooks, the login-process needs to be
Expand All @@ -182,7 +184,7 @@ public static function loginWithApache(\OCP\Authentication\IApacheBackend $backe
'post_login',
[
'uid' => $uid,
'password' => '',
'password' => $pw,
Copy link
Member

@juliusknorr juliusknorr Jul 21, 2021

Choose a reason for hiding this comment

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

Suggested change
'password' => $pw,
'password' => $password,

'isTokenLogin' => false,
]
);
Expand Down
8 changes: 8 additions & 0 deletions lib/public/Authentication/IApacheBackend.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -62,4 +62,12 @@ public function getLogoutUrl();
* @since 6.0.0
*/
public function getCurrentUserId();

/**
* 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
Copy link
Member

@juliusknorr juliusknorr Jul 21, 2021

Choose a reason for hiding this comment

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

Suggested change
* @since 21.0.0
* @since 23.0.0

Copy link
Member

@juliusknorr juliusknorr Jul 21, 2021

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.

Copy link
Member

@MichaIng MichaIng Jul 21, 2021

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?

Copy link
Member

@juliusknorr juliusknorr Jul 22, 2021

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 👍

*/
public function getCurrentUserSecret();
}