Copilot commented on code in PR #844:
URL: https://github.com/apache/unomi/pull/844#discussion_r3935504183
##########
graphql/cxs-impl/src/main/java/org/apache/unomi/graphql/servlet/auth/GraphQLServletSecurityValidator.java:
##########
@@ -109,13 +110,55 @@ private boolean isPublicOperation(String query) {
return true;
}
+ /**
+ * Authenticates a WebSocket upgrade that carries an {@code Authorization}
header. Subscriptions are
+ * never public, so only Basic JAAS credentials are accepted.
+ *
+ * @return true when the caller is authenticated; false after a 401 has
been sent
+ */
+ public boolean validateWebSocketUpgrade(HttpServletRequest req,
HttpServletResponse res) throws IOException {
+ if (req.getHeader("Authorization") == null) {
+ res.addHeader("WWW-Authenticate", "Basic realm=\"karaf\"");
+ res.sendError(HttpServletResponse.SC_UNAUTHORIZED);
+ return false;
+ }
+ if (isAuthenticatedUser(req)) {
+ return true;
+ }
+ res.sendError(HttpServletResponse.SC_UNAUTHORIZED);
Review Comment:
A 401 response must include a `WWW-Authenticate` challenge, but invalid or
malformed credentials reach this branch without one. Add the same Basic
challenge used by the missing-header branch so upgrade clients receive a
standards-compliant response.
##########
graphql/cxs-impl/src/main/java/org/apache/unomi/graphql/servlet/websocket/SubscriptionWebSocket.java:
##########
@@ -82,9 +169,85 @@ public void onWebSocketText(String textMessage) {
}
}
+ /**
+ * Authenticates the socket from the {@code connection_init} payload,
which is how a browser client
+ * presents credentials (it cannot set request headers on the handshake).
A socket that already
+ * authenticated on the upgrade is left as it is.
+ *
+ * @return true when the socket may proceed, false when it has been closed
+ */
+ private boolean handleConnectionInit(GraphQLMessage message) {
+ if (authenticated) {
+ return true;
+ }
+ if (isDeadlineExpired()) {
+ return refuseAfterDeadline(message);
+ }
+
+ final String credential = basicCredentialFrom(message.getPayload());
+ if (credential == null || validator == null ||
!validator.authenticateBasicCredential(credential)) {
+ LOGGER.warn("Refusing GraphQL WebSocket connection_init without a
valid credential");
+ sendMessage(GraphQLMessage.create(message.getId())
+ .type(GraphQLMessage.TYPE_CONNECTION_ERROR)
+ .errors(Collections.singletonList("Not authenticated"))
+ .build());
+ closeConnection(message, CLOSE_POLICY_VIOLATION, "Not
authenticated");
+ return false;
+ }
+
+ synchronized (authenticationLock) {
+ // The deadline may have fired while the credential was being
checked.
+ if (deadlineExpired) {
+ return refuseAfterDeadline(message);
+ }
+ this.authenticated = true;
+ }
+ cancelAuthenticationDeadline();
+ final Session session = getSession();
+ if (session != null) {
+ // Authenticated: drop the short unauthenticated deadline.
+ session.setIdleTimeout(0);
Review Comment:
After browser-side `connection_init` authentication, Jetty interprets an
idle timeout of `0` as disabled. This makes these sockets bypass the factory's
normal configured idle limit indefinitely, unlike sockets authenticated during
the upgrade. The scheduled task already enforces the authentication deadline,
so preserve and restore the session's original timeout (or avoid changing it at
all).
##########
graphql/cxs-impl/src/main/java/org/apache/unomi/graphql/servlet/auth/GraphQLServletSecurityValidator.java:
##########
@@ -109,13 +110,55 @@ private boolean isPublicOperation(String query) {
return true;
}
+ /**
+ * Authenticates a WebSocket upgrade that carries an {@code Authorization}
header. Subscriptions are
+ * never public, so only Basic JAAS credentials are accepted.
+ *
+ * @return true when the caller is authenticated; false after a 401 has
been sent
+ */
+ public boolean validateWebSocketUpgrade(HttpServletRequest req,
HttpServletResponse res) throws IOException {
+ if (req.getHeader("Authorization") == null) {
+ res.addHeader("WWW-Authenticate", "Basic realm=\"karaf\"");
+ res.sendError(HttpServletResponse.SC_UNAUTHORIZED);
+ return false;
+ }
+ if (isAuthenticatedUser(req)) {
+ return true;
+ }
+ res.sendError(HttpServletResponse.SC_UNAUTHORIZED);
+ return false;
+ }
+
+ /**
+ * Authenticates a Basic credential that did not arrive on an HTTP
request, such as the one a browser
+ * client sends in the WebSocket {@code connection_init} payload. Same
JAAS check as the HTTP path.
+ */
+ public boolean authenticateBasicCredential(String authorizationValue) {
+ return authenticateBasic(authorizationValue, null);
+ }
+
private boolean isAuthenticatedUser(HttpServletRequest req) {
req.setAttribute(AUTHENTICATION_TYPE, HttpServletRequest.BASIC_AUTH);
+ return authenticateBasic(req.getHeader("Authorization"), req);
+ }
- String authHeader = req.getHeader("Authorization");
-
- String usernameAndPassword = new
String(Base64.getDecoder().decode(authHeader.substring(6).getBytes()));
+ /**
+ * @param req the originating request, or {@code null} when the credential
did not arrive on one
+ */
+ private boolean authenticateBasic(String authHeader, HttpServletRequest
req) {
+ if (authHeader == null || !authHeader.startsWith("Basic ")) {
Review Comment:
HTTP authentication scheme names are case-insensitive, so this exact-case
check rejects valid headers such as `basic <credentials>` or `BASIC
<credentials>` on both HTTP and WebSocket paths. Use a case-insensitive prefix
comparison while retaining the existing credential decoding.
--
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]
