RFC 40: User account management changes #40
Conversation
kaedroho
changed the title
kaedroho
force-pushed
the
user-account-management
branch
from
2f5d29a
to
73926b0
Compare
|
|
||
| When users are created, currently the admin sets their email address and password. The email address is never verified and passwords set in this way are usually weak and don’t have to be changed which can lead to multiple users having the same password. | ||
|
|
||
| We should remove the password fields from the create user form, and instead email the user a link to a form where they can set their password themselves. Similar to the password reset feature. |
There was a problem hiding this comment.
What do you think to adding another setting to allow this behaviour to be toggled?
| - Their email address is verified - which is important in case they want notifications or need to use the password reset feature in the future | ||
| - Their password is likely to be stronger and unguessable by other users | ||
|
|
||
| A disadvantage is this requires email to be configured in order to create new users. I think we could either rely on the admin finding the change password box on the edit page or keep those fields on the create user form but conceal them a bit to discourage their use. |
There was a problem hiding this comment.
Perhaps we could use a radio select to solve this problem? Options could be something like:
- Send this user a link to set their own password (recommended)
- Allow me to set the password for this user
We'd keep the first as the default option, and only show password fields if the user deliberately chose the second option.
|
|
||
| ## Task B - Add setting to require verified email addresses | ||
|
|
||
| Right now, admins can create users with any email address and users can change their email address to anything they want. |
There was a problem hiding this comment.
Could we perhaps make the function that sends the notification swappable? I've worked on a few projects before where it was necessary use different backends for different types of email, so such a feature would be handy in those situations. It would also be cool to provide flexibility for triggering things other than emails... e.g. sending a notification via Slack or some other service.
|
|
||
| - Convert the users admin interface into an admin module (Groups is already a module) | ||
| - Allow admin modules to be easily unregistered | ||
| - Add a setting to allow disabling the “Change email” function for users `WAGTAIL_USER_CHANGE_EMAIL_ENABLED` |
There was a problem hiding this comment.
This is now done: wagtail/wagtail#5293
|
Wondering if we could also add a concept of "Locked groups". Groups that can't be assigned to/unassigned from users by using the Wagtail admin. This would prevent admins from fiddling with groups that are managed by an external system. |
|
@kaedroho I really like that idea! It's a pity we can't add fields to the group model directly though. We'd need some extra model to store this. Probably one to tackle separately :) |
|
Maybe we could do this with a setting? |
Only issue there is, any third-party app that did want to create and lock down groups would have to make it clear to the developer that they'll have to manually update their hardcoded config (or change env vars) for things to work as expected, which is a bit limiting. Also, the group name would probably have to be fixed forever. |
I can't think of any examples where a third-party app would need to lock down groups? Third party apps shouldn't be working with Groups directly.
Yep, I agree this is a problem. We could use a hook instead? For example: from custom_sso_app.models import SSOManagedGroup
@hooks.register('get_admin_managable_groups')
def get_admin_managable_groups(groups):
return [
group for group in groups
if not SSOManagedGroup.objects.filter(group=group).exists()
]But I think a static list would be much more common since these mappings are usually hardcoded. If we implement this, I think it should only prevent:
|
I can :) e.g. a multi-tenancy app that automatically created a 'base' group for each site and assigned users to it directly based on some other trigger. I can also see separate 'is_editable' and 'is_assignable' flags being useful. |
|
In a multi-tenancy environment, would you even use Django groups? Each user/group would need to be linked to a tenant to allow tenants can create their own groups without other tenants seeing them, or it might be better to disable the groups interface for tenants altogether and come up with something different. |
|
I suppose it all depends on how strict the model of multi-tenancy is, and how much of Wagtail's own permission enforcement is being respected or overridden. Details aside, I think there's a decent argument for making things dynamic - it would certainly be more flexible that way. |
Rendered