From d187271f8154b3f313847c625ba9bfecae9581c7 Mon Sep 17 00:00:00 2001 From: Lukas Reschke Date: Mon, 31 Jul 2017 11:43:23 +0200 Subject: [PATCH 1/5] Add test for ensuring keeping of displayname Signed-off-by: Lukas Reschke --- tests/integration/features/Shibboleth.feature | 1 + tests/integration/features/bootstrap/FeatureContext.php | 2 +- 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/tests/integration/features/Shibboleth.feature b/tests/integration/features/Shibboleth.feature index 4d2a047c9..94a7990e0 100644 --- a/tests/integration/features/Shibboleth.feature +++ b/tests/integration/features/Shibboleth.feature @@ -63,6 +63,7 @@ Feature: Shibboleth And I should be redirected to "http://localhost/index.php/apps/files/" Then The user value "id" should be "student1" Then The user value "email" should be "" + And The user value "display-name" should be "Default displayname of student1" Scenario: Authenticating using Shibboleth with SAML in provisioning mode and custom mapped attributes Given The setting "type" is set to "saml" diff --git a/tests/integration/features/bootstrap/FeatureContext.php b/tests/integration/features/bootstrap/FeatureContext.php index af7cc5195..fb0ff7241 100644 --- a/tests/integration/features/bootstrap/FeatureContext.php +++ b/tests/integration/features/bootstrap/FeatureContext.php @@ -226,7 +226,7 @@ public function thUserValueShouldBe($key, $value) { public function aLocalUserWithUidExists($uid) { shell_exec( sprintf( - 'sudo -u apache OC_PASS=password /opt/rh/rh-php56/root/usr/bin/php %s user:add %s --password-from-env', + 'sudo -u apache OC_PASS=password /opt/rh/rh-php56/root/usr/bin/php %s user:add %s --display-name "Default displayname of '.$uid.'" --password-from-env', __DIR__ . '/../../../../../../occ', $uid ) From 034e2d7c8a856ad6cadb0ad2e68289cfeacdf0d2 Mon Sep 17 00:00:00 2001 From: Lukas Reschke Date: Mon, 31 Jul 2017 15:44:34 +0200 Subject: [PATCH 2/5] Claim to always support \OC\User\Backend::GET_DISPLAYNAME `\OC_User::loginWithApache` sets the display name otherwise to the login name which is wrong. Signed-off-by: Lukas Reschke --- lib/userbackend.php | 11 +++-------- 1 file changed, 3 insertions(+), 8 deletions(-) diff --git a/lib/userbackend.php b/lib/userbackend.php index c7a0b15a5..d10e2a2f3 100644 --- a/lib/userbackend.php +++ b/lib/userbackend.php @@ -116,12 +116,7 @@ public function createUserIfNotExists($uid) { */ public function implementsActions($actions) { $availableActions = \OC\User\Backend::CHECK_PASSWORD; - if($this->autoprovisionAllowed() - && $this->config->getAppValue('user_saml', 'saml-attribute-mapping-displayName_mapping', '') !== '') { - - $availableActions |= \OC\User\Backend::GET_DISPLAYNAME; - } - + $availableActions |= \OC\User\Backend::GET_DISPLAYNAME; return (bool)($availableActions & $actions); } @@ -413,7 +408,7 @@ public function registerBackends(array $backends) { private function getAttributeValue($name, array $attributes) { $keys = explode(' ', $this->config->getAppValue('user_saml', $name, '')); - if(count($keys) === 1 && $keys[1] === '') { + if(count($keys) === 1 && $keys[0] === '') { throw new \InvalidArgumentException('Attribute is not configured'); } @@ -460,7 +455,7 @@ public function updateAttributes($uid, $currentDisplayname = (string)$this->getDisplayName($uid); if($newDisplayname !== null && $currentDisplayname !== $newDisplayname) { - $this->setDisplayName($uid, $newDisplayname); + $user->setDisplayName($newDisplayname); } } } From d9245ae533ef3190c6b2e0a2f8723f84e4fa0111 Mon Sep 17 00:00:00 2001 From: Lukas Reschke Date: Mon, 31 Jul 2017 15:45:56 +0200 Subject: [PATCH 3/5] Follow PSR file naming Signed-off-by: Lukas Reschke --- lib/{userbackend.php => UserBackend.php} | 0 1 file changed, 0 insertions(+), 0 deletions(-) rename lib/{userbackend.php => UserBackend.php} (100%) diff --git a/lib/userbackend.php b/lib/UserBackend.php similarity index 100% rename from lib/userbackend.php rename to lib/UserBackend.php From a5a406fb1fcbf8f7467d7acf13c76899b331da79 Mon Sep 17 00:00:00 2001 From: Lukas Reschke Date: Mon, 31 Jul 2017 16:17:58 +0200 Subject: [PATCH 4/5] Backend also supports setting a displayname Signed-off-by: Lukas Reschke --- lib/UserBackend.php | 1 + 1 file changed, 1 insertion(+) diff --git a/lib/UserBackend.php b/lib/UserBackend.php index d10e2a2f3..67e244cef 100644 --- a/lib/UserBackend.php +++ b/lib/UserBackend.php @@ -117,6 +117,7 @@ public function createUserIfNotExists($uid) { public function implementsActions($actions) { $availableActions = \OC\User\Backend::CHECK_PASSWORD; $availableActions |= \OC\User\Backend::GET_DISPLAYNAME; + $availableActions |= \OC\User\Backend::SET_DISPLAYNAME; return (bool)($availableActions & $actions); } From 4691870887e1af75772ffd4f686425cc76860b96 Mon Sep 17 00:00:00 2001 From: Lukas Reschke Date: Mon, 31 Jul 2017 16:58:51 +0200 Subject: [PATCH 5/5] Don't advertise ability to change display name Otherwise users can change their display name and email address which is in a SAML scenario undesired. Signed-off-by: Lukas Reschke --- lib/UserBackend.php | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/lib/UserBackend.php b/lib/UserBackend.php index 67e244cef..5d8e14b39 100644 --- a/lib/UserBackend.php +++ b/lib/UserBackend.php @@ -117,7 +117,6 @@ public function createUserIfNotExists($uid) { public function implementsActions($actions) { $availableActions = \OC\User\Backend::CHECK_PASSWORD; $availableActions |= \OC\User\Backend::GET_DISPLAYNAME; - $availableActions |= \OC\User\Backend::SET_DISPLAYNAME; return (bool)($availableActions & $actions); } @@ -456,7 +455,7 @@ public function updateAttributes($uid, $currentDisplayname = (string)$this->getDisplayName($uid); if($newDisplayname !== null && $currentDisplayname !== $newDisplayname) { - $user->setDisplayName($newDisplayname); + $this->setDisplayName($uid, $newDisplayname); } } }