refactor(common/auth): move login credentials to auth_provider

[Neilk1021]

What changes were proposed in this PR?

As discussed in #6716 we'd like to split authentication handling into its own table auth_provider. For future support of multiple sources of authentication (e.g. github, IEEE accounts, etc.). This PR migrate the Schema from the image on the left to the one on the right.

This PR:

1. Creates a new auth_provider table in texera_ddl.
2. Providers a migration script 33.sql to migrate old account data to the new schema.
3. Refactors backend to match new schema.
4. Refactors portions of GoogleAuthService to a generic ExternalAuthProvisioner to be used in future PRs adding new external logins.

Any related issues, documentation, discussions?

Closes #7048

How was this PR tested?

Compiled and ran all tests as well as deployed locally to verify functionality.

Was this PR authored or co-authored using generative AI tooling?

Co-authored with Claude Opus 4.8

[github-actions]

Automated Reviewer Suggestions

Based on the git blame history of the changed files, we recommend the following reviewers:

• Contributors with relevant context: @mengw15, @aglinxinyuan, @xuang7
  You can notify them by mentioning @mengw15, @aglinxinyuan, @xuang7 in a comment.

[codecov-commenter]

Codecov Report

❌ Patch coverage is 83.68794% with 23 lines in your changes missing coverage. Please review.
✅ Project coverage is 86.04%. Comparing base (ab4d25e) to head (8be10a1).
⚠️ Report is 25 commits behind head on main.
✅ All tests successful. No failed tests found.

Files with missing lines	Patch %	Lines
...ra/web/resource/auth/ExternalAuthProvisioner.scala	85.41%	4 Missing and 3 partials ⚠️
...apache/texera/web/resource/auth/AuthResource.scala	80.00%	4 Missing and 2 partials ⚠️
.../texera/web/resource/auth/GoogleAuthResource.scala	75.00%	4 Missing and 1 partial ⚠️
...exera/web/resource/auth/LocalAuthProvisioner.scala	92.30%	0 Missing and 2 partials ⚠️
...c/main/scala/org/apache/texera/dao/SqlStates.scala	0.00%	2 Missing ⚠️
...era/web/resource/dashboard/DashboardResource.scala	0.00%	1 Missing ⚠️

Additional details and impacted files

@@             Coverage Diff              @@
##               main    #7055      +/-   ##
============================================
+ Coverage     85.51%   86.04%   +0.52%     
- Complexity     4150     4225      +75     
============================================
  Files          1169     1172       +3     
  Lines         46740    46789      +49     
  Branches       5202     5206       +4     
============================================
+ Hits          39971    40259     +288     
+ Misses         5055     4790     -265     
- Partials       1714     1740      +26     

Flag	Coverage Δ		*Carryforward flag
access-control-service	70.00% <ø> (ø)
agent-service	85.50% <ø> (ø)		Carriedforward from 88ba36b
amber	82.09% <83.57%> (+1.21%)	⬆️
computing-unit-managing-service	60.38% <100.00%> (+9.66%)	⬆️
config-service	65.97% <ø> (ø)
file-service	69.05% <ø> (ø)
frontend	88.29% <ø> (ø)		Carriedforward from 88ba36b
notebook-migration-service	78.89% <ø> (ø)
pyamber	97.51% <ø> (ø)		Carriedforward from 88ba36b
workflow-compiling-service	26.31% <ø> (ø)

*This pull request uses carry forward flags. Click here to find out more.

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

[github-actions]

⚠️ Benchmark changes need a look

🟢 0 better · 🔴 10 worse · ⚪ 5 noise (<±5%) · 0 without baseline

CI benchmark results are noisy; treat <±5% as noise unless repeated.

Dashboard · Run

	config	throughput	MB/s	latency	max Δ latest / 7d
🔴	bs=10 sw=10 sl=64	425	0.26	23,260/32,185/32,185 us	🔴 +113.9% / 🔴 +93.7%
🔴	bs=100 sw=10 sl=64	945	0.577	102,739/137,353/137,353 us	🔴 +35.7% / 🔴 +21.3%
⚪	bs=1000 sw=10 sl=64	1,094	0.668	908,779/1,019,844/1,019,844 us	⚪ within ±5% / 🟢 +13.3%

Baseline details

Latest main 42d08a3 from 2026-08-10T13:03:59.622Z

