diff --git a/openmetadata-service/src/main/java/org/openmetadata/service/security/AuthenticationCodeFlowHandler.java b/openmetadata-service/src/main/java/org/openmetadata/service/security/AuthenticationCodeFlowHandler.java index a285b3374a36..5daa7cbc301c 100644 --- a/openmetadata-service/src/main/java/org/openmetadata/service/security/AuthenticationCodeFlowHandler.java +++ b/openmetadata-service/src/main/java/org/openmetadata/service/security/AuthenticationCodeFlowHandler.java @@ -613,6 +613,11 @@ public void handleCallback(HttpServletRequest req, HttpServletResponse resp) { // Build Credentials OidcCredentials credentials = buildCredentials(successResponse); + // The identity below must come from the ID token the back-channel exchange returns, and that + // exchange only runs when the callback carries an authorization code. Reject a callback that + // brings only a front-channel token: it never passed through the provider's token endpoint. + requireAuthorizationCode(pendingSession, credentials); + // Validations validateAndSendTokenRequest(pendingSession, credentials, computedCallbackUrl); @@ -914,6 +919,32 @@ static Map buildLoginParams(OidcConfiguration configuration) { return new HashMap<>(authParams); } + private void requireAuthorizationCode(UserSession session, OidcCredentials credentials) { + if (expectsAuthorizationCode() && credentials.getCode() == null) { + LOG.warn( + "Rejecting OIDC callback without an authorization code for session {}", + SessionService.truncateId(session.getId())); + throw new IllegalArgumentException("Authorization code is required"); + } + } + + private boolean expectsAuthorizationCode() { + // The default and documented confidential mode is the authorization-code flow. Per OIDC, any + // response_type that includes "code" (plain or hybrid) returns an authorization code, so the + // callback must carry one. A response_type configured for pure implicit delivery is not a code + // flow and keeps its existing handling. + String responseType = client.getConfiguration().getResponseType(); + if (nullOrEmpty(responseType)) { + return true; + } + for (String part : responseType.trim().split("\\s+")) { + if ("code".equalsIgnoreCase(part)) { + return true; + } + } + return false; + } + private void validateAndSendTokenRequest( UserSession session, OidcCredentials oidcCredentials, String computedCallbackUrl) throws URISyntaxException { diff --git a/openmetadata-service/src/test/java/org/openmetadata/service/security/AuthenticationCodeFlowHandlerTest.java b/openmetadata-service/src/test/java/org/openmetadata/service/security/AuthenticationCodeFlowHandlerTest.java index e28428da13b2..b04ee2376959 100644 --- a/openmetadata-service/src/test/java/org/openmetadata/service/security/AuthenticationCodeFlowHandlerTest.java +++ b/openmetadata-service/src/test/java/org/openmetadata/service/security/AuthenticationCodeFlowHandlerTest.java @@ -1805,6 +1805,73 @@ private static List roleNames(User user) { return user.getRoles().stream().map(EntityReference::getName).toList(); } + @Test + void handleCallback_codeFlowFrontChannelTokenWithoutCode_rejectedWithoutActivatingSession() + throws Exception { + UserSession pendingSession = + UserSession.builder() + .id("pending-session") + .state("state-abc") + .redirectUri(TEST_SERVER_URL + "/auth/callback") + .build(); + when(sessionService.getPendingSession(request, response)) + .thenReturn(Optional.of(pendingSession)); + when(oidcClient.getCallbackUrl()).thenReturn(TEST_SERVER_URL + "/callback"); + when(oidcClient.getConfiguration()).thenReturn(oidcConfiguration); + when(oidcConfiguration.isWithState()).thenReturn(false); + when(oidcConfiguration.getResponseType()).thenReturn("code"); + OIDCProviderMetadata metadata = mock(OIDCProviderMetadata.class); + when(metadata.supportsAuthorizationResponseIssuerParam()).thenReturn(false); + IOidcOpMetadataResolver metadataResolver = mock(IOidcOpMetadataResolver.class); + when(metadataResolver.load()).thenReturn(metadata); + when(oidcConfiguration.getOpMetadataResolver()).thenReturn(metadataResolver); + AuthenticationConfiguration authConfig = mock(AuthenticationConfiguration.class); + when(authConfig.getCallbackUrl()).thenReturn(TEST_SERVER_URL + "/callback"); + + // A syntactically valid but unsigned, claim-less token stands in for a front-channel token that + // never passed through the provider's token endpoint. The callback carries it with no code. + String frontChannelToken = new PlainJWT(new JWTClaimsSet.Builder().build()).serialize(); + when(request.getParameterMap()) + .thenReturn( + Map.of( + "state", new String[] {"state-abc"}, + "id_token", new String[] {frontChannelToken})); + + AuthenticationCodeFlowHandler handler = + createHandlerWithMockedInternals(sessionService, oidcClient); + setField(handler, "serverUrl", TEST_SERVER_URL); + setField(handler, "authenticationConfiguration", authConfig); + + handler.handleCallback(request, response); + + verify(response).setStatus(HttpServletResponse.SC_BAD_REQUEST); + assertTrue(captureOutputStream.getCapturedOutput().contains("Authorization code is required")); + verify(sessionService, never()) + .activatePendingSession(any(), any(), any(), any(), any(), any()); + } + + @Test + void expectsAuthorizationCode_trueForCodeAndHybridAndDefault_falseForImplicit() throws Exception { + when(oidcClient.getConfiguration()).thenReturn(oidcConfiguration); + AuthenticationCodeFlowHandler handler = + createHandlerWithMockedInternals(sessionService, oidcClient); + Method expectsCode = + AuthenticationCodeFlowHandler.class.getDeclaredMethod("expectsAuthorizationCode"); + expectsCode.setAccessible(true); + + when(oidcConfiguration.getResponseType()).thenReturn("code"); + assertEquals(Boolean.TRUE, expectsCode.invoke(handler)); + + when(oidcConfiguration.getResponseType()).thenReturn("code id_token"); + assertEquals(Boolean.TRUE, expectsCode.invoke(handler)); + + when(oidcConfiguration.getResponseType()).thenReturn(null); + assertEquals(Boolean.TRUE, expectsCode.invoke(handler)); + + when(oidcConfiguration.getResponseType()).thenReturn("id_token"); + assertEquals(Boolean.FALSE, expectsCode.invoke(handler)); + } + private AuthenticationCodeFlowHandler createHandlerWithMockedInternals( SessionService sessionService, OidcClient client) throws Exception { sun.misc.Unsafe unsafe = getUnsafe();