This is an automated email from the ASF dual-hosted git repository. github-merge-queue[bot] pushed a commit to branch gh-readonly-queue/main/pr-5305-0e5686ec2fff54a9b1a9b44840c5de3ceb7dd178 in repository https://gitbox.apache.org/repos/asf/texera.git
commit eb287f3fc2cbba5416d851078affc10537123e68 Author: Yicong Huang <[email protected]> AuthorDate: Mon Jun 1 00:22:31 2026 -0700 feat(config-service): split /config/pre-login from authenticated endpoints (#5305) ### What changes were proposed in this PR? `config-service` no longer broadcasts the full GUI configuration to anonymous callers. The four fields the frontend actually needs before login (`localLogin`, `googleLogin`, `defaultLocalUser`, `attributionEnabled`) move to a new `GET /api/config/pre-login` (`@PermitAll`). `GET /api/config/gui` and `GET /api/config/user-system` are now `@RolesAllowed("REGULAR", "ADMIN")` and only answer authenticated traffic. On the frontend, `GuiConfigService.load()` always fetches `/config/pre-login` at `APP_INITIALIZER`. When a JWT is already in `localStorage` (browser reload while logged in), it chains `/config/gui` + `/config/user-system` in the same await so the full config is in memory before any post-login component mounts. `UserService.handleAccessToken` does the same chaining on a fresh login so `loginWithExistingToken` (which reads `config.env.inviteOnly`) runs only after the authenticated config has resolved. Expired-token 403s on the post-login fetch are caught so a stale `localStorage` token cannot block bootstrap — that was the exact failure mode that caused #5025 to revert the earlier eager-401 lockdown. ### Any related issues, documentation, discussions? Closes #5304. Related: #4901 (eager-401 from `JwtAuthFilter`), #5025 (revert that broke `ConfigService` bootstrap last time we attempted a similar lockdown), #5199 (re-applied `@RolesAllowed` enforcement on the microservices, with `@PermitAll` opt-out for the two pre-login endpoints). ### How was this PR tested? Added unit tests covering pre-login / gui / user-system access with and without a valid Bearer token, the orchestrator's branching on a stored token, the 403 fallback, and the `UserService.handleAccessToken` ordering guarantee. ### Was this PR authored or co-authored using generative AI tooling? Generated-by: Claude Opus 4.7 --------- Co-authored-by: Claude Opus 4.7 (1M context) <[email protected]> --- .../texera/service/resource/ConfigResource.scala | 33 ++-- .../service/resource/ConfigResourceAuthSpec.scala | 114 +++++++++--- .../app/common/service/gui-config.service.mock.ts | 4 + .../app/common/service/gui-config.service.spec.ts | 194 +++++++++++++++++++++ .../src/app/common/service/gui-config.service.ts | 86 ++++++--- .../app/common/service/user/user.service.spec.ts | 62 +++++++ .../src/app/common/service/user/user.service.ts | 29 ++- 7 files changed, 457 insertions(+), 65 deletions(-) diff --git a/config-service/src/main/scala/org/apache/texera/service/resource/ConfigResource.scala b/config-service/src/main/scala/org/apache/texera/service/resource/ConfigResource.scala index ace8e8618d..1db52e2dc8 100644 --- a/config-service/src/main/scala/org/apache/texera/service/resource/ConfigResource.scala +++ b/config-service/src/main/scala/org/apache/texera/service/resource/ConfigResource.scala @@ -19,7 +19,7 @@ package org.apache.texera.service.resource -import jakarta.annotation.security.PermitAll +import jakarta.annotation.security.{PermitAll, RolesAllowed} import jakarta.ws.rs.core.MediaType import jakarta.ws.rs.{GET, Path, Produces} import org.apache.texera.config.{AuthConfig, ComputingUnitConfig, GuiConfig, UserSystemConfig} @@ -28,12 +28,26 @@ import org.apache.texera.config.{AuthConfig, ComputingUnitConfig, GuiConfig, Use @Produces(Array(MediaType.APPLICATION_JSON)) class ConfigResource { - // These two endpoints are fetched by the frontend during app initialization, - // before any login, so they must answer unauthenticated callers — hence @PermitAll. - // They are the only endpoints in this resource, so role enforcement gates nothing - // here; @PermitAll is what keeps them reachable when role enforcement is enabled. + // Anonymous endpoint loaded by the frontend's APP_INITIALIZER before any user has + // logged in. Only fields that the login page (or the logged-out branches of the + // dashboard shell) actually need belong here — anything else lives on /gui or + // /user-system, both of which require authentication. @GET @PermitAll + @Path("/pre-login") + def getPreLoginConfig: Map[String, Any] = + Map( + "localLogin" -> GuiConfig.guiLoginLocalLogin, + "googleLogin" -> GuiConfig.guiLoginGoogleLogin, + "defaultLocalUser" -> Map( + "username" -> GuiConfig.guiLoginDefaultLocalUserUsername, + "password" -> GuiConfig.guiLoginDefaultLocalUserPassword + ), + "attributionEnabled" -> GuiConfig.guiAttributionEnabled + ) + + @GET + @RolesAllowed(Array("REGULAR", "ADMIN")) @Path("/gui") def getGuiConfig: Map[String, Any] = Map( @@ -41,8 +55,6 @@ class ConfigResource { "exportExecutionResultEnabled" -> GuiConfig.guiWorkflowWorkspaceExportExecutionResultEnabled, "autoAttributeCorrectionEnabled" -> GuiConfig.guiWorkflowWorkspaceAutoAttributeCorrectionEnabled, "selectingFilesFromDatasetsEnabled" -> GuiConfig.guiWorkflowWorkspaceSelectingFilesFromDatasetsEnabled, - "localLogin" -> GuiConfig.guiLoginLocalLogin, - "googleLogin" -> GuiConfig.guiLoginGoogleLogin, "userPresetEnabled" -> GuiConfig.guiWorkflowWorkspaceUserPresetEnabled, "workflowExecutionsTrackingEnabled" -> GuiConfig.guiWorkflowWorkspaceWorkflowExecutionsTrackingEnabled, "linkBreakpointEnabled" -> GuiConfig.guiWorkflowWorkspaceLinkBreakpointEnabled, @@ -55,20 +67,15 @@ class ConfigResource { "sharingComputingUnitEnabled" -> ComputingUnitConfig.sharingComputingUnitEnabled, "operatorConsoleMessageBufferSize" -> GuiConfig.guiWorkflowWorkspaceOperatorConsoleMessageBufferSize, "pythonLanguageServerPort" -> GuiConfig.guiWorkflowWorkspacePythonLanguageServerPort, - "defaultLocalUser" -> Map( - "username" -> GuiConfig.guiLoginDefaultLocalUserUsername, - "password" -> GuiConfig.guiLoginDefaultLocalUserPassword - ), "activeTimeInMinutes" -> GuiConfig.guiWorkflowWorkspaceActiveTimeInMinutes, "copilotEnabled" -> GuiConfig.guiWorkflowWorkspaceCopilotEnabled, "limitColumns" -> GuiConfig.guiWorkflowWorkspaceLimitColumns, - "attributionEnabled" -> GuiConfig.guiAttributionEnabled, // flags from the auth.conf if needed "expirationTimeInMinutes" -> AuthConfig.jwtExpirationMinutes ) @GET - @PermitAll + @RolesAllowed(Array("REGULAR", "ADMIN")) @Path("/user-system") def getUserSystemConfig: Map[String, Any] = Map( diff --git a/config-service/src/test/scala/org/apache/texera/service/resource/ConfigResourceAuthSpec.scala b/config-service/src/test/scala/org/apache/texera/service/resource/ConfigResourceAuthSpec.scala index 56cde8c263..3475682ef8 100644 --- a/config-service/src/test/scala/org/apache/texera/service/resource/ConfigResourceAuthSpec.scala +++ b/config-service/src/test/scala/org/apache/texera/service/resource/ConfigResourceAuthSpec.scala @@ -35,11 +35,12 @@ import org.scalatest.flatspec.AnyFlatSpec import org.scalatest.matchers.should.Matchers // Wires ConfigResource through the same Jersey auth pipeline production uses -// (JwtAuthFilter + RolesAllowedDynamicFeature) and fires HTTP requests with no -// Authorization header. Regression guard for the bootstrap break that caused -// PR #5049 to be reverted in #5173: /config/gui and /config/user-system are -// loaded by the frontend's APP_INITIALIZER before any login, so they must -// return 200 to unauthenticated callers even with role enforcement enabled. +// (JwtAuthFilter + RolesAllowedDynamicFeature) and fires HTTP requests with and +// without an Authorization header. /config/pre-login is the only @PermitAll +// endpoint and must answer unauthenticated callers (bootstrap regression guard, +// same shape as the break that caused PR #5049 to be reverted in #5173). +// /config/gui and /config/user-system are now @RolesAllowed; they must reject +// anonymous traffic and accept callers with a valid Bearer token. class ConfigResourceAuthSpec extends AnyFlatSpec with Matchers with BeforeAndAfterAll { // Mirror production's mapper: ConfigService bootstraps Dropwizard's default mapper @@ -59,18 +60,94 @@ class ConfigResourceAuthSpec extends AnyFlatSpec with Matchers with BeforeAndAft override protected def beforeAll(): Unit = resources.before() override protected def afterAll(): Unit = resources.after() - "GET /config/gui" should "return 200 without an Authorization header" in { + private def regularToken(): String = { + val u = new User() + u.setUid(2) + u.setName("test-regular") + u.setEmail("[email protected]") + u.setGoogleId(null) + u.setRole(UserRoleEnum.REGULAR) + JwtAuth.jwtToken(JwtAuth.jwtClaims(u, expireInDays = 1)) + } + + private def adminToken(): String = { + val u = new User() + u.setUid(1) + u.setName("test-admin") + u.setEmail("[email protected]") + u.setGoogleId(null) + u.setRole(UserRoleEnum.ADMIN) + JwtAuth.jwtToken(JwtAuth.jwtClaims(u, expireInDays = 1)) + } + + "GET /config/pre-login" should "return 200 without an Authorization header" in { + val response = resources.target("/config/pre-login").request(MediaType.APPLICATION_JSON).get() + response.getStatus shouldBe 200 + } + + it should "expose exactly the fields the login UI needs and nothing else" in { + // Locking down the payload keeps anonymous callers from reading workspace flags, + // feature toggles, or session timers. If a new field is needed before login, it + // must be added here explicitly; the assertion forces that decision into review. + val payload = resources + .target("/config/pre-login") + .request(MediaType.APPLICATION_JSON) + .get(classOf[Map[String, Any]]) + payload.keySet shouldBe Set( + "localLogin", + "googleLogin", + "defaultLocalUser", + "attributionEnabled" + ) + } + + "GET /config/gui" should "return 403 without an Authorization header" in { val response = resources.target("/config/gui").request(MediaType.APPLICATION_JSON).get() + response.getStatus shouldBe 403 + } + + it should "return 200 with a valid Bearer token whose role matches @RolesAllowed" in { + val response = resources + .target("/config/gui") + .request(MediaType.APPLICATION_JSON) + .header("Authorization", s"Bearer ${regularToken()}") + .get() response.getStatus shouldBe 200 } - "GET /config/user-system" should "return 200 without an Authorization header" in { + it should "not leak any pre-login field through the authenticated payload" in { + // The split is only meaningful if /gui drops the fields that /pre-login owns. + // Without this, a future refactor could re-add them under the @RolesAllowed + // endpoint, doubling the surface and creating two sources of truth. + val payload = resources + .target("/config/gui") + .request(MediaType.APPLICATION_JSON) + .header("Authorization", s"Bearer ${regularToken()}") + .get(classOf[Map[String, Any]]) + payload.keySet should contain noneOf ( + "localLogin", + "googleLogin", + "defaultLocalUser", + "attributionEnabled" + ) + } + + "GET /config/user-system" should "return 403 without an Authorization header" in { val response = resources.target("/config/user-system").request(MediaType.APPLICATION_JSON).get() + response.getStatus shouldBe 403 + } + + it should "return 200 with a valid Bearer token whose role matches @RolesAllowed" in { + val response = resources + .target("/config/user-system") + .request(MediaType.APPLICATION_JSON) + .header("Authorization", s"Bearer ${regularToken()}") + .get() response.getStatus shouldBe 200 } - "GET an @RolesAllowed endpoint" should "return 403 without an Authorization header" in { + "GET an @RolesAllowed probe endpoint" should "return 403 without an Authorization header" in { // Sanity: with no SecurityContext set by JwtAuthFilter, RolesAllowedDynamicFeature // must reject. Catches the case where the feature is registered but somehow // disabled (e.g. swallowed exception during setup). @@ -82,22 +159,15 @@ class ConfigResourceAuthSpec extends AnyFlatSpec with Matchers with BeforeAndAft it should "return 200 with a valid Bearer token whose role matches @RolesAllowed" in { // Positive-direction sibling to the previous test. Without this, a filter- // priority bug that lets RolesAllowedRequestFilter run *before* JwtAuthFilter - // is invisible to the spec: the no-auth case still 403s, the @PermitAll cases - // still 200, and the only path that actually exercises auth → authz ordering - // is "valid JWT → 200". Manual integration testing of PR #5199 found this: - // a real admin JWT was getting 403 on every @RolesAllowed endpoint until - // JwtAuthFilter was pinned to Priorities.AUTHENTICATION. - val u = new User() - u.setUid(1) - u.setName("test-admin") - u.setEmail("[email protected]") - u.setGoogleId(null) - u.setRole(UserRoleEnum.ADMIN) - val token = JwtAuth.jwtToken(JwtAuth.jwtClaims(u, expireInDays = 1)) + // is invisible to the spec: the no-auth case still 403s, and the only path + // that actually exercises auth → authz ordering is "valid JWT → 200". Manual + // integration testing of PR #5199 found this: a real admin JWT was getting + // 403 on every @RolesAllowed endpoint until JwtAuthFilter was pinned to + // Priorities.AUTHENTICATION. val response = resources .target("/auth-probe") .request(MediaType.APPLICATION_JSON) - .header("Authorization", s"Bearer $token") + .header("Authorization", s"Bearer ${adminToken()}") .get() response.getStatus shouldBe 200 } @@ -106,7 +176,7 @@ class ConfigResourceAuthSpec extends AnyFlatSpec with Matchers with BeforeAndAft object ConfigResourceAuthSpec { // A deliberately @RolesAllowed companion to ConfigResource, so the same setup also // proves the feature actually rejects when it should — a 200 on the @PermitAll - // endpoints would otherwise be consistent with the feature being silently no-op'd. + // endpoint would otherwise be consistent with the feature being silently no-op'd. @Path("/auth-probe") @Produces(Array(MediaType.APPLICATION_JSON)) class ProtectedProbe { diff --git a/frontend/src/app/common/service/gui-config.service.mock.ts b/frontend/src/app/common/service/gui-config.service.mock.ts index 0e20da8024..9165c19b9c 100644 --- a/frontend/src/app/common/service/gui-config.service.mock.ts +++ b/frontend/src/app/common/service/gui-config.service.mock.ts @@ -63,6 +63,10 @@ export class MockGuiConfigService { return of(this._config); } + loadPostLogin(): Observable<Partial<GuiConfig>> { + return of(this._config); + } + setConfig(config: Partial<GuiConfig>): void { this._config = { ...this._config, ...config }; } diff --git a/frontend/src/app/common/service/gui-config.service.spec.ts b/frontend/src/app/common/service/gui-config.service.spec.ts new file mode 100644 index 0000000000..0294ea8307 --- /dev/null +++ b/frontend/src/app/common/service/gui-config.service.spec.ts @@ -0,0 +1,194 @@ +/** + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +import { TestBed } from "@angular/core/testing"; +import { HttpClientTestingModule, HttpTestingController } from "@angular/common/http/testing"; +import { firstValueFrom } from "rxjs"; + +import { GuiConfigService } from "./gui-config.service"; +import { AppSettings } from "../app-setting"; + +const API = "api"; +const TOKEN_KEY = "access_token"; + +const PRE_LOGIN_PAYLOAD = { + localLogin: true, + googleLogin: false, + defaultLocalUser: { username: "demo", password: "demo-pw" }, + attributionEnabled: true, +}; + +const GUI_PAYLOAD = { + copilotEnabled: true, + limitColumns: 42, + defaultExecutionMode: "PIPELINED", +}; + +const USER_SYSTEM_PAYLOAD = { + inviteOnly: true, +}; + +describe("GuiConfigService", () => { + let service: GuiConfigService; + let http: HttpTestingController; + + beforeEach(() => { + localStorage.removeItem(TOKEN_KEY); + TestBed.configureTestingModule({ + imports: [HttpClientTestingModule], + providers: [GuiConfigService], + }); + service = TestBed.inject(GuiConfigService); + http = TestBed.inject(HttpTestingController); + vi.spyOn(AppSettings, "getApiEndpoint").mockReturnValue(API); + }); + + afterEach(() => { + localStorage.removeItem(TOKEN_KEY); + http.verify(); + }); + + // ─── loadPreLogin ───────────────────────────────────────────────────────── + + it("loadPreLogin fetches /config/pre-login and merges the response into env", async () => { + const pending = firstValueFrom(service.loadPreLogin()); + + const req = http.expectOne(`${API}/config/pre-login`); + expect(req.request.method).toBe("GET"); + req.flush(PRE_LOGIN_PAYLOAD); + + await pending; + expect(service.env.localLogin).toBe(true); + expect(service.env.googleLogin).toBe(false); + expect(service.env.defaultLocalUser).toEqual({ username: "demo", password: "demo-pw" }); + expect(service.env.attributionEnabled).toBe(true); + }); + + it("loadPreLogin propagates errors so APP_INITIALIZER can surface them", async () => { + // Network failure at bootstrap means the login page can't render correctly. + // The orchestrator and APP_INITIALIZER both rely on this rejection to log + // and (in production) show an error banner, so the rejection must escape. + const pending = firstValueFrom(service.loadPreLogin()); + http.expectOne(`${API}/config/pre-login`).error(new ProgressEvent("net error")); + await expect(pending).rejects.toThrow(/pre-login configuration/); + }); + + // ─── loadPostLogin ──────────────────────────────────────────────────────── + + it("loadPostLogin fetches /config/gui and /config/user-system in parallel and merges", async () => { + const pending = firstValueFrom(service.loadPostLogin()); + + const gui = http.expectOne(`${API}/config/gui`); + const userSystem = http.expectOne(`${API}/config/user-system`); + expect(gui.request.method).toBe("GET"); + expect(userSystem.request.method).toBe("GET"); + + gui.flush(GUI_PAYLOAD); + userSystem.flush(USER_SYSTEM_PAYLOAD); + + await pending; + expect(service.env.copilotEnabled).toBe(true); + expect(service.env.limitColumns).toBe(42); + expect(service.env.inviteOnly).toBe(true); + }); + + it("loadPostLogin preserves pre-login fields when both phases run", async () => { + // Guards a refactor that overwrites the merged map with the gui/user-system + // response shape. Without this assertion, pre-login fields could silently + // disappear from env after post-login completes. + const preLoginPending = firstValueFrom(service.loadPreLogin()); + http.expectOne(`${API}/config/pre-login`).flush(PRE_LOGIN_PAYLOAD); + await preLoginPending; + + const postLoginPending = firstValueFrom(service.loadPostLogin()); + http.expectOne(`${API}/config/gui`).flush(GUI_PAYLOAD); + http.expectOne(`${API}/config/user-system`).flush(USER_SYSTEM_PAYLOAD); + await postLoginPending; + + expect(service.env.localLogin).toBe(true); + expect(service.env.defaultLocalUser).toEqual({ username: "demo", password: "demo-pw" }); + expect(service.env.copilotEnabled).toBe(true); + expect(service.env.inviteOnly).toBe(true); + }); + + // ─── load() orchestrator ────────────────────────────────────────────────── + + it("load() only hits /config/pre-login when no access token is in localStorage", async () => { + const pending = firstValueFrom(service.load()); + http.expectOne(`${API}/config/pre-login`).flush(PRE_LOGIN_PAYLOAD); + // /config/gui and /config/user-system must not be requested when anonymous; + // the no-Authorization-header request would 403 and pollute network logs. + http.expectNone(`${API}/config/gui`); + http.expectNone(`${API}/config/user-system`); + await pending; + expect(service.env.localLogin).toBe(true); + }); + + it("load() chains /config/gui + /config/user-system when a token is stored", async () => { + localStorage.setItem(TOKEN_KEY, "stored-token"); + const pending = firstValueFrom(service.load()); + + http.expectOne(`${API}/config/pre-login`).flush(PRE_LOGIN_PAYLOAD); + // forkJoin fires both requests in parallel after pre-login resolves. + http.expectOne(`${API}/config/gui`).flush(GUI_PAYLOAD); + http.expectOne(`${API}/config/user-system`).flush(USER_SYSTEM_PAYLOAD); + + await pending; + expect(service.env.localLogin).toBe(true); + expect(service.env.copilotEnabled).toBe(true); + expect(service.env.inviteOnly).toBe(true); + }); + + it("load() swallows post-login 403s so bootstrap is not blocked by an expired token", async () => { + // Stale token in localStorage would otherwise leave the app stuck on a + // blank screen — this is the exact failure mode that caused PR #5025 to + // revert the earlier eager-401 lockdown. + localStorage.setItem(TOKEN_KEY, "expired-token"); + const pending = firstValueFrom(service.load()); + + http.expectOne(`${API}/config/pre-login`).flush(PRE_LOGIN_PAYLOAD); + const guiReq = http.expectOne(`${API}/config/gui`); + const userSystemReq = http.expectOne(`${API}/config/user-system`); + guiReq.flush({}, { status: 403, statusText: "Forbidden" }); + // forkJoin tears down the sibling observable on first error, so the + // user-system request is already cancelled by the time we get here. + // Flushing a cancelled TestRequest throws. + if (!userSystemReq.cancelled) { + userSystemReq.flush({}, { status: 403, statusText: "Forbidden" }); + } + + await pending; + // Pre-login fields stay, post-login fields stay unset; the app degrades + // rather than failing to start. + expect(service.env.localLogin).toBe(true); + expect(service.env.copilotEnabled).toBeUndefined(); + expect(service.env.inviteOnly).toBeUndefined(); + }); + + it("load() rejects when /config/pre-login itself fails so the bootstrap error is surfaced", async () => { + localStorage.setItem(TOKEN_KEY, "stored-token"); + const pending = firstValueFrom(service.load()); + http.expectOne(`${API}/config/pre-login`).error(new ProgressEvent("offline")); + // /config/gui must NOT be attempted if pre-login fails — the catchError on + // the inner pipe must not catch the outer pre-login rejection. + http.expectNone(`${API}/config/gui`); + http.expectNone(`${API}/config/user-system`); + await expect(pending).rejects.toThrow(/pre-login configuration/); + }); +}); diff --git a/frontend/src/app/common/service/gui-config.service.ts b/frontend/src/app/common/service/gui-config.service.ts index a2e164efeb..e96d98b191 100644 --- a/frontend/src/app/common/service/gui-config.service.ts +++ b/frontend/src/app/common/service/gui-config.service.ts @@ -20,45 +20,83 @@ import { HttpClient } from "@angular/common/http"; import { Injectable } from "@angular/core"; import { GuiConfig } from "../type/gui-config"; -import { catchError, forkJoin, map, Observable, tap, throwError } from "rxjs"; +import { catchError, forkJoin, map, Observable, of, switchMap, tap, throwError } from "rxjs"; import { AppSettings } from "../app-setting"; +// AuthService also owns this key but injects GuiConfigService, so importing it +// back here would create a dependency cycle. Duplicating the constant is the +// least invasive fix. +const ACCESS_TOKEN_KEY = "access_token"; + +type PreLoginConfig = Pick<GuiConfig, "localLogin" | "googleLogin" | "defaultLocalUser" | "attributionEnabled">; +type GuiOnlyConfig = Omit<GuiConfig, keyof PreLoginConfig | "inviteOnly">; +type UserSystemConfig = Pick<GuiConfig, "inviteOnly">; + @Injectable({ providedIn: "root" }) export class GuiConfigService { - private config!: GuiConfig; + private config: Partial<GuiConfig> = {}; constructor(private http: HttpClient) {} - load(): Observable<GuiConfig> { - // Fetch both GUI config and user system config in parallel - const guiConfig$ = this.http.get<Omit<GuiConfig, "inviteOnly">>(`${AppSettings.getApiEndpoint()}/config/gui`); - const userSystemConfig$ = this.http.get<{ inviteOnly: boolean }>( - `${AppSettings.getApiEndpoint()}/config/user-system` + /** + * APP_INITIALIZER entry point. Always loads /config/pre-login (anonymous). If + * a JWT is already in localStorage (browser reload while logged in), chains + * /config/gui + /config/user-system in the same await so the full config is + * available before any post-login component mounts. + */ + load(): Observable<Partial<GuiConfig>> { + return this.loadPreLogin().pipe( + switchMap(() => { + if (!GuiConfigService.hasStoredAccessToken()) { + return of(this.config); + } + return this.loadPostLogin().pipe( + // Expired or malformed token → /config/gui returns 403. Continue + // with pre-login only; UserService.loginWithExistingToken detects + // expiry on its own, so we shouldn't block bootstrap on it. + catchError((err: unknown) => { + console.warn("Failed to load authenticated config; continuing with pre-login only.", err); + return of(this.config); + }) + ); + }) ); + } - return forkJoin([guiConfig$, userSystemConfig$]).pipe( - map(([guiConfig, userSystemConfig]) => { - // Merge both configurations - return { - ...guiConfig, - ...userSystemConfig, - } as GuiConfig; - }), - tap(config => { - this.config = config; - console.log("GUI configuration loaded successfully from backend"); + loadPreLogin(): Observable<Partial<GuiConfig>> { + return this.http.get<PreLoginConfig>(`${AppSettings.getApiEndpoint()}/config/pre-login`).pipe( + tap(preLogin => { + this.config = { ...this.config, ...preLogin }; }), + map(() => this.config), catchError((error: unknown) => { - console.error("Failed to load GUI configuration:", error); - return throwError(() => new Error(`Failed to load GUI configuration from backend: ${error}`)); + console.error("Failed to load pre-login configuration:", error); + return throwError(() => new Error(`Failed to load pre-login configuration from backend: ${error}`)); }) ); } + /** + * Fetches the authenticated portion of the configuration. Runs after the + * frontend has a valid JWT — called from APP_INITIALIZER on bootstrap when a + * stored token exists, and from UserService.handleAccessToken on fresh login. + */ + loadPostLogin(): Observable<Partial<GuiConfig>> { + const guiConfig$ = this.http.get<GuiOnlyConfig>(`${AppSettings.getApiEndpoint()}/config/gui`); + const userSystemConfig$ = this.http.get<UserSystemConfig>(`${AppSettings.getApiEndpoint()}/config/user-system`); + return forkJoin([guiConfig$, userSystemConfig$]).pipe( + tap(([guiConfig, userSystemConfig]) => { + this.config = { ...this.config, ...guiConfig, ...userSystemConfig }; + }), + map(() => this.config) + ); + } + get env(): GuiConfig { - if (!this.config) { - throw new Error("GUI configuration not loaded yet. Make sure load() is called during app initialization"); - } - return this.config; + return this.config as GuiConfig; + } + + private static hasStoredAccessToken(): boolean { + return typeof localStorage !== "undefined" && localStorage.getItem(ACCESS_TOKEN_KEY) != null; } } diff --git a/frontend/src/app/common/service/user/user.service.spec.ts b/frontend/src/app/common/service/user/user.service.spec.ts index 8c99f7f279..199a9ff44b 100644 --- a/frontend/src/app/common/service/user/user.service.spec.ts +++ b/frontend/src/app/common/service/user/user.service.spec.ts @@ -24,11 +24,14 @@ import { UserService } from "./user.service"; import { AuthService } from "./auth.service"; import { StubAuthService } from "./stub-auth.service"; import { skip } from "rxjs/operators"; +import { firstValueFrom, Subject, throwError } from "rxjs"; import { commonTestProviders } from "../../testing/test-utils"; import { HttpClientTestingModule } from "@angular/common/http/testing"; +import { GuiConfigService } from "../gui-config.service"; describe("UserService", () => { let service: UserService; + let config: GuiConfigService; beforeEach(() => { AuthService.removeAccessToken(); @@ -38,6 +41,7 @@ describe("UserService", () => { }); service = TestBed.inject(UserService); + config = TestBed.inject(GuiConfigService); }); afterAll(() => { @@ -108,4 +112,62 @@ describe("UserService", () => { expect((service as any).currentUser).toBeFalsy(); }); })); + + // ─── post-login config fetch coordination ───────────────────────────────── + + it("loads the authenticated config when a fresh login succeeds", async () => { + // /config/gui and /config/user-system are @RolesAllowed; their values must + // be in memory before any post-login component reads config.env, otherwise + // the dashboard renders against undefined flags. + const spy = vi.spyOn(config, "loadPostLogin"); + await firstValueFrom(service.login("test", "password")); + expect(spy).toHaveBeenCalledTimes(1); + expect(service.isLogin()).toBe(true); + }); + + it("loads the authenticated config when a googleLogin succeeds", async () => { + // googleLogin shares the same handleAccessToken plumbing as username/password + // login, so the post-login config fetch must fire here too — otherwise a + // user who only ever signs in through Google would see undefined flags. + const spy = vi.spyOn(config, "loadPostLogin"); + await firstValueFrom(service.googleLogin("any-credential")); + expect(spy).toHaveBeenCalledTimes(1); + expect(service.isLogin()).toBe(true); + }); + + it("orders the post-login config fetch before the userChanged event fires", async () => { + // Subscribers to userChanged (header, sidebar, routing guards) drive the + // initial dashboard render. If userChanged fires before loadPostLogin + // resolves, those subscribers see env without the authenticated fields and + // mis-render (e.g. copilot button missing, inviteOnly check skipped). + const gate = new Subject<unknown>(); + vi.spyOn(config, "loadPostLogin").mockReturnValue(gate.asObservable() as any); + + const userEmissions: Array<unknown> = []; + service + .userChanged() + .pipe(skip(1)) + .subscribe(u => userEmissions.push(u)); + + const loginPromise = firstValueFrom(service.login("test", "password")); + // Login is in-flight; loadPostLogin has not resolved yet, so userChanged + // must NOT have emitted a logged-in user yet. + expect(userEmissions).toEqual([]); + + gate.next({}); + gate.complete(); + await loginPromise; + + expect(userEmissions.length).toBe(1); + expect(userEmissions[0]).toBeTruthy(); + }); + + it("still completes login when loadPostLogin fails", async () => { + // Backend hiccup on /config/gui must not strand the user on a blank screen. + // The JwtAuthFilter on every protected endpoint is the authoritative gate; + // degraded config is preferable to a stuck spinner. + vi.spyOn(config, "loadPostLogin").mockReturnValue(throwError(() => new Error("simulated 500"))); + await firstValueFrom(service.login("test", "password")); + expect(service.isLogin()).toBe(true); + }); }); diff --git a/frontend/src/app/common/service/user/user.service.ts b/frontend/src/app/common/service/user/user.service.ts index c5f2d43635..9a55df6487 100644 --- a/frontend/src/app/common/service/user/user.service.ts +++ b/frontend/src/app/common/service/user/user.service.ts @@ -24,7 +24,7 @@ import { Observable, of, ReplaySubject } from "rxjs"; import { Role, User } from "../../type/user"; import { AuthService } from "./auth.service"; import { GuiConfigService } from "../gui-config.service"; -import { catchError, map, shareReplay } from "rxjs/operators"; +import { catchError, map, shareReplay, switchMap } from "rxjs/operators"; /** * User Service manages User information. It relies on different @@ -55,11 +55,13 @@ export class UserService { // validate the credentials with backend return this.authService .auth(username, password) - .pipe(map(({ accessToken }) => this.handleAccessToken(accessToken))); + .pipe(switchMap(({ accessToken }) => this.handleAccessToken(accessToken))); } public googleLogin(credential: string): Observable<void> { - return this.authService.googleAuth(credential).pipe(map(({ accessToken }) => this.handleAccessToken(accessToken))); + return this.authService + .googleAuth(credential) + .pipe(switchMap(({ accessToken }) => this.handleAccessToken(accessToken))); } public isLogin(): boolean { @@ -82,7 +84,7 @@ export class UserService { public register(username: string, password: string): Observable<void> { return this.authService .register(username, password) - .pipe(map(({ accessToken }) => this.handleAccessToken(accessToken))); + .pipe(switchMap(({ accessToken }) => this.handleAccessToken(accessToken))); } /** @@ -101,9 +103,24 @@ export class UserService { this.userChangeSubject.next(this.currentUser); } - private handleAccessToken(accessToken: string): void { + // Returns Observable<void> rather than void so callers (login / googleLogin / + // register) can switchMap through the post-login config fetch. The /config/gui + // and /config/user-system endpoints are @RolesAllowed, so we must wait for the + // new JWT to be in localStorage before they will answer; loginWithExistingToken + // also reads config.env.inviteOnly, so it must run after loadPostLogin resolves. + private handleAccessToken(accessToken: string): Observable<void> { AuthService.setAccessToken(accessToken); - this.changeUser(this.authService.loginWithExistingToken()); + return this.config.loadPostLogin().pipe( + catchError((err: unknown) => { + // If the authenticated config fetch fails, still complete login with + // whatever we have. The JwtAuthFilter on every protected endpoint is + // the authoritative gate; degraded config is preferable to a stuck + // login flow. + console.warn("Failed to load authenticated config after login; continuing.", err); + return of(undefined); + }), + map(() => this.changeUser(this.authService.loginWithExistingToken())) + ); } /**
