feat(auth): support built-in OAuth methods#439
Conversation
yordis
commented
Jul 15, 2026
yordis
commented
- PostgreSQL's method-oriented authentication model gives operators a familiar way to reason about password and OAuth access side by side.
- Modern deployments need external IdP bearer-token authentication without making database auth depend on plugin-only extension points.
- Password authentication should remain available while OAuth is introduced so operators can migrate deliberately.
Signed-off-by: Yordis Prieto <yordis.prieto@gmail.com>
PR SummaryHigh Risk Overview Introduces a built-in OAuth stack: OIDC/JWT Bearer validation ( The admin UI gains a PKCE authorization-code browser flow (challenge/callback endpoints, protected PKCE cookie, token exchange), stores the access token in an Configuration, telemetry, and startup validation now key off authentication methods (built-in user store for password/OAuth-only setups, default-user password rules) instead of a single Reviewed by Cursor Bugbot for commit 0a09f35. Bugbot is set up for automated code reviews on this repo. Configure here. |
|
Warning Review limit reached
Next review available in: 28 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (3)
WalkthroughAuthentication now supports multiple configured methods, composite provider routing, OAuth JWT validation, and an OAuth PKCE browser sign-in flow. Configuration, middleware challenges, telemetry, cookies, UI rendering, startup wiring, and automated tests were updated. ChangesAuthentication and OAuth integration
Validation coverage
Estimated code review effort: 4 (Complex) | ~60 minutes Possibly related PRs
Sequence Diagram(s)sequenceDiagram
participant SignInPage
participant OAuthBrowserFlowService
participant OAuthProvider
participant UiCredentialCookie
SignInPage->>OAuthBrowserFlowService: request PKCE challenge
OAuthBrowserFlowService-->>SignInPage: challenge and correlation state
SignInPage->>OAuthProvider: browser authorization
OAuthProvider->>OAuthBrowserFlowService: callback with code and state
OAuthBrowserFlowService->>OAuthProvider: exchange authorization code
OAuthBrowserFlowService->>UiCredentialCookie: append OAuth token
OAuthBrowserFlowService-->>SignInPage: redirect to /ui/signin
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
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 |
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
Signed-off-by: Yordis Prieto <yordis.prieto@gmail.com>
Signed-off-by: Yordis Prieto <yordis.prieto@gmail.com>
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/EventStore.ClusterNode/Components/Pages/SignIn.razor (1)
52-88: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winPrevent duplicate error messages when multiple auth methods are supported.
Because
Messageis conditionally rendered inside both theAuthentication.SupportsOAuthBrowserFlowandAuthentication.SupportsBasicblocks, an error (such as invalid credentials or a failed OAuth flow) will be displayed twice on the screen if both methods are enabled.Consider lifting the message display above the individual authentication sections so it is rendered only once.
🎨 Proposed UI fix
else { + `@if` (!string.IsNullOrWhiteSpace(Message)) + { + <div class="mb-6 rounded-2xl border border-red-200 bg-red-50 p-4 text-sm font-bold text-red-800">`@Message`</div> + } + `@if` (Authentication.SupportsOAuthBrowserFlow) { <div class="mb-6 rounded-[1.5rem] border border-es-ink/10 bg-es-cloud/70 p-5"> <h2 class="text-2xl font-black tracking-tight text-es-ink">Continue with OAuth</h2> <p class="mt-2 text-sm leading-6 text-es-muted">Use the configured browser sign-in flow for this node.</p> <button class="mt-4 rounded-2xl bg-es-ink px-5 py-3 text-sm font-black text-white shadow-lg shadow-es-ink/15 transition hover:bg-es-green" type="button" data-ui-oauth-signin data-ui-oauth-return="`@Destination`" data-ui-oauth-type="`@Authentication.Type`" data-ui-oauth-properties="`@Authentication.PropertiesJson`">Continue with OAuth</button> - `@if` (!string.IsNullOrWhiteSpace(Message)) - { - <p class="mt-4 rounded-2xl border border-red-200 bg-red-50 p-4 text-sm font-bold text-red-800">`@Message`</p> - } <p class="mt-4 hidden rounded-2xl border border-red-200 bg-red-50 p-4 text-sm font-bold text-red-800" data-ui-oauth-status></p> </div> } `@if` (Authentication.SupportsBasic) { <EditForm Model="Input" FormName="ui-signin" OnValidSubmit="SignInUser"> <AntiforgeryToken /> <DataAnnotationsValidator /> <div class="grid gap-4"> <label class="text-sm font-bold text-es-muted"> Username <InputText autocomplete="username" autofocus class="mt-2 w-full rounded-2xl border border-es-ink/10 bg-white px-4 py-3 text-es-ink outline-none transition focus:border-es-green focus:ring-4 focus:ring-es-green/15" `@bind-Value`="Input.Username" /> <ValidationMessage For="() => Input.Username" /> </label> <label class="text-sm font-bold text-es-muted"> Password <InputText autocomplete="current-password" type="password" class="mt-2 w-full rounded-2xl border border-es-ink/10 bg-white px-4 py-3 text-es-ink outline-none transition focus:border-es-green focus:ring-4 focus:ring-es-green/15" `@bind-Value`="Input.Password" /> <ValidationMessage For="() => Input.Password" /> </label> </div> - `@if` (!string.IsNullOrWhiteSpace(Message)) - { - <p class="mt-5 rounded-2xl border border-red-200 bg-red-50 p-4 text-sm font-bold text-red-800">`@Message`</p> - } - <div class="mt-6 flex flex-wrap gap-2">🤖 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 `@src/EventStore.ClusterNode/Components/Pages/SignIn.razor` around lines 52 - 88, Move the shared Message display out of the Authentication.SupportsOAuthBrowserFlow and Authentication.SupportsBasic sections and render it once above those authentication method blocks. Preserve the existing conditional visibility and styling, while removing both duplicated inline Message elements.
🧹 Nitpick comments (3)
src/EventStore.Core/Authentication/CompositeAuthenticationProvider.cs (2)
34-35: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valuePrevent potential duplicate keys in aggregated public properties.
If multiple underlying providers return the same property key,
SelectManywill yield duplicates. If downstream consumers load these properties into aDictionary, this will throw anArgumentException. Consider filtering by distinct keys to make the aggregate sequence safer.♻️ Proposed refactor
public override IEnumerable<KeyValuePair<string, string>> GetPublicProperties() => - providers.SelectMany(provider => provider.GetPublicProperties() ?? []); + providers + .SelectMany(provider => provider.GetPublicProperties() ?? []) + .DistinctBy(p => p.Key);🤖 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 `@src/EventStore.Core/Authentication/CompositeAuthenticationProvider.cs` around lines 34 - 35, Update CompositeAuthenticationProvider.GetPublicProperties to deduplicate aggregated properties by their key after flattening provider results. Preserve each key’s first property value and continue ignoring null property collections, ensuring downstream consumers receive unique keys.
67-75: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueHardcoded scheme selection limits extensibility.
The
SelectSchememethod explicitly binds the composite provider routing toBearer,UserCertificate, andBasic. While this covers the immediate built-in methods, it makes the composite provider rigid and unaware of additional future schemes (such as those introduced by external plugins).Consider mapping the intended scheme dynamically from the
AuthenticationRequestif possible, so that routing adheres better to the Open-Closed Principle for future auth types.🤖 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 `@src/EventStore.Core/Authentication/CompositeAuthenticationProvider.cs` around lines 67 - 75, Update SelectScheme to derive the authentication scheme from AuthenticationRequest or the existing extensible scheme-resolution mechanism instead of hardcoding Bearer, UserCertificate, and Basic. Preserve the current built-in routing behavior and fallback while allowing externally registered authentication schemes to be selected without modifying this method.src/EventStore.Core.Tests/EventStore.Core.Tests.csproj (1)
29-29: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚖️ Poor tradeoffConsider avoiding upward dependencies in test projects.
Adding a project reference from
EventStore.Core.TeststoEventStore.ClusterNodebreaks the dependency direction (Core$\rightarrow$ ClusterNode).If this reference is needed to test
ClusterNodecomponents (such as the newly added browser flow endpoints), consider moving those tests to anEventStore.ClusterNode.Testsproject to preserve clear architectural boundaries.🤖 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 `@src/EventStore.Core.Tests/EventStore.Core.Tests.csproj` at line 29, The EventStore.Core.Tests project now depends upward on EventStore.ClusterNode. Remove the EventStore.ClusterNode project reference from EventStore.Core.Tests and move the ClusterNode-specific tests, including browser flow endpoint coverage, into an EventStore.ClusterNode.Tests project.
🤖 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 `@src/EventStore.ClusterNode/Components/Services/OAuthBrowserFlowEndpoints.cs`:
- Around line 42-54: Replace the unbounded _pendingChallenges state used by
CreateCodeChallenge with a short-lived encrypted browser cookie containing the
PKCE verifier, and recover and validate it during the OAuth callback. Remove the
per-request PruneExpired iteration and correlationId dictionary dependency,
ensuring cookie settings bind the challenge to the user agent and prevent Login
CSRF. If server-side storage is retained, use a bounded IMemoryCache with
expiration and require an explicit correlation cookie during callback
validation.
In `@src/EventStore.ClusterNode/Components/Services/SecurityBrowserService.cs`:
- Around line 28-32: Complete the OAuth browser-flow property contract in
SecurityBrowserService by requiring response_type and scope alongside the
existing properties before setting SupportsOAuthBrowserFlow. In
src/EventStore.ClusterNode/ui-assets/js/ui-auth.js lines 106-108, update the
flow-start validation to require code_challenge_uri, redirect_uri,
response_type, and scope before proceeding.
In `@src/EventStore.ClusterNode/Program.cs`:
- Line 297: Update the OAuthBrowserFlowService registration in Program so its
HttpClient uses a SocketsHttpHandler configured with an appropriate
PooledConnectionLifetime instead of an unmanaged new HttpClient(). Preserve the
existing OAuth options and TimeProvider.System arguments.
In `@src/EventStore.Core/Authentication/AuthenticationMethodNames.cs`:
- Around line 32-33: Update AuthenticationMethodNames.Normalize to handle a null
method before calling Trim, returning the appropriate normalized fallback
without throwing. Preserve the existing IsLegacyInternal behavior and
normalization for non-null methods.
- Around line 13-19: Update FromOptions to null-coalesce options.Methods to an
empty array before the LINQ Where call, preserving the existing filtering,
normalization, and deduplication behavior when methods are provided.
---
Outside diff comments:
In `@src/EventStore.ClusterNode/Components/Pages/SignIn.razor`:
- Around line 52-88: Move the shared Message display out of the
Authentication.SupportsOAuthBrowserFlow and Authentication.SupportsBasic
sections and render it once above those authentication method blocks. Preserve
the existing conditional visibility and styling, while removing both duplicated
inline Message elements.
---
Nitpick comments:
In `@src/EventStore.Core.Tests/EventStore.Core.Tests.csproj`:
- Line 29: The EventStore.Core.Tests project now depends upward on
EventStore.ClusterNode. Remove the EventStore.ClusterNode project reference from
EventStore.Core.Tests and move the ClusterNode-specific tests, including browser
flow endpoint coverage, into an EventStore.ClusterNode.Tests project.
In `@src/EventStore.Core/Authentication/CompositeAuthenticationProvider.cs`:
- Around line 34-35: Update CompositeAuthenticationProvider.GetPublicProperties
to deduplicate aggregated properties by their key after flattening provider
results. Preserve each key’s first property value and continue ignoring null
property collections, ensuring downstream consumers receive unique keys.
- Around line 67-75: Update SelectScheme to derive the authentication scheme
from AuthenticationRequest or the existing extensible scheme-resolution
mechanism instead of hardcoding Bearer, UserCertificate, and Basic. Preserve the
current built-in routing behavior and fallback while allowing externally
registered authentication schemes to be selected without modifying this method.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 6b104ea7-2516-47e1-84e1-ff04932deea7
📒 Files selected for processing (27)
src/Directory.Packages.propssrc/EventStore.ClusterNode/ClusterVNodeHostedService.cssrc/EventStore.ClusterNode/Components/Pages/SignIn.razorsrc/EventStore.ClusterNode/Components/Services/ConfigurationBrowserService.cssrc/EventStore.ClusterNode/Components/Services/OAuthBrowserFlowEndpoints.cssrc/EventStore.ClusterNode/Components/Services/SecurityBrowserService.cssrc/EventStore.ClusterNode/Components/Services/UiCredentialCookie.cssrc/EventStore.ClusterNode/Program.cssrc/EventStore.ClusterNode/ui-assets/js/ui-auth.jssrc/EventStore.Core.Tests/Authentication/CompositeAuthenticationProviderTests.cssrc/EventStore.Core.Tests/Authentication/OAuthAuthenticationProviderTests.cssrc/EventStore.Core.Tests/Authentication/OAuthBrowserFlowServiceTests.cssrc/EventStore.Core.Tests/EventStore.Core.Tests.csprojsrc/EventStore.Core.Tests/Services/Transport/Http/Authentication/authentication_middleware_should.cssrc/EventStore.Core.XUnit.Tests/Authentication/AuthenticationMethodNamesTests.cssrc/EventStore.Core/Authentication/AuthenticationMethodNames.cssrc/EventStore.Core/Authentication/CompositeAuthenticationProvider.cssrc/EventStore.Core/Authentication/CompositeAuthenticationProviderFactory.cssrc/EventStore.Core/Authentication/OAuth/OAuthAuthenticationProvider.cssrc/EventStore.Core/Authentication/OAuth/OAuthAuthenticationProviderFactory.cssrc/EventStore.Core/ClusterVNode.cssrc/EventStore.Core/Configuration/ClusterVNodeOptions.cssrc/EventStore.Core/Configuration/ClusterVNodeOptionsValidator.cssrc/EventStore.Core/EventStore.Core.csprojsrc/EventStore.Core/Services/Transport/Http/AuthenticationMiddleware.cssrc/EventStore.Core/Telemetry/TelemetryService.cssrc/EventStore.Core/Util/Opts.cs
💤 Files with no reviewable changes (1)
- src/EventStore.Core/Util/Opts.cs
Signed-off-by: Yordis Prieto <yordis.prieto@gmail.com>
Signed-off-by: Yordis Prieto <yordis.prieto@gmail.com>
Signed-off-by: Yordis Prieto <yordis.prieto@gmail.com>
Signed-off-by: Yordis Prieto <yordis.prieto@gmail.com>
Signed-off-by: Yordis Prieto <yordis.prieto@gmail.com>
Signed-off-by: Yordis Prieto <yordis.prieto@gmail.com>
Signed-off-by: Yordis Prieto <yordis.prieto@gmail.com>
There was a problem hiding this comment.
🧹 Nitpick comments (2)
src/EventStore.Core.Tests/Authentication/CompositeAuthenticationProviderTests.cs (1)
54-54: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winDispose the generated test certificate.
X509Certificate2encapsulates unmanaged resources. To prevent handle leaks and memory exhaustion in the test runner, ensure the certificate is properly disposed after the request is created.♻️ Proposed fix
- var request = HttpAuthenticationRequest.CreateWithValidCertificate(context, "admin", CreateCertificate()); + using var certificate = CreateCertificate(); + var request = HttpAuthenticationRequest.CreateWithValidCertificate(context, "admin", certificate);🤖 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 `@src/EventStore.Core.Tests/Authentication/CompositeAuthenticationProviderTests.cs` at line 54, Update the test around HttpAuthenticationRequest.CreateWithValidCertificate and CreateCertificate so the generated X509Certificate2 is disposed after the request is created, using scoped disposal while preserving the existing request setup.src/EventStore.Core.Tests/Authentication/OAuthBrowserFlowServiceTests.cs (1)
26-26: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueDispose
OAuthBrowserFlowServiceinstances.The
OAuthBrowserFlowServiceimplementsIDisposable(and manages anHttpClient). To prevent resource exhaustion in test environments, ensure that these instances are cleanly disposed of by making themusingvariables.
src/EventStore.Core.Tests/Authentication/OAuthBrowserFlowServiceTests.cs#L26-L26: change tousing var service = Service(new TokenHandler());src/EventStore.Core.Tests/Authentication/OAuthBrowserFlowServiceTests.cs#L45-L45: change tousing var service = Service(handler);src/EventStore.Core.Tests/Authentication/OAuthBrowserFlowServiceTests.cs#L72-L72: change tousing var service = Service(handler);src/EventStore.Core.Tests/Authentication/OAuthBrowserFlowServiceTests.cs#L93-L93: change tousing var service = Service(handler);src/EventStore.Core.Tests/Authentication/OAuthBrowserFlowServiceTests.cs#L117-L117: change tousing var service = Service(handler);src/EventStore.Core.Tests/Authentication/OAuthBrowserFlowServiceTests.cs#L141-L141: change tousing var service = Service(handler);🤖 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 `@src/EventStore.Core.Tests/Authentication/OAuthBrowserFlowServiceTests.cs` at line 26, OAuthBrowserFlowService test instances are not being disposed. In src/EventStore.Core.Tests/Authentication/OAuthBrowserFlowServiceTests.cs at lines 26, 45, 72, 93, 117, and 141, declare each Service(...) result as a using variable so every OAuthBrowserFlowService is disposed at the end of its test.
🤖 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.
Nitpick comments:
In
`@src/EventStore.Core.Tests/Authentication/CompositeAuthenticationProviderTests.cs`:
- Line 54: Update the test around
HttpAuthenticationRequest.CreateWithValidCertificate and CreateCertificate so
the generated X509Certificate2 is disposed after the request is created, using
scoped disposal while preserving the existing request setup.
In `@src/EventStore.Core.Tests/Authentication/OAuthBrowserFlowServiceTests.cs`:
- Line 26: OAuthBrowserFlowService test instances are not being disposed. In
src/EventStore.Core.Tests/Authentication/OAuthBrowserFlowServiceTests.cs at
lines 26, 45, 72, 93, 117, and 141, declare each Service(...) result as a using
variable so every OAuthBrowserFlowService is disposed at the end of its test.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: dcbda19c-e113-426f-a627-0d31aaf46c7c
📒 Files selected for processing (16)
src/EventStore.ClusterNode/ClusterVNodeHostedService.cssrc/EventStore.ClusterNode/Components/Services/OAuthBrowserFlowEndpoints.cssrc/EventStore.ClusterNode/Components/Services/SecurityBrowserService.cssrc/EventStore.ClusterNode/Program.cssrc/EventStore.ClusterNode/ui-assets/js/ui-auth.jssrc/EventStore.Core.Tests/Authentication/CompositeAuthenticationProviderTests.cssrc/EventStore.Core.Tests/Authentication/OAuthAuthenticationProviderTests.cssrc/EventStore.Core.Tests/Authentication/OAuthBrowserFlowServiceTests.cssrc/EventStore.Core.Tests/Authentication/SecurityBrowserServiceTests.cssrc/EventStore.Core.Tests/EventStore.Core.Tests.csprojsrc/EventStore.Core.XUnit.Tests/Authentication/AuthenticationMethodNamesTests.cssrc/EventStore.Core/Authentication/AuthenticationMethodNames.cssrc/EventStore.Core/Authentication/OAuth/OAuthAuthenticationProvider.cssrc/EventStore.Core/Authentication/UserCertificateAuthenticationProvider.cssrc/EventStore.Core/Authentication/UserCertificateAuthenticationProviderFactory.cssrc/EventStore.Core/Configuration/ClusterVNodeOptions.cs
🚧 Files skipped from review as they are similar to previous changes (8)
- src/EventStore.Core.XUnit.Tests/Authentication/AuthenticationMethodNamesTests.cs
- src/EventStore.ClusterNode/ClusterVNodeHostedService.cs
- src/EventStore.Core/Authentication/AuthenticationMethodNames.cs
- src/EventStore.Core.Tests/EventStore.Core.Tests.csproj
- src/EventStore.ClusterNode/Program.cs
- src/EventStore.Core/Configuration/ClusterVNodeOptions.cs
- src/EventStore.Core/Authentication/OAuth/OAuthAuthenticationProvider.cs
- src/EventStore.ClusterNode/ui-assets/js/ui-auth.js
Signed-off-by: Yordis Prieto <yordis.prieto@gmail.com>
Signed-off-by: Yordis Prieto <yordis.prieto@gmail.com>
Signed-off-by: Yordis Prieto <yordis.prieto@gmail.com>
|
Caution Failed to replace (edit) comment. This is likely due to insufficient permissions or the comment being deleted. Error details |
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/EventStore.Core/Configuration/ClusterVNodeOptionsValidator.cs (1)
153-164: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winValidate default passwords based on password authentication, not the built-in user store.
IncludesBuiltInUserStoreis also true for OAuth-only configurations, so custom default admin/ops passwords are accepted even though password authentication is disabled and those passwords cannot be used. UseIncludesPasswordhere; retainIncludesBuiltInUserStorefor user-management visibility.Proposed fix
- var needsDefaultUsers = AuthenticationMethodNames.IncludesBuiltInUserStore(options.Auth); - if (options.Application.AuthDisabled() || !needsDefaultUsers) + var passwordAuthenticationEnabled = AuthenticationMethodNames.IncludesPassword(options.Auth); + if (options.Application.AuthDisabled() || !passwordAuthenticationEnabled)🤖 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 `@src/EventStore.Core/Configuration/ClusterVNodeOptionsValidator.cs` around lines 153 - 164, Update the default admin/ops password validation in ClusterVNodeOptionsValidator to determine needsDefaultUsers with AuthenticationMethodNames.IncludesPassword(options.Auth) instead of IncludesBuiltInUserStore. Preserve IncludesBuiltInUserStore for user-management visibility elsewhere.
🤖 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.
Outside diff comments:
In `@src/EventStore.Core/Configuration/ClusterVNodeOptionsValidator.cs`:
- Around line 153-164: Update the default admin/ops password validation in
ClusterVNodeOptionsValidator to determine needsDefaultUsers with
AuthenticationMethodNames.IncludesPassword(options.Auth) instead of
IncludesBuiltInUserStore. Preserve IncludesBuiltInUserStore for user-management
visibility elsewhere.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 1128c4fa-63ca-470c-89c9-51fae0ec9436
📒 Files selected for processing (11)
src/EventStore.ClusterNode/Components/Services/ConfigurationBrowserService.cssrc/EventStore.ClusterNode/Components/Services/SecurityBrowserService.cssrc/EventStore.ClusterNode/Program.cssrc/EventStore.Core.Tests/Authentication/CompositeAuthenticationProviderTests.cssrc/EventStore.Core.Tests/Authentication/SecurityBrowserServiceTests.cssrc/EventStore.Core.XUnit.Tests/Authentication/AuthenticationMethodNamesTests.cssrc/EventStore.Core.XUnit.Tests/Configuration/ClusterVNodeOptionsValidatorTests.cssrc/EventStore.Core/Authentication/AuthenticationMethodNames.cssrc/EventStore.Core/Authentication/UserCertificateAuthenticationProvider.cssrc/EventStore.Core/ClusterVNode.cssrc/EventStore.Core/Configuration/ClusterVNodeOptionsValidator.cs
🚧 Files skipped from review as they are similar to previous changes (3)
- src/EventStore.Core/Authentication/UserCertificateAuthenticationProvider.cs
- src/EventStore.Core/Authentication/AuthenticationMethodNames.cs
- src/EventStore.ClusterNode/Program.cs
Signed-off-by: Yordis Prieto <yordis.prieto@gmail.com>
Signed-off-by: Yordis Prieto <yordis.prieto@gmail.com>
yordis
force-pushed
the
yordis/feat-built-in-oauth-authentication
branch
from
e404de6 to
675962f
Compare
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes using default effort and found 1 potential issue.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, have a team admin enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 675962f. Configure here.
Signed-off-by: Yordis Prieto <yordis.prieto@gmail.com>
yordis
force-pushed
the
yordis/feat-built-in-oauth-authentication
branch
from
675962f to
0a09f35
Compare
1 participant
