From 8aa76cf3b3da3564a144580c4b185bd49070f077 Mon Sep 17 00:00:00 2001 From: Marc Nuri Date: Thu, 18 Apr 2024 12:31:27 +0200 Subject: [PATCH] refactor(openid-connection): provide structure to token persistence Signed-off-by: Marc Nuri --- .../client/utils/OpenIDConnectionUtils.java | 117 +++++++--------- .../OpenIDConnectionUtilsBehaviorTest.java | 129 ++++++++++++++++++ .../utils/OpenIDConnectionUtilsTest.java | 22 ++- .../internal/OpenShiftOAuthInterceptor.java | 12 +- 4 files changed, 188 insertions(+), 92 deletions(-) diff --git a/kubernetes-client-api/src/main/java/io/fabric8/kubernetes/client/utils/OpenIDConnectionUtils.java b/kubernetes-client-api/src/main/java/io/fabric8/kubernetes/client/utils/OpenIDConnectionUtils.java index 500ec438da6..ef5fe66061c 100644 --- a/kubernetes-client-api/src/main/java/io/fabric8/kubernetes/client/utils/OpenIDConnectionUtils.java +++ b/kubernetes-client-api/src/main/java/io/fabric8/kubernetes/client/utils/OpenIDConnectionUtils.java @@ -20,7 +20,6 @@ import io.fabric8.kubernetes.api.model.AuthInfo; import io.fabric8.kubernetes.api.model.AuthProviderConfig; import io.fabric8.kubernetes.api.model.NamedAuthInfo; -import io.fabric8.kubernetes.api.model.NamedContext; import io.fabric8.kubernetes.client.Config; import io.fabric8.kubernetes.client.KubernetesClientException; import io.fabric8.kubernetes.client.http.HttpClient; @@ -47,7 +46,6 @@ import java.util.Map; import java.util.Optional; import java.util.concurrent.CompletableFuture; -import java.util.function.Consumer; import javax.net.ssl.KeyManager; import javax.net.ssl.TrustManager; @@ -92,20 +90,13 @@ public static CompletableFuture resolveOIDCTokenFromAuthConfig( final HttpClient httpClient = initHttpClientWithPemCert(idpCert, clientBuilder); final CompletableFuture result = getOpenIdConfiguration(httpClient, currentAuthProviderConfig) .thenCompose(openIdConfiguration -> refreshOpenIdToken(httpClient, currentAuthProviderConfig, openIdConfiguration)) + .thenApply(oAuthToken -> persistOAuthToken(currentConfig, oAuthToken, null)) .thenApply(oAuthToken -> { if (oAuthToken == null || Utils.isNullOrEmpty(oAuthToken.idToken)) { LOGGER.warn("token response did not contain an id_token, either the scope \\\"openid\\\" wasn't " + "requested upon login, or the provider doesn't support id_tokens as part of the refresh response."); return originalToken; } - - // Persist new config and if successful, update the in memory config. - try { - persistKubeConfigWithUpdatedToken(currentConfig, oAuthToken); - } catch (IOException e) { - LOGGER.warn("oidc: failure while persisting new tokens into KUBECONFIG", e); - } - return oAuthToken.idToken; }); result.whenComplete((s, t) -> httpClient.close()); @@ -187,73 +178,61 @@ private static CompletableFuture refreshOpenIdToken( } /** - * Well known URL for getting OpenID Connect metadata. - * https://openid.net/specs/openid-connect-discovery-1_0.html#ProviderConfig + * Save Updated Access and Refresh token in local KubeConfig file and in-memory Config object. * - * @param authProviderConfig containing the issuing authority URL - * @return well known URL for corresponding OpenID provider + * @param currentConfig current Config object. + * @param oAuthToken OAuth token information as received from OpenID provider. + * @param token new token to be persisted in KubeConfig (if not null). + * @return the oAuthToken for chaining and further processing. */ - private static String resolveWellKnownUrlForOpenIDIssuer(Map authProviderConfig) { - return URLUtils.join(authProviderConfig.get(ISSUER_KUBECONFIG), "/", WELL_KNOWN_OPENID_CONFIGURATION); - } - - /** - * Save Updated Access and Refresh token in local KubeConfig file. - * - * @param currentConfig current KubeConfig object - * @param oAuthToken OAuth token information as received from OpenID provider - * @return boolean value whether update was successful or not - * @throws IOException in case of any failure while writing file - */ - static boolean persistKubeConfigWithUpdatedToken(Config currentConfig, OAuthToken oAuthToken) - throws IOException { - return persistKubeConfigWithUpdatedAuthInfo(currentConfig, a -> { - Map authProviderConfig = a.getAuthProvider().getConfig(); + public static OAuthToken persistOAuthToken(Config currentConfig, OAuthToken oAuthToken, String token) { + final Map authProviderConfig = new HashMap<>(); + if (oAuthToken != null) { authProviderConfig.put(ID_TOKEN_KUBECONFIG, oAuthToken.idToken); authProviderConfig.put(REFRESH_TOKEN_KUBECONFIG, oAuthToken.refreshToken); - }); + // Persist in memory + Optional.of(currentConfig).map(Config::getAuthProvider).map(AuthProviderConfig::getConfig) + .ifPresent(c -> c.putAll(authProviderConfig)); + } + // Persist in file + if (currentConfig.getFile() != null && currentConfig.getCurrentContext() != null) { + try { + final io.fabric8.kubernetes.api.model.Config kubeConfig = KubeConfigUtils.parseConfig(currentConfig.getFile()); + final String userName = currentConfig.getCurrentContext().getContext().getUser(); + NamedAuthInfo namedAuthInfo = kubeConfig.getUsers().stream().filter(n -> n.getName().equals(userName)).findFirst() + .orElseGet(() -> { + NamedAuthInfo result = new NamedAuthInfo(userName, new AuthInfo()); + kubeConfig.getUsers().add(result); + return result; + }); + if (namedAuthInfo.getUser() == null) { + namedAuthInfo.setUser(new AuthInfo()); + } + if (namedAuthInfo.getUser().getAuthProvider() == null) { + namedAuthInfo.getUser().setAuthProvider(new AuthProviderConfig()); + } + namedAuthInfo.getUser().getAuthProvider().getConfig().putAll(authProviderConfig); + if (Utils.isNotNullOrEmpty(token)) { + namedAuthInfo.getUser().setToken(token); + } + KubeConfigUtils.persistKubeConfigIntoFile(kubeConfig, currentConfig.getFile().getAbsolutePath()); + } catch (IOException ex) { + LOGGER.warn("oidc: failure while persisting new tokens into KUBECONFIG", ex); + } + } + + return oAuthToken; } /** - * Return true if the Config can be updated. false if not for a variety of reasons: - * - a kubeconfig file was not used - * - there's no current context + * Well known URL for getting OpenID Connect metadata. + * https://openid.net/specs/openid-connect-discovery-1_0.html#ProviderConfig + * + * @param authProviderConfig containing the issuing authority URL + * @return well known URL for corresponding OpenID provider */ - public static boolean persistKubeConfigWithUpdatedAuthInfo(Config currentConfig, Consumer updateAction) - throws IOException { - AuthInfo authInfo = new AuthInfo(); - authInfo.setAuthProvider(new AuthProviderConfig(new HashMap<>(2), currentConfig.getAuthProvider().getName())); - updateAction.accept(authInfo); - //update new auth info to in-memory config - currentConfig.getAuthProvider().getConfig().putAll(authInfo.getAuthProvider().getConfig()); - - if (currentConfig.getFile() == null) { - return false; - } - io.fabric8.kubernetes.api.model.Config config = KubeConfigUtils.parseConfig(currentConfig.getFile()); - NamedContext currentNamedContext = currentConfig.getCurrentContext(); - - if (currentNamedContext == null) { - return false; - } - String userName = currentNamedContext.getContext().getUser(); - - NamedAuthInfo namedAuthInfo = config.getUsers().stream().filter(n -> n.getName().equals(userName)).findFirst() - .orElseGet(() -> { - NamedAuthInfo result = new NamedAuthInfo(userName, new AuthInfo()); - config.getUsers().add(result); - return result; - }); - //update new auth info to kubeConfig - if (namedAuthInfo.getUser() == null) { - namedAuthInfo.setUser(authInfo); - } else { - Optional.ofNullable(authInfo.getToken()).ifPresent(t -> namedAuthInfo.getUser().setToken(t)); - namedAuthInfo.getUser().getAuthProvider().getConfig().putAll(authInfo.getAuthProvider().getConfig()); - } - // Persist changes to KUBECONFIG - KubeConfigUtils.persistKubeConfigIntoFile(config, currentConfig.getFile().getAbsolutePath()); - return true; + private static String resolveWellKnownUrlForOpenIDIssuer(Map authProviderConfig) { + return URLUtils.join(authProviderConfig.get(ISSUER_KUBECONFIG), "/", WELL_KNOWN_OPENID_CONFIGURATION); } private static HttpClient initHttpClientWithPemCert(String idpCert, HttpClient.Builder clientBuilder) { diff --git a/kubernetes-client-api/src/test/java/io/fabric8/kubernetes/client/utils/OpenIDConnectionUtilsBehaviorTest.java b/kubernetes-client-api/src/test/java/io/fabric8/kubernetes/client/utils/OpenIDConnectionUtilsBehaviorTest.java index 9d2721a4567..290ed88284a 100644 --- a/kubernetes-client-api/src/test/java/io/fabric8/kubernetes/client/utils/OpenIDConnectionUtilsBehaviorTest.java +++ b/kubernetes-client-api/src/test/java/io/fabric8/kubernetes/client/utils/OpenIDConnectionUtilsBehaviorTest.java @@ -25,6 +25,7 @@ import io.fabric8.kubernetes.client.http.TestStandardHttpClient; import io.fabric8.kubernetes.client.http.TestStandardHttpClientBuilder; import io.fabric8.kubernetes.client.http.TestStandardHttpClientFactory; +import io.fabric8.kubernetes.client.internal.KubeConfigUtils; import org.assertj.core.api.InstanceOfAssertFactories; import org.bouncycastle.asn1.x500.X500Name; import org.bouncycastle.cert.X509CertificateHolder; @@ -39,6 +40,7 @@ import org.junit.jupiter.api.io.TempDir; import java.io.ByteArrayOutputStream; +import java.io.File; import java.io.IOException; import java.io.PrintStream; import java.math.BigInteger; @@ -59,9 +61,11 @@ import javax.net.ssl.X509ExtendedTrustManager; import static io.fabric8.kubernetes.client.http.TestStandardHttpClientFactory.Mode.SINGLETON; +import static io.fabric8.kubernetes.client.utils.OpenIDConnectionUtils.persistOAuthToken; import static io.fabric8.kubernetes.client.utils.OpenIDConnectionUtils.resolveOIDCTokenFromAuthConfig; import static org.assertj.core.api.Assertions.assertThat; import static org.assertj.core.api.Assertions.assertThatThrownBy; +import static org.assertj.core.data.MapEntry.entry; class OpenIDConnectionUtilsBehaviorTest { @@ -388,4 +392,129 @@ void tokenRefreshRequestContainsValidFormData() { } } } + + @Nested + @DisplayName("persistOAuthToken()") + class PersistOAuthToken { + + @TempDir + private Path tempDir; + private File kubeConfig; + private Config originalConfig; + private OpenIDConnectionUtils.OAuthToken oAuthTokenResponse; + + @BeforeEach + void setUp() { + kubeConfig = tempDir.resolve("kubeconfig").toFile(); + originalConfig = Config.empty(); + oAuthTokenResponse = new OpenIDConnectionUtils.OAuthToken(); + oAuthTokenResponse.setIdToken("new-token"); + oAuthTokenResponse.setRefreshToken("new-refresh-token"); + } + + @Test + void skipsInMemoryWhenNoOAuthToken() { + persistOAuthToken(originalConfig, null, "fake.token"); + assertThat(originalConfig) + .returns(null, Config::getAuthProvider); + } + + @Test + void skipsInMemoryWhenOriginalConfigHasNoAuthProvider() { + persistOAuthToken(originalConfig, oAuthTokenResponse, "fake.token"); + assertThat(originalConfig) + .returns(null, Config::getAuthProvider); + } + + @Test + void updatesInMemory() { + originalConfig = new ConfigBuilder(originalConfig).withAuthProvider(new AuthProviderConfig()).build(); + persistOAuthToken(originalConfig, oAuthTokenResponse, "fake.token"); + assertThat(originalConfig) + .extracting(c -> c.getAuthProvider().getConfig()) + .asInstanceOf(InstanceOfAssertFactories.map(String.class, String.class)) + .containsOnly( + entry("id-token", "new-token"), + entry("refresh-token", "new-refresh-token")); + } + + @Test + void skipsInFileWhenOriginalConfigHasNoFile() { + persistOAuthToken(originalConfig, oAuthTokenResponse, "fake.token"); + assertThat(kubeConfig).doesNotExist(); + } + + @Test + void skipsInFileWhenOriginalConfigHasNoCurrentContext() { + originalConfig.setFile(kubeConfig); + persistOAuthToken(originalConfig, oAuthTokenResponse, "fake.token"); + assertThat(kubeConfig).doesNotExist(); + } + + @Test + void logsWarningIfReferencedFileIsMissing() { + originalConfig.setFile(kubeConfig); + originalConfig = new ConfigBuilder(originalConfig) + .withCurrentContext(new NamedContextBuilder().withName("context").build()).build(); + persistOAuthToken(originalConfig, oAuthTokenResponse, "fake.token"); + assertThat(systemErr.toString()) + .contains("oidc: failure while persisting new tokens into KUBECONFIG") + .contains("FileNotFoundException"); + } + + @Nested + @DisplayName("With valid kube config") + class WithValidKubeConfig { + @BeforeEach + void setUp() throws IOException { + Files.write(kubeConfig.toPath(), ("---" + + "users:\n" + + "- name: user\n").getBytes(StandardCharsets.UTF_8)); + } + + @Test + void persistsTokenInFile() throws IOException { + originalConfig.setFile(kubeConfig); + originalConfig = new ConfigBuilder(originalConfig) + .withCurrentContext(new NamedContextBuilder() + .withName("context") + .withNewContext().withUser("user").endContext().build()) + .build(); + persistOAuthToken(originalConfig, oAuthTokenResponse, "fake.token"); + assertThat(KubeConfigUtils.parseConfig(kubeConfig)) + .returns("fake.token", c -> c.getUsers().iterator().next().getUser().getToken()); + } + + @Test + void skipsTokenInFileIfNull() throws IOException { + originalConfig.setFile(kubeConfig); + originalConfig = new ConfigBuilder(originalConfig) + .withCurrentContext(new NamedContextBuilder() + .withName("context") + .withNewContext().withUser("user").endContext().build()) + .build(); + persistOAuthToken(originalConfig, oAuthTokenResponse, null); + assertThat(KubeConfigUtils.parseConfig(kubeConfig)) + .returns(null, c -> c.getUsers().iterator().next().getUser().getToken()); + } + + @Test + void persistsOAuthTokenInFile() throws IOException { + originalConfig.setFile(kubeConfig); + originalConfig = new ConfigBuilder(originalConfig) + .withCurrentContext(new NamedContextBuilder() + .withName("context") + .withNewContext().withUser("user").endContext().build()) + .build(); + persistOAuthToken(originalConfig, oAuthTokenResponse, "fake.token"); + assertThat(KubeConfigUtils.parseConfig(kubeConfig)) + .extracting(c -> c.getUsers().iterator().next().getUser().getAuthProvider().getConfig()) + .asInstanceOf(InstanceOfAssertFactories.map(String.class, String.class)) + .containsOnly( + entry("id-token", "new-token"), + entry("refresh-token", "new-refresh-token")); + } + } + + } } diff --git a/kubernetes-client-api/src/test/java/io/fabric8/kubernetes/client/utils/OpenIDConnectionUtilsTest.java b/kubernetes-client-api/src/test/java/io/fabric8/kubernetes/client/utils/OpenIDConnectionUtilsTest.java index a7f82e2f50c..e5a7886a061 100644 --- a/kubernetes-client-api/src/test/java/io/fabric8/kubernetes/client/utils/OpenIDConnectionUtilsTest.java +++ b/kubernetes-client-api/src/test/java/io/fabric8/kubernetes/client/utils/OpenIDConnectionUtilsTest.java @@ -31,11 +31,10 @@ import org.mockito.MockedStatic; import java.io.File; -import java.io.FileInputStream; import java.io.IOException; import java.nio.charset.StandardCharsets; import java.nio.file.Files; -import java.nio.file.Paths; +import java.nio.file.Path; import java.nio.file.StandardCopyOption; import java.time.Instant; import java.util.Base64; @@ -71,24 +70,23 @@ void setUp() { } @Test - void persistKubeConfigWithUpdatedToken() throws IOException { + // TODO: remove in favor of specific tests, kept for checking compatibility + void persistOAuthTokenWithUpdatedToken(@TempDir Path tempDir) throws IOException { // Given final OpenIDConnectionUtils.OAuthToken oAuthTokenResponse = new OpenIDConnectionUtils.OAuthToken(); oAuthTokenResponse.setIdToken("id-token-updated"); oAuthTokenResponse.setRefreshToken("refresh-token-updated"); - File tempFile = Files.createTempFile("test", "kubeconfig").toFile(); - Files.copy(getClass().getResourceAsStream("/test-kubeconfig-oidc"), Paths.get(tempFile.getPath()), + Path kubeConfig = Files.createTempFile(tempDir, "test", "kubeconfig"); + Files.copy(OpenIDConnectionUtilsTest.class.getResourceAsStream("/test-kubeconfig-oidc"), kubeConfig, StandardCopyOption.REPLACE_EXISTING); - - Config theConfig = Config.fromKubeconfig(null, IOHelpers.readFully(new FileInputStream(tempFile), StandardCharsets.UTF_8), - tempFile.getAbsolutePath()); + Config originalConfig = Config.fromKubeconfig(null, new String(Files.readAllBytes(kubeConfig), StandardCharsets.UTF_8), + kubeConfig.toFile().getAbsolutePath()); // When - boolean isPersisted = OpenIDConnectionUtils.persistKubeConfigWithUpdatedToken(theConfig, oAuthTokenResponse); + OpenIDConnectionUtils.persistOAuthToken(originalConfig, oAuthTokenResponse, null); // Then - assertTrue(isPersisted); - io.fabric8.kubernetes.api.model.Config config = KubeConfigUtils.parseConfig(tempFile); + io.fabric8.kubernetes.api.model.Config config = KubeConfigUtils.parseConfig(kubeConfig.toFile()); assertNotNull(config); NamedContext currentNamedContext = KubeConfigUtils.getCurrentContext(config); assertNotNull(currentNamedContext); @@ -97,7 +95,7 @@ void persistKubeConfigWithUpdatedToken() throws IOException { Map authProviderConfigInFile = config.getUsers().get(currentUserIndex).getUser().getAuthProvider() .getConfig(); assertFalse(authProviderConfigInFile.isEmpty()); - Map authProviderConfigInMemory = theConfig.getAuthProvider().getConfig(); + Map authProviderConfigInMemory = originalConfig.getAuthProvider().getConfig(); //auth info should be updated in memory assertEquals("id-token-updated", authProviderConfigInMemory.get(ID_TOKEN_KUBECONFIG)); assertEquals("refresh-token-updated", authProviderConfigInMemory.get(REFRESH_TOKEN_KUBECONFIG)); diff --git a/openshift-client/src/main/java/io/fabric8/openshift/client/internal/OpenShiftOAuthInterceptor.java b/openshift-client/src/main/java/io/fabric8/openshift/client/internal/OpenShiftOAuthInterceptor.java index a3cf9aca844..35b942dd046 100644 --- a/openshift-client/src/main/java/io/fabric8/openshift/client/internal/OpenShiftOAuthInterceptor.java +++ b/openshift-client/src/main/java/io/fabric8/openshift/client/internal/OpenShiftOAuthInterceptor.java @@ -33,10 +33,7 @@ import io.fabric8.openshift.api.model.SelfSubjectRulesReview; import io.fabric8.openshift.api.model.SubjectAccessReview; import io.fabric8.openshift.api.model.SubjectRulesReview; -import org.slf4j.Logger; -import org.slf4j.LoggerFactory; -import java.io.IOException; import java.net.MalformedURLException; import java.net.URL; import java.time.Instant; @@ -56,8 +53,6 @@ */ public class OpenShiftOAuthInterceptor extends TokenRefreshInterceptor { - private static final Logger LOGGER = LoggerFactory.getLogger(OpenShiftOAuthInterceptor.class); - private static final String AUTHORIZATION = "Authorization"; private static final String LOCATION = "Location"; private static final String AUTHORIZATION_SERVER_PATH = ".well-known/oauth-authorization-server"; @@ -153,13 +148,8 @@ protected boolean shouldFail(HttpResponse response) { private static String persistNewOAuthTokenIntoKubeConfig(Config config, String token) { if (token != null) { - try { - OpenIDConnectionUtils.persistKubeConfigWithUpdatedAuthInfo(config, a -> a.setToken(token)); - } catch (IOException e) { - LOGGER.warn("failure while persisting new token into KUBECONFIG", e); - } + OpenIDConnectionUtils.persistOAuthToken(config, null, token); } - return token; }