From edf166237a240f141502e62c0ebb42da096f0b59 Mon Sep 17 00:00:00 2001 From: sahusanket Date: Wed, 12 Aug 2026 22:32:31 +0530 Subject: [PATCH] CDAP-21261 : Add support for Refresh Token Rotation --- .../cdap/datapipeline/oauth/AuthType.java | 31 +++++ .../datapipeline/oauth/OAuthProvider.java | 19 ++- .../cdap/datapipeline/oauth/OAuthStore.java | 46 ++++++- .../oauth/PutOAuthCredentialRequest.java | 12 +- .../oauth/PutOAuthProviderRequest.java | 9 +- .../datapipeline/service/OAuthHandler.java | 121 ++++++++++++++++-- .../cdap/datapipeline/OAuthServiceTest.java | 58 +++++++-- .../cdap/datapipeline/OAuthStoreTest.java | 2 +- 8 files changed, 266 insertions(+), 32 deletions(-) create mode 100644 cdap-app-templates/cdap-etl/cdap-data-pipeline-base/src/main/java/io/cdap/cdap/datapipeline/oauth/AuthType.java diff --git a/cdap-app-templates/cdap-etl/cdap-data-pipeline-base/src/main/java/io/cdap/cdap/datapipeline/oauth/AuthType.java b/cdap-app-templates/cdap-etl/cdap-data-pipeline-base/src/main/java/io/cdap/cdap/datapipeline/oauth/AuthType.java new file mode 100644 index 000000000000..e1e9acd0289e --- /dev/null +++ b/cdap-app-templates/cdap-etl/cdap-data-pipeline-base/src/main/java/io/cdap/cdap/datapipeline/oauth/AuthType.java @@ -0,0 +1,31 @@ +/* + * Copyright © 2024 Cask Data, Inc. + * Licensed under the Apache License, Version 2.0 (the "License"); you may not + * use this file except in compliance with the License. You may obtain a copy of + * the License at + * http://www.apache.org/licenses/LICENSE-2.0 + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT + * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the + * License for the specific language governing permissions and limitations under + * the License. + * + */ + +package io.cdap.cdap.datapipeline.oauth; + +/** + * The type of OAuth 2.0 authorization flow to use. + */ +public enum AuthType { + /** + * Standard OAuth 2.0 Authorization Code flow. + */ + STANDARD, + + /** + * OAuth 2.0 Authorization Code flow with Proof Key for Code Exchange (PKCE). + * Provides enhanced security by dynamically generating a code challenge and verifier. + */ + PKCE +} diff --git a/cdap-app-templates/cdap-etl/cdap-data-pipeline-base/src/main/java/io/cdap/cdap/datapipeline/oauth/OAuthProvider.java b/cdap-app-templates/cdap-etl/cdap-data-pipeline-base/src/main/java/io/cdap/cdap/datapipeline/oauth/OAuthProvider.java index 0aeea2777325..ba2fca32af3e 100644 --- a/cdap-app-templates/cdap-etl/cdap-data-pipeline-base/src/main/java/io/cdap/cdap/datapipeline/oauth/OAuthProvider.java +++ b/cdap-app-templates/cdap-etl/cdap-data-pipeline-base/src/main/java/io/cdap/cdap/datapipeline/oauth/OAuthProvider.java @@ -33,18 +33,23 @@ public class OAuthProvider { @Nullable private final String userAgent; + @Nullable + private final AuthType authType; + public OAuthProvider(String name, String loginURL, String tokenRefreshURL, @Nullable OAuthClientCredentials clientCreds, @Nullable CredentialEncodingStrategy strategy, - @Nullable String userAgent) { + @Nullable String userAgent, + @Nullable AuthType authType) { this.name = name; this.loginURL = loginURL; this.tokenRefreshURL = tokenRefreshURL; this.clientCreds = clientCreds; this.strategy = strategy; this.userAgent = userAgent; + this.authType = authType != null ? authType : AuthType.STANDARD; } public String getName() { @@ -74,6 +79,10 @@ public String getUserAgent() { return userAgent; } + public AuthType getAuthType() { + return authType; + } + public enum CredentialEncodingStrategy { // (default) Sends client ID & secret as part of the POST request body FORM_BODY, @@ -95,6 +104,7 @@ public static class Builder { private OAuthClientCredentials clientCreds; private CredentialEncodingStrategy strategy; private String userAgent; + private AuthType authType; public Builder() {} @@ -128,6 +138,11 @@ public Builder withUserAgent(@Nullable String userAgent) { return this; } + public Builder withAuthType(@Nullable AuthType authType) { + this.authType = authType; + return this; + } + public OAuthProvider build() { Preconditions.checkNotNull(name, "OAuth provider name missing"); Preconditions.checkNotNull(loginURL, "Login URL missing"); @@ -136,7 +151,7 @@ public OAuthProvider build() { if (strategy == null) { this.strategy = CredentialEncodingStrategy.FORM_BODY; } - return new OAuthProvider(name, loginURL, tokenRefreshURL, clientCreds, strategy, userAgent); + return new OAuthProvider(name, loginURL, tokenRefreshURL, clientCreds, strategy, userAgent, authType); } } } diff --git a/cdap-app-templates/cdap-etl/cdap-data-pipeline-base/src/main/java/io/cdap/cdap/datapipeline/oauth/OAuthStore.java b/cdap-app-templates/cdap-etl/cdap-data-pipeline-base/src/main/java/io/cdap/cdap/datapipeline/oauth/OAuthStore.java index 6f3a2ea67de3..c5b826ca893e 100644 --- a/cdap-app-templates/cdap-etl/cdap-data-pipeline-base/src/main/java/io/cdap/cdap/datapipeline/oauth/OAuthStore.java +++ b/cdap-app-templates/cdap-etl/cdap-data-pipeline-base/src/main/java/io/cdap/cdap/datapipeline/oauth/OAuthStore.java @@ -46,13 +46,16 @@ public class OAuthStore { private static final String TOKEN_REFRESH_URL_COL = "tokenrefreshurl"; private static final String CREDENTIAL_ENCODING_STRATEGY_COL = "credentialencodingstrategy"; private static final String USER_AGENT_COL = "useragent"; + private static final String AUTH_TYPE_COL = "authtype"; private static final String CLIENT_CREDS_KEY_PREFIX = "oauthclientcreds"; private static final String ACCESS_TOKEN_KEY_PREFIX = "oauthaccesstoken"; private static final String REFRESH_TOKEN_KEY_PREFIX = "oauthrefreshtoken"; + private static final String PKCE_KEY_PREFIX = "oauth-pkce"; private static final Gson GSON = new Gson(); private final TransactionRunner transactionRunner; private final SecureStore secureStore; private final SecureStoreManager secureStoreManager; + private final long codeVerifierTTL; public static final StructuredTableId TABLE_ID = new StructuredTableId("oauth"); public static final StructuredTableSpecification TABLE_SPEC = new StructuredTableSpecification.Builder() @@ -61,17 +64,20 @@ public class OAuthStore { Fields.stringType(LOGIN_URL_COL), Fields.stringType(TOKEN_REFRESH_URL_COL), Fields.stringType(CREDENTIAL_ENCODING_STRATEGY_COL), - Fields.stringType(USER_AGENT_COL)) + Fields.stringType(USER_AGENT_COL), + Fields.stringType(AUTH_TYPE_COL)) .withPrimaryKeys(OAUTH_PROVIDER_COL) .build(); public OAuthStore( TransactionRunner transactionRunner, SecureStore secureStore, - SecureStoreManager secureStoreManager) { + SecureStoreManager secureStoreManager, + long codeVerifierTTL) { this.transactionRunner = transactionRunner; this.secureStore = secureStore; this.secureStoreManager = secureStoreManager; + this.codeVerifierTTL = codeVerifierTTL; } /** @@ -299,6 +305,10 @@ private static String getAccessTokenKey(String oauthProvider, String credentialI return String.format("%s-%s-%s", ACCESS_TOKEN_KEY_PREFIX, oauthProvider.toLowerCase(), credentialId.toLowerCase()); } + private static String getPKCEKey(String oauthProvider, String state) { + return String.format("%s-%s-%s", PKCE_KEY_PREFIX, oauthProvider.toLowerCase(), state.toLowerCase()); + } + private static List> getKey(String name) { List> keyFields = new ArrayList<>(1); keyFields.add(Fields.stringField(OAUTH_PROVIDER_COL, name)); @@ -311,6 +321,7 @@ private static OAuthProvider fromRow(StructuredRow row, OAuthClientCredentials c String tokenRefreshURL = row.getString(TOKEN_REFRESH_URL_COL); String credentialEncodingStrategy = row.getString(CREDENTIAL_ENCODING_STRATEGY_COL); String userAgent = row.getString(USER_AGENT_COL); + String authTypeStr = row.getString(AUTH_TYPE_COL); return OAuthProvider.newBuilder() .withName(name) @@ -321,6 +332,7 @@ private static OAuthProvider fromRow(StructuredRow row, OAuthClientCredentials c Optional.ofNullable(credentialEncodingStrategy) .map(OAuthProvider.CredentialEncodingStrategy::valueOf).orElse(null)) .withUserAgent(userAgent) + .withAuthType(authTypeStr != null ? AuthType.valueOf(authTypeStr) : AuthType.STANDARD) .build(); } @@ -333,6 +345,36 @@ private static List> getRow(OAuthProvider oauthProvider) { CREDENTIAL_ENCODING_STRATEGY_COL, oauthProvider.getCredentialEncodingStrategy().toString())); fields.add(Fields.stringField(USER_AGENT_COL, oauthProvider.getUserAgent())); + fields.add(Fields.stringField(AUTH_TYPE_COL, oauthProvider.getAuthType().toString())); return fields; } + + public void writePKCECodeVerifier(String provider, String state, String codeVerifier) throws Exception { + String key = getPKCEKey(provider, state); + secureStoreManager.put(NamespaceId.SYSTEM.getNamespace(), key, codeVerifier, + "PKCE Code Verifier", Collections.emptyMap(), codeVerifierTTL); + } + + public String getPKCECodeVerifier(String provider, String state) throws OAuthStoreException { + String key = getPKCEKey(provider, state); + try { + return new String(secureStore.getData(NamespaceId.SYSTEM.getNamespace(), key), StandardCharsets.UTF_8); + } catch (IOException e) { + throw new OAuthStoreException("Failed to read PKCE code verifier from secure storage", e); + } catch (Exception e) { + throw new OAuthStoreException("PKCE code verifier not found or expired for state or an " + + "unexpected error reading from secure store: ", e); + } + } + + public void deletePKCECodeVerifier(String provider, String state) throws OAuthStoreException { + String key = getPKCEKey(provider, state); + try { + secureStoreManager.delete(NamespaceId.SYSTEM.getNamespace(), key); + } catch (IOException e) { + throw new OAuthStoreException("Failed to delete PKCE code verifier from secure storage", e); + } catch (Exception e) { + // Ignore if not found + } + } } diff --git a/cdap-app-templates/cdap-etl/cdap-data-pipeline-base/src/main/java/io/cdap/cdap/datapipeline/oauth/PutOAuthCredentialRequest.java b/cdap-app-templates/cdap-etl/cdap-data-pipeline-base/src/main/java/io/cdap/cdap/datapipeline/oauth/PutOAuthCredentialRequest.java index bcf445e79e23..4f07d1f05b42 100644 --- a/cdap-app-templates/cdap-etl/cdap-data-pipeline-base/src/main/java/io/cdap/cdap/datapipeline/oauth/PutOAuthCredentialRequest.java +++ b/cdap-app-templates/cdap-etl/cdap-data-pipeline-base/src/main/java/io/cdap/cdap/datapipeline/oauth/PutOAuthCredentialRequest.java @@ -17,16 +17,21 @@ package io.cdap.cdap.datapipeline.oauth; +import javax.annotation.Nullable; + /** * OAuth credential REST PUT request body. */ public class PutOAuthCredentialRequest { private final String oneTimeCode; private final String redirectURI; + @Nullable + private final String state; - public PutOAuthCredentialRequest(String oneTimeCode, String redirectURI) { + public PutOAuthCredentialRequest(String oneTimeCode, String redirectURI, @Nullable String state) { this.oneTimeCode = oneTimeCode; this.redirectURI = redirectURI; + this.state = state; } public String getOneTimeCode() { @@ -36,4 +41,9 @@ public String getOneTimeCode() { public String getRedirectURI() { return redirectURI; } + + @Nullable + public String getState() { + return state; + } } diff --git a/cdap-app-templates/cdap-etl/cdap-data-pipeline-base/src/main/java/io/cdap/cdap/datapipeline/oauth/PutOAuthProviderRequest.java b/cdap-app-templates/cdap-etl/cdap-data-pipeline-base/src/main/java/io/cdap/cdap/datapipeline/oauth/PutOAuthProviderRequest.java index b872f881ae56..b062f9afe13b 100644 --- a/cdap-app-templates/cdap-etl/cdap-data-pipeline-base/src/main/java/io/cdap/cdap/datapipeline/oauth/PutOAuthProviderRequest.java +++ b/cdap-app-templates/cdap-etl/cdap-data-pipeline-base/src/main/java/io/cdap/cdap/datapipeline/oauth/PutOAuthProviderRequest.java @@ -27,6 +27,7 @@ public class PutOAuthProviderRequest { private final String clientSecret; private final OAuthProvider.CredentialEncodingStrategy strategy; private final String userAgent; + private final AuthType authType; public PutOAuthProviderRequest( String loginURL, @@ -34,13 +35,15 @@ public PutOAuthProviderRequest( String clientId, String clientSecret, OAuthProvider.CredentialEncodingStrategy strategy, - String userAgent) { + String userAgent, + AuthType authType) { this.loginURL = loginURL; this.tokenRefreshURL = tokenRefreshURL; this.clientId = clientId; this.clientSecret = clientSecret; this.strategy = strategy; this.userAgent = userAgent; + this.authType = authType; } public String getLoginURL() { @@ -66,4 +69,8 @@ public OAuthProvider.CredentialEncodingStrategy getCredentialEncodingStrategy() public String getUserAgent() { return userAgent; } + + public AuthType getAuthType() { + return authType; + } } diff --git a/cdap-app-templates/cdap-etl/cdap-data-pipeline-base/src/main/java/io/cdap/cdap/datapipeline/service/OAuthHandler.java b/cdap-app-templates/cdap-etl/cdap-data-pipeline-base/src/main/java/io/cdap/cdap/datapipeline/service/OAuthHandler.java index 673d19c38c8e..f5d0464a679c 100644 --- a/cdap-app-templates/cdap-etl/cdap-data-pipeline-base/src/main/java/io/cdap/cdap/datapipeline/service/OAuthHandler.java +++ b/cdap-app-templates/cdap-etl/cdap-data-pipeline-base/src/main/java/io/cdap/cdap/datapipeline/service/OAuthHandler.java @@ -23,12 +23,14 @@ import io.cdap.cdap.api.service.http.AbstractSystemHttpServiceHandler; import io.cdap.cdap.api.service.http.HttpServiceRequest; import io.cdap.cdap.api.service.http.HttpServiceResponder; +import io.cdap.cdap.api.security.AccessException; import io.cdap.cdap.api.service.http.SystemHttpServiceContext; import io.cdap.cdap.datapipeline.oauth.CredentialIsValidResponse; import io.cdap.cdap.datapipeline.oauth.GetAccessTokenResponse; import io.cdap.cdap.datapipeline.oauth.OAuthAccessToken; import io.cdap.cdap.datapipeline.oauth.OAuthClientCredentials; import io.cdap.cdap.datapipeline.oauth.OAuthProvider; +import io.cdap.cdap.datapipeline.oauth.AuthType; import io.cdap.cdap.datapipeline.oauth.OAuthProvider.CredentialEncodingStrategy; import io.cdap.cdap.datapipeline.oauth.OAuthRefreshToken; import io.cdap.cdap.datapipeline.oauth.OAuthStore; @@ -36,6 +38,7 @@ import io.cdap.cdap.datapipeline.oauth.PutOAuthCredentialRequest; import io.cdap.cdap.datapipeline.oauth.PutOAuthProviderRequest; import io.cdap.cdap.datapipeline.oauth.RefreshTokenResponse; +import io.cdap.cdap.proto.id.NamespaceId; import io.cdap.common.http.HttpRequest; import io.cdap.common.http.HttpRequests; import io.cdap.common.http.HttpResponse; @@ -45,8 +48,13 @@ import java.net.MalformedURLException; import java.net.URL; import java.nio.charset.StandardCharsets; +import java.security.MessageDigest; +import java.security.SecureRandom; import java.util.Base64; +import java.util.Collections; +import java.util.Map; import java.util.Optional; +import java.util.UUID; import javax.ws.rs.DELETE; import javax.ws.rs.DefaultValue; import javax.ws.rs.GET; @@ -69,13 +77,29 @@ public class OAuthHandler extends AbstractSystemHttpServiceHandler { .setPrettyPrinting() .registerTypeAdapterFactory(new ErrorHandlingGsonTypeAdapterFactory()) .create(); + private static final SecureRandom SECURE_RANDOM = new SecureRandom(); + + // The following settings can be overridden via CDAP System Preferences + // Time To Live in seconds for PCKE based code verifier in Secure Store. + private static final String PREF_PKCE_CODE_VERIFIER_TTL = "oauth.pkce.code.verifier.ttl.sec"; + + private static final long DEFAULT_PKCE_CODE_VERIFIER_TTL = 900; private OAuthStore oauthStore; @Override public void initialize(SystemHttpServiceContext context) throws Exception { + Map prefs = Collections.emptyMap(); + try { + prefs = context.getPreferencesForNamespace(NamespaceId.SYSTEM.getNamespace(), true); + } catch (IOException | IllegalArgumentException | AccessException e) { + LOG.warn("Failed to load preferences for OAuth RTR configuration. Using default values.", e); + } + super.initialize(context); - this.oauthStore = new OAuthStore(context, context, context.getAdmin()); + + this.oauthStore = new OAuthStore(context, context, context.getAdmin(), + getPrefOrDefault(prefs, PREF_PKCE_CODE_VERIFIER_TTL, DEFAULT_PKCE_CODE_VERIFIER_TTL)); } @GET @@ -103,6 +127,22 @@ public void getAuthURL(HttpServiceRequest request, HttpServiceResponder responde String response = String.format( formatURL, loginUrl, oauthProvider.getClientCredentials().getClientId(), redirectURI); + + if (oauthProvider.getAuthType() == AuthType.PKCE) { + String state = UUID.randomUUID().toString(); + String codeVerifier = generateCodeVerifier(); + String codeChallenge = generateCodeChallenge(codeVerifier); + + try { + oauthStore.writePKCECodeVerifier(provider, state, codeVerifier); + } catch (Exception e) { + throw new OAuthServiceException( + HttpURLConnection.HTTP_INTERNAL_ERROR, "Failed to store PKCE code verifier", e); + } + + response += String.format("&state=%s&code_challenge=%s&code_challenge_method=S256", state, codeChallenge); + } + responder.sendString(response); } catch (OAuthServiceException e) { e.respond(responder); @@ -141,6 +181,7 @@ public void putOAuthProvider(HttpServiceRequest request, HttpServiceResponder re .withClientCredentials(clientCredentials) .withCredentialEncodingStrategy(strategy) .withUserAgent(userAgent) + .withAuthType(putOAuthProviderRequest.getAuthType()) .build(); oauthStore.writeProvider(provider, reuseClientCredentials); responder.sendStatus(HttpURLConnection.HTTP_OK); @@ -190,7 +231,8 @@ public void putOAuthCredential(HttpServiceRequest request, HttpServiceResponder PutOAuthCredentialRequest.class); if (putOAuthCredentialRequest.getOneTimeCode() == null || putOAuthCredentialRequest.getOneTimeCode().isEmpty()) { - throw new OAuthServiceException(HttpURLConnection.HTTP_BAD_REQUEST, "Invalid request: missing one-time code"); + throw new OAuthServiceException( + HttpURLConnection.HTTP_BAD_REQUEST, "Invalid request: missing one-time code"); } if (putOAuthCredentialRequest.getRedirectURI() == null || putOAuthCredentialRequest.getRedirectURI().isEmpty()) { @@ -202,14 +244,33 @@ public void putOAuthCredential(HttpServiceRequest request, HttpServiceResponder OAuthProvider oauthProvider = getProvider(provider); + String state = putOAuthCredentialRequest.getState(); + String codeVerifier = null; + if (oauthProvider.getAuthType() == AuthType.PKCE) { + if (state == null || state.isEmpty() || !state.matches("^[a-zA-Z0-9-]+$")) { + throw new OAuthServiceException( + HttpURLConnection.HTTP_BAD_REQUEST, "State is required and must be valid for " + + "PKCE authentication"); + } + try { + codeVerifier = oauthStore.getPKCECodeVerifier(provider, state); + oauthStore.deletePKCECodeVerifier(provider, state); + } catch (OAuthStoreException e) { + throw new OAuthServiceException(HttpURLConnection.HTTP_INTERNAL_ERROR, + "Failed to process PKCE code verifier", e); + } + } + HttpResponse response; try { response = HttpRequests.execute(createGetRefreshTokenRequest( oauthProvider, putOAuthCredentialRequest.getOneTimeCode(), - putOAuthCredentialRequest.getRedirectURI())); + putOAuthCredentialRequest.getRedirectURI(), + codeVerifier)); } catch (IOException e) { - throw new OAuthServiceException(HttpURLConnection.HTTP_INTERNAL_ERROR, "Error while fetching refresh token", e); + throw new OAuthServiceException( + HttpURLConnection.HTTP_INTERNAL_ERROR, "Error while fetching refresh token", e); } if (response.getResponseCode() != 200) { @@ -387,7 +448,8 @@ public void getOAuthCredentialValidity(HttpServiceRequest request, HttpServiceRe try { response = HttpRequests.execute(createGetAccessTokenRequest(oauthProvider, refreshToken.getRefreshToken())); } catch (IOException e) { - throw new OAuthServiceException(HttpURLConnection.HTTP_INTERNAL_ERROR, "Error while fetching refresh token", e); + throw new OAuthServiceException( + HttpURLConnection.HTTP_INTERNAL_ERROR, "Error while fetching refresh token", e); } responder.sendString(GSON.toJson(new CredentialIsValidResponse(checkCredIsValid(response)))); @@ -427,20 +489,28 @@ private String buildRequestBody(CredentialEncodingStrategy strategy, String code, String redirectURI, String refreshToken, - OAuthClientCredentials clientCreds) { + OAuthClientCredentials clientCreds, + String codeVerifier) { + String body; switch (strategy) { case BASIC_AUTH: - return grantType.equals("authorization_code") + body = grantType.equals("authorization_code") ? String.format("code=%s&redirect_uri=%s&grant_type=%s", code, redirectURI, grantType) : String.format("grant_type=%s&refresh_token=%s", grantType, refreshToken); + break; case FORM_BODY: // fall-through default: - return grantType.equals("authorization_code") + body = grantType.equals("authorization_code") ? String.format("code=%s&redirect_uri=%s&client_id=%s&client_secret=%s&grant_type=%s", code, redirectURI, clientCreds.getClientId(), clientCreds.getClientSecret(), grantType) : String.format("grant_type=%s&client_id=%s&client_secret=%s&refresh_token=%s", grantType, clientCreds.getClientId(), clientCreds.getClientSecret(), refreshToken); + break; } + if (codeVerifier != null && !codeVerifier.isEmpty()) { + body += "&code_verifier=" + codeVerifier; + } + return body; } /** Build HTTP request for getting tokens */ @@ -468,18 +538,36 @@ private HttpRequest.Builder buildHttpRequest(String body, return requestBuilder; } + private String generateCodeVerifier() { + byte[] codeVerifierBytes = new byte[32]; + SECURE_RANDOM.nextBytes(codeVerifierBytes); + return Base64.getUrlEncoder().withoutPadding().encodeToString(codeVerifierBytes); + } + + private String generateCodeChallenge(String codeVerifier) throws OAuthServiceException { + try { + MessageDigest md = MessageDigest.getInstance("SHA-256"); + md.update(codeVerifier.getBytes(StandardCharsets.US_ASCII)); + return Base64.getUrlEncoder().withoutPadding().encodeToString(md.digest()); + } catch (Exception e) { + throw new OAuthServiceException( + HttpURLConnection.HTTP_INTERNAL_ERROR, "Failed to generate SHA-256 code challenge", e); + } + } + /** * Build the HttpRequest to request a refresh token from the OAuth provider * @param provider * @param code the authorization code given after the user accepts OAuth from the provider * @param redirectURI */ - private HttpRequest createGetRefreshTokenRequest(OAuthProvider provider, String code, String redirectURI) + private HttpRequest createGetRefreshTokenRequest(OAuthProvider provider, String code, String redirectURI, + String codeVerifier) throws OAuthServiceException { OAuthClientCredentials clientCreds = provider.getClientCredentials(); CredentialEncodingStrategy strategy = provider.getCredentialEncodingStrategy(); String tokenRefreshURL = provider.getTokenRefreshURL(); - String body = buildRequestBody(strategy, "authorization_code", code, redirectURI, null, clientCreds); + String body = buildRequestBody(strategy, "authorization_code", code, redirectURI, null, clientCreds, codeVerifier); String userAgent = provider.getUserAgent(); try { @@ -499,7 +587,7 @@ private HttpRequest createGetAccessTokenRequest(OAuthProvider provider, String r OAuthClientCredentials clientCreds = provider.getClientCredentials(); CredentialEncodingStrategy strategy = provider.getCredentialEncodingStrategy(); String tokenRefreshURL = provider.getTokenRefreshURL(); - String body = buildRequestBody(strategy, "refresh_token", null, null, refreshToken, clientCreds); + String body = buildRequestBody(strategy, "refresh_token", null, null, refreshToken, clientCreds, null); String userAgent = provider.getUserAgent(); try { @@ -590,4 +678,15 @@ void respond(HttpServiceResponder responder) { } } } + + private long getPrefOrDefault(Map prefs, String key, long defaultValue) { + if (prefs.containsKey(key)) { + try { + return Long.parseLong(prefs.get(key)); + } catch (NumberFormatException e) { + LOG.warn("Invalid number format for preference {}. Using default value: {}", key, defaultValue); + } + } + return defaultValue; + } } diff --git a/cdap-app-templates/cdap-etl/cdap-data-pipeline-base/src/test/java/io/cdap/cdap/datapipeline/OAuthServiceTest.java b/cdap-app-templates/cdap-etl/cdap-data-pipeline-base/src/test/java/io/cdap/cdap/datapipeline/OAuthServiceTest.java index b99e8f77dedd..a3e64582f525 100644 --- a/cdap-app-templates/cdap-etl/cdap-data-pipeline-base/src/test/java/io/cdap/cdap/datapipeline/OAuthServiceTest.java +++ b/cdap-app-templates/cdap-etl/cdap-data-pipeline-base/src/test/java/io/cdap/cdap/datapipeline/OAuthServiceTest.java @@ -19,8 +19,8 @@ import com.google.gson.GsonBuilder; import io.cdap.cdap.common.http.DefaultHttpRequestConfig; import io.cdap.cdap.datapipeline.oauth.OAuthProvider; +import io.cdap.cdap.datapipeline.oauth.AuthType; import io.cdap.cdap.datapipeline.oauth.PutOAuthProviderRequest; -import io.cdap.cdap.datapipeline.oauth.PutOAuthCredentialRequest; import io.cdap.common.http.HttpMethod; import io.cdap.common.http.HttpRequest; import io.cdap.common.http.HttpRequests; @@ -50,7 +50,7 @@ public void testCreateProvider() throws IOException { clientId, clientSecret, OAuthProvider.CredentialEncodingStrategy.FORM_BODY, - null); + null, null); HttpResponse createResponse = makePutCall("provider/testprovider", request); Assert.assertEquals(200, createResponse.getResponseCode()); @@ -72,7 +72,7 @@ public void testCreateProviderWithClientCredentialsMissing() throws IOException null, null, OAuthProvider.CredentialEncodingStrategy.FORM_BODY, - null); + null, null); HttpResponse createResponse = makePutCall("provider/testprovider", request); Assert.assertEquals(400, createResponse.getResponseCode()); } @@ -89,7 +89,7 @@ public void testCreateProviderWithReuseClientCredentialsTrue() throws IOExceptio null, null, OAuthProvider.CredentialEncodingStrategy.FORM_BODY, - null); + null, null); HttpResponse createResponse = makePutCall("provider/testprovider10?reuse_client_credentials=true", request); Assert.assertEquals(500, createResponse.getResponseCode()); } @@ -107,7 +107,7 @@ public void testCreateProviderReuseCredentialsWithReuseClientCredentialsTrue() t clientId, clientSecret, OAuthProvider.CredentialEncodingStrategy.FORM_BODY, - null); + null, null); HttpResponse createResponse = makePutCall("provider/testprovider20", request); Assert.assertEquals(200, createResponse.getResponseCode()); @@ -121,7 +121,7 @@ public void testCreateProviderReuseCredentialsWithReuseClientCredentialsTrue() t null, null, OAuthProvider.CredentialEncodingStrategy.FORM_BODY, - null); + null, null); createResponse = makePutCall("provider/testprovider20?reuse_client_credentials=true", request); Assert.assertEquals(200, createResponse.getResponseCode()); } @@ -138,7 +138,7 @@ public void testCreateProviderWithReuseClientCredentialsFalse() throws IOExcepti null, null, OAuthProvider.CredentialEncodingStrategy.FORM_BODY, - null); + null, null); HttpResponse createResponse = makePutCall("provider/testprovider30?reuse_client_credentials=false", request); Assert.assertEquals(400, createResponse.getResponseCode()); } @@ -156,7 +156,7 @@ public void testCreateProviderWithBasicAuth() throws IOException { clientId, clientSecret, OAuthProvider.CredentialEncodingStrategy.BASIC_AUTH, - null); + null, null); HttpResponse createOauthProviderResponse = makePutCall("provider/testprovider31", request); Assert.assertEquals(200, createOauthProviderResponse.getResponseCode()); @@ -180,7 +180,7 @@ public void testCreateProviderWithBasicAuthAndUserAgent() throws IOException { clientId, clientSecret, OAuthProvider.CredentialEncodingStrategy.BASIC_AUTH, - "cdap-test"); + "cdap-test", null); HttpResponse createOauthProviderResponse = makePutCall("provider/testprovider32", request); Assert.assertEquals(200, createOauthProviderResponse.getResponseCode()); @@ -191,6 +191,36 @@ public void testCreateProviderWithBasicAuthAndUserAgent() throws IOException { Assert.assertEquals("http://www.example.com/login32?client_id=clientid&redirect_uri=null", authURL); } + @Test + public void testCreateProviderWithPkceAuthType() throws IOException { + String loginURL = "http://www.example.com/login_pkce"; + String tokenRefreshURL = "http://www.example.com/token_pkce"; + String clientId = "clientid"; + String clientSecret = "clientsecret"; + PutOAuthProviderRequest request = new PutOAuthProviderRequest( + loginURL, + tokenRefreshURL, + clientId, + clientSecret, + OAuthProvider.CredentialEncodingStrategy.FORM_BODY, + null, AuthType.PKCE); + HttpResponse createOauthProviderResponse = makePutCall("provider/testprovider_pkce", request); + Assert.assertEquals(200, createOauthProviderResponse.getResponseCode()); + + // Grab OAuth login URL to verify write succeeded + HttpResponse getAuthUrlResponse = makeGetCall("provider/testprovider_pkce/authurl"); + Assert.assertEquals(200, getAuthUrlResponse.getResponseCode()); + String authURL = getAuthUrlResponse.getResponseBodyAsString(); + + // Verify base URL + Assert.assertTrue(authURL.startsWith("http://www.example.com/login_pkce?client_id=clientid&redirect_uri=null")); + + // Verify PKCE specific query params + Assert.assertTrue(authURL.contains("&state=")); + Assert.assertTrue(authURL.contains("&code_challenge=")); + Assert.assertTrue(authURL.contains("&code_challenge_method=S256")); + } + @Test public void testGetAuthURLForMissingClientCredentials() throws IOException { // Attempt to create provider with missing client credentials and 'reuse_client_credentials' @@ -203,7 +233,7 @@ public void testGetAuthURLForMissingClientCredentials() throws IOException { null, null, OAuthProvider.CredentialEncodingStrategy.FORM_BODY, - null); + null, null); HttpResponse createResponse = makePutCall("provider/testprovider40?reuse_client_credentials=false", request); Assert.assertEquals(400, createResponse.getResponseCode()); @@ -225,7 +255,7 @@ public void testGetAuthURLForReusedClientCredentials() throws IOException { clientId, clientSecret, OAuthProvider.CredentialEncodingStrategy.FORM_BODY, - null); + null, null); HttpResponse createResponse = makePutCall("provider/testprovider50", request); Assert.assertEquals(200, createResponse.getResponseCode()); @@ -245,7 +275,7 @@ public void testGetAuthURLForReusedClientCredentials() throws IOException { null, null, OAuthProvider.CredentialEncodingStrategy.FORM_BODY, - null); + null, null); createResponse = makePutCall("provider/testprovider50?reuse_client_credentials=true", request); Assert.assertEquals(200, createResponse.getResponseCode()); @@ -269,7 +299,7 @@ public void testCreateProviderBadLoginURL() throws IOException { clientId, clientSecret, OAuthProvider.CredentialEncodingStrategy.FORM_BODY, - null); + null, null); HttpResponse createResponse = makePutCall("provider/testprovider", request); Assert.assertEquals(400, createResponse.getResponseCode()); } @@ -287,7 +317,7 @@ public void testCreateProviderBadTokenRefreshURL() throws IOException { clientId, clientSecret, OAuthProvider.CredentialEncodingStrategy.FORM_BODY, - null); + null, null); HttpResponse createResponse = makePutCall("provider/testprovider", request); Assert.assertEquals(400, createResponse.getResponseCode()); } diff --git a/cdap-app-templates/cdap-etl/cdap-data-pipeline-base/src/test/java/io/cdap/cdap/datapipeline/OAuthStoreTest.java b/cdap-app-templates/cdap-etl/cdap-data-pipeline-base/src/test/java/io/cdap/cdap/datapipeline/OAuthStoreTest.java index b560e18478a2..0ad78acd9481 100644 --- a/cdap-app-templates/cdap-etl/cdap-data-pipeline-base/src/test/java/io/cdap/cdap/datapipeline/OAuthStoreTest.java +++ b/cdap-app-templates/cdap-etl/cdap-data-pipeline-base/src/test/java/io/cdap/cdap/datapipeline/OAuthStoreTest.java @@ -65,7 +65,7 @@ public void setUp() { mockTable = mock(StructuredTable.class); mockRow = mock(StructuredRow.class); - oauthStore = new OAuthStore(mockTransactionRunner, mockSecureStore, mockSecureStoreManager); + oauthStore = new OAuthStore(mockTransactionRunner, mockSecureStore, mockSecureStoreManager, 900L); } @Test