xiangfu0 commented on code in PR #18933:
URL: https://github.com/apache/pinot/pull/18933#discussion_r3566179028
##########
pinot-controller/src/main/java/org/apache/pinot/controller/api/resources/PinotQueryResource.java:
##########
@@ -805,10 +814,32 @@ private String getTimeSeriesQueryURL(String protocol,
String hostName, int port,
}
}
+ /**
+ * Injects the stored Basic-auth token from the server-side session store
into the forwarded
+ * broker request headers. This is needed because browser UI requests carry
only the session
+ * cookie, not an Authorization header; the broker still requires one for
server-to-server auth.
+ */
+ private void injectSessionCredentialIfNeeded(Map<String, String> headers,
HttpHeaders httpHeaders) {
+ if (_sessionManager == null || _controllerConf == null
+ ||
!_controllerConf.getProperty(ControllerConf.CONTROLLER_UI_SESSION_ENABLED,
false)) {
+ return;
+ }
+ Cookie sessionCookie =
httpHeaders.getCookies().get(SessionManager.SESSION_COOKIE_NAME);
+ if (sessionCookie != null && sessionCookie.getValue() != null) {
+ _sessionManager.getBasicAuthToken(sessionCookie.getValue()).ifPresent(
+ authToken -> headers.put(HttpHeaders.AUTHORIZATION, authToken));
+ }
+ }
+
private Map<String, String> extractHeaders(HttpHeaders httpHeaders) {
+ // In SESSION mode, exclude the Authorization header from being forwarded
to the broker.
+ // The stored session credential is injected separately via
injectSessionCredentialIfNeeded().
+ boolean isSessionMode = _controllerConf != null
+ &&
_controllerConf.getProperty(ControllerConf.CONTROLLER_UI_SESSION_ENABLED,
false);
return httpHeaders.getRequestHeaders().entrySet().stream()
- .filter(entry -> !entry.getValue().isEmpty())
- .collect(Collectors.toMap(Entry::getKey, entry ->
entry.getValue().get(0)));
+ .filter(entry -> !entry.getValue().isEmpty())
+ .filter(entry -> !isSessionMode ||
!entry.getKey().equalsIgnoreCase(HttpHeaders.AUTHORIZATION))
Review Comment:
This drops `Authorization` for every controller `/sql` and time-series proxy
request whenever session mode is enabled, including API clients that
authenticated with Basic/Bearer and do not have a session cookie. The
controller-side access check can pass with the original header, but the
forwarded broker request then arrives without credentials and fails on
broker-auth clusters. Keep the caller-supplied `Authorization` when there is no
valid session cookie, or only replace it when injecting a session credential.
--
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]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