config	metric	PR	latest main	7d avg	Δ latest	Δ 7d
bs=10 sw=10 sl=64	throughput	425 tuples/sec	818.75 tuples/sec	740.3 tuples/sec	-48.1%	-42.6%
bs=10 sw=10 sl=64	MB/s	0.26 MB/s	0.5 MB/s	0.452 MB/s	-48.0%	-42.5%
bs=10 sw=10 sl=64	p50	23,260 us	11,857 us	13,173 us	+96.2%	+76.6%
bs=10 sw=10 sl=64	p95	32,185 us	15,045 us	16,615 us	+113.9%	+93.7%
bs=10 sw=10 sl=64	p99	32,185 us	17,739 us	19,272 us	+81.4%	+67.0%
bs=100 sw=10 sl=64	throughput	945 tuples/sec	1,057 tuples/sec	939.86 tuples/sec	-10.6%	+0.5%
bs=100 sw=10 sl=64	MB/s	0.577 MB/s	0.645 MB/s	0.574 MB/s	-10.6%	+0.6%
bs=100 sw=10 sl=64	p50	102,739 us	93,404 us	106,062 us	+10.0%	-3.1%
bs=100 sw=10 sl=64	p95	137,353 us	101,214 us	113,212 us	+35.7%	+21.3%
bs=100 sw=10 sl=64	p99	137,353 us	112,746 us	123,853 us	+21.8%	+10.9%
bs=1000 sw=10 sl=64	throughput	1,094 tuples/sec	1,081 tuples/sec	966.11 tuples/sec	+1.2%	+13.2%
bs=1000 sw=10 sl=64	MB/s	0.668 MB/s	0.66 MB/s	0.59 MB/s	+1.2%	+13.3%
bs=1000 sw=10 sl=64	p50	908,779 us	923,474 us	1,038,733 us	-1.6%	-12.5%
bs=1000 sw=10 sl=64	p95	1,019,844 us	983,118 us	1,085,289 us	+3.7%	-6.0%
bs=1000 sw=10 sl=64	p99	1,019,844 us	1,015,764 us	1,115,555 us	+0.4%	-8.6%

Raw CSV

config_idx,batch_size,schema_width,string_len,num_batches,total_ms,total_tuples,total_bytes,tuples_per_sec,mb_per_sec,lat_p50_us,lat_p95_us,lat_p99_us
0,10,10,64,20,470.12,200,128000,425,0.260,23259.56,32185.26,32185.26
1,100,10,64,20,2115.64,2000,1280000,945,0.577,102739.48,137353.04,137353.04
2,1000,10,64,20,18284.96,20000,12800000,1094,0.668,908778.93,1019843.88,1019843.88

[Yicong-Huang]

