search

inveniosoftware / invenio-oauthclient

Invenio module that provides OAuth web authorization support.
https://invenio-oauthclient.readthedocs.io
MIT License
6 stars 76 forks source link

cern: allow users based on 'IdentityClass' #181

Closed zzacharo closed 5 years ago

zzacharo commented 5 years ago

closes #178 .

zzacharo commented 5 years ago

Lightweight accounts do not have a uidNumber so we get a 500 error. I use instead the PersonID field as they have a negative integer as an id in that case which can be used for our purposes. Do you see something that can go wrong with this solution?

lnielsen commented 5 years ago

The external id is treated as a string so probably not an issue as long as the two doesn't conflict. Did you try it out?

ntarocco commented 5 years ago

@lnielsen it appears that all of them are treated as string, no? I would go for string to be safe, and then let who is using that value do the parsing and eventually fail in a controlled manner.

@zzacharo I would also add, if the @lnielsen agrees, a new config variable for cern contrib, something like OAUTHCLIENT_CERN_ALLOWED_IDENTITY_CLASS = ['CERN Registered', 'CERN Shared'] (primary and secondary accounts, and probably also add service accounts by default, but I don't know the identity class for that) and fail login if not in the list. With this, we can block lightweight account if needed (probably my most of the websites). Probably you need to throw an exception in account_info, to be checked.

lnielsen commented 5 years ago

What was the conclusion in the end - that we close or integrate this PR?

ntarocco commented 5 years ago

There was no further discussion I guess... Integrate? @zzacharo?

zzacharo commented 5 years ago

@lnielsen @ntarocco @pamfilos I upgraded the PR with a new solution. We enable the cern accounts that belong to a specific IdentityClass so in that way be default we disable the lightweight accounts. Please review again.

zzacharo commented 5 years ago

@tiborsimko can you please check this PR so we can merge it if you don't see any problem?