✨ OPENID: Added option to create users on login #4421
Conversation
actual-github-bot
bot
changed the title
lelemm
changed the title
✅ Deploy Preview for actualbudget ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
lelemm
changed the title
WalkthroughThe changes introduce modifications to user authentication and configuration management within the application. The Suggested labels
Suggested reviewers
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
⏰ Context from checks skipped due to timeout of 90000ms (8)
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
packages/sync-server/src/accounts/openid.js (1)
223-223: Consider enforcing unique user_name constraint
The insertion query looks correct. For added data integrity, consider placing a unique constraint onuser_namein the database. This will prevent duplicates if any transaction concurrency occurs.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
upcoming-release-notes/4421.mdis excluded by!**/*.md
📒 Files selected for processing (3)
packages/sync-server/src/accounts/openid.js(3 hunks)packages/sync-server/src/config-types.ts(1 hunks)packages/sync-server/src/load-config.js(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: Build Docker image (alpine)
🔇 Additional comments (6)
packages/sync-server/src/accounts/openid.js (4)
196-197: Ensure final fallback sub remains stable
UsinguserInfo.subas the ultimate fallback is typically correct for a unique, provider-specific ID. However, please verify with your OpenID provider thatsubremains stable across sessions for the same user to avoid unexpected account duplication.
209-212: Good application of userCreationMode
This condition cleanly handles the newuserCreationModelogic, allowing automatic user creation upon login. No issues found.
228-229: Logical assignment for owner and role
Correctly assigns the first user asADMINand subsequent users asBASIC. This logic appears consistent with the new workflow.
233-237: Verify edge cases in file transfer
Transferring files from a user with an emptyuser_nameis logical for migrating previously password-created data. However, confirm that no other users share an emptyuser_nameto avoid unintended file transfers.packages/sync-server/src/config-types.ts (1)
43-43: New userCreationMode property
The optionaluserCreationMode?: 'manual' | 'login';property is well-defined and expands configuration possibilities.packages/sync-server/src/load-config.js (1)
224-226: Environment-based override for userCreationMode
Nicely allowsuserCreationModeto be configured via an environment variable, enabling flexible deployment setups.
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
packages/sync-server/src/load-config.js (1)
229-241: Add debug logging foruserCreationMode.For consistency with other configuration properties, consider adding debug logging for the new
userCreationModesetting.debug(`using login method ${finalConfig.loginMethod}`); debug(`using trusted proxies ${finalConfig.trustedProxies.join(', ')}`); +debug(`using user creation mode ${finalConfig.userCreationMode}`); debug( `using trusted auth proxies ${ finalConfig.trustedAuthProxies?.join(', ') ?? 'same as trusted proxies'
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
packages/sync-server/src/load-config.js(2 hunks)
🧰 Additional context used
🪛 ESLint
packages/sync-server/src/load-config.js
[error] 91-91: Insert ,
(prettier/prettier)
🪛 GitHub Check: lint
packages/sync-server/src/load-config.js
[failure] 91-91:
Insert ,
🪛 GitHub Actions: Test
packages/sync-server/src/load-config.js
[error] 91-91: Prettier formatting check failed: Insert ,.
⏰ Context from checks skipped due to timeout of 90000ms (7)
- GitHub Check: build (macos-latest)
- GitHub Check: build (windows-latest)
- GitHub Check: Wait for Netlify build to finish
- GitHub Check: Analyze
- GitHub Check: build (ubuntu-latest)
- GitHub Check: Build Docker image (alpine)
- GitHub Check: Build Docker image (ubuntu)
lelemm
changed the title
After feedback, some users using self hosted openid provider wants to be able to create users on login.
Added the variable
ACTUAL_USER_CREATION_MODEthat can bemanualorlogin.If set to
loginit will create user at the first login of that user. The created user will be aBASICuser.The default behavior is the
manual