IdentityPython / djangosaml2

Django SAML2 Service Provider based on pySAML2
Apache License 2.0
256 stars 143 forks source link

Documentation seems to be incorrect with regards to User profile objects. #334

Open dino8890 opened 2 years ago

dino8890 commented 2 years ago

Django profile objects refers to objects that are typically related to the User object using OneToOneField and store extra information about user. The documentation states:

If you are using Django user profile objects to store extra attributes about your user you can add those attributes to the SAML_ATTRIBUTE_MAPPING dictionary. For each (key, value) pair, djangosaml2 will try to store the attribute in the User model if there is a matching field in that model. Otherwise it will try to do the same with your profile custom model.

However, this is not the case and it confused the hell out of me trying to get it to work. This excerpt from authentication backend method handles attribute updates and it does not appear to look for profile object:

for attr in django_attrs:
    if attr == user_lookup_key:
        # Don't update user_lookup_key (e.g. username) (issue #245)
        # It was just used to find/create this user and might have
        # been changed by `clean_user_main_attribute`
        continue
    elif hasattr(user, attr):
        user_attr = getattr(user, attr)
        if callable(user_attr):
            modified = user_attr(attr_value_list)
        else:
            modified = set_attribute(user, attr, attr_value_list[0])

        has_updated_fields = has_updated_fields or modified
    else:
        logger.debug(f'Could not find attribute "{attr}" on user "{user}"')

If this is indeed true, let me know and I will send a PR with updated description.

peppelinux commented 2 years ago

I use a custom user model and always the django primitive get_user_model()

it works as it is with the current documentation, anyway if you've been confused I believe that it's time for a PR! thank you indeed

dino8890 commented 2 years ago

Yes, that approach works perfectly, but reading documentation I was under the impression that you can somehow specify profile object without having to subclass User model, e.g. by using specific related_name in OneToOneField or configure it in settings.py like you can AUTH_USER_MODEL.

It is not a huge issue, but it is better to minimize confusion anyway. Give me a day or two and I'll get back to you with a patch.