🔴 10 resolved · 0 open · 5 new (5 new — the newly-introduced vs. late-catch split is unavailable this round: the prior review's commit 1abdc70 was force-pushed away)

1 must-fix · 1 advisory · 3 polish. Everything from round 2 is resolved, and resolved the right way. The email-casing regression is fixed with two tests that genuinely fail on the old code, and the four behavior changes were withdrawn rather than argued for — which is what makes refactor(auth) an honest type now. What is left is the same case-sensitivity defect in a guard round 2 did not reach, plus three comments the withdrawal left stale.

Correctness (1)

• AuthResource.scala:115 — the new admin-bootstrap email guard is case-sensitive, so a case-variant admin username slips it and a duplicate account is created (must-fix, see inline)

Simplifications (1)

• JwtAuth.scala:68 — expireInDays is never read and names days for a minutes value; this signature is already being rewritten (advisory, see inline)

Polish: 3 comments left stale by the round-2 withdrawal (see inline comments).

Verification trace

I re-traced the four behavior changes I asked to be split out, since the refactor type now rests on their withdrawal. All four are genuinely gone rather than relocated. (1) Avatar representation: profileOf is back to the last-path-segment form (GoogleAuthResource.scala:44-46), matching pre-PR :96-98. The part that matters is that the migration moved with it — 32.sql carries no value-rewrite, and says so at :24-26. Code and data agree. (2) No unverified-email gate: ExternalProfile (ExternalAuthProvisioner.scala:39-45) has no emailVerified field. (3) verifiedPayload (GoogleAuthResource.scala:75-76) has no try/catch, so a malformed credential behaves as pre-PR. (4) The display-name refresh is restored at :152-155.

What still changes observably is inherent to the schema move. register asks handleExists (:170) because the handle now lives in auth_provider, which restores the pre-PR meaning rather than altering it. The 23505 to 409 mapping (LocalAuthProvisioner.scala:98) is the error contract the new two-row insert needs.

I also confirmed the casing fix rather than taking it on report. Both sites route through the tx-scoped userByEmailIgnoreCase (:63-69, called at :100 and :124), and the new tests at ExternalAuthProvisionerSpec.scala:194 and :208 seed and query different casings — so unlike the old :374 assertion, they can observe a duplicate.

[Yicong-Huang]

🔴 5 resolved · 0 open · 6 new (6 new — the newly-introduced vs. late-catch split is unavailable this round: the prior review's commit 38db65d was force-pushed away)

1 must-fix · 3 advisory · 2 polish. All five round-3 items are resolved, and the refactor claim still holds — the four withdrawn behavior changes are genuinely absent rather than relocated. The blocker is not a new mistake so much as an unverified fix: the race handler applied from Copilot's autofix sits inside the transaction it means to recover from.

Correctness (1)

• ExternalAuthProvisioner.scala:115 — the 23505 recovery runs inside the already-aborted transaction, so it cannot recover and the request still 500s (must-fix, see inline)

Design & architecture (1)

• AuthResource.scala:110 — two seams extracted this round "so the path is reachable from a test" have no test caller, leaving this round's own fix unguarded (advisory, see inline)

Simplifications (1)

• LocalAuthProvisioner.scala:114 — claimWithLocalCredential duplicates createLocalAccount but for one line (advisory, see inline)

Conventions (1)

• Description: names migration 32.sql (the added file is 33.sql; 32.sql is the unrelated user_warehouse migration) and calls the table auth_providers, not auth_provider. (advisory)

Polish: 2 quick touch-ups (see inline comments).

Verification trace

The must-fix rests on one claim, so I checked its premises rather than assuming Postgres semantics. SqlServer.withTransaction (SqlServer.scala:96) is a bare dsl.transaction(...), and SqlServer installs no custom jOOQ Settings or TransactionProvider — so this is one JDBC transaction, with savepoints only for nested transaction() calls, never per statement. That leaves the insert's 23505 aborting the transaction outright, which is why the handler's own SELECT cannot run. The decisive part is the in-repo counter-example: LocalAuthProvisioner does the same 23505 to 409 mapping but puts its try outside withTransaction, so its handler runs after rollback.

I also re-traced the refactor claim, since it rests on the round-2 withdrawals. All four are still absent rather than relocated: no sanitizedAvatar anywhere in the tree, ExternalProfile has no emailVerified, verifiedPayload (GoogleAuthResource.scala:68-69) has no try/catch, and the display-name refresh lives in refresh (ExternalAuthProvisioner.scala:144-147). The claim contract holds too: JwtAuth.scala:71-75 still writes googleAvatar and writes googleId whenever one is in hand, auth.service.ts:167-168 and flarum.service.ts:39,48 both still resolve, and a grep for getGoogleId|getGoogleAvatar|GOOGLE_AVATAR|GOOGLE_ID across all Scala returns nothing. The two jwtClaims sites that omit googleId are service-to-service — an internal upload header (ResultExportService.scala:553) and a computing-unit token (ComputingUnitManagingResource.scala:383) — neither reaching the browser, as the new scaladoc claims.

[Yicong-Huang]

🔴 3 resolved · 3 open · 6 new (6 new = 1 newly introduced · 5 late catches — my misses from earlier rounds)

1 must-fix · 4 advisory · 2 polish. The round-4 blocker is genuinely fixed: the 23505 catch now sits outside the transaction it recovers from. What the fix did not do is look at its sibling — LocalAuthProvisioner's handler has the mirror-image problem, right place but wrong message. Both still-open code items are the ones deferred to a follow-up issue.

Correctness (1)

• LocalAuthProvisioner.scala:100 — one 23505 handler covers two unique constraints, so an email collision is reported as a taken handle (must-fix, see inline)

Design & architecture (1)

• ExternalAuthProvisioner.scala:80 — bare "23505" beside LocalAuthProvisioner.UNIQUE_VIOLATION in the same package (advisory, see inline)

Simplifications (2)

• AuthResource.scala:45 — userDao lost its last caller in this diff, and no Scala -Wunused pass will flag it (advisory, see inline)
• JwtAuthSpec.scala:71 — the expiry test's expireInDays parameter is unread, so both call sites assert the same thing (advisory, see inline)

Conventions (1)

• Description: the added migration is sql/updates/33.sql, not 32.sql (that one is user_warehouse), and the table is auth_provider, not auth_providers. Carried over from round 4. (advisory)

Polish: 2 doc lines the delta made inaccurate (see inline comments).

Verification trace

Two claims were load-bearing this round, so I traced both rather than reading the reply.

That the round-4 recovery now actually runs: SqlServer.withTransaction (SqlServer.scala:96) is a bare dsl.transaction, so the rollback completes before the new catch at ExternalAuthProvisioner.scala:79-82 executes — the shape LocalAuthProvisioner:83-104 already uses. Re-running provision resolves either violation it can raise: losing user_email_key finds the winner through the lower(email) lookup (:107), and losing uq_provider_identity finds it through the provider join (:91-99).

That createLocalAccount's transaction really has two constraints inside it: txUserDao.insert (:88) writes "user", whose email is UNIQUE (texera_ddl.sql:108), and txAuthDao.insert (:95) writes uq_provider_identity (:129). Both raise 23505, and both land on the single handler at :98.

I also re-checked the frozen wire contract, since that is what keeps this a refactor: the googleAvatar and googleId claim names are unchanged (JwtAuth.scala:74-75), a repo-wide grep for getGoogleId|getGoogleAvatar|GOOGLE_ID|GOOGLE_AVATAR across Scala and Java returns nothing, and no google_id/google_avatar survives in TS, SQL, YAML, Python or shell outside sql/updates/. The DDL and migration 33 converge on the same end state, and AdminUserResource.list still projects 12 columns onto 12 UserInfo fields in matching order.

[Yicong-Huang]

🔴 3 addressed · 4 open · 4 new (4 new = 1 newly introduced · 3 late catches — my misses from earlier rounds)

1 must-fix · 1 advisory · 2 polish. All three suggestion blocks from round 5 were applied verbatim and verified in the tree. The one must-fix is the sibling handler that fix skipped. The schema, the migration and the frozen wire names all re-verified clean.

Correctness (1)

• LocalAuthProvisioner.scala:130 — the sibling handler still names the handle for a collision that is actually on PRIMARY KEY (uid, provider_type) (must-fix, see inline)

Conventions (1)

• Title scope: auth names a feature, not a module. CONTRIBUTING.md asks for the module carrying the change — here amber. Possibly meant as common/auth, so worth confirming (advisory)

Polish: 2 scaladoc lines, one of them wording I supplied last round (see inline comments).

Still open from earlier rounds: the bare "23505" at ExternalAuthProvisioner.scala:81, the unreferenced userDao at AuthResource.scala:45, the unread expireInDays at JwtAuthSpec.scala:71, and the description's auth_providers / 32.sql (the table is auth_provider, the migration is 33.sql).

Verification trace

Two claims were load-bearing this round, so I traced both rather than reading the commit messages.

That the three suggestions really landed: LocalAuthProvisioner.scala:98-102, ExternalAuthProvisioner.scala:70-74 and JwtAuth.scala:65-67 now carry the suggested text verbatim, and SqlServer.withTransaction (SqlServer.scala:96) is still a bare dsl.transaction, so every catch continues to run after the rollback it recovers from.

Which constraint the remaining handler can actually meet: claimWithLocalCredential updates an existing user row and then inserts (uid, LOCAL, handle). The email is not changed on that path, so user_email_key cannot fire; PRIMARY KEY (uid, provider_type) (texera_ddl.sql:125) is what a second claim of the same placeholder hits, not uq_provider_identity. That is why re-checking handleExists alone is not the whole answer here.

I also checked whether the retry at :82 could 500 permanently on a user_email_key violation raised by refresh + update (:102-104), which would need two accounts whose emails differ only in case. It cannot arise from anything this PR touches: DatasetResource.resolveContributorUid (:212-217) looks up lower(email) before inserting a placeholder, and both register and provision match case-insensitively. The one exact-match check left is AdminUserResource.updateUser (:116), which this PR does not change.