-
Notifications
You must be signed in to change notification settings - Fork 14
Description
See comment by @pbiering here: #55 (comment)
See comment by @bimimicah here: #55 (comment)
What I was trying to say is that our current code (and this PR) just skips the domain (everything after the '@'), which could potentially be a security hole if the system is linked to multiple domains which both have a user with the same name.
I was wondering if the newer functions (such as
getgrouplist) now support specifying domains (e.g. '[email protected]' or 'domain.com\user'), so maybe we can stop skipping the domain?But that is beyond the scope of this PR, so we can take a look at it sometime down the road. I think this PR looks good. If there are no comments by next week, I will merge it and make a new release.
I see it also dangerous to strip the domain by default.
Either add a particular option to strip a particular domain or skip users at all having domain inside by default.
- Test whether
getgrouplist()supports domain syntax (e.g. 'user@domain' or 'domain\user') - if supported, remove domain stripping code
- if not supported, develop a method to specify domain(s) that are accepted and can be stripped