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

plone.api.get_roles(username=userid) breaks if username != userid #204

Open
fongtech opened this issue Dec 28, 2014 · 14 comments
Open

plone.api.get_roles(username=userid) breaks if username != userid #204

fongtech opened this issue Dec 28, 2014 · 14 comments
Milestone

Comments

@fongtech
Copy link

When use with collective.emaillogin4, userid can be different from username.

So the api should be
plone.api.user.get_roles(userid=None, user=None, obj=None, inherit=True)
not
plone.api.user.get_roles(username=None, user=None, obj=None, inherit=True)

@zupo
Copy link
Member

zupo commented Jan 5, 2015

IIUC, the proper way would be to update all api methods that use the "username" parameter to also take the "userid" parameter (in cases where username does not equal userid).

@adamcheasley
Copy link
Contributor

Yesterday @mattss and I discovered a potential bug in api.user.get that may indeed be related to the above issue. I will see if I can write some tests to expose this problem.

@jaroel
Copy link
Member

jaroel commented Feb 3, 2015

Different userids and usernames are really common.
People get married and get a different username (emailaddress or fullname). The userid usually stays the same.
Please do not assume username and userid are the same. They are not.

@cedricmessiant
Copy link

We also had a problem once with the difference between userid and username. It would be nice to be able to use either username or userid/user_id

@adamcheasley
Copy link
Contributor

Out current work around is to first use api.user.get to lookup the user, as this takes either username or userid, then pass the user object into api.user.get_roles

@jaroel
Copy link
Member

jaroel commented Jun 9, 2015

What needs to be done, and who is going to do it?

@adamcheasley
Copy link
Contributor

IMO, what needs to be done is we should change the signatures of every function (except possibly api.user.create) that accepts username to also accept userid, as @zupo suggests.
I should be able to look at this at some point.

@adamcheasley
Copy link
Contributor

Sorry, I've been too busy to look at this one recently. Anyone should feel free to take it on.

@jensens
Copy link
Member

jensens commented Jul 14, 2015

the whole naming is crap, usally we have in authorization systems a login and a userid, and thats the confusion.
BUT PAS itself uses the terms userid and username, where username is used for login.
https://github.com/zopefoundation/Products.PluggableAuthService/blob/master/Products/PluggableAuthService/interfaces/authservice.py#L30

THEN PAS uses the term login in its credentials, see
https://github.com/zopefoundation/Products.PluggableAuthService/blob/master/Products/PluggableAuthService/interfaces/plugins.py#L65 and
https://github.com/zopefoundation/Products.PluggableAuthService/blob/master/Products/PluggableAuthService/interfaces/plugins.py#L87

So I would prefer to unify naming in Plone to

  • userid as the thing that never changes
  • login as the thing one uses to type into the login field.

@jaroel
Copy link
Member

jaroel commented Jul 14, 2015

YES! Awesome! 💃

@pbauer
Copy link
Member

pbauer commented Jul 14, 2015

👍

@jaroel
Copy link
Member

jaroel commented Jul 19, 2015

Any takers? I would really love to see this resolved before shipping plone.api with Plone 5.

@jaroel
Copy link
Member

jaroel commented Jul 20, 2015

@jaroel
Copy link
Member

jaroel commented Aug 7, 2015

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

No branches or pull requests

7 participants