-
Notifications
You must be signed in to change notification settings - Fork 2.8k
Closes #16137: Remove is_staff
boolean from User model
#20306
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
Conversation
is_staff
boolean from User model
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Had one question about why were still setting user.is_staff
in RemoteUserAuth
, but this looks good otherwise.
# Set is_staff attribute for compatibility with pre-v4.5 | ||
user.is_staff = user.is_superuser | ||
if user.is_staff: | ||
logger.debug(f"Marked user {user} as staff due to superuser status") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hopefully it's just my lack of familiarity with our use of RemoteUserBackend
, but I don't understand why we need to do this (temporarily setting user.is_staff
) here, now.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Honestly I'm not sure that we do, but I figured it might be less disruptive to keep it. That said, I'm okay removing it for v4.5.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm +0 on removing it. Happy to defer if you think it's best to leave it, but otherwise it's just one more thing for us to remember to get back to later.
Going to approve this PR and let you decide.
Closes: #16137
is_staff
boolean field from our custom User modelREMOTE_AUTH_STAFF_GROUPS
andREMOTE_AUTH_STAFF_USERS
configuration parametersIsSuperuser
REST API permission to replace DRF'sIsAdminUser
(which evaluatesis_staff
)RemoteUserBackend
now infersis_staff
fromis_superuser
PluginMenuItem
still supports thestaff_only
attribute, but it now maps to theis_superuser
attribute on a useris_staff
from the LDAP authentication docs