conwetlab / ckanext-oauth2

OAuth2 support for CKAN
GNU Affero General Public License v3.0
25 stars 56 forks source link

CKAN WSO2 Login - changing email in CKAN user doesn't work #41

Closed marcyborg closed 3 years ago

marcyborg commented 3 years ago

Hi, I'm using CKAN 2.6.8 https://github.com/italia/ckan-it and WSO2 IDM, the plugin works correctly installing in Dockerfile via pip install. I have _CKAN_PROFILE_API_USERFIELD env variable enhanced with name attribute, which corresponds to CKAN username

In oauth2.py class, in the method _user_json(self, userdata), I have noticed this instruction user = None users = model.User.by_email(email)

If I try to change email in an existing CKAN user, the login doesn't work because CKAN returns this error: username Integrity Violation Error - username already exists

Looking at CKAN source code, I have found a method get of user.pyclass, https://github.com/ckan/ckan/blob/master/ckan/model/user.py

If i change instruction at line 180 of oauth2.py class, users = model.User.by_email(email), with this one users = model.User.get(user_name) can I resolve this kind of issue? Otherwise can you suggest me alternatives?

Thanks.

LukusV commented 3 years ago

Same issue for me

aitormagan commented 3 years ago

The main issue here is that some IdP allows users to change their username. If that happens and you apply the suggested patch, this kind of users will loose their accounts.

I don’t get what you are referring when you say “I try to change email in an existing CKAN user”.

I’ll be please to help you if I get the whole context :)

aitormagan commented 3 years ago

I think I've understood your problem. The problem arises when you change the user's email in WSO2 IDM and you try to log in again in CKAN, right? The extension is thought taking into account that the email is a field that will never change, but if the IDM offers that option, problems as the one you describe can arise.

However, if you are pretty sure that the user_name in your IDM cannot be changed, I think the solution you describe can be applied. However, if the user name can be changed, take into account that once the user has changed their user name, they will lose access to their CKAN account. Moreover, another "malicious" user can pick that user name and take control of this account in the CKAN instance.

marcyborg commented 3 years ago

Hi @aitormagan, thanks very much for your quick answer.

Yes, the problem is exactly what you described in your second comment: through WSO2 I have to handle login from LDAP in CKAN, so the mapping of _CKAN_PROFILE_API_USERFIELD is made through username.

I try to change email in a CKAN user, through CKAN administration panel , if I try to login again that error appears; the same issue will appear changing also email to an existing user in WSO2 administration panel.

I am pretty sure that user cannot be changed, because of functional requirements, so I have thought to that solution about Ckan native methods.

If I want to make this change, how can I manage this? I installed plugin via Dockerfile with this command pip install ckanext-oauth2; how can I handle a modified version of your plugin in this installation phase?

Thanks very much for your support.

aitormagan commented 3 years ago

But, there's something I don't get, why are you trying to change the user's email through the CKAN administration panel? That change must be done in both sides: CKAN and WSO2 so user don't loose their account.

Anyway, you can install the plugin manually. My approach will be the following:

  1. Clone this repo
  2. Change the code you need to change
  3. Change the reference in the docker file from ckanext-oauth2 to git+https://[THE_GITHUB_PATH_WHERE_YOUR_CLONE_IS_TORED]. (pip install git+https://...)

Note: I don't remember if the git+https:// part is requierd or you can use just https://.

marcyborg commented 3 years ago

@aitormagan you're right and I agree with you, but unfortunately functional requirements that I have to handle are like that one. For this reason I'm wondering about that, and I have forked the repo in my account and changed oauth2.py class, committing that CKAN native method variation. ASAP I'm going to check your installation instructions and I'll be back soon with a feedback.

marcyborg commented 3 years ago

@aitormagan it works! I've used this command RUN pip install git+https://github.com/marcyborg/ckanext-oauth2 in Dockerfile.