Mango icon indicating copy to clipboard operation
Mango copied to clipboard

Subdivide User Permission

Open Leeingnyo opened this issue 2 years ago • 6 comments

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

  • overall library
    • scan
    • generate thumbnails
    • get the thumbnail progress
    • get / remove missing titles / entries
  • user management
    • delete a user
    • create a user (it's on router/admin.cr, not the router/api.cr)
    • change a user password (it's on router/admin.cr, not the router/api.cr)
  • chapter (titles / entries)
    • change a display name
    • change a sort title
    • tag / untag
  • plugin
    • download entries by plugins

Features that would require an permission

  • chapter
    • download a chapter
    • delete a chapter (not yet implemented)

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!

Leeingnyo avatar Jul 24 '22 05:07 Leeingnyo

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 :)

hkalexling avatar Jul 24 '22 14:07 hkalexling

@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 😆

Leeingnyo avatar Jul 24 '22 16:07 Leeingnyo

~~BTW I think this can also cover #300~~

hkalexling avatar Aug 01 '22 11:08 hkalexling

Could you explain more about that?

Leeingnyo avatar Aug 10 '22 13:08 Leeingnyo

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 avatar Aug 10 '22 14:08 hkalexling

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

hkalexling avatar Aug 18 '22 10:08 hkalexling