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

fix: Implement IGetDisplayNameBackend #771

Merged
merged 1 commit into from
Oct 10, 2023

Conversation

juliusknorr
Copy link
Member

@juliusknorr juliusknorr commented Oct 10, 2023

@juliusknorr juliusknorr requested a review from blizzz October 10, 2023 06:20
@juliusknorr juliusknorr force-pushed the bugfix/noid/user-backend-displayname-interface branch from 86ece90 to 253d71a Compare October 10, 2023 06:22
@@ -328,7 +331,7 @@ public function getDisplayName($uid) {
}
}

return false;
return $uid;
Copy link
Member

Choose a reason for hiding this comment

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

did you check internal usage of this method? are we somewhere expecting or acting specifically to false?

Copy link
Member Author

Choose a reason for hiding this comment

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

All other user backends implement it that way. The interface actually requires a string type and doesn't allow boolean values anymore

Copy link
Member

Choose a reason for hiding this comment

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

No doubt about this, my point was to double check we don't have odd internal behavior that relies on the false.

Copy link
Member

Choose a reason for hiding this comment

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

… but that is not the case ✔️

lib/UserBackend.php Outdated Show resolved Hide resolved
@blizzz blizzz force-pushed the bugfix/noid/user-backend-displayname-interface branch from efeef61 to 0caf43f Compare October 10, 2023 18:58
@blizzz blizzz force-pushed the bugfix/noid/user-backend-displayname-interface branch from 0caf43f to 91e6004 Compare October 10, 2023 19:02
@blizzz
Copy link
Member

blizzz commented Oct 10, 2023

Amended a fix to another static analysis finding https://github.com/nextcloud/user_saml/actions/runs/6473643774/job/17576817710?pr=771

@blizzz blizzz merged commit 991660f into master Oct 10, 2023
20 checks passed
@blizzz blizzz deleted the bugfix/noid/user-backend-displayname-interface branch October 10, 2023 19:11
@blizzz
Copy link
Member

blizzz commented Oct 10, 2023

@juliushaertl do we need a backport?

@juliusknorr
Copy link
Member Author

/backport to stable5.2

@juliusknorr
Copy link
Member Author

/backport to stable-5.2

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

Successfully merging this pull request may close these issues.

3 participants