Ma77Ball commented on code in PR #5199:
URL: https://github.com/apache/texera/pull/5199#discussion_r3321711439
##########
config-service/src/main/scala/org/apache/texera/service/resource/ConfigResource.scala:
##########
@@ -28,8 +28,15 @@ import org.apache.texera.config.{AuthConfig,
ComputingUnitConfig, GuiConfig, Use
@Produces(Array(MediaType.APPLICATION_JSON))
class ConfigResource {
+ // The frontend loads /config/gui and /config/user-system as an
APP_INITIALIZER
+ // (GuiConfigService.load() in gui-config.service.ts) — i.e. before any
login. They
+ // must answer unauthenticated callers so the login page can render. PR
#5049 left
+ // @RolesAllowed on both endpoints, which once RolesAllowedDynamicFeature was
+ // registered started returning 403 during bootstrap and broke the whole
app; that
+ // PR was reverted in #5173. @PermitAll keeps enforcement on for the rest of
the
+ // service while explicitly whitelisting these two pre-login endpoints.
@GET
- @RolesAllowed(Array("REGULAR", "ADMIN"))
+ @PermitAll
Review Comment:
Heads up on what `@PermitAll` here exposes (pre-existing, not introduced by
this PR): `getGuiConfig` returns `defaultLocalUser.password` (line 63), so
marking `/config/gui` `@PermitAll` formally serves the default local-user
password to fully unauthenticated callers. On `main` this endpoint was already
public — `RolesAllowedDynamicFeature` was never registered there, so the old
`@RolesAllowed` was dead code — so this is not a *new* exposure. But since this
PR cements the access policy, it is worth deciding whether the default password
belongs in a pre-login payload at all; if something needs it before login,
consider returning only the username (or a boolean) and resolving the password
server-side.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
