Skip to content
New issue

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

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

Already on GitHub? Sign in to your account

OAuthでclient credentials flowが利用可能なように #2403

Open
kyosu-1 opened this issue May 5, 2024 · 3 comments · May be fixed by #2433
Open

OAuthでclient credentials flowが利用可能なように #2403

kyosu-1 opened this issue May 5, 2024 · 3 comments · May be fixed by #2433
Labels
kind/enhancement 機能改善に関するもの

Comments

@kyosu-1
Copy link
Member

kyosu-1 commented May 5, 2024

middlewareでtokenからuserIDを取っているが、client credential flowだとuuid.Nilとなる。

newToken, err := h.Repo.IssueToken(client, uuid.Nil, client.RedirectURI, validScopes, h.AccessTokenExp, false)

user, err := repo.GetUser(uid, true)

ここでuidがuuid.Nilだと500エラーが返されてしまう

client credentials flowを利用するにはここら辺を改修する必要がある

なお、client credentials flowを利用するにはconfidentialなclientである必要があるので、#2402 の対応も必要

@kyosu-1 kyosu-1 added the kind/enhancement 機能改善に関するもの label May 5, 2024
@kyosu-1
Copy link
Member Author

kyosu-1 commented May 20, 2024

ユーザー取得をUserAuthenticateのミドルウェアで回避しても、その後のRBACでユーザーがcontextから取れることを前提としてユーザーの権限をチェックする部分がある。(なお、AccessControlMiddlewareGenerator は scope パラメータによるrbac とユーザー自身の持つrbacの両方をチェックしている)

user := c.Get(consts.KeyUser).(model.UserInfo)

このフローによるTokenを使ったアクセス時はユーザーの権限チェックを回避し、(ユーザー同意なしで)事前にclientとしてアクセス可能なリソースの範囲を定めた上で、その範囲内のアクセスかをチェックするのが良さそう

https://datatracker.ietf.org/doc/html/rfc6749#section-1.3.4 より

The client credentials (or other forms of client authentication) can be used as an authorization grant when the authorization scope is limited to the protected resources under the control of the client, or to protected resources previously arranged with the authorization server. Client credentials are used as an authorization grant typically when the client is acting on its own behalf (the client is also the resource owner) or is requesting access to protected resources based on an authorization previously arranged with the authorization server.

日本語(https://openid-foundation-japan.github.io/rfc6749.ja.html#anchor7)

認可範囲がクライアントの管理下にある保護されたリソース, もしくはあらかじめ認可サーバーとの間で調整済の保護されたリソースに制限されている場合は, 認可グラントとしてクライアントクレデンシャル (もしくは他のクライアント認証形式) が使用できる. クライアントがクライアント自身の権限においてに行動している (つまりクライアントがリソースオーナーである) か, クライアントがあらかじめ認可サーバーとの間で調整済の保護されたリソースアクセスを求めている場合, クライアントクレデンシャルが認可グラントとして使用される.

権限は
https://github.com/traPtitech/traQ/tree/efcd171bde834a5fac018f8124f34b58c3e36865/service/rbac/role
辺りでclient.goを作って、clientのロールを定義するイメージ

つまり、


でtoken.UserIDがnilだったらその後のユーザー取得などの部分をskipして

user := c.Get(consts.KeyUser).(model.UserInfo)

のところでconsts.KeyUser が取れなかったら、client credentials flowと判断して、client roleの範囲内の操作かをチェックする

もしくは明示的にユーザーに紐づいているかどうかに関するkeyをcontextに入れて、それをチェックする形でclient credentials flowであることを認識する形でも

なお、少なくとも

userID := getRequestUserID(c)

のようにhandler中でuser情報をcontextから取得するようなものはclient credentials flowで許可するのは不可能

@kyosu-1
Copy link
Member Author

kyosu-1 commented May 20, 2024

clinet credentials flowだと、アカウント失効されても引退者が現役時に作ったclientでリソース取得が可能になるというハックがありそう。引退時にそのユーザーが作成したOAuth clientは削除されるという運用をしていれば問題はない。

sysad系のサービスで使う場合は逆に途中で失効するとまずいけど、ユーザー失効のない@.traPアカウントで作成する方針になっていそうだから大丈夫そう

一応secretを見れていた人が引退することもあるので、年度替わりとかでsecret_idのローテーションを運用として行った方が良さそう

@kyosu-1
Copy link
Member Author

kyosu-1 commented May 20, 2024

client は
https://github.com/traPtitech/traQ/blob/master/service/rbac/role/read.go
のうち、GetMeとかGetOIDCUserInfoといった自分自身に関する情報取得以外の参照系は許可する形が良さそう。

@kyosu-1 kyosu-1 linked a pull request May 22, 2024 that will close this issue
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/enhancement 機能改善に関するもの
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant