Discussion: Better alternative to user abilities based on roles
まだ誰も着手していません。
評価
- 難易度
- 5/5
- 見積もり時間
- 1週間以上
- 初心者へのやさしさ
- 20/100
- issue の種類
- 機能追加
- 明瞭さ
- 説明が足りない
- 活発さ
- 停滞
- 技術スタック
- javascript
- 領域
- api, authorization, frontend
調査の方向性
Issue 内の ability の例から始め、特に projectMembership、userRole、route before/afterModel checks を確認してください。記載されている current-user sideloading と API abilities の選択肢を検討してください。完了の条件は、非同期の ability チェックと過剰な project membership requests に対する合意済みの解決策があることですが、ファイルやテストは指定されていません。
索引モデルが issue の本文から書いたものです。
説明
Problem
Right now, most of our authorization stuff is based in a user's membership level in a project. They can be unrelated, they can have a pending membership (meaning they requested membership, but they are yet to be approved), or they can be any of [contributor, admin owner].
Based on the level of their role, they are allowed or not allowed to perform certain project-related actions.
First Problem: Amount of requests
In order to determine a role level of a user in a project, we usually need to perform the following:
- fetch
project.projectUsers - for each
projectUser, actually load the record from the backend, because just having theidis not enough.
Right now, this is not a big deal performance-wise. However, we expect projects to grow, and this is, worst-case, n+1 requests for a project with n members. Luckily, we use colasced id requests to significantly reduce this, but it's still something that's potentially problematic.
Second Problem: Ember-Can does not play well with async
The ability above is a computed, the content of which is partially dependant on async network requests.
Basically, in most cases we have something like this:
projectMembership: computed('project.projectUsers', 'currentUser.user.id', function() {
let currentUserId = get(this, 'currentUser.user.id');
if (isEmpty(currentUserId)) {
return false;
} else {
return get(this, 'project.projectUsers').find((item) => {
return get(item, 'user.id') === currentUserId;
});
}
}),
userRole: alias('projectMembership.role'),
userIsContributor: equal('userRole', 'contributor'),
userIsAdmin: equal('userRole', 'admin'),
userIsOwner: equal('userRole', 'owner'),
canEdit: or('{userIsAuthor,userIsAdmin,userIsOwner}'),
canAssign: or('canEdit', 'userIsContributor'),
canReposition: alias('canAssign')
With template usage, where we do or do not render parts of the UI based on abilities, this works amazingly well, mostly thanks to DS.PromiseObject and DS.PromiseArray classes.
As results are being fetched, eventually, we will get a currentUserId and we will get a projectMembership.role. Until that happens, the ability is evaluated as false. For templates, this is almost exactly what we need. Eventually, when we determine a user is able to do something, that part of the UI will render.
For the case of routes and redirecting in case of lack of abilities, this is problematic. In those cases, we basically get the current value of the ability, which is usually false at the moment of a route's before/afterModel hook. Due to that, we are forced to use a dirty solution of explicitly fetching records before checking the ability:
return get(project, 'projectUsers').then(() => {
if (this.cannot('manage project', project)) {
return this.transitionTo('project');
}
});
Its a very clear code smell and it will end up being highly problematic as the number of users grows.
We need a solution, primarily for the second, but also for the first problem
Potential solutions
We sideload the projectUser relationship as part of the user request
This works, but also means a lot of data we don't need being loaded. Most user will not need information for other user records.
We sideload the projectUser relationship, but only for the current user.
Our API could sideload it in this specific case, if the GET /users/:id matches the currently authenticated user.
Really, this would tackle most cases. We'd have to rethink our currentUser/abilities structure, but it could work quite nicely, I think.
We could go further and have the authentication request return a user record with all the sideloaded information, which we can then push into the ember store manually.
The more I think about it, the more I like this solution.
We send some sort of computed ability table structure for each user
Probably highly complex and something I'm not looking forward to tackling and would vote against, but wanted to get the option listed here anyway.
Have some sort of ability endpoint on the API, which can tell each user if they can/cannot do something.
Again, a solution I do not personally like, but listing it for the sake of discussion.
Other options
Listed above are the options I could think of, but there may be other.
Side effects
Depending on how we deal with this, we could eliminate the need to fix #1151
- 主要言語
- JavaScript
- スター
- 120
- フォーク
- 75
- PR マージ指標
- 30日以内にマージされた PR はありません
コントリビューションガイド
はじめの一歩
- issue を最後まで読み、次にプロジェクトのコントリビューションガイドを読みます。
- 着手することを issue にコメントします — 二人が同じ作業をするのを防げます。
- リポジトリをフォークし、ブランチを切って変更します。
- issue 番号を参照したプルリクエストを送ります。
code-corps/code-corps-ember のほかの issue
-
難易度 2/5 1〜3時間 初心者へのやさしさ 68/100
code-corps/code-corps-ember#1616 ·
-
難易度 2/5 1〜3時間 初心者へのやさしさ 65/100
code-corps/code-corps-ember#1613 ·
-
難易度 2/5 1〜3時間 初心者へのやさしさ 72/100
code-corps/code-corps-ember#1612 ·
-
Difficulty: Medium Skill: ember-cli-page-object
難易度 2/5 1〜3時間 初心者へのやさしさ 62/100
code-corps/code-corps-ember#1063 · リアクション 1 件 ·
-
greenkeeper
難易度 3/5 1〜2日 初心者へのやさしさ 25/100
code-corps/code-corps-ember#1765 · コメント 1 件 ·
code-corps/code-corps-ember の issue をすべて見る
似ている issue
-
bug confirmed issue
難易度 2/5 1〜3時間 初心者へのやさしさ 75/100
open-webui/open-webui#30750 · コメント 1 件 ·
-
難易度 2/5 1〜3時間 初心者へのやさしさ 75/100
-
難易度 2/5 1〜3時間 初心者へのやさしさ 70/100
-
難易度 2/5 1〜3時間 初心者へのやさしさ 75/100
-
Mend: dependency security vulnerability untriaged
難易度 2/5 1〜3時間 初心者へのやさしさ 70/100