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

IdP の機能分離 #41

Merged
merged 11 commits into from
Nov 12, 2024
Merged

IdP の機能分離 #41

merged 11 commits into from
Nov 12, 2024

Conversation

a01sa01to
Copy link
Member

No description provided.

@a01sa01to a01sa01to self-assigned this Nov 10, 2024
@a01sa01to a01sa01to requested a review from sor4chi November 10, 2024 06:47
Copy link

cloudflare-workers-and-pages bot commented Nov 10, 2024

Deploying saitamau-maximum-auth with  Cloudflare Pages  Cloudflare Pages

Latest commit: dc57f91
Status: ✅  Deploy successful!
Preview URL: https://8b7c9e31.saitamau-maximum-auth.pages.dev
Branch Preview URL: https://separate-idp.saitamau-maximum-auth.pages.dev

View logs

@sor4chi
Copy link
Member

sor4chi commented Nov 10, 2024

機能分離???

@sor4chi
Copy link
Member

sor4chi commented Nov 11, 2024

なんかcamelだった変数がsnakeになってたりで色々間違えてたりしない?

@a01sa01to
Copy link
Member Author

a01sa01to commented Nov 11, 2024

機能分離???

これはそもそもoauth機能とaccount機能を一緒に置いてるのが悪くて、基本的に別システムなのでパッケージ or サービス分けるくらいにしたほうが良さそう (#39)

これって IdP と OAuth の分離しろってことじゃないんかい

なんかcamelだった変数がsnakeになってたりで色々間違えてたりしない?

snake になってるのは DB 読み出しの処理なのでどうしようもなく...

@sor4chi
Copy link
Member

sor4chi commented Nov 11, 2024

これって IdP と OAuth の分離しろってことじゃないんかい

普通に別パッケージ別サービスとして分けるイメージだった
これだと結局Remix配下に共存するっていう問題点は解決できてなくない?

@sor4chi
Copy link
Member

sor4chi commented Nov 11, 2024

snake になってるのは DB 読み出しの処理なのでどうしようもなく...

OK

@a01sa01to
Copy link
Member Author

普通に別パッケージ別サービスとして分けるイメージだった これだと結局Remix配下に共存するっていう問題点は解決できてなくない?

もう auth 側は Hono only でやって (たぶん Pages から Workers になりそう)、 id 側は Remix only っていう認識だった

@sor4chi
Copy link
Member

sor4chi commented Nov 11, 2024

あーそういうことか
俺はOAuth + ID (1Worker)、ID UI (Remix)だったww

@sor4chi
Copy link
Member

sor4chi commented Nov 11, 2024

どっちがやりやすい?

@sor4chi
Copy link
Member

sor4chi commented Nov 11, 2024

まあ3 workerに分けるのも一つ手だけど

@sor4chi
Copy link
Member

sor4chi commented Nov 11, 2024

ちなみに俺はRemixからDBを呼びたくない

@a01sa01to
Copy link
Member Author

a01sa01to commented Nov 11, 2024

今思ってるのは OAuth (Hono, 1 Workers) + IdP (Remix, 1 Pages) でこれなら割とすぐできそう (だと勝手に思ってる) んだけど、

まあ3 workerに分けるのも一つ手だけど

何をどう 3 つに分ける想定?

ちなみに俺はRemixからDBを呼びたくない

これの理由を聞きたい

@sor4chi
Copy link
Member

sor4chi commented Nov 11, 2024

何をどう 3 つに分ける想定?

ID (API Server), OAuth (API Server), ID UI(BFF)

これの理由を聞きたい

そもそもUIサーバー(BFF)から直接DBを叩くのはマイクロサービスシステムとしてはアンチパターンという認識

@a01sa01to
Copy link
Member Author

なるほど とりあえず OAuth は Workers だけでよさそうね
よくよく考えると外部から IdP API 呼ぶ場面も出てきそうなので Remix only は無理か
個人的には IdP UI と API でドメインは分けたくない (id.maximum.vc/api 下とかで配信したい どこの API なのか一目で分かったほうが嬉しい + 別々だと管理・引継ぎコスト上がりそう?) ので IdP 側で Hono Remix Adapter とかするのが良い?

@sor4chi
Copy link
Member

sor4chi commented Nov 11, 2024

個人的には IdP UI と API でドメインは分けたくない (id.maximum.vc/api 下とかで配信したい

これは色々やり方あると思うけど、普通にRemix側のPagesのmiddlewareからapi workersへproxyでいい気がしてる
それか、Workers RPCを使って内部通信する方法もある(これはベンダーロックインになりそうだけど)

どこの API なのか一目で分かったほうが嬉しい + 別々だと管理・引継ぎコスト上がりそう?) ので IdP 側で Hono Remix Adapter とかするのが良い?

それでもいいし、UIとIdP APIをそれぞれprivate workers/pagesで作って、それをまとめ上げるもう1プロキシworkersあってもいい

@a01sa01to
Copy link
Member Author

@sor4chi Review plz

Copy link
Member

@sor4chi sor4chi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

とりあえず :yosaso:

色々突っ込んですまん

@a01sa01to a01sa01to merged commit 1715777 into main Nov 12, 2024
7 checks passed
@a01sa01to a01sa01to deleted the separate-idp branch November 12, 2024 13:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants