Skip to content

Support several required fields during User creation #95

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

Merged
merged 1 commit into from
Apr 29, 2020
Merged

Support several required fields during User creation #95

merged 1 commit into from
Apr 29, 2020

Conversation

francoisfreitag
Copy link
Contributor

Custom User models may have several required attributes, for example a
username and an email. If create is called directly based on
DJANGO_USER_MAIN_ATTRIBUTE, the NOT NULL constraint will be violated,
preventing to save that user. Instead, rely on update_user to call
save when most attributes have been defined on the user.

A nice side effect is that a database query might be saved, because the
INSERT query for new users could contain all attributes when save() is
called from update_user, while a first query (INSERT) was issued to
create the user, then update user would call save(), issuing an UPDATE
query.

@derchrisuk
Copy link

Hi,

can you check the PR against the current 0.17.1 version?
I tried to implement just the changes in the backend.py file, but it fails

AttributeError at /saml2/acs/

'function' object has no attribute 'DoesNotExist'

Here are some details:

Django Version: |1.11.15
Exception Type:|AttributeError
Exception Value:|'function' object has no attribute 'DoesNotExist'
Exception Location:|/usr/local/lib/python2.7/site-packages/djangosaml2/backends.py in _get_or_create_saml2_user, line 166
Python Executable:|/usr/local/bin/python
Python Version:|2.7.15

I have to write some more attributes into the django user table, and this would help me a lot.

Thanks,
chris

@francoisfreitag
Copy link
Contributor Author

francoisfreitag commented Aug 8, 2018

I'm surprised by the issue, User is supposed to be a class (subclass of django.contrib.auth.models.AbstractUser), not a function.

What is your SAML_USER_MODEL setting set to?

I may be able to help you further if you can post a failing test case or a minimal django project that fails?

Edit: Link to AbstractUser, not User.

@peppelinux
Copy link
Member

peppelinux commented Apr 28, 2020

Hi, I think that this should be merged soon for the next release, v0.18.2.
Can you refactor your code using get_user_model() instead of User?

This Will let us to use djangosaml2 with a custom user app if you agree.

@peppelinux peppelinux added this to the v0.18.2 milestone Apr 29, 2020
@francoisfreitag
Copy link
Contributor Author

Can you precise your request?
User = get_saml_user_model() ensures settings.SAML_USER_MODEL is used when it is defined, otherwise defaults to Django’s auth.get_user_model().

I rebased the PR on master.

Custom User models may have several required attributes, for example a
username and an email. If create is called directly based on
DJANGO_USER_MAIN_ATTRIBUTE, the NOT NULL constraint will be violated,
preventing to save that user. Instead, rely on update_user to call
save when most attributes have been defined on the user.

A nice side effect is that a database query might be saved, because the
INSERT query for new users could contain all attributes when save() is
called from update_user, while a first query (INSERT) was issued to
create the user, then update user would call save(), issuing an UPDATE
query.
@peppelinux
Copy link
Member

Can you precise your request?
User = get_saml_user_model() ensures settings.SAML_USER_MODEL is used when it is defined, otherwise defaults to Django’s auth.get_user_model().

I rebased the PR on master.

Yes, you got it, that's it

@peppelinux peppelinux merged commit 521089e into IdentityPython:master Apr 29, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants