getmango / Mango

Mango is a self-hosted manga server and web reader
https://getmango.app
MIT License
1.68k stars 118 forks source link

Subdivide User Permission #325

Open Leeingnyo opened 2 years ago

Leeingnyo commented 2 years ago

Currently, the admin permission of Mango is all-or-nothing. There are many requests for splitting admin permissions (https://github.com/getmango/Mango/issues/249, https://github.com/getmango/Mango/issues/66) (just 2?).

Current features that requires an admin permission

Features that would require an permission

If there is anything that you think it's an action that requires permission, please comment freely! Some features I listed would be questionable that they are proper to be subdivided.

Suggestion

A user whose is_admin is true has a full permission (A current behavior isn't changed), others would be checked if they have a permission to do. Permissions are represented as a string such as 'library.all' (full permission on library related), 'chapter.download' (only able to download chapters).

Implementation Plan

  1. Add user_permissions table.
name type
user_id string
permission_name string

This will be better than adding boolean columns by each permission to the user table.

  1. Check logged-in user has a proper permission to request an api by a handler (by editing AuthHandler)
// this is a pseudo code (not a crystal-lang)
// permission.cr
permission_map = { [api-name]: [permission-name, permission-name], ... } // this should be in a code to prevent to edit
/*
permission_map = {
  '/api/download': ['chapter.all', 'chapter.donwload_chapter'],
  '/api/scan': ['library.all', 'library.scan'],
  ...
}
*/

// in admin_handler.cr
  for api-name of keys(permission_map)
    if request_path_startswith env, api-name
      pass = Storage.default.username_has_permission username, permission_map[api-name]
      unless pass
        response 403
        return
      end
    end
  end

Migration

go ->

create an empty permissions table preserve the is_admin column

<- back

drop the permissions table preserve the is_admin column

UI

In user management page (admin only?) there is a permission section to see and to change permission of selected user. use the above permission_map to make a table.

[Select username ▼ (dropdown)] [Save Changes (button)] [Reset Changes (button)]

.-----------------------------------------------------------------.
| permission name      | comments                      | checkbox |
|----------------------|-------------------------------|----------|
| library.all          | full permission about library |    [o]   |
| library.scan         | to scan library               |    [ ]   |
| library.thumbnail    |                               |    [ ]   |
| library.missing      | to scan library               |    [ ]   |
| user.all             | full permission about user    |    [ ]   |
| user.delete          | to scan library               |    [ ]   |
| user.create          | to scan library               |    [ ]   |
| user.change-password | to scan library               |    [ ]   |
| chapter.all          | to scan library               |    [o]   |
| ...                                                             |
'-----------------------------------------------------------------'

Please comment freely about this issue!

hkalexling commented 2 years ago

Thanks for the write-up! I like the design and agree with all points. A few comments:

  1. Perhaps there should be a way to list/edit user permissions using the mango admin CLI tool. But this is a secondary feature and can come after what you proposed.separate
  2. I think this can somehow be extended and be integrated with the existing tagging system to achieve what you described in #13. But again this can be done as a separate feature.

Overall this looks like a big task, so maybe it will be helpful to split it into multiple smaller issues so they are easier to develop and track. If you are interested we can then each pick one or more sub-tasks to work on :)

Leeingnyo commented 2 years ago

@hkalexling Thanks for the comments!

  1. okay, I see.
  2. Oh, nice idea. a directory permission would be represented as library.allow.directory:/path/to/allow and for tag, library.allow.tag:allowed-tag. or, we could add a column data to user_permissions table instead of using :data on permission name. Actually, since I'm the only user to use the Mango of my own server, I don't need that feature strongly anymore though 😆
hkalexling commented 1 year ago

BTW I think this can also cover #300

Leeingnyo commented 1 year ago

Could you explain more about that?

hkalexling commented 1 year ago

Oops sorry on second thought that doesn't make sense. A user should be able to change their own password regardless the of permission level. I must be drunk when writing the comment XD

hkalexling commented 1 year ago

We can also have separate permissions for creating/viewing/managing subscriptions. Currently only admin users have access to these