search

AzMoo / django-okta-auth

Django Okta Auth is a library that acts as a client for the Okta OpenID Connect provider.
MIT License
30 stars 23 forks source link

Only set Django User's staff/superuser on first login. #6

Closed KoreyPeters closed 3 years ago

KoreyPeters commented 3 years ago

This pull request modifies the code such that the setting of superuser/staff status is only done on account creation.

AzMoo commented 3 years ago

Hi @KoreyPeters thanks for the pull request. The problem I see with this approach is that you can't add a user to the superuser group after user creation, you would have to manage adding and removing from super/staff through the django admin and that's not desirable for me. The goal is to be able to manage users without django being involved at all.

You mentioned in the other thread that your usecase is to allow super and staff statuses to be mutually exclusive, and that makes a lot of sense. How do you feel about a second django setting, STAFF_GROUP, which would only set the staff flag if the user is a member of that group.

That gives the ability to set the two flags individually, but also allows you to add and remove the statuses from okta.

AzMoo commented 3 years ago

0654255a6b96ad048dbd5c3838d7bb17b6185c92

New branch staff-flag which demonstrates what I mean.

KoreyPeters commented 3 years ago

Thanks for you comments. I'm not understanding your use case (which is fine! It's not my software! :)) As far as I can tell, the only way to add an admin/staff/superuser with the current code is to edit your settings.py file and re-deploy the app, which seems much more difficult/error-prone that using Django Admin. But maybe I'm missing something?

If editing/re-deploying is required for staff access, then "STAFF_GROUP" doesn't really help me. I definitely would rather manage users through Django Admin. But again, I feel like I'm missing something about how your managing users. So the idea may well have value.

I'll probably keep using my patch for now. Thanks for the excellent package. I really appreciate it.

AzMoo commented 3 years ago

The STAFF_GROUP and SUPERUSER_GROUP settings each reference a group name in Okta. To manage staff users and superusers the idea is to add them to the appropriate group within Okta that is referenced by these settings.

The goal of this library is to delegate all Django user management to Okta. Is that your use case as well? If it's not, I'm not sure what you're looking for from this library.

KoreyPeters commented 3 years ago

Ah, I read it as I needed users in that setting, not Okta groups. What you say makes much more sense. :) But yes, the STAFF_GROUP would certainly be valuable for me. Thanks!

KoreyPeters commented 3 years ago

Now that I understand how SUPERUSER_GROUP (and potentially in the future STAFF_GROUP) are supposed to be used, this pull request isn't relevant any more.

AzMoo commented 3 years ago

Great. I will merge in the STAFF_GROUP change because it makes sense to me.

Also opened up #8 to update the documentation for SUPERUSER_GROUP: