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

Auto created users are missing email when using 'email' as USERNAME_FIELD and are not able to login afterwards #244

Open
73VW opened this issue Jan 30, 2024 · 10 comments · May be fixed by #245
Labels
work-in-progress Stale action

Comments

@73VW
Copy link

73VW commented Jan 30, 2024

Hello there,

I ran into a problem using the package with the following configuration:

  • Custom user model
  • USERNAME_FIELD set as 'email'

When a user does not exist, he is correctly created but after logout and logging back in, he is not found anymore. The packages tries to create it but as he already exists with the same username, it crashes.

The mail is never set in the create_new_user method even if USERNAME_FIELD is set as email. Check here

I have been able to patch this behaviour using the following TRIGGER.CREATE_USER :

def post_create_user(user_dict, *args, **kwargs):
    user_model = get_user_model()
    user_id = user_dict.get(user_model.USERNAME_FIELD)
    user = user_model.objects.get(username=user_id)
    user.email = user_id
    user.save()
@73VW 73VW linked a pull request Jan 30, 2024 that will close this issue
Copy link

github-actions bot commented Mar 1, 2024

This issue is stale because it has been open 30 days with no activity. Remove stale label or comment or this will be closed in 5 days.

@github-actions github-actions bot added the no-issue-activity Stale action label Mar 1, 2024
Copy link

github-actions bot commented Mar 7, 2024

This issue was closed because it has been stalled for 5 days with no activity.

@github-actions github-actions bot closed this as not planned Won't fix, can't repro, duplicate, stale Mar 7, 2024
@mostafa mostafa linked a pull request Mar 26, 2024 that will close this issue
@mostafa mostafa reopened this Mar 26, 2024
@mostafa mostafa removed the no-issue-activity Stale action label Mar 26, 2024
@mostafa
Copy link
Member

mostafa commented Mar 26, 2024

Hey @73VW,

The reason is that I presumed that Django would be set to use the email field as the auth user ID. I see that this is a useful change, so I'll merge it and will release a version at some point.

@dejonestheadmin
Copy link

in the users.py file I was able to get my application to put both the username and email field correctly. I am using mailman with django so idk if this will particular pertain directly to anyone else situation. The part with the asterisks is where I want to bring attention to I added that to the file and on top of that I added what the OP put in a hook method with one tweak please see below. The combination of these two things is what is working for me currently. In case someone else has this issue I want to make sure I clearly explain.

In the user.py file find the section with the def seen below at the end of that def include what the asterisk are around the og code just reads as return "user_id.lower() if user_id else None"

Once you've made that change then just create a .py file in the django_saml2_auth path insert the code below the def post create user. save it

make sure its called correctly in your settings.py file and then it should work. i havent seen any consquences on my end yet for this change.. @mostafa wants i can post the entire file but again i only made those two changes..

def get_user_id(user: Union[str, Dict[str, Any]]) -> Optional[str]:
"""Get user_id (username or email) from user object

Args:
    user (Union[str, Dict[str, Any]]): A cleaned user info object

Returns:
    Optional[str]: user_id, which is either email or username
"""
user_model = get_user_model()
user_id = None

if isinstance(user, dict):
    user_id = user["email"] if user_model.USERNAME_FIELD == "email" else user["username"]

if isinstance(user, str):
    user_id = user

****return user_id if user_id and '@' in user_id else None****

def post_create_user(user_dict, *args, **kwargs):
user_model = get_user_model()
user_id = user_dict.get(user_model.USERNAME_FIELD)
user = user_model.objects.get(username=user_id)
user.email = user_id
user.username = user_id.split('@')[0]
user.save()

@dejonestheadmin

This comment was marked as spam.

@mostafa
Copy link
Member

mostafa commented Apr 25, 2024

Hey @dejonestheadmin,

Please do not post the entire code base in a comment, instead create a fork and change that or use GitHub Gists to show the changes.

@dejonestheadmin
Copy link

@mostafa my apologies for the mistake I will try to make the changes the appropriate way

@dejonestheadmin
Copy link

@mostafa I got it figured out 100% finally Ill update code asap most likely send what I have tonight after work

@dejonestheadmin
Copy link

dejonestheadmin commented Apr 26, 2024

Okay after testing with multiple users I have a lot of confidence that this works just as expected I will post the entire code or make the changes where deemed appropriate but I believe and please correct if I am wrong but I can post at least a snippet of the areas that need changes
ALL CHANGES ARE ONLY MADE IN THE user.py file.
Change #1 can be found in 'def create_new_user'
user = user_model.objects.create_user(email.split('@')[0], **kwargs)
user.is_active = is_active
user.is_staff = is_staff
user.is_superuser = is_superuser
user.email = email
user.save()

Change #2 can be found in 'def get_user'
return user_model.objects.get(**{id_field: user_id.split('@')[0]})

Ill be honest again im very very new to messing with any of these technologies but for me this did solve the problem. I will put the full code in the appropriate locations. Apologies again if this type of info doesnt belong in this section I am just overly excited that I could finally figure something like this out .. although i know its small compared to the things other brilliant minds that have helped here as well.

This also should allow the username to be created without the @domain.com and let the email address remain as is

Copy link

This issue is stale because it has been open 30 days with no activity. Remove stale label or comment or this will be closed in 5 days.

@github-actions github-actions bot added the no-issue-activity Stale action label May 27, 2024
@mostafa mostafa added work-in-progress Stale action and removed no-issue-activity Stale action labels May 27, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
work-in-progress Stale action
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants