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

User model mapping issue w/ get_user when User model uses "email" field as USERNAME_FIELD #7

Open
anyeone opened this issue Apr 30, 2018 · 4 comments

Comments

@anyeone
Copy link

anyeone commented Apr 30, 2018

Hi, in my user model I am using the email address field as the username, i.e. in my User model:
USERNAME_FIELD = 'email'

This means that my User has no 'username' property.

As such, when I call CognitoUser.get_user(), I am getting an error when it tries to reconstruct my User model from the database:

File "/Users/anye/bcve/lib/python3.5/site-packages/warrant/init.py", line 460, in get_user
metadata=user_metadata,attr_map=attr_map)
File "/Users/anye/bcve/lib/python3.5/site-packages/django_warrant/backend.py", line 37, in get_user_obj
defaults=user_attrs)
File "/Users/anye/bcve/lib/python3.5/site-packages/django/db/models/manager.py", line 85, in manager_method
return getattr(self.get_queryset(), name)(*args, **kwargs)
File "/Users/anye/bcve/lib/python3.5/site-packages/django/db/models/query.py", line 476, in update_or_create
lookup, params = self._extract_model_params(defaults, **kwargs)
File "/Users/anye/bcve/lib/python3.5/site-packages/django/db/models/query.py", line 534, in _extract_model_params
"', '".join(sorted(invalid_params)),
django.core.exceptions.FieldError: Invalid field name(s) for model User: 'username'.

I tried adding a mapping between username and email but that did not help the issue. My mapping looks like:
COGNITO_ATTR_MAPPING={
'email': 'email',
'given_name': 'first_name',
'family_name': 'last_name',
'username': 'email'
}

Update:
I think the problem is actually twofold; it happens either when trying to create unknown users or get the User from cognito:
backend.py

it's the CognitoUser.user_class.objects.update_or_create() and CognitoUser.user_class.objects.get(username=username) methods that are throwing the exception because it assumes there is a username field on the user_class.

def get_user_obj(self,username=None,attribute_list=[],metadata={},attr_map={}):
user_attrs = cognito_to_dict(attribute_list,CognitoUser.COGNITO_ATTR_MAPPING)
django_fields = [f.name for f in CognitoUser.user_class._meta.get_fields()]
extra_attrs = {}
for k, v in user_attrs.items():
if k not in django_fields:
extra_attrs.update({k: user_attrs.pop(k, None)})
if getattr(settings, 'CREATE_UNKNOWN_USERS', True):
user, created = CognitoUser.user_class.objects.update_or_create(
username=username,
defaults=user_attrs)

else:
try:
user = CognitoUser.user_class.objects.get(username=username)
for k, v in iteritems(user_attrs):
setattr(user, k, v)
user.save()
except CognitoUser.user_class.DoesNotExist:
user = None
if user:
for k, v in extra_attrs.items():
setattr(user, k, v)
return user

I'm not sure if fixing this is going to require overriding these methods from the Warrant library, since those are assuming a username field, or whether you can simply add conditional logic here to either get/update_or_create by either username or USERNAME_FIELD?

@anyeone
Copy link
Author

anyeone commented Apr 30, 2018

Also - although the documentation says it is looking for a setting called COGNITO_CREATE_UNKNOWN_USERS, the method above is looking for a setting called CREATE_UNKNOWN_USERS; I had COGNITO_CREATE_UNKNOWN_USERS in my settings and it was ignored, CREATE_UNKNOWN_USERS resolving to true by default.

@anyeone
Copy link
Author

anyeone commented Apr 30, 2018

It's actually a little more complicated than I thought. What you have to find the local User record is the cognito id, which in my scenario is stored in a separate field (cognito_id) rather than in the username field.

So by subclassing CognitoUser I can get around this by changing the get / update_or_create to look for the cognito_id field, i.e.
user = CognitoUser.user_class.objects.get(cognito_id=username)
instead of username=username

But the 'cognito_id' field name for the field holding the cognito guid is not going to be universal for everyone who uses a different USERNAME_FIELD setting.

To handle this scenario generically I think requires an additional setting, i.e. COGNITO_ID_MODEL_FIELDNAME (which in my case would hold
cognito_id) and then the get / update_or_create methods in the CognitoUser.get_user_obj class would need to dynamically query based on this field name.

@tabdon
Copy link

tabdon commented Sep 28, 2018

@anyeone Did you have any luck resolving this? I'm running into the same problem.

@pinktoadette
Copy link

a year later, i am also having this issue.
this library doesnt seem active.

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

No branches or pull requests

3 participants