Changed get_or_create to update_or_create

[fservida]

Changed get_or_create to update_or_create, this way if an attribute is changed on the IdP (eg. email/first_name/phone number) the user object and profiles if existing are automatically updated. As is now if they change the lookup fails and therefore get_or_create tries to create a user object but incurs in an IntegrityError on the username field.

I don't know if I'm using in some way the library wrong, but it seems to me that only one attribute should be used to lookup the user object and all the others should be updated if needed, but not used to filter the objects.

I wasn't able to test the edit now, will try asap.

[fservida]

Ok, never mind the commit, it looks like I'm unable to read the docs, I'm using update_or_create in a wrong way, testing locally now other ways to use correctly update_or_create to solve the above problem, will commit as soon as I get it to work.

[fservida]

Done, I've configured update_or_create to use a lookup key that the user can define in the settings under "SAML_USERS_LOOKUP_ATTRIBUTE", it defaults to the username if nothing is specified.

In the settings.py set: SAML_USERS_LOOKUP_ATTRIBUTE = "username" or your attribute of choice, the attribute has to be one of those specified in the SAML_USERS_MAP, and ideally should be a unique attribute in your user model (by default username)

The additional attributes are used to update the model fields via the argument "defaults" of update_or_create, which is a dictionary of fields and respective values to be updated.

(Does anyone know if update_or_create can update fields in related models (eg. extended profiles)?)

[juliedavila]

@FranceX hmm, I like this capability. However, what do you think of making it an option to opt for get_or_create vs update_or_create? In other words, make this a flag dictated by config. I could see a use case where someone does not want their IDP to be the "source of truth" for how the user model should be. What do you think?

Also, I'm not 100% sure, but I'm fairly confident update_or_create doesn't trickle over into related models.

[fservida]

Yeah, it seems like a good idea, I didn’t think of a mixed scenario where you’d want to login in an account using an IDP but not updating it, but it seems only right to give the option to choose if using get_or_create or update_or_create, defaulting to one of them if there’s no setting specified in settings.py.

Il giorno 20 feb 2018, alle ore 13:27, Jonathan Davila notifications@github.com ha scritto:

@FranceX https://github.com/francex hmm, I like this capability. However, what do you think of making it an option to opt for get_or_create vs update_or_create? In other words, make this a flag dictated by config. I could see a use case where someone does not want their IDP to be the "source of truth" for how the user model should be. What do you think?

Also, I'm not 100% sure, but I'm fairly confident update_or_create doesn't trickle over into related models.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/MindPointGroup/django-saml2-pro-auth/pull/17#issuecomment-366962151, or mute the thread https://github.com/notifications/unsubscribe-auth/AAeoxvW90jacDGx5ZmS2f16hNUkFViwHks5tWrozgaJpZM4SKvgY.

[juliedavila]

Cool beans. I'll wait for you to update your PR and we'll go from there.

[fservida]

I've updated it with the new settings flag, left get_or_create as the default in order to not change the behavior of existing applications. Let me know if there's anything you'd change.

[juliedavila]

@FranceX Thank you for this work!

[juliedavila]

Pushed to pypi and should be available in version 0.0.5

[fservida]

You're welcome 👍 I'll probably be back if I find something else interesting to contribute (I saw you'd like to implement SLO and ADFS support)