Skip to content
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

Dont reset displayname to uid on sso login #29981

Merged
merged 1 commit into from
Jan 8, 2018
Merged

Conversation

tomneedham
Copy link
Contributor

Description

If you login with SSO your displayname is set to your uid. Metadata sync is handled at the moment via shibboleth but will eventually be handled by the sync service on login. This is not necessary.

Related Issue

Motivation and Context

Breaks sso dispalynames

How Has This Been Tested?

  • setup ldap user backend with a server (use a custom internal user id)
  • use a custom displayname attribute
  • sync the users
  • displaynamse are correct
  • setup shibboleth login
  • login using sso
  • see that dispalyname is still correct and is no longer converted to your uid

Screenshots (if appropriate):

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)

Checklist:

  • My code follows the code style of this project.
  • My change requires a change to the documentation.
  • I have updated the documentation accordingly.
  • I have read the CONTRIBUTING document.
  • I have added tests to cover my changes.
  • All new and existing tests passed.

@tomneedham tomneedham added this to the development milestone Dec 27, 2017
@tomneedham tomneedham self-assigned this Dec 27, 2017
@tomneedham tomneedham changed the title Dont reset dispalyname to uid on sso login Dont reset displayname to uid on sso login Dec 27, 2017
@codecov
Copy link

codecov bot commented Dec 27, 2017

Codecov Report

Merging #29981 into master will increase coverage by <.01%.
The diff coverage is n/a.

Impacted file tree graph

@@             Coverage Diff              @@
##             master   #29981      +/-   ##
============================================
+ Coverage     58.21%   58.22%   +<.01%     
  Complexity    18508    18508              
============================================
  Files          1093     1093              
  Lines         63693    63692       -1     
============================================
  Hits          37082    37082              
+ Misses        26611    26610       -1
Impacted Files Coverage Δ Complexity Δ
drone/src/lib/private/legacy/user.php 20.61% <0%> (+0.1%) 75% <0%> (ø) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update de71a31...10e2bce. Read the comment docs.

Copy link
Member

@butonic butonic left a comment

Choose a reason for hiding this comment

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

yes, makes sense. if no displayname is set the fallback is the userid anyway

@lock
Copy link

lock bot commented Aug 1, 2019

This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.

@lock lock bot locked as resolved and limited conversation to collaborators Aug 1, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants