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

imprv: Initialization for Passport strategies #9353

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

yuki-takei
Copy link
Member

@yuki-takei yuki-takei commented Nov 1, 2024

経緯 on Slack

Yu
Friday at 08:02
@yuki
大変お世話になっております。
本日 7.0.23 から 7.1.0 にアップグレードしたところ、OIDC 設定 が「セットアップが完了していません。」に戻っておりました。
何らかの理由で未設定状態に戻ることはあるでしょうか?
当方、普段は OIDC のみを有効にしているため、認証プロバイダとの接続が外れてしまうとログインできなくなってしまいます。
前バージョン中でログインしていたマシンから OIDC 設定の「更新」ボタンを押すことでセットアップが完了し、
再度 OIDC 認証できるようになりましたが、何か対策がありましたらご教授いただけると助かります。
(CLIからID/PASSを有効にしたりできたりしますか?)
設定が外れた環境は、docker v20.10.23 (Synology NAS/Container Manager)です。

yuki
Saturday at 05:09
v7.1 で特にそのあたりに変更は加えていないため、勝手に設定が変更されるということは考えにくいです。
ただ初期化時のソースを眺めてみて、疑わしい箇所はありました。

yuki
Saturday at 05:09
この両方で await していないので
https://github.com/weseek/growi/blob/master/apps/app/src/server/crowi/index.js#L385-L390
https://github.com/weseek/growi/blob/master/apps/app/src/server/service/passport.ts#L200
結果として初期化が完了しているフラグが true になるタイミングがかなり遅くなることがありそうです。
https://github.com/weseek/growi/blob/master/apps/app/src/server/service/passport.ts#L639

yuki
Saturday at 05:09
もしくはなんらかの外的原因で同メソッドで試行している OIDCIssuer.discover() に失敗したような時でも、同フラグが true にならないです。

yuki
Saturday at 05:10
そのため、「設定は生きているが、起動時にセットアップが完了していないように見えた」ということは起こり得ると思います。
画面からはそこの区別はつかないので申し訳ないです。ログを洗うと特定できるかもしれません。
仮に今書いたような憶測通りの症状が再度起こった場合は、特に設定を変更せずサーバーを再起動してみるだけで回避できると思います。 (edited)

Copy link

changeset-bot bot commented Nov 1, 2024

⚠️ No Changeset found

Latest commit: 4e16200

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

Successfully merging this pull request may close these issues.

1 participant