From d791b217373f7ea50744583518f39f8ad7e83cb6 Mon Sep 17 00:00:00 2001 From: mohitdeuex Date: Sat, 17 Oct 2026 14:52:32 +0530 Subject: [PATCH 1/2] fix(auth): require an authorization code on the confidential OIDC callback Establish the login session only from the ID token the back-channel token exchange returns. Require the authorization code on the callback and reject a request that carries only a front-channel token. --- .../AuthenticationCodeFlowHandler.java | 14 +++++++ .../AuthenticationCodeFlowHandlerTest.java | 40 +++++++++++++++++++ 2 files changed, 54 insertions(+) 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..4df1c3d22d7f 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,15 @@ static Map buildLoginParams(OidcConfiguration configuration) { return new HashMap<>(authParams); } + private static void requireAuthorizationCode(UserSession session, OidcCredentials credentials) { + if (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 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..1ff14ff52492 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,46 @@ private static List roleNames(User user) { return user.getRoles().stream().map(EntityReference::getName).toList(); } + @Test + void requireAuthorizationCode_noCode_throwsAndIsRejected() throws Exception { + UserSession session = UserSession.builder().id("pending-session").build(); + // A callback that carries only a front-channel id token, with no authorization code. + OidcCredentials credentials = new OidcCredentials(); + credentials.setIdToken("front.channel.token"); + + Method requireCode = + AuthenticationCodeFlowHandler.class.getDeclaredMethod( + "requireAuthorizationCode", UserSession.class, OidcCredentials.class); + requireCode.setAccessible(true); + + IllegalArgumentException thrown = + assertThrows( + IllegalArgumentException.class, + () -> { + try { + requireCode.invoke(null, session, credentials); + } catch (InvocationTargetException e) { + throw e.getCause(); + } + }); + + assertEquals("Authorization code is required", thrown.getMessage()); + } + + @Test + void requireAuthorizationCode_withCode_doesNotThrow() throws Exception { + UserSession session = UserSession.builder().id("pending-session").build(); + OidcCredentials credentials = new OidcCredentials(); + credentials.setCode("valid-authorization-code"); + + Method requireCode = + AuthenticationCodeFlowHandler.class.getDeclaredMethod( + "requireAuthorizationCode", UserSession.class, OidcCredentials.class); + requireCode.setAccessible(true); + + assertDoesNotThrow(() -> requireCode.invoke(null, session, credentials)); + } + private AuthenticationCodeFlowHandler createHandlerWithMockedInternals( SessionService sessionService, OidcClient client) throws Exception { sun.misc.Unsafe unsafe = getUnsafe(); From c09551b449ddf99d51b51e08a3d646d85cdc790e Mon Sep 17 00:00:00 2001 From: mohitdeuex Date: Sat, 17 Oct 2026 16:47:07 +0530 Subject: [PATCH 2/2] fix(auth): scope the callback code requirement to code-flow configs Require an authorization code only when the configured response_type is a code flow (plain or hybrid), so a deployment set to a pure implicit response_type is not rejected. Cover the callback rejection end-to-end and the response_type handling with tests. --- .../AuthenticationCodeFlowHandler.java | 21 ++++- .../AuthenticationCodeFlowHandlerTest.java | 85 ++++++++++++------- 2 files changed, 75 insertions(+), 31 deletions(-) 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 4df1c3d22d7f..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 @@ -919,8 +919,8 @@ static Map buildLoginParams(OidcConfiguration configuration) { return new HashMap<>(authParams); } - private static void requireAuthorizationCode(UserSession session, OidcCredentials credentials) { - if (credentials.getCode() == null) { + 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())); @@ -928,6 +928,23 @@ private static void requireAuthorizationCode(UserSession session, OidcCredential } } + 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 1ff14ff52492..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 @@ -1806,43 +1806,70 @@ private static List roleNames(User user) { } @Test - void requireAuthorizationCode_noCode_throwsAndIsRejected() throws Exception { - UserSession session = UserSession.builder().id("pending-session").build(); - // A callback that carries only a front-channel id token, with no authorization code. - OidcCredentials credentials = new OidcCredentials(); - credentials.setIdToken("front.channel.token"); + 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"); - Method requireCode = - AuthenticationCodeFlowHandler.class.getDeclaredMethod( - "requireAuthorizationCode", UserSession.class, OidcCredentials.class); - requireCode.setAccessible(true); + // 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})); - IllegalArgumentException thrown = - assertThrows( - IllegalArgumentException.class, - () -> { - try { - requireCode.invoke(null, session, credentials); - } catch (InvocationTargetException e) { - throw e.getCause(); - } - }); + AuthenticationCodeFlowHandler handler = + createHandlerWithMockedInternals(sessionService, oidcClient); + setField(handler, "serverUrl", TEST_SERVER_URL); + setField(handler, "authenticationConfiguration", authConfig); - assertEquals("Authorization code is required", thrown.getMessage()); + 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 requireAuthorizationCode_withCode_doesNotThrow() throws Exception { - UserSession session = UserSession.builder().id("pending-session").build(); - OidcCredentials credentials = new OidcCredentials(); - credentials.setCode("valid-authorization-code"); + 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); - Method requireCode = - AuthenticationCodeFlowHandler.class.getDeclaredMethod( - "requireAuthorizationCode", UserSession.class, OidcCredentials.class); - requireCode.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)); - assertDoesNotThrow(() -> requireCode.invoke(null, session, credentials)); + when(oidcConfiguration.getResponseType()).thenReturn("id_token"); + assertEquals(Boolean.FALSE, expectsCode.invoke(handler)); } private AuthenticationCodeFlowHandler createHandlerWithMockedInternals(