search

scylladb / argus

Apache License 2.0
4 stars 11 forks source link

feature(service/admin): User manager #422

Closed k0machi closed 3 months ago

k0machi commented 3 months ago

This commit adds a user manager for administrators, allowing them to change email, passwords and admin state (as well as deleting) other users. The interface is accessible on a previously placeholder page inside admin panel. Additionally, login page can now be configured for which authentication methods it provides, allowing fully disabling login via password or github.

k0machi commented 3 months ago

Demo:

image image image

fruch commented 3 months ago

@k0machi

is there some context to this one ? i.e. some asked for it, it was blocking something we couldn't do before, helping with work with staging ?

k0machi commented 3 months ago

@k0machi

is there some context to this one ? i.e. some asked for it, it was blocking something we couldn't do before, helping with work with staging ?

This is something that's been needed for a while, to be able to change user passwords in case they want to use password login for some reason (e.g. staging) or fixing their email addresses or deleting old users.

k0machi commented 3 months ago

Added configuration for the login form (and service functions) to disable password or github login:

image


# Which login methods to enable
LOGIN_METHODS:
  - password
  - gh
k0machi commented 3 months ago

Generally, I'm not convinced to 'deleting' users - I'd rather to anonymize them instead to keep all the references valid. But this is only my opinion and I can still merge it and deploy. @roydahan @fruch WDYT?

In general, that's what Argus does already - the entity is deleted and every frontend component (backend logic as well), save for a few spots I've found while doing this PR already switches non-existent user to a placeholder 'Ghost' user, I should've included some screenshots with that. We could keep the entity and just 'anonymize' the fields but this presents a problem of getting multiple "Ghost" users, where each one adds an entry to the user table, in which case we would either have to filter those out or hide them in the queries and such. Keep in mind since Scylla doesn't enforce relations it's no big deal if there's a non-existent user on the backend. One problem I can see with this PR is deleting very prolific user - since currently every single entity created by them is preserved - job assignments, comments, user profile picture, and so on, so this could potentially bloat the database somewhat. For that case we can introduce cleanup for storage intensive entities (which we currently don't have so I don't think that's needed right now).

fruch commented 3 months ago

Generally, I'm not convinced to 'deleting' users - I'd rather to anonymize them instead to keep all the references valid. But this is only my opinion and I can still merge it and deploy. @roydahan @fruch WDYT?

I don't see the value of keeping anonymous users,

We would have jobs assigned to anonymous ?

Comments from anonymous ?

where else do we have references ?

From an admin perspective I need a way to not to see this user anywhere that we need to select a user, and unassigne it from everything.

In comments I would rather still see the github handle if the user doesn't exist, so I can read old discussion without too much confusion, if we keep disabled users for that purpose only, that o.k., but I rather avoid that if possible.