search

inveniosoftware / invenio-oauthclient

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

Login/logout should know the method used #228

Closed ntarocco closed 3 years ago

ntarocco commented 3 years ago

Currently, CERN contribs (and probably others) relies on signals to perform login/logout operations. Unfortunately, all the contribs are "registered" on app init and they will all receive signals.

Problem

On login/logout, each contribs receive the signal and perform operations. The correct behaviour would be that only the contrib used to login should receive such signals. This is also creating an issue on logout: given that invenio-oauthclient is not aware of which login method was used, the logout signal is emitted to all contribs and they all perform logout operations, causing issues in case of redirections.

Possible solution

The solution could be to use DB information or add any extra information to store which was the authentication method used by the user. This could allow to stop firing signals to all contribs and emit only to the contrib used on login.

GraemeWatt commented 3 years ago

For the HEPData service (hepdata.net) we allow login with either CERN, ORCID, or local password, using the invenio-oauthclient package. We've had an issue (HEPData/hepdata#106) for the last five years where if a user has previously connected to their CERN account, but uses one of the other methods (ORCID or local password) to log in, they get an exception KeyError: 'Group' from this line of cern.py. We had hoped that this issue would be fixed in recent versions of invenio-oauthclient or when we moved to the new CERN SSO (merged PR HEPData/hepdata#343, but not yet deployed). But we still get a similar exception KeyError: 'cern_roles' from this line of cern_openid.py. These exceptions have generated complaints from HEPData users multiple times over the last five years. Would you be able to prioritise fixing this issue for a future invenio-oauthclient release?

ntarocco commented 3 years ago

@GraemeWatt thanks for reporting this. If you are moving to the more recent cern_openid then I would focus on that, and not on the old cern.py. cern_roles should be there by default at CERN, I am surprised it is not the case. Do you have access to the CERN authentication documentation? I am available to discuss this in the Discord chat.

GraemeWatt commented 3 years ago

Thanks for the quick response, @ntarocco. Yes, I can see these docs. To clarify, the problem is not with CERN login itself. It's with local (or ORCID) login where the CERN contrib still gets a signal according to this issue. @alisonrclarke has investigated the problem in more detail. We can discuss further in the inveniosoftware Discord chat if you prefer.

ntarocco commented 3 years ago

It is clear now, thank you. I might have a solution (not ideal...), I will test it.

ntarocco commented 3 years ago

Thanks for the quick response, @ntarocco. Yes, I can see these docs. To clarify, the problem is not with CERN login itself. It's with local (or ORCID) login where the CERN contrib still gets a signal according to this issue. @alisonrclarke has investigated the problem in more detail. We can discuss further in the inveniosoftware Discord chat if you prefer.

See my PR: https://github.com/inveniosoftware/invenio-oauthclient/pull/254

ntarocco commented 3 years ago

@GraemeWatt v1.5.1 released :)