-
Notifications
You must be signed in to change notification settings - Fork 81
add displayname #61
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
add displayname #61
Conversation
lib/Controller/SAMLController.php
Outdated
| */ | ||
| private function autoprovisionIfPossible(array $auth) { | ||
| $uidMapping = $this->config->getAppValue('user_saml', 'general-uid_mapping'); | ||
| $displayName = $auth['urn:oid:2.5.4.42'][0] . ' ' . $auth['urn:oid:1.2.40.0.10.2.1.1.261.20'][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.
From my understanding those should be configurable values – right?
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.
good point.
Two values like displayname.firstpart, displayname.secondpart to leave the admin a choice if first,last or last,first?
| private function autoprovisionIfPossible(array $auth) { | ||
| $uidMapping = $this->config->getAppValue('user_saml', 'general-uid_mapping'); | ||
| $displaynamefirstpartMapping = $this->config->getAppValue('user_saml','general-displaynamefirstpart_mapping'); | ||
| $displaynamesecondpartMapping = $this->config->getAppValue('user_saml','general-displaynamesecondpart_mapping'); |
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.
I personally would prefer to not separate between first name and second name here. Rather just a single input field that can take multiple fields. (such as {{fieldBar}} {{fieldFoo}})
If you can give me commit access to your fork I can try to come up with something like that later.
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.
I gave you commit Access (i hope...)
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.
Yes. I just finished the automated integration test suite for Shibboleth. I'll see if I can find some time to dig into this here. :) – But I'm pretty busy at the moment with a lot of other stuff…
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.
btw: already got mailadress working, not ready for commit yet....
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.
Any update on this mailaddress item? I am storing the mail address(preferences database, as email configkey for given uid, like LDAP is doing this too), but it is not shown in the users personal page. Also, i have configured the displayname and its shown in the upper right side of the WebUI, but also not on the personal page. Somewhat strange.
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.
Displayname works for me everywhere: upper right side, personal page, search field for sharing items.
regarding mailaddress: i have not found time to implement this better.
My current solution is: writing the email the moment the user gets created, no update, no changes:
SAMLController.php
$mailMapping = "urn:oid:0.9.2342.19200300.100.1.3";
$mail='';
if(isset($auth[$mailMapping])) {
if(is_array($auth[$mailMapping])) {
$mail = $auth[$mailMapping][0];
} else {
$mail = $auth[$mailMapping];
}
}`
if(!$userExists && !$autoProvisioningAllowed) {
throw new NoUserFoundException();
} elseif(!$userExists && $autoProvisioningAllowed) {
$this->userBackend->createUserIfNotExists($uid,$displayName);
$user = $this->userManager->get($uid);
error_log("set email");
if (!is_null($user)) {
$user->setEMailAddress($mail);
}
error_log("set email");
return;
}
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.
Thank you very much for your response!
|
Hi, Any hints? |
|
@bne86 From the look at your code I'd say this is because your backend doesn't define https://github.com/nextcloud/user_saml/pull/61/files#diff-eec818f31669ac6d6b4d1161cc38794bR113 |
|
Thanks for the pointer, now i am getting the Log output and a error with the query that i can fix. |
|
As discussed out-of-band with @bne86 I'll look into getting this into shape and merged into master soonish. |
|
Let's close this in favour of #90 where I also added integration tests. @rzmf @bne86 Can I ask you to give it a test? Any feedback appreciated. Note that you also require nextcloud/server#3507 |
No description provided.