Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Role #9437

Merged
merged 49 commits into from
Jan 12, 2023
Merged

Role #9437

merged 49 commits into from
Jan 12, 2023

Conversation

syuilo
Copy link
Member

@syuilo syuilo commented Dec 31, 2022

What

#6305

Why

Additional info (optional)

  • 指定ロールのユーザー一覧
  • 自分の持っている権限一覧を取得できるように i
  • インスタンスのデフォルト権限を取得できる曜日 meta
  • defaultRoleOverride更新API
  • API呼び出し時の権限判定
  • migration
    • rename isAdmin to isRoot
  • ロール設定UI
    • デフォルトロール編集(=オーバーライド)
    • アサイン

moderator manager admin

@github-actions github-actions bot added the packages/backend Server side specific issue/PR label Dec 31, 2022
@codecov
Copy link

codecov bot commented Dec 31, 2022

Codecov Report

Merging #9437 (a99b1c2) into develop (60e545b) will increase coverage by 0.09%.
The diff coverage is 22.44%.

❗ Current head a99b1c2 differs from pull request most recent head f702c40. Consider uploading reports for the commit f702c40 to get more accurate results

@@             Coverage Diff             @@
##           develop    #9437      +/-   ##
===========================================
+ Coverage    22.31%   22.41%   +0.09%     
===========================================
  Files          724      731       +7     
  Lines        67170    67896     +726     
  Branches      2171     2200      +29     
===========================================
+ Hits         14991    15219     +228     
- Misses       52179    52677     +498     
Impacted Files Coverage Δ
packages/backend/src/core/DeleteAccountService.ts 51.16% <0.00%> (-3.84%) ⬇️
...ckages/backend/src/server/NodeinfoServerService.ts 0.00% <0.00%> (ø)
packages/backend/src/server/api/ApiCallService.ts 0.00% <0.00%> (ø)
packages/backend/src/server/api/EndpointsModule.ts 0.00% <0.00%> (ø)
packages/backend/src/server/api/endpoints.ts 0.00% <0.00%> (ø)
.../src/server/api/endpoints/admin/accounts/create.ts 0.00% <0.00%> (ø)
.../src/server/api/endpoints/admin/accounts/delete.ts 0.00% <0.00%> (ø)
.../api/endpoints/admin/delete-all-files-of-a-user.ts 0.00% <0.00%> (ø)
.../src/server/api/endpoints/admin/drive/show-file.ts 0.00% <0.00%> (ø)
.../src/server/api/endpoints/admin/get-index-stats.ts 0.00% <0.00%> (ø)
... and 51 more

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

@tamaina
Copy link
Contributor

tamaina commented Dec 31, 2022

adminはロールではないのね

@syuilo
Copy link
Member Author

syuilo commented Dec 31, 2022

その予定

@syuilo
Copy link
Member Author

syuilo commented Dec 31, 2022

また、特定のユーザーのみファイルをアップロード出来なくする(スパム対応)等もあれば便利かと思います。

(実際に実装するかどうかは別にして)このユースケースを実現するにはどうすれば良いか考えてる

@syuilo
Copy link
Member Author

syuilo commented Dec 31, 2022

  • fileUploadみたいな権限を作る
  • 全てのユーザーにデフォルトで割り当てられる権限(not Role)を設定できるようにする

の二本立てでできそう

@tamaina
Copy link
Contributor

tamaina commented Dec 31, 2022

ファイル容量をロールごとに変更したいみたいなことをしたいけどここでやるのは混乱するか

@syuilo
Copy link
Member Author

syuilo commented Dec 31, 2022

  • fileUploadみたいな権限を作る
  • 全てのユーザーにデフォルトで割り当てられる権限(not Role)を設定できるようにする

の二本立てでできそう

いや出来なさそう

@tamaina
Copy link
Contributor

tamaina commented Dec 31, 2022

  • 一般ユーザーのロールを作って新規ユーザーは全員それに割り当てる
  • 制限したいユーザーには制限ユーザーのロールを割り当てる

みたいなのでいいのでは

@syuilo
Copy link
Member Author

syuilo commented Dec 31, 2022

  • 一般ユーザーのロールを作って新規ユーザーは全員それに割り当てる
  • 制限したいユーザーには制限ユーザーのロールを割り当てる

みたいなのでいいのでは

まあそうなるかあ
全てのユーザーに対して少なくとも1つロール割り当てるとパフォーマンス上の懸念があったからできれば避けたかった

@syuilo
Copy link
Member Author

syuilo commented Dec 31, 2022

例えばioだとリモート含め60万くらいUserレコードあるからロール割り当ても最低60万になって、頻繁に発生すると思われるユーザーがどのロール持ってるか判定する処理が少し遅くなりそう

@syuilo
Copy link
Member Author

syuilo commented Dec 31, 2022

あとマイグレーションがめんどくさそう

@tamaina
Copy link
Contributor

tamaina commented Dec 31, 2022

というかそもそもUser関連のエンティティに直接ロールのID/リレーションを持たせるわけではないのね

@syuilo
Copy link
Member Author

syuilo commented Dec 31, 2022

というかそもそもUser関連のエンティティに直接ロールのID/リレーションを持たせるわけではないのね

複数のロール持つ可能性あるからね

@tamaina
Copy link
Contributor

tamaina commented Dec 31, 2022

複数のロール持つ可能性あるからね

oneToManyでもよくね説

@tamaina
Copy link
Contributor

tamaina commented Dec 31, 2022

(ユーザーが)複数のロール持つ可能性あるからね

これそもそも一つでよくねと思ったりした、ロール2つのうち1つで許可されていてもう1つでは不許可の場合どちらかの場合は…許可されていると見做せばいいのか()
ちなみにDiscordではロールはユーザーに対して複数あって、設定画面上でのロールの並び順で適用の優先順位が決まるらしい

(ロールを拡張するロールを設定できるようにするとかでもいい気がした)

@tamaina
Copy link
Contributor

tamaina commented Dec 31, 2022

あとグループのロール(現在存在しない概念)と被りそうなのでInstanceRoleとかにしたほうがいい気がした

@syuilo
Copy link
Member Author

syuilo commented Dec 31, 2022

例えばioだとリモート含め60万くらいUserレコードあるからロール割り当ても最低60万になって、頻繁に発生すると思われるユーザーがどのロール持ってるか判定する処理が少し遅くなりそう

これのスマートな解決策が思いつかないため挫折

@syuilo
Copy link
Member Author

syuilo commented Dec 31, 2022

また、特定のユーザーのみファイルをアップロード出来なくする(スパム対応)等もあれば便利かと思います。

(実際に実装するかどうかは別にして)このユースケースを実現するにはどうすれば良いか考えてる

こういうユースケースは諦めるか

@syuilo
Copy link
Member Author

syuilo commented Dec 31, 2022

やっぱadmin/moderator/一般ユーザーの区別だけで十分じゃね

@tamaina
Copy link
Contributor

tamaina commented Dec 31, 2022

(そんなバカスカ作られるものではないのでアンテナみたいにオンメモリで良さそうな気も、それなら負荷高くないような)

@syuilo
Copy link
Member Author

syuilo commented Dec 31, 2022

処理速度的にはそれで良いけど、ユーザーと同じ数だけレコードがあるとDB側のインデックスのメモリ使用量がバカにならなそう

@tamaina
Copy link
Contributor

tamaina commented Dec 31, 2022

RoleAssignment反対派(UserにロールIDを保存すればいい派)だからそこまでインデックスが食うとは思ってない

@syuilo
Copy link
Member Author

syuilo commented Dec 31, 2022

配列で持つってことかしら

@tamaina
Copy link
Contributor

tamaina commented Dec 31, 2022

配列で持つってことかしら

yes

@syuilo
Copy link
Member Author

syuilo commented Dec 31, 2022

まあ配列で持ったとしても結局はそこにインデックス張ることになるからそんなに変わらなそう

@tamaina
Copy link
Contributor

tamaina commented Dec 31, 2022

すでにcreatedAtにインデックスが貼られているし怖いことはないのでは()

@syuilo
Copy link
Member Author

syuilo commented Jan 10, 2023

アサインされているロールが存在しない場合はシステム上デフォルトロールにアサインされている状態と見做すようにして、さらにロールに「デフォルトロールを上書き」オプションを用意すれば、全ユーザーにロールのアサインレコードが存在するのを防ぎつつ、#9437 (comment) を実現できる気がした

@syuilo
Copy link
Member Author

syuilo commented Jan 10, 2023

そこまでしてロール機能を導入する必要があるかというのは謎

@syuilo syuilo marked this pull request as ready for review January 12, 2023 09:16
@syuilo
Copy link
Member Author

syuilo commented Jan 12, 2023

できた

@syuilo
Copy link
Member Author

syuilo commented Jan 12, 2023

色設定要る?

@tamaina
Copy link
Contributor

tamaina commented Jan 12, 2023

あった方が楽しい

@syuilo
Copy link
Member Author

syuilo commented Jan 12, 2023

カラーピッカーコンポーネント欲しくなってきた

@syuilo
Copy link
Member Author

syuilo commented Jan 12, 2023

色追加

@syuilo
Copy link
Member Author

syuilo commented Jan 12, 2023

ロールで設定したい項目募集中

現在:
image

@syuilo
Copy link
Member Author

syuilo commented Jan 12, 2023

サイレンスもロールに統合しちゃってもいい気がしてきたな

@syuilo
Copy link
Member Author

syuilo commented Jan 12, 2023

するか

@tamaina
Copy link
Contributor

tamaina commented Jan 12, 2023

カスタム絵文字の作成・編集とかは要望多いかも

@syuilo
Copy link
Member Author

syuilo commented Jan 12, 2023

ロールでカスタム絵文字の作成を許可できても、現在はコントロールパネルからしかそういった操作は行えないからフロントエンドの改修も要りそう

@syuilo
Copy link
Member Author

syuilo commented Jan 12, 2023

作成/編集/削除はひとまとめでもいいのかな

@tamaina
Copy link
Contributor

tamaina commented Jan 12, 2023

作成/編集/削除はひとまとめでもいいのかな

いいと思う

@syuilo
Copy link
Member Author

syuilo commented Jan 12, 2023

一般ユーザーによるカスタム絵文字の編集はそれだけでちょっとした機能になりそうだから別の機会かも

@syuilo syuilo merged commit 2470afa into develop Jan 12, 2023
@syuilo syuilo deleted the role branch January 12, 2023 12:02
@tamaina
Copy link
Contributor

tamaina commented Jan 12, 2023

招待コード発行・管理…もカスタム絵文字と同様か

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
packages/backend Server side specific issue/PR packages/frontend Client side specific issue/PR
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants