From 6d6b796dcf8fcdd4ccf610ac388464d812cdf69b Mon Sep 17 00:00:00 2001 From: Yicong Huang <17627829+Yicong-Huang@users.noreply.github.com> Date: Sat, 4 Jul 2026 14:58:13 -0700 Subject: [PATCH 1/6] refactor(config-service): move site_settings write API from texera-web to config-service Fold AdminSettingsResource's GET/PUT/reset endpoints into ConfigResource as /config/settings/{key}, so the service that seeds site_settings also owns its read/write API. PUT and reset stay ADMIN-only; the single-key GET keeps REGULAR+ADMIN because non-admin pages (dashboard logo/tabs, dataset upload limits) read it. The frontend service now rides the existing /api/config routing, so no proxy or nginx changes are needed. Co-Authored-By: Claude Fable 5 --- .../texera/web/TexeraWebApplication.scala | 2 - .../settings/AdminSettingsResource.scala | 108 ------------------ .../service/resource/ConfigResource.scala | 87 +++++++++++++- .../resource/ConfigResourceAuthSpec.scala | 56 ++++++++- .../admin/settings/admin-settings.service.ts | 2 +- 5 files changed, 141 insertions(+), 114 deletions(-) delete mode 100644 amber/src/main/scala/org/apache/texera/web/resource/dashboard/admin/settings/AdminSettingsResource.scala diff --git a/amber/src/main/scala/org/apache/texera/web/TexeraWebApplication.scala b/amber/src/main/scala/org/apache/texera/web/TexeraWebApplication.scala index 20ce01008b5..73e473ba7a4 100644 --- a/amber/src/main/scala/org/apache/texera/web/TexeraWebApplication.scala +++ b/amber/src/main/scala/org/apache/texera/web/TexeraWebApplication.scala @@ -36,7 +36,6 @@ import org.apache.texera.web.resource._ import org.apache.texera.web.resource.auth.{AuthResource, GoogleAuthResource} import org.apache.texera.web.resource.dashboard.DashboardResource import org.apache.texera.web.resource.dashboard.admin.execution.AdminExecutionResource -import org.apache.texera.web.resource.dashboard.admin.settings.AdminSettingsResource import org.apache.texera.web.resource.dashboard.admin.user.AdminUserResource import org.apache.texera.web.resource.dashboard.hub.HubResource import org.apache.texera.web.resource.dashboard.user.UserResource @@ -159,7 +158,6 @@ class TexeraWebApplication environment.jersey.register(classOf[GmailResource]) environment.jersey.register(classOf[AdminExecutionResource]) environment.jersey.register(classOf[UserQuotaResource]) - environment.jersey.register(classOf[AdminSettingsResource]) environment.jersey.register(classOf[AIAssistantResource]) environment.jersey.register(classOf[HuggingFaceModelResource]) diff --git a/amber/src/main/scala/org/apache/texera/web/resource/dashboard/admin/settings/AdminSettingsResource.scala b/amber/src/main/scala/org/apache/texera/web/resource/dashboard/admin/settings/AdminSettingsResource.scala deleted file mode 100644 index a1880f3c3ca..00000000000 --- a/amber/src/main/scala/org/apache/texera/web/resource/dashboard/admin/settings/AdminSettingsResource.scala +++ /dev/null @@ -1,108 +0,0 @@ -/* - * 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. - */ - -package org.apache.texera.web.resource.dashboard.admin.settings - -import com.fasterxml.jackson.annotation.JsonProperty -import io.dropwizard.auth.Auth -import org.apache.texera.auth.SessionUser -import org.apache.texera.common.config.DefaultsConfig -import org.apache.texera.dao.SqlServer -import org.jooq.impl.DSL - -import javax.annotation.security.RolesAllowed -import javax.ws.rs._ -import javax.ws.rs.core.{MediaType, Response} - -case class AdminSettingsPojo( - @JsonProperty("key") settingKey: String, - @JsonProperty("value") settingValue: String -) - -@Path("/admin/settings") -@Produces(Array(MediaType.APPLICATION_JSON)) -class AdminSettingsResource { - - private def ctx = SqlServer.getInstance().createDSLContext() - private val siteSettings = DSL.table("site_settings") - private val key = DSL.field("key", classOf[String]) - private val value = DSL.field("value", classOf[String]) - private val updatedBy = DSL.field("updated_by", classOf[String]) - - @GET - @Path("{key}") - def getSetting(@PathParam("key") keyParam: String): AdminSettingsPojo = { - ctx - .select(key, value) - .from(siteSettings) - .where(key.eq(keyParam)) - .fetchOneInto(classOf[AdminSettingsPojo]) - } - - @PUT - @Path("{key}") - @RolesAllowed(Array("ADMIN")) - @Consumes(Array(MediaType.APPLICATION_JSON)) - def updateSetting( - @Auth currentUser: SessionUser, - @PathParam("key") keyParam: String, - setting: AdminSettingsPojo - ): Response = { - if (setting.settingValue != null && keyParam.nonEmpty) { - upsertSetting(keyParam, setting.settingValue, currentUser.getName) - } - Response.ok().build() - } - - /** - * Resets the specified configuration key to its default value defined in default.conf. - */ - @POST - @Path("/reset/{key}") - @RolesAllowed(Array("ADMIN")) - def resetSetting( - @Auth currentUser: SessionUser, - @PathParam("key") keyParam: String - ): Response = { - DefaultsConfig.allDefaults.get(keyParam) match { - case Some(defaultValue) => - upsertSetting(keyParam, defaultValue, currentUser.getName) - Response.ok().build() - case None => - Response - .status(Response.Status.NOT_FOUND) - .entity(s"No default for key '$keyParam'") - .build() - } - } - - private def upsertSetting(keyParam: String, valueParam: String, userName: String): Unit = { - ctx - .insertInto(siteSettings) - .set(key, keyParam) - .set(value, valueParam) - .set(updatedBy, userName) - .onConflict(key) - .doUpdate() - .set(value, valueParam) - .set(DSL.field("updated_by", classOf[String]), userName) - .set(DSL.field("updated_at", classOf[java.sql.Timestamp]), DSL.currentTimestamp()) - .execute() - } -} 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 72f346349a2..ce91bc42746 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,21 +19,38 @@ package org.apache.texera.service.resource +import com.fasterxml.jackson.annotation.JsonProperty +import io.dropwizard.auth.Auth import jakarta.annotation.security.{PermitAll, RolesAllowed} -import jakarta.ws.rs.core.MediaType -import jakarta.ws.rs.{GET, Path, Produces} +import jakarta.ws.rs.core.{MediaType, Response} +import jakarta.ws.rs.{Consumes, GET, POST, PUT, Path, PathParam, Produces} +import org.apache.texera.auth.SessionUser import org.apache.texera.common.config.{ ApplicationConfig, AuthConfig, ComputingUnitConfig, + DefaultsConfig, GuiConfig, UserSystemConfig } +import org.apache.texera.dao.SqlServer +import org.jooq.impl.DSL + +case class ConfigSettingPojo( + @JsonProperty("key") settingKey: String, + @JsonProperty("value") settingValue: String +) @Path("/config") @Produces(Array(MediaType.APPLICATION_JSON)) class ConfigResource { + private def ctx = SqlServer.getInstance().createDSLContext() + private val siteSettings = DSL.table("site_settings") + private val key = DSL.field("key", classOf[String]) + private val value = DSL.field("value", classOf[String]) + private val updatedBy = DSL.field("updated_by", classOf[String]) + // 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 @@ -99,4 +116,70 @@ class ConfigResource { // flags from the user-system.conf "inviteOnly" -> UserSystemConfig.inviteOnly ) + + // Write side of the config API, backed by the site_settings table this + // service seeds at startup. Reads stay open to any logged-in user because + // non-admin pages (dashboard logo/tabs, dataset upload limits) consume + // individual keys; only mutation is ADMIN-gated. + @GET + @RolesAllowed(Array("REGULAR", "ADMIN")) + @Path("/settings/{key}") + def getSetting(@PathParam("key") keyParam: String): ConfigSettingPojo = { + ctx + .select(key, value) + .from(siteSettings) + .where(key.eq(keyParam)) + .fetchOneInto(classOf[ConfigSettingPojo]) + } + + @PUT + @RolesAllowed(Array("ADMIN")) + @Path("/settings/{key}") + @Consumes(Array(MediaType.APPLICATION_JSON)) + def updateSetting( + @Auth currentUser: SessionUser, + @PathParam("key") keyParam: String, + setting: ConfigSettingPojo + ): Response = { + if (setting.settingValue != null && keyParam.nonEmpty) { + upsertSetting(keyParam, setting.settingValue, currentUser.getName) + } + Response.ok().build() + } + + /** + * Resets the specified configuration key to its default value defined in default.conf. + */ + @POST + @RolesAllowed(Array("ADMIN")) + @Path("/settings/reset/{key}") + def resetSetting( + @Auth currentUser: SessionUser, + @PathParam("key") keyParam: String + ): Response = { + DefaultsConfig.allDefaults.get(keyParam) match { + case Some(defaultValue) => + upsertSetting(keyParam, defaultValue, currentUser.getName) + Response.ok().build() + case None => + Response + .status(Response.Status.NOT_FOUND) + .entity(s"No default for key '$keyParam'") + .build() + } + } + + private def upsertSetting(keyParam: String, valueParam: String, userName: String): Unit = { + ctx + .insertInto(siteSettings) + .set(key, keyParam) + .set(value, valueParam) + .set(updatedBy, userName) + .onConflict(key) + .doUpdate() + .set(value, valueParam) + .set(updatedBy, userName) + .set(DSL.field("updated_at", classOf[java.sql.Timestamp]), DSL.currentTimestamp()) + .execute() + } } 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 e395f5f89d1..47b81a87ba5 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 @@ -21,12 +21,14 @@ package org.apache.texera.service.resource import com.fasterxml.jackson.databind.ObjectMapper import com.fasterxml.jackson.module.scala.DefaultScalaModule +import io.dropwizard.auth.AuthValueFactoryProvider import io.dropwizard.jackson.Jackson import io.dropwizard.testing.junit5.ResourceExtension import jakarta.annotation.security.RolesAllowed +import jakarta.ws.rs.client.Entity import jakarta.ws.rs.core.MediaType import jakarta.ws.rs.{GET, Path, Produces} -import org.apache.texera.auth.{JwtAuth, JwtAuthFilter, UnauthorizedExceptionMapper} +import org.apache.texera.auth.{JwtAuth, JwtAuthFilter, SessionUser, UnauthorizedExceptionMapper} import org.apache.texera.dao.jooq.generated.enums.UserRoleEnum import org.apache.texera.dao.jooq.generated.tables.pojos.User import org.glassfish.jersey.server.filter.RolesAllowedDynamicFeature @@ -56,6 +58,10 @@ class ConfigResourceAuthSpec extends AnyFlatSpec with Matchers with BeforeAndAft .addProvider(classOf[JwtAuthFilter]) .addProvider(classOf[UnauthorizedExceptionMapper]) .addProvider(classOf[RolesAllowedDynamicFeature]) + // Production (AuthFeatures.register) binds this so @Auth SessionUser + // parameters resolve; without it the /config/settings write endpoints + // fail resource-model validation at startup. + .addProvider(new AuthValueFactoryProvider.Binder(classOf[SessionUser])) .addResource(new ConfigResource) .addResource(new ConfigResourceAuthSpec.ProtectedProbe) .build() @@ -188,6 +194,54 @@ class ConfigResourceAuthSpec extends AnyFlatSpec with Matchers with BeforeAndAft guiPayload.keySet should not contain "defaultDataTransferBatchSize" } + // /config/settings is the write side over site_settings: reads are open to + // any logged-in user (non-admin pages consume individual keys), mutation is + // ADMIN-only. Positive read/write paths need a database, so this spec only + // pins the auth gates plus the one ADMIN path that never reaches the DB + // (reset of a key absent from default.conf → 404). + "GET /config/settings/{key}" should "return 401 with a Bearer challenge without an Authorization header" in { + val response = + resources.target("/config/settings/logo").request(MediaType.APPLICATION_JSON).get() + response.getStatus shouldBe 401 + response.getHeaderString("WWW-Authenticate") shouldBe JwtAuthFilter.BearerChallenge + } + + "PUT /config/settings/{key}" should "return 401 without an Authorization header" in { + val response = resources + .target("/config/settings/logo") + .request(MediaType.APPLICATION_JSON) + .put(Entity.json("""{"key":"logo","value":"x"}""")) + response.getStatus shouldBe 401 + response.getHeaderString("WWW-Authenticate") shouldBe JwtAuthFilter.BearerChallenge + } + + it should "return 403 for a REGULAR user" in { + val response = resources + .target("/config/settings/logo") + .request(MediaType.APPLICATION_JSON) + .header("Authorization", s"Bearer ${regularToken()}") + .put(Entity.json("""{"key":"logo","value":"x"}""")) + response.getStatus shouldBe 403 + } + + "POST /config/settings/reset/{key}" should "return 403 for a REGULAR user" in { + val response = resources + .target("/config/settings/reset/logo") + .request(MediaType.APPLICATION_JSON) + .header("Authorization", s"Bearer ${regularToken()}") + .post(Entity.json("{}")) + response.getStatus shouldBe 403 + } + + it should "pass the role gate for an ADMIN (404 for a key with no default)" in { + val response = resources + .target("/config/settings/reset/no-such-key") + .request(MediaType.APPLICATION_JSON) + .header("Authorization", s"Bearer ${adminToken()}") + .post(Entity.json("{}")) + response.getStatus shouldBe 404 + } + "GET an @RolesAllowed probe endpoint" should "return 401 without an Authorization header" in { // Sanity: JwtAuthFilter is now eager — missing Authorization is rejected // by the filter itself with a 401 + Bearer challenge, before diff --git a/frontend/src/app/dashboard/service/admin/settings/admin-settings.service.ts b/frontend/src/app/dashboard/service/admin/settings/admin-settings.service.ts index 9a9b40d3da7..72d38ca99e0 100644 --- a/frontend/src/app/dashboard/service/admin/settings/admin-settings.service.ts +++ b/frontend/src/app/dashboard/service/admin/settings/admin-settings.service.ts @@ -31,7 +31,7 @@ import { map } from "rxjs/operators"; providedIn: "root", }) export class AdminSettingsService { - private readonly BASE_URL = "/api/admin/settings"; + private readonly BASE_URL = "/api/config/settings"; constructor(private http: HttpClient) {} getSetting(key: string): Observable { From 2b7ed36b10dd320d69470d76976ac22742f6cdfa Mon Sep 17 00:00:00 2001 From: Yicong Huang <17627829+Yicong-Huang@users.noreply.github.com> Date: Sat, 4 Jul 2026 15:03:06 -0700 Subject: [PATCH 2/6] refactor(config-service): use generated jOOQ SITE_SETTINGS table instead of string-based DSL Co-Authored-By: Claude Fable 5 --- .../service/resource/ConfigResource.scala | 29 +++++++++---------- 1 file changed, 14 insertions(+), 15 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 ce91bc42746..3928cfb2fd8 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 @@ -34,8 +34,11 @@ import org.apache.texera.common.config.{ UserSystemConfig } import org.apache.texera.dao.SqlServer +import org.apache.texera.dao.jooq.generated.Tables.SITE_SETTINGS import org.jooq.impl.DSL +// Wire DTO for /config/settings: the JSON contract is exactly {key, value}; +// the generated jOOQ pojo would also expose updated_by/updated_at. case class ConfigSettingPojo( @JsonProperty("key") settingKey: String, @JsonProperty("value") settingValue: String @@ -46,10 +49,6 @@ case class ConfigSettingPojo( class ConfigResource { private def ctx = SqlServer.getInstance().createDSLContext() - private val siteSettings = DSL.table("site_settings") - private val key = DSL.field("key", classOf[String]) - private val value = DSL.field("value", classOf[String]) - private val updatedBy = DSL.field("updated_by", classOf[String]) // 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 @@ -126,9 +125,9 @@ class ConfigResource { @Path("/settings/{key}") def getSetting(@PathParam("key") keyParam: String): ConfigSettingPojo = { ctx - .select(key, value) - .from(siteSettings) - .where(key.eq(keyParam)) + .select(SITE_SETTINGS.KEY, SITE_SETTINGS.VALUE) + .from(SITE_SETTINGS) + .where(SITE_SETTINGS.KEY.eq(keyParam)) .fetchOneInto(classOf[ConfigSettingPojo]) } @@ -171,15 +170,15 @@ class ConfigResource { private def upsertSetting(keyParam: String, valueParam: String, userName: String): Unit = { ctx - .insertInto(siteSettings) - .set(key, keyParam) - .set(value, valueParam) - .set(updatedBy, userName) - .onConflict(key) + .insertInto(SITE_SETTINGS) + .set(SITE_SETTINGS.KEY, keyParam) + .set(SITE_SETTINGS.VALUE, valueParam) + .set(SITE_SETTINGS.UPDATED_BY, userName) + .onConflict(SITE_SETTINGS.KEY) .doUpdate() - .set(value, valueParam) - .set(updatedBy, userName) - .set(DSL.field("updated_at", classOf[java.sql.Timestamp]), DSL.currentTimestamp()) + .set(SITE_SETTINGS.VALUE, valueParam) + .set(SITE_SETTINGS.UPDATED_BY, userName) + .set(SITE_SETTINGS.UPDATED_AT, DSL.currentTimestamp()) .execute() } } From 86b76b7aa263cb57b6b8b6d4e0818a08423b03db Mon Sep 17 00:00:00 2001 From: Yicong Huang <17627829+Yicong-Huang@users.noreply.github.com> Date: Sat, 4 Jul 2026 15:24:23 -0700 Subject: [PATCH 3/6] test(config-service): cover /config/settings positive paths with embedded Postgres Co-Authored-By: Claude Fable 5 --- build.sbt | 1 + .../resource/ConfigSettingsCrudSpec.scala | 113 ++++++++++++++++++ 2 files changed, 114 insertions(+) create mode 100644 config-service/src/test/scala/org/apache/texera/service/resource/ConfigSettingsCrudSpec.scala diff --git a/build.sbt b/build.sbt index 6d7674fbd24..1fb5fb2c7fe 100644 --- a/build.sbt +++ b/build.sbt @@ -78,6 +78,7 @@ lazy val Auth = (project in file("common/auth")) .dependsOn(DAO % "test->test") // reuse MockTexeraDB embedded Postgres in tests lazy val ConfigService = (project in file("config-service")) .dependsOn(Auth, Config, Resource) + .dependsOn(DAO % "test->test") // reuse MockTexeraDB embedded Postgres in tests .settings(commonModuleSettings) .settings( dependencyOverrides ++= Seq( diff --git a/config-service/src/test/scala/org/apache/texera/service/resource/ConfigSettingsCrudSpec.scala b/config-service/src/test/scala/org/apache/texera/service/resource/ConfigSettingsCrudSpec.scala new file mode 100644 index 00000000000..246454652ae --- /dev/null +++ b/config-service/src/test/scala/org/apache/texera/service/resource/ConfigSettingsCrudSpec.scala @@ -0,0 +1,113 @@ +/* + * 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. + */ + +package org.apache.texera.service.resource + +import org.apache.texera.auth.SessionUser +import org.apache.texera.common.config.DefaultsConfig +import org.apache.texera.dao.MockTexeraDB +import org.apache.texera.dao.jooq.generated.Tables.SITE_SETTINGS +import org.apache.texera.dao.jooq.generated.tables.pojos.User +import org.scalatest.BeforeAndAfterAll +import org.scalatest.flatspec.AnyFlatSpec +import org.scalatest.matchers.should.Matchers + +// Positive-path coverage for the /config/settings endpoints against an +// embedded Postgres: ConfigResourceAuthSpec pins the auth gates (401/403), +// this spec exercises the bodies — read-miss, insert, upsert-on-conflict, +// reset-to-default — by calling the resource methods directly. +class ConfigSettingsCrudSpec + extends AnyFlatSpec + with Matchers + with BeforeAndAfterAll + with MockTexeraDB { + + private val resource = new ConfigResource + + private def adminSession(name: String = "test-admin"): SessionUser = { + val u = new User() + u.setUid(1) + u.setName(name) + new SessionUser(u) + } + + override protected def beforeAll(): Unit = { + initializeDBAndReplaceDSLContext() + } + + override protected def afterAll(): Unit = { + shutdownDB() + } + + "GET /config/settings/{key}" should "return null for a key that has no row" in { + resource.getSetting("no-such-key") shouldBe null + } + + "PUT /config/settings/{key}" should "insert a new row and record who wrote it" in { + val response = + resource.updateSetting(adminSession(), "logo", ConfigSettingPojo("logo", "custom.png")) + response.getStatus shouldBe 200 + + val stored = resource.getSetting("logo") + stored.settingKey shouldBe "logo" + stored.settingValue shouldBe "custom.png" + + getDSLContext + .select(SITE_SETTINGS.UPDATED_BY) + .from(SITE_SETTINGS) + .where(SITE_SETTINGS.KEY.eq("logo")) + .fetchOne(SITE_SETTINGS.UPDATED_BY) shouldBe "test-admin" + } + + it should "update the existing row on a repeated PUT (upsert conflict path)" in { + resource.updateSetting(adminSession(), "logo", ConfigSettingPojo("logo", "v1.png")) + resource.updateSetting( + adminSession("second-admin"), + "logo", + ConfigSettingPojo("logo", "v2.png") + ) + + resource.getSetting("logo").settingValue shouldBe "v2.png" + getDSLContext.fetchCount(SITE_SETTINGS, SITE_SETTINGS.KEY.eq("logo")) shouldBe 1 + getDSLContext + .select(SITE_SETTINGS.UPDATED_BY) + .from(SITE_SETTINGS) + .where(SITE_SETTINGS.KEY.eq("logo")) + .fetchOne(SITE_SETTINGS.UPDATED_BY) shouldBe "second-admin" + } + + it should "be a no-op when the payload carries a null value" in { + resource.updateSetting(adminSession(), "logo", ConfigSettingPojo("logo", "kept.png")) + val response = resource.updateSetting(adminSession(), "logo", ConfigSettingPojo("logo", null)) + response.getStatus shouldBe 200 + resource.getSetting("logo").settingValue shouldBe "kept.png" + } + + "POST /config/settings/reset/{key}" should "restore the default.conf value for a known key" in { + resource.updateSetting(adminSession(), "logo", ConfigSettingPojo("logo", "overridden.png")) + + val response = resource.resetSetting(adminSession(), "logo") + response.getStatus shouldBe 200 + resource.getSetting("logo").settingValue shouldBe DefaultsConfig.allDefaults("logo") + } + + it should "return 404 for a key that has no default" in { + resource.resetSetting(adminSession(), "no-such-key").getStatus shouldBe 404 + } +} From fb3b9ef127ffe489fcf466176c1914165d2d244c Mon Sep 17 00:00:00 2001 From: Yicong Huang <17627829+Yicong-Huang@users.noreply.github.com> Date: Sat, 4 Jul 2026 15:25:51 -0700 Subject: [PATCH 4/6] build: declare ConfigService's direct DAO dependency explicitly ConfigService main code uses SqlServer and the generated jOOQ tables directly but only got DAO transitively through Auth; declare it like the other services that touch the database do. Co-Authored-By: Claude Fable 5 --- build.sbt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/build.sbt b/build.sbt index 1fb5fb2c7fe..99e58912dfb 100644 --- a/build.sbt +++ b/build.sbt @@ -77,7 +77,7 @@ lazy val Auth = (project in file("common/auth")) .dependsOn(DAO, Config) .dependsOn(DAO % "test->test") // reuse MockTexeraDB embedded Postgres in tests lazy val ConfigService = (project in file("config-service")) - .dependsOn(Auth, Config, Resource) + .dependsOn(Auth, Config, DAO, Resource) .dependsOn(DAO % "test->test") // reuse MockTexeraDB embedded Postgres in tests .settings(commonModuleSettings) .settings( From f719799383e214c21b8772ef73f38112b54ba27d Mon Sep 17 00:00:00 2001 From: Yicong Huang <17627829+Yicong-Huang@users.noreply.github.com> Date: Sat, 4 Jul 2026 17:27:21 -0700 Subject: [PATCH 5/6] refactor(config-service): split user-facing settings reads from the admin single-key GET Non-admin pages only need 19 whitelisted keys (branding, sidebar tabs, upload limits); serve those in one payload via GET /config/settings/public (REGULAR+ADMIN) and make the arbitrary single-key GET ADMIN-only. The frontend reads public settings through one cached request instead of a request per key. Co-Authored-By: Claude Fable 5 --- .../service/resource/ConfigResource.scala | 50 +++++++++++++++++-- .../resource/ConfigResourceAuthSpec.scala | 26 ++++++++-- .../resource/ConfigSettingsCrudSpec.scala | 13 +++++ .../component/dashboard.component.spec.ts | 2 +- .../component/dashboard.component.ts | 8 +-- .../files-uploader.component.ts | 2 +- .../dataset-detail.component.ts | 6 +-- .../admin/settings/admin-settings.service.ts | 22 +++++++- 8 files changed, 110 insertions(+), 19 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 3928cfb2fd8..d7c262a0429 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 @@ -37,6 +37,8 @@ import org.apache.texera.dao.SqlServer import org.apache.texera.dao.jooq.generated.Tables.SITE_SETTINGS import org.jooq.impl.DSL +import scala.jdk.CollectionConverters._ + // Wire DTO for /config/settings: the JSON contract is exactly {key, value}; // the generated jOOQ pojo would also expose updated_by/updated_at. case class ConfigSettingPojo( @@ -116,12 +118,52 @@ class ConfigResource { "inviteOnly" -> UserSystemConfig.inviteOnly ) - // Write side of the config API, backed by the site_settings table this - // service seeds at startup. Reads stay open to any logged-in user because - // non-admin pages (dashboard logo/tabs, dataset upload limits) consume - // individual keys; only mutation is ADMIN-gated. + // The site_settings keys that logged-in, non-admin pages consume: dashboard + // branding, sidebar tab toggles, and dataset upload limits. Everything else + // in the table (e.g. csv_parser_max_columns) is management-only. Adding a + // key here makes it readable by every user — the explicit list forces that + // decision into review, same as the /pre-login payload above. + private val publicSettingKeys: Set[String] = Set( + "logo", + "mini_logo", + "favicon", + "hub_enabled", + "home_enabled", + "workflow_enabled", + "dataset_enabled", + "your_work_enabled", + "projects_enabled", + "workflows_enabled", + "datasets_enabled", + "compute_enabled", + "quota_enabled", + "forum_enabled", + "about_enabled", + "single_file_upload_max_size_mib", + "multipart_upload_chunk_size_mib", + "max_number_of_concurrent_uploading_file", + "max_number_of_concurrent_uploading_file_chunks" + ) + + // Read side for regular users: the public keys in one payload, so the + // dashboard doesn't fire a request per key. @GET @RolesAllowed(Array("REGULAR", "ADMIN")) + @Path("/settings/public") + def getPublicSettings: Map[String, String] = { + ctx + .select(SITE_SETTINGS.KEY, SITE_SETTINGS.VALUE) + .from(SITE_SETTINGS) + .where(SITE_SETTINGS.KEY.in(publicSettingKeys.asJava)) + .fetchMap(SITE_SETTINGS.KEY, SITE_SETTINGS.VALUE) + .asScala + .toMap + } + + // Management read over the site_settings table this service seeds at + // startup: any key, including the ones not exposed through /settings/public. + @GET + @RolesAllowed(Array("ADMIN")) @Path("/settings/{key}") def getSetting(@PathParam("key") keyParam: String): ConfigSettingPojo = { ctx 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 47b81a87ba5..d00168af8d4 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 @@ -194,11 +194,18 @@ class ConfigResourceAuthSpec extends AnyFlatSpec with Matchers with BeforeAndAft guiPayload.keySet should not contain "defaultDataTransferBatchSize" } - // /config/settings is the write side over site_settings: reads are open to - // any logged-in user (non-admin pages consume individual keys), mutation is - // ADMIN-only. Positive read/write paths need a database, so this spec only - // pins the auth gates plus the one ADMIN path that never reaches the DB - // (reset of a key absent from default.conf → 404). + // /config/settings is the site_settings API: /settings/public serves the + // whitelisted user-visible keys to any logged-in user; the single-key read + // and all mutation are ADMIN-only. Positive read/write paths need a + // database, so this spec only pins the auth gates plus the one ADMIN path + // that never reaches the DB (reset of a key absent from default.conf → 404). + "GET /config/settings/public" should "return 401 with a Bearer challenge without an Authorization header" in { + val response = + resources.target("/config/settings/public").request(MediaType.APPLICATION_JSON).get() + response.getStatus shouldBe 401 + response.getHeaderString("WWW-Authenticate") shouldBe JwtAuthFilter.BearerChallenge + } + "GET /config/settings/{key}" should "return 401 with a Bearer challenge without an Authorization header" in { val response = resources.target("/config/settings/logo").request(MediaType.APPLICATION_JSON).get() @@ -206,6 +213,15 @@ class ConfigResourceAuthSpec extends AnyFlatSpec with Matchers with BeforeAndAft response.getHeaderString("WWW-Authenticate") shouldBe JwtAuthFilter.BearerChallenge } + it should "return 403 for a REGULAR user (management read is ADMIN-only)" in { + val response = resources + .target("/config/settings/logo") + .request(MediaType.APPLICATION_JSON) + .header("Authorization", s"Bearer ${regularToken()}") + .get() + response.getStatus shouldBe 403 + } + "PUT /config/settings/{key}" should "return 401 without an Authorization header" in { val response = resources .target("/config/settings/logo") diff --git a/config-service/src/test/scala/org/apache/texera/service/resource/ConfigSettingsCrudSpec.scala b/config-service/src/test/scala/org/apache/texera/service/resource/ConfigSettingsCrudSpec.scala index 246454652ae..733eab14ddb 100644 --- a/config-service/src/test/scala/org/apache/texera/service/resource/ConfigSettingsCrudSpec.scala +++ b/config-service/src/test/scala/org/apache/texera/service/resource/ConfigSettingsCrudSpec.scala @@ -99,6 +99,19 @@ class ConfigSettingsCrudSpec resource.getSetting("logo").settingValue shouldBe "kept.png" } + "GET /config/settings/public" should "serve whitelisted keys and hide management-only ones" in { + resource.updateSetting(adminSession(), "favicon", ConfigSettingPojo("favicon", "fav.ico")) + resource.updateSetting( + adminSession(), + "csv_parser_max_columns", + ConfigSettingPojo("csv_parser_max_columns", "4096") + ) + + val publicSettings = resource.getPublicSettings + publicSettings("favicon") shouldBe "fav.ico" + publicSettings should not contain key("csv_parser_max_columns") + } + "POST /config/settings/reset/{key}" should "restore the default.conf value for a known key" in { resource.updateSetting(adminSession(), "logo", ConfigSettingPojo("logo", "overridden.png")) diff --git a/frontend/src/app/dashboard/component/dashboard.component.spec.ts b/frontend/src/app/dashboard/component/dashboard.component.spec.ts index cad7ed91be2..245258144ad 100644 --- a/frontend/src/app/dashboard/component/dashboard.component.spec.ts +++ b/frontend/src/app/dashboard/component/dashboard.component.spec.ts @@ -130,7 +130,7 @@ describe("DashboardComponent", () => { }; adminSettingsServiceMock = { - getSetting: vi.fn().mockReturnValue(EMPTY), + getPublicSetting: vi.fn().mockReturnValue(EMPTY), }; activatedRouteMock = { diff --git a/frontend/src/app/dashboard/component/dashboard.component.ts b/frontend/src/app/dashboard/component/dashboard.component.ts index 88dc04bb690..29f6b7bd43a 100644 --- a/frontend/src/app/dashboard/component/dashboard.component.ts +++ b/frontend/src/app/dashboard/component/dashboard.component.ts @@ -176,21 +176,21 @@ export class DashboardComponent implements OnInit { loadLogos(): void { this.adminSettingsService - .getSetting("logo") + .getPublicSetting("logo") .pipe(untilDestroyed(this)) .subscribe(dataUri => { this.logo = dataUri; }); this.adminSettingsService - .getSetting("mini_logo") + .getPublicSetting("mini_logo") .pipe(untilDestroyed(this)) .subscribe(dataUri => { this.miniLogo = dataUri; }); this.adminSettingsService - .getSetting("favicon") + .getPublicSetting("favicon") .pipe(untilDestroyed(this)) .subscribe(dataUri => { document.querySelectorAll("link[rel*='icon']").forEach(el => ((el as HTMLLinkElement).href = dataUri)); @@ -200,7 +200,7 @@ export class DashboardComponent implements OnInit { loadTabs(): void { (Object.keys(this.sidebarTabs) as (keyof SidebarTabs)[]).forEach(tab => { this.adminSettingsService - .getSetting(tab) + .getPublicSetting(tab) .pipe(untilDestroyed(this)) .subscribe(value => { this.sidebarTabs[tab] = value === "true"; diff --git a/frontend/src/app/dashboard/component/user/files-uploader/files-uploader.component.ts b/frontend/src/app/dashboard/component/user/files-uploader/files-uploader.component.ts index 6967d46b61a..5fd00c4a8aa 100644 --- a/frontend/src/app/dashboard/component/user/files-uploader/files-uploader.component.ts +++ b/frontend/src/app/dashboard/component/user/files-uploader/files-uploader.component.ts @@ -82,7 +82,7 @@ export class FilesUploaderComponent { private modal: NzModalService ) { this.adminSettingsService - .getSetting("single_file_upload_max_size_mib") + .getPublicSetting("single_file_upload_max_size_mib") .pipe(untilDestroyed(this)) .subscribe(value => (this.singleFileUploadMaxSizeMiB = parseInt(value))); } diff --git a/frontend/src/app/dashboard/component/user/user-dataset/user-dataset-explorer/dataset-detail.component.ts b/frontend/src/app/dashboard/component/user/user-dataset/user-dataset-explorer/dataset-detail.component.ts index 2287050ed50..492ded2f0a5 100644 --- a/frontend/src/app/dashboard/component/user/user-dataset/user-dataset-explorer/dataset-detail.component.ts +++ b/frontend/src/app/dashboard/component/user/user-dataset/user-dataset-explorer/dataset-detail.component.ts @@ -462,15 +462,15 @@ export class DatasetDetailComponent implements OnInit { private loadUploadSettings(): void { this.adminSettingsService - .getSetting("multipart_upload_chunk_size_mib") + .getPublicSetting("multipart_upload_chunk_size_mib") .pipe(untilDestroyed(this)) .subscribe(value => (this.chunkSizeMiB = parseInt(value))); this.adminSettingsService - .getSetting("max_number_of_concurrent_uploading_file_chunks") + .getPublicSetting("max_number_of_concurrent_uploading_file_chunks") .pipe(untilDestroyed(this)) .subscribe(value => (this.maxConcurrentChunks = parseInt(value))); this.adminSettingsService - .getSetting("max_number_of_concurrent_uploading_file") + .getPublicSetting("max_number_of_concurrent_uploading_file") .pipe(untilDestroyed(this)) .subscribe(value => { this.maxConcurrentFiles = parseInt(value); diff --git a/frontend/src/app/dashboard/service/admin/settings/admin-settings.service.ts b/frontend/src/app/dashboard/service/admin/settings/admin-settings.service.ts index 72d38ca99e0..6d5017a0196 100644 --- a/frontend/src/app/dashboard/service/admin/settings/admin-settings.service.ts +++ b/frontend/src/app/dashboard/service/admin/settings/admin-settings.service.ts @@ -20,7 +20,7 @@ import { Injectable } from "@angular/core"; import { HttpClient } from "@angular/common/http"; import { Observable } from "rxjs"; -import { map } from "rxjs/operators"; +import { map, shareReplay } from "rxjs/operators"; /** * Service for managing site-wide settings (key-value pairs) via REST API. @@ -32,8 +32,28 @@ import { map } from "rxjs/operators"; }) export class AdminSettingsService { private readonly BASE_URL = "/api/config/settings"; + + // One request for all user-visible settings, shared by every consumer. + // The admin settings page reloads the whole window after saving, so a + // per-page-load cache never serves stale values. + private publicSettings$?: Observable>; + constructor(private http: HttpClient) {} + /** + * Reads one of the user-visible settings (branding, sidebar tabs, upload + * limits) through the aggregated REGULAR-accessible endpoint. + */ + getPublicSetting(key: string): Observable { + if (!this.publicSettings$) { + this.publicSettings$ = this.http.get>(`${this.BASE_URL}/public`).pipe(shareReplay(1)); + } + return this.publicSettings$.pipe(map(settings => settings[key] ?? null)); + } + + /** + * Reads any setting by key. ADMIN-only; used by the admin settings page. + */ getSetting(key: string): Observable { return this.http .get<{ key: string; value: string }>(`${this.BASE_URL}/${key}`) From 0cac2addee0034abc95dbbc811768c2f62946a43 Mon Sep 17 00:00:00 2001 From: Yicong Huang <17627829+Yicong-Huang@users.noreply.github.com> Date: Sat, 4 Jul 2026 19:36:09 -0700 Subject: [PATCH 6/6] fix(frontend): don't let shareReplay cache a failed public-settings fetch Address review: if the first /config/settings/public request errors, shareReplay(1) would replay that error to every consumer with no retry. Drop the cached observable on error so the next read retries. Co-Authored-By: Claude Fable 5 --- .../admin/settings/admin-settings.service.ts | 15 ++++++++++++--- 1 file changed, 12 insertions(+), 3 deletions(-) diff --git a/frontend/src/app/dashboard/service/admin/settings/admin-settings.service.ts b/frontend/src/app/dashboard/service/admin/settings/admin-settings.service.ts index 6d5017a0196..1fd0c5398cc 100644 --- a/frontend/src/app/dashboard/service/admin/settings/admin-settings.service.ts +++ b/frontend/src/app/dashboard/service/admin/settings/admin-settings.service.ts @@ -19,8 +19,8 @@ import { Injectable } from "@angular/core"; import { HttpClient } from "@angular/common/http"; -import { Observable } from "rxjs"; -import { map, shareReplay } from "rxjs/operators"; +import { Observable, throwError } from "rxjs"; +import { catchError, map, shareReplay } from "rxjs/operators"; /** * Service for managing site-wide settings (key-value pairs) via REST API. @@ -46,7 +46,16 @@ export class AdminSettingsService { */ getPublicSetting(key: string): Observable { if (!this.publicSettings$) { - this.publicSettings$ = this.http.get>(`${this.BASE_URL}/public`).pipe(shareReplay(1)); + this.publicSettings$ = this.http.get>(`${this.BASE_URL}/public`).pipe( + // shareReplay would otherwise cache a failed fetch and replay the + // error to every consumer forever; drop the cached observable so the + // next getPublicSetting call retries the request. + catchError((err: unknown) => { + this.publicSettings$ = undefined; + return throwError(() => err); + }), + shareReplay(1) + ); } return this.publicSettings$.pipe(map(settings => settings[key] ?? null)); }