Repository navigation
Conversation
ee3d4ec to
f174a57
Compare
f174a57 to
92ee222
Compare
92ee222 to
c33052c
Compare
c33052c to
de6dba7
Compare
de6dba7 to
ac8d008
Compare
ac8d008 to
9b46d71
Compare
9b46d71 to
eb0fab9
Compare
eb0fab9 to
6a6aa78
Compare
6a6aa78 to
c1a99bd
Compare
16e9543 to
1d13fe9
Compare
1d13fe9 to
5258916
Compare
38a41c7 to
77141f7
Compare
a7b95bf to
ea8d962
Compare
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: ea8d962b26
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 869c91d029
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: d99fc68df6
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 89d85141a2
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 217e1f87b3
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| return; | ||
|
|
||
| bool isFirstUser = await userRepository.CountAsync() == 0; | ||
| bool isFirstUser = await userRepository.CountAsync().WaitAsync(cancellationToken) == 0; |
There was a problem hiding this comment.
When the first account is created through OAuth and the request or 15-second admission deadline is canceled immediately after this count completes, AddGlobalAdminRoleIfFirstUserAsync has already set _isFirstUserChecked but CompleteExternalAccountAdmission aborts before the user is persisted. A subsequent successful signup in the same process then skips the check and creates the installation's first user without GlobalAdmin; after that, restarting cannot repair it because the count is nonzero. Publish or reset this process-wide flag only after the first user has been stored successfully.
Useful? React with 👍 / 👎.
|
Refresh an existing OAuth account's username and supplied profile data on linking and login, retaining one canonical entry for duplicate identities. Prevent concurrent sign-ins and transfers from creating duplicate owners or losing links. Native user version checks reject stale saves, including metadata refresh racing a password change.
Latest validation (
89d85141): backend test-project build passed with zero warnings/errors; 74 focused backend tests, 10 focused frontend auth tests, and frontend validation passed. The stale-email and OAuth 409 regressions failed before their fixes. CI at this head passed all four backend shards, frontend validation/tests, and image builds. Five browser shards passed; shard 1 reached a saved-view 404 and passed its automatic retry, so the flaky-test gate correctly failed the run. The trace shows a successful 201 create, an immediate 200 list omitting the new view, and a later list containing it. Saved-view creation/listing and their repository paths are unchanged from main and do not write user authentication state. This existing saved-view consistency race is outside this authentication change; the failed jobs are being rerun once. No test gate, timeout, or retry policy was changed.API behavior: stale user writes can return 409. Unlink accepts canonical provider names only (422 otherwise); blank subject IDs retain their existing 400 response and body contract.
Deployment note: pre-version cached users expire normally; writes using those old snapshots may fail until their cache TTL expires.
Microsoft email ownership: this PR retains the existing email-matching and email-verification policy. Graph profile email is not a mailbox-ownership assertion; draft #2687, stacked on this branch, documents the concern and separately proposes Microsoft checks while preserving signup, established-identity sign-in, and authenticated linking. That Microsoft-only implementation does not depend on OAuth2 #163.
Verification and implementation details
Reconciled merged Replace Windows Live authentication with Microsoft and remove Facebook login #2403 with the canonical Microsoft provider and unlink validation, retained legacy-provider cleanup and last-usable-login protection (including duplicate identities), and preserved the generic 403 endpoint description. The frontend displays the new 409 failures; password-reset cancellation stays on the page when it fails.
A fresh email-owner read must still match the provider address before attachment. A handler regression reproduced a successful login after the email changed between lookup and admission; the request now returns 401 without changing the account or issuing a token.
Earlier validation on
d99fc68df: 323 affected backend tests, frontend validation, and two browser runs without retries passed locally; full CI passed.Existing account refresh preserves the same instance, merges supplied data, retains unspecified keys, and removes legacy duplicates so stale provider email metadata cannot survive in another copy. Authenticated relinking and anonymous OAuth login both refresh metadata while preserving the user's application email/name. Unit and real HTTP regressions cover both paths.
Plain
IVersionedand its repository/serializer tests moved here from Protect user updates and clean up orphaned private views #2631 because OAuth refresh needs protection before that dependent PR lands. A native repository persistence hook performs a real password-change HTTP request before OAuth's save; both login modes return 409, preserve the new credentials, and retain a working replacement token. Elasticsearch supplies versions when legacy records are read; no migration, resave, cache repair, explicit interface wrapper, or user-specific serializer modifier is required. Existing cached records expire normally. See Foundatio versioning.The original one-parameter cached email helper remains unchanged. OAuth admission uses a native uncached query: a canceled cached lookup can finish later and publish a miss hiding an existing owner. The real stale-negative-cache regression requires unchanged user count and the stored owner's token. Late cache publication is tracked in Foundatio.Repositories #323.
Identity lookup returns all exact ordinal provider/subject matches, with native pagination and ordinary cancellation checks. The handler rejects ambiguous ownership under identity/email/user locks; the repository never throws an authentication exception. Exact pairing is necessary because Elasticsearch flattens ordinary object arrays. Lock acquisition and admission reads have a 15-second cancellation deadline; admitted persistence and token issuance complete inside the locks. Integration tests use native repositories and locks; unavailable-lock injection remains in unit tests.
Standard request validation rejects invalid providers. Blank subjects are handled directly by the handler; the custom unlink response filter is removed. OpenAPI snapshots cover 409 responses on affected user writes. Forgot-password consumes version conflicts internally, returns the unknown-email success response, and sends no uncommitted reset token. Repository single/bulk deletes do not apply version preconditions; their existing API contract is retained.
CI already runs npm ci. A failing AppHost topology regression proved that redundant installation was still automatic; CI mode now uses the supported installer-disable option, while development retains installation and both modes wait for the API. A separate browser failure clicked a loading skeleton. The actual scenario reproduced this with a delayed stack-list response, passed with a selectable-row locator, then passed twice with normal responses and retries disabled. Temporary fault injection was removed. Timeouts, retries, flaky-test gates, required checks, and shard discovery are unchanged.