Skip to content

Add test for ensuring keeping of displayname #137

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

Jump to bottom
Merged
LukasReschke merged 5 commits into from
Jul 31, 2017
Merged

Add test for ensuring keeping of displayname #137

LukasReschke merged 5 commits into from
Jul 31, 2017

Conversation

@LukasReschke
Copy link
Member

@LukasReschke LukasReschke commented Jul 31, 2017 •
edited
Loading

There seems to be a bug here, let's try to track it down with integration tests.

Fixes #119, fixes #125

Signed-off-by: Lukas Reschke [email protected]

@LukasReschke
Copy link
Member Author

LukasReschke commented Jul 31, 2017 •
edited
Loading 

# FeatureContext::thUserValueShouldBe()
And The user value "display-name" should be "Default displayname of student1" 
# FeatureContext::thUserValueShouldBe()
Expected Default displayname of student1 as value but got student1 (UnexpectedValueException)

@codecov
Copy link

codecov bot commented Jul 31, 2017 •
edited
Loading

Codecov Report

Merging #137 into master will increase coverage by 0.06%.
The diff coverage is 0%.

Impacted file tree graph

@@             Coverage Diff              @@
##             master     #137      +/-   ##
============================================
+ Coverage     33.67%   33.74%   +0.06%     
+ Complexity      123      121       -2     
============================================
  Files            10       10              
  Lines           490      489       -1     
============================================
  Hits            165      165              
+ Misses          325      324       -1
Impacted Files Coverage Δ Complexity Δ
lib/UserBackend.php 4.34% <0%> (ø) 59 <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 740ebb4...4691870. Read the comment docs.

This was referenced Jul 31, 2017
Closed
Closed
LukasReschke added a commit to nextcloud/server that referenced this pull request Jul 31, 2017
@LukasReschke
Remove setUidAsDisplayName
bef4f01 
This code is a leftover from the old Shibboleth implementation from ownCloud days, nowadays it causes more issues than it does good (such as nextcloud/user_saml#137). Let's remove it for the future thus.

Signed-off-by: Lukas Reschke <[email protected]>
@LukasReschke LukasReschke mentioned this pull request Jul 31, 2017
Merged
@LukasReschke
Add test for ensuring keeping of displayname
d187271 
Signed-off-by: Lukas Reschke <[email protected]>
@LukasReschke
Claim to always support \OC\User\Backend::GET_DISPLAYNAME
034e2d7 
`\OC_User::loginWithApache` sets the display name otherwise to the login name which is wrong.

Signed-off-by: Lukas Reschke <[email protected]>
@LukasReschke
Follow PSR file naming
d9245ae 
Signed-off-by: Lukas Reschke <[email protected]>
@LukasReschke
Backend also supports setting a displayname
a5a406f 
Signed-off-by: Lukas Reschke <[email protected]>
@LukasReschke LukasReschke force-pushed the add-test-for-testing-displayname-2 branch from c13c09e to a5a406f Compare July 31, 2017 14:31
@LukasReschke
Don't advertise ability to change display name
4691870 
Otherwise users can change their display name and email address which is in a SAML scenario undesired.

Signed-off-by: Lukas Reschke <[email protected]>
@LukasReschke LukasReschke changed the title [WIP] Add test for ensuring keeping of displayname Add test for ensuring keeping of displayname Jul 31, 2017
@LukasReschke LukasReschke merged commit 688c32a into master Jul 31, 2017
@LukasReschke LukasReschke deleted the add-test-for-testing-displayname-2 branch July 31, 2017 15:17
@LukasReschke
Copy link
Member Author

LukasReschke commented Jul 31, 2017

Sent to owner of affected instance and awaiting feedback.

@MorrisJobke
Copy link
Member

MorrisJobke commented Jul 31, 2017

This array index reminds me of another PR or was this in user_saml? 🤔

@MorrisJobke
Copy link
Member

MorrisJobke commented Aug 1, 2017

This array index reminds me of another PR or was this in user_saml? 🤔

https://github.com/nextcloud/user_saml/pull/132/files?w=1

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

4. to release

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

SAML provisioned users don't see "Full Name" after login Displayname / Full Name not displayed / searchable

3 participants

@LukasReschke @MorrisJobke