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

Log warning when no uid was found for user #11499

Merged
merged 1 commit into from
Oct 10, 2014

Conversation

PVince81
Copy link
Contributor

@PVince81 PVince81 commented Oct 9, 2014

In some incomplete setups (like mine) it can happen that the uid
attribute of users is missing.

To be able to find out that something is wrong, a debug message is now
logged when it has not been found.

To test:

  1. Create a user but omit the "uid" attribute (I had "cn" set and did not know I had to set "uid" as well)
  2. Add that user in a LDAP group and make sure the group memberships are visible in OC
  3. Go to the admin page
  4. Exclude the user's group from sharing
  5. Refresh the page

Before this PR: no warnings message
After this PR: a warning message appears in the log (debug level)

Note that "exclude user group from sharing" does not work when uid is not set, but this is not ownCloud's fault.

@blizzz

In some incomplete setups (like mine) it can happen that the uid
attribute of users is missing.

To be able to find out that something is wrong, a debug message is now
logged when it has not been found.
@PVince81
Copy link
Contributor Author

PVince81 commented Oct 9, 2014

@blizzz can you confirm that it is expected that every user has a "uid" attribute and no fallback to "cn" is required ?

@blizzz
Copy link
Contributor

blizzz commented Oct 9, 2014

Depends on LDAP server, uid is most common, samaccoutname it is on AD.

In this context, AD will not fall into this path and i did find an LDAP server yet which does not incorporate uid attribute. So this is pretty safe to assume here, also because it is specified in an RFC for login uses.

@PVince81
Copy link
Contributor Author

PVince81 commented Oct 9, 2014

Ok, good to know. Then this PR will help people like me who incorrectly setup their LDAP users.

@blizzz
Copy link
Contributor

blizzz commented Oct 9, 2014

Yes. 👍

@ghost
Copy link

ghost commented Oct 9, 2014

🚀 Test PASSed. 🚀
Refer to this link for build results (access rights to CI server needed):
https://ci.owncloud.org//job/pull-request-analyser-ng/183/:rocket: Test PASSed. 🚀

@scrutinizer-notifier
Copy link

The inspection completed: 1 new issues

@LukasReschke
Copy link
Member

👍

LukasReschke added a commit that referenced this pull request Oct 10, 2014
Log warning when no uid was found for user
@LukasReschke LukasReschke merged commit 4a4ea1d into master Oct 10, 2014
@LukasReschke LukasReschke deleted the ldap-warningwhenuidismissing branch October 10, 2014 10:42
@PVince81
Copy link
Contributor Author

@karlitschek backport to stable7 ? can be useful when troubleshooting LDAP issues

@karlitschek
Copy link
Contributor

Yes. Backport

@PVince81
Copy link
Contributor Author

stable7: b297ddf

@lock lock bot locked as resolved and limited conversation to collaborators Aug 15, 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.

5 participants