Add DisableEmailPasswordLogin setting for Microsoft-only sign-in#230
Conversation
When enabled, admins can't set a password on new accounts and the Login handler rejects any email/password attempt outright.
📝 WalkthroughWalkthroughChangesPassword login control
Estimated code review effort: 3 (Moderate) | ~20 minutes Suggested labels: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://cdn.github.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@SW.Bitween.Api/Resources/Accounts/Create.cs`:
- Around line 35-42: Update the Account domain property and constructor contract
to use nullable string types for Password, allowing Create to pass null when
DisableEmailPasswordLogin is enabled. Preserve existing SetPassword and Login
behavior for accounts with hashes, and add the required migration separately if
deployed schemas still enforce a non-null Accounts.Password column.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository: simplify9/coderabbit/.coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: bff89896-4832-4333-9c72-96bcf158748d
📒 Files selected for processing (4)
SW.Bitween.Api/Resources/Accounts/Create.csSW.Bitween.Api/Resources/Accounts/Login.csSW.Bitween.Api/Resources/Settings/Config.csSW.Bitween.Api/Services/BitweenOptions.cs
📜 Review details
🔇 Additional comments (4)
SW.Bitween.Api/Services/BitweenOptions.cs (1)
48-55: LGTM!SW.Bitween.Api/Resources/Settings/Config.cs (1)
26-26: LGTM!SW.Bitween.Api/Resources/Accounts/Login.cs (1)
64-70: LGTM!SW.Bitween.Api/Resources/Accounts/Create.cs (1)
53-57: 🩺 Stability & AvailabilityEnsure nested
Validatevalidators can resolve through DI.
SW.CqApiregisters validators from the handler assembly, butprivate class Validate : AbstractValidator<...>constructors still need to be resolvable.Accounts/Create.cs:53requiresBitweenOptions; if DI resolution is not used here, account creation validation will fail before command handling. Add/confirm DI registration and a validation-path test for create-account requests.
| var password = _bitweenOptions.DisableEmailPasswordLogin | ||
| ? null | ||
| : SecurePasswordHasher.Hash(request.Password); | ||
|
|
||
| var newAccount = new Account( | ||
| request.Name, | ||
| request.Email, | ||
| SecurePasswordHasher.Hash(request.Password), | ||
| password, |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
rg -n -C3 '<Nullable>|Password|IsRequired' \
--glob '*.csproj' --glob '*.cs' --glob '*.sql' SW.Bitween.ApiRepository: simplify9/Bitween-api
Length of output: 29759
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf 'Project files:\n'
git ls-files '*.*proj' | sed -n '1,80p'
printf '\nSW.Bitween.Api.csproj:\n'
cat -n SW.Bitween.Api/SW.Bitween.Api.csproj
printf '\nAccount.cs:\n'
cat -n SW.Bitween.Api/Domain/Accounts/Account.cs | sed -n '1,80p'
printf '\nDbContext Account mapping relevant lines:\n'
cat -n SW.Bitween.Api/Data/BitweenDbContext.cs | sed -n '330,375p'
printf '\nMigrations mentioning Password field/column/constraint:\n'
git ls-files | grep -E '(^|/Migrations/.*\.cs$|\.sql$|\.md$)' | xargs rg -n -i 'password|account' --glob '*.cs' --glob '*.sql' --glob '*.md' | sed -n '1,220p'Repository: simplify9/Bitween-api
Length of output: 47971
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf 'Project files:\n'
git ls-files '*.*proj' | sed -n '1,80p'
printf '\nSW.Bitween.Api.csproj:\n'
cat -n SW.Bitween.Api/SW.Bitween.Api.csproj
printf '\nAccount.cs:\n'
cat -n SW.Bitween.Api/Domain/Accounts/Account.cs | sed -n '1,80p'
printf '\nDbContext Account mapping relevant lines:\n'
cat -n SW.Bitween.Api/Data/BitweenDbContext.cs | sed -n '330,375p'
printf '\nMigrations mentioning Password field/column/constraint:'
git ls-files | grep -E '(^|/Migrations/.*\.cs$|\.sql$|\.md$)' | xargs rg -n -i 'password|account' --glob '*.cs' --glob '*.sql' --glob '*.md' | sed -n '1,220p'Repository: simplify9/Bitween-api
Length of output: 47903
Make Password nullable before passing null into Account.
Account’s constructor and Password are non-nullable string. EF mapping leaves Accounts.Password nullable, but the model still allows null while other paths like SetPassword/Login still expect a hash. Change the domain property/constructor contract to string? and add an out-of-band migration if deployed schemas don’t allow null yet.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@SW.Bitween.Api/Resources/Accounts/Create.cs` around lines 35 - 42, Update the
Account domain property and constructor contract to use nullable string types
for Password, allowing Create to pass null when DisableEmailPasswordLogin is
enabled. Preserve existing SetPassword and Login behavior for accounts with
hashes, and add the required migration separately if deployed schemas still
enforce a non-null Accounts.Password column.
2 participants
When enabled, admins can't set a password on new accounts and the Login handler rejects any email/password attempt outright.