Official way of setting is_staff and is_superuser

[Rjevski]

Does anyone know what's the "official" way of setting is_staff and is_superuser?

My IdP is sending the status as a "true" or "false" string in a specific attribute. However, simply putting the attribute in SAML_ATTRIBUTE_MAPPING is not enough as djangosaml2 will try to assign the string value to the boolean field on the user model where it'll raise an exception upon save.

At the moment I am working around that with a custom authentication backend which overrides the clean_attributes method and "booleanizes" the values there, but I wonder if there's a better way?

Questions:

• Does SAML have the concept of "boolean" values? Do I just need to tell my IdP to send that value as a specific attribute and it'll automatically be interpreted as a boolean by djangosaml2?
• If not, is there a better way to do this?
• If not, should we have a better way? I feel like subclassing the backend for such a basic thing is overkill, and I'd prefer being able to pass arbitrary functions/lambdas in the SAML_ATTRIBUTE_MAPPING which will be called with the original value and return the transformed value. Then I could just set it to {"IsSuperuser": lambda v: [x == 'true' for x in v]} and be done with it.

[peppelinux]

Simply use a custom account app and add a @Property getter and setter to handle that behaviour

[peppelinux]

Otherwise, the "stop feature" would be an Attribute post processor that can optionally rewrite the attrs upon some rules.

I implemented a similar approach in another library, called pymultildap

https://github.com/peppelinux/pyMultiLDAP/blob/5dcb32955f4ba0eda1300405c69885d6321f5945/examples/settings.py.example#L62

Would It Be a good feature for you?

[peppelinux]

Also here:
#192

[Rjevski]

I don't think the rewrite feature as in pymultildap is a good idea. It's a large amount of work to implement all potential rewrite operations and you still wouldn't be able to cover every case. Why not just use actual Python code, by passing a callable (which takes the original value as argument)?

[peppelinux]

I'd prefer a @Property but yes, callable Just works. Feel free to close this issue if you're satisfied, I'd wait a week before doing that, if there won't be other posts

[peppelinux]

Hi @Rjevski is there any update on this?
Would you like to purpose a PR or have you found alternative solutions based on a custom user account model?
Let me know, I'd close this issues in days

[Rjevski]

I've worked around this by subclassing the SAML authentication backend and setting the attributes there. I do think however the package should be extended to take lambdas in settings directly. I'd say it's worth leaving this open for the foreseeable future as a discussion thread and see if other users come forward with their opinions.

…

On 20 Oct 2020, at 12:22, Giuseppe De Marco ***@***.***> wrote: Hi @Rjevski is there any update on this? Would you like to purpose a PR or have you found alternative solutions based on a custom user account model? Let me know, I'd close this issues in days — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub, or unsubscribe.

[peppelinux]

Are you working on v0.50.0 or v1.0.0-rc (current main)?

[Rjevski]

I'm no longer working on that particular project (it was delivered and the client is happy ✨) so I don't remember, I think I was using ~0.30. I'm afraid I can't contribute any more details to this for now, but I would prefer to leave this discussion open in case someone else has a similar use-case and wants to share their thoughts.

…

On 17 Dec 2020, at 10:41, Giuseppe De Marco ***@***.***> wrote: Are you working on v0.50.0 or v1.0.0-rc (current main)? — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub, or unsubscribe.

[pauldekkers]

Hi! I was looking at the same issue actually.

I don't need to update the User model, so I didn't change AUTH_USER_MODEL but set SAML_USER_MODEL to a proxy model for User, like:

from django.contrib.auth.models import User

class MyUser(User):
    class Meta:
        proxy = True

    def process_entitlement(self, values):
        if 'specific-entitlement' in values:
            self.is_staff = True
            self.save()
        else:
            self.is_staff = False
            self.save()

And in settings I have

SAML_ATTRIBUTE_MAPPING = {
    'eduPersonPrincipalName': ('username', ),
    'eduPersonEntitlement': ('process_entitlement', ),
}

Now, a different option seems to be a different backend, like:

from djangosaml2.backends import Saml2Backend

class MySaml2Backend(Saml2Backend):
    def _update_user(self, user, attributes: dict, attribute_mapping: dict, force_save: bool = False):
        if 'eduPersonEntitlement' in attributes:
            if 'some-entitlement' in attributes['eduPersonEntitlement']:
                user.is_staff = True
                force_save = True
            else:
                user.is_staff = False
                force_save = True
        return super()._update_user(user, attributes, attribute_mapping, force_save)

At first I thought about the save_user function, but that has no access to attributes, and it's not called if there are no other updates.

Comments welcome to either approach - and maybe one of these is useful in the documentation?

Now I thought there was a third option that @peppelinux suggested; but I was unable to use the @receiver(post_authenticated) or @receiver(pre_user_save) from djangosaml2.signals - perhaps that requires a custom User model after all, though I tried with and without sender, and sender also set to get_user_model() or User. It could also be I didn't put it in the right place.)

[peppelinux]

Hi @pauldekkers
happy to see you here :)

Yes, I like your approach, so feel free to do a PullRequest to dev branch (dev, not master!)
I'll update docs asap

[pauldekkers]

Thanks @peppelinux. So, both approaches? I was in doubt about the _update_user as it being 'internal', but it's the shortest and most flexible example actually and possibly calls user.save less times.

[peppelinux]

Yes, it would be great to having both, it's a good hint for developers!

[Rjevski]

The proxy model seems like a good approach in this case - it would've worked for me too but I didn't think about it at that time.