Implement `IProvideUserSecretBackend` compatibility for per-user encryption by summersab · Pull Request #697 · nextcloud/user_saml

summersab · 2023-02-10T00:48:51Z

This PR is intended to implement the functionality from PR #537. The upstream code changed enough that it was easier to close the original PR and submit a new one.

Now that the IProvideUserSecretBackend class has been added to the Nextcloud core with PR nextcloud/server#24837 / nextcloud/server#27929, this PR adds the necessary logic to support per-user SAML provided secrets.

summersab · 2023-02-24T17:29:56Z

@blizzz, you look like one of the top contributors to this repo. Is the CI check broken?

blizzz · 2023-05-12T14:55:07Z

@blizzz, you look like one of the top contributors to this repo. Is the CI check broken?

Only partly, it should be green up to and including stable25. We're working on getting it back for 26 and master.

immerda · 2023-05-20T20:55:41Z

Hey, this is just our yearly reminder that we are still testing this patch in our instance. In fact I just applied this new PR to the newest released nextcloud version and it works perfectly well and is stable enough that we can still decrypt the data from 2.5 years ago when we initially proposed this patch :) We are still eagerly awaiting upstream adoption...

mglants · 2023-11-02T01:35:15Z

Any news, when it will be supported, readlly want to use saml for server-side encryption

Signed-off-by: summersab <18727110+summersab@users.noreply.github.com> perform a little lint cleanup Signed-off-by: summersab <18727110+summersab@users.noreply.github.com> Signed-off-by: Andrew Summers <18727110+summersab@users.noreply.github.com>

blizzz

Thank you for your contribution! Rebased (CI is repaired since) and left a first review.

blizzz · 2023-11-03T17:37:02Z

lib/UserBackend.php

 		return $value;
 	}

+	private function getUserSecret($uid, array $attributes) {


$uid is unused and can be removed.

blizzz · 2023-11-03T17:38:03Z

lib/UserBackend.php

 		return $value;
 	}

+	private function getUserSecret($uid, array $attributes) {


Suggested change

private function getUserSecret($uid, array $attributes) {

private function getUserSecret(array $attributes): ?string {

blizzz · 2023-11-03T17:42:52Z

lib/UserBackend.php

 		}
 	}

+	private function getUserSecretHash($uid) {


Suggested change

private function getUserSecretHash($uid) {

private function getUserSecretHashes($uid): array {

may return up to ten, should be reflected in the name. Is 10 an arbitrary number, or where does it come from?

blizzz · 2023-11-03T17:44:41Z

lib/UserBackend.php

+		return $data;
+	}
+
+	private function checkUserSecretHash($uid, $userSecret) {


Suggested change

private function checkUserSecretHash($uid, $userSecret) {

private function checkUserSecretHash($uid, $userSecret): bool {

blizzz · 2023-11-03T17:46:08Z

lib/UserBackend.php

+		$data = $this->getUserSecretHash($uid);
+		foreach($data as $row) {
+			$storedHash = $row['token'];
+			if (\OC::$server->getHasher()->verify($userSecret, $storedHash, $newHash)) {


please use ' \OCP\Server::get(OCP\Security\IHasher::class)over deprecated and private\OC::$server->getHasher()`. Also on in the other places.

blizzz · 2023-11-03T17:46:59Z

lib/UserBackend.php

+					'name' => $qb->createNamedParameter('sso_secret_hash'),
+				]);
+		}
+		return $qb->execute();


Suggested change

return $qb->execute();

return $qb->executeStatement() > 0;

blizzz · 2023-11-03T17:47:34Z

lib/UserBackend.php

+		return false;
+	}
+
+	private function updateUserSecretHash($uid, $userSecret, $exists = false) {


Suggested change

private function updateUserSecretHash($uid, $userSecret, $exists = false) {

private function updateUserSecretHash($uid, $userSecret, $exists = false): bool {

blizzz · 2023-11-03T17:51:31Z

lib/UserBackend.php

+			if ($userSecret !== null) {
+				$this->updateUserSecretHash($uid, $userSecret);
+				// Emit a post login action to initialize the encryption module with the user secret provided by the idp.
+				\OC_Hook::emit('OC_User', 'post_login', ['run' => true, 'uid' => $uid, 'password' => $userSecret, 'isTokenLogin' => false]);


please dispatch the typed event \OCP\User\Events\PostLoginEvent instead of deprecated OC_Hook::emit(…).

blizzz · 2023-11-03T17:53:53Z

lib/UserBackend.php

+			$storedHash = $row['token'];
+			if (\OC::$server->getHasher()->verify($userSecret, $storedHash, $newHash)) {
+				if (!empty($newHash)) {
+					$this->updateUserSecretHash($uid, $userSecret, true);


mh, the function is said to check the hash, not update it.

blizzz · 2025-05-02T09:02:44Z

@summersab @immerda is this still a thing?

summersab · 2025-05-03T00:18:40Z

I mean... it should have been. I had to shift away from my NC plans, but this is a feature that provides some advanced encryption capabilities. I wish it would have been merged when I was more active with the project to be honest.

blizzz · 2025-05-06T15:26:19Z

Understandably 😓

summersab force-pushed the IProvideUserSecretBackend branch 2 times, most recently from 2835706 to cd0c669 Compare February 10, 2023 20:18

summersab marked this pull request as draft July 11, 2023 13:54

summersab force-pushed the IProvideUserSecretBackend branch from cd0c669 to d5e4825 Compare July 11, 2023 14:24

summersab marked this pull request as ready for review July 11, 2023 14:25

blizzz added enhancement 3. to review labels Jul 14, 2023

summersab force-pushed the IProvideUserSecretBackend branch from d5e4825 to 17b1210 Compare August 24, 2023 16:42

summersab requested review from blizzz and juliusknorr as code owners August 24, 2023 16:42

summersab force-pushed the IProvideUserSecretBackend branch 2 times, most recently from becfef3 to f5de408 Compare August 24, 2023 16:52

summersab force-pushed the IProvideUserSecretBackend branch from f5de408 to 128a9d3 Compare September 3, 2023 18:24

blizzz force-pushed the IProvideUserSecretBackend branch from 128a9d3 to 6e1548a Compare November 3, 2023 17:35

blizzz requested changes Nov 3, 2023

View reviewed changes

summersab mentioned this pull request Sep 2, 2025

Provide mechanism to override libraries with custom/patched versions nextcloud/server#54825

Open

	private function getUserSecret($uid, array $attributes) {
	private function getUserSecret(array $attributes): ?string {

	private function getUserSecretHash($uid) {
	private function getUserSecretHashes($uid): array {

	private function checkUserSecretHash($uid, $userSecret) {
	private function checkUserSecretHash($uid, $userSecret): bool {

	private function updateUserSecretHash($uid, $userSecret, $exists = false) {
	private function updateUserSecretHash($uid, $userSecret, $exists = false): bool {

Conversation

summersab commented Feb 10, 2023 • edited by joshtrichards Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

summersab commented Feb 24, 2023

Uh oh!

blizzz commented May 12, 2023

Uh oh!

immerda commented May 20, 2023

Uh oh!

mglants commented Nov 2, 2023

Uh oh!

blizzz left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

blizzz commented May 2, 2025

Uh oh!

summersab commented May 3, 2025

Uh oh!

blizzz commented May 6, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

summersab commented Feb 10, 2023 •

edited by joshtrichards

Loading