From 442833b06ad8ad92d26bb1f5c84220c56ac4b0c8 Mon Sep 17 00:00:00 2001 From: Anatolii Bazko Date: Fri, 17 Nov 2023 10:49:31 +0100 Subject: [PATCH] Refactoring Signed-off-by: Anatolii Bazko --- .../KubernetesPersonalAccessTokenManager.java | 101 ++++++++++++---- ...ernetesPersonalAccessTokenManagerTest.java | 109 ++++++++++++++++-- 2 files changed, 175 insertions(+), 35 deletions(-) diff --git a/infrastructures/infrastructure-factory/src/main/java/org/eclipse/che/api/factory/server/scm/kubernetes/KubernetesPersonalAccessTokenManager.java b/infrastructures/infrastructure-factory/src/main/java/org/eclipse/che/api/factory/server/scm/kubernetes/KubernetesPersonalAccessTokenManager.java index 35c38fabba..15ce7e16e2 100644 --- a/infrastructures/infrastructure-factory/src/main/java/org/eclipse/che/api/factory/server/scm/kubernetes/KubernetesPersonalAccessTokenManager.java +++ b/infrastructures/infrastructure-factory/src/main/java/org/eclipse/che/api/factory/server/scm/kubernetes/KubernetesPersonalAccessTokenManager.java @@ -12,6 +12,7 @@ package org.eclipse.che.api.factory.server.scm.kubernetes; import static com.google.common.base.Strings.isNullOrEmpty; +import static org.eclipse.che.commons.lang.StringUtils.trimEnd; import com.google.common.annotations.VisibleForTesting; import com.google.common.collect.ImmutableMap; @@ -43,7 +44,6 @@ import org.eclipse.che.api.workspace.server.spi.InfrastructureException; import org.eclipse.che.commons.annotation.Nullable; import org.eclipse.che.commons.env.EnvironmentContext; import org.eclipse.che.commons.lang.NameGenerator; -import org.eclipse.che.commons.lang.StringUtils; import org.eclipse.che.commons.subject.Subject; import org.eclipse.che.workspace.infrastructure.kubernetes.CheServerKubernetesClientFactory; import org.eclipse.che.workspace.infrastructure.kubernetes.api.shared.KubernetesNamespaceMeta; @@ -182,36 +182,31 @@ public class KubernetesPersonalAccessTokenManager implements PersonalAccessToken .secrets() .get(KUBERNETES_PERSONAL_ACCESS_TOKEN_LABEL_SELECTOR); for (Secret secret : secrets) { - Map annotations = secret.getMetadata().getAnnotations(); - String trimmedUrl = StringUtils.trimEnd(annotations.get(ANNOTATION_SCM_URL), '/'); - if (annotations.get(ANNOTATION_CHE_USERID).equals(cheUser.getUserId()) - && (oAuthProviderName == null - || oAuthProviderName.equals( - annotations.get(ANNOTATION_SCM_PERSONAL_ACCESS_TOKEN_NAME))) - && (!isNullOrEmpty(trimmedUrl)) // ignore PAT with empty server endpoint URL - && (scmServerUrl == null - || trimmedUrl.equals(StringUtils.trimEnd(scmServerUrl, '/')))) { - String token = - new String(Base64.getDecoder().decode(secret.getData().get("token"))).trim(); - String providerName = annotations.get(ANNOTATION_SCM_PERSONAL_ACCESS_TOKEN_NAME); - String tokenId = annotations.get(ANNOTATION_SCM_PERSONAL_ACCESS_TOKEN_ID); - String organization = annotations.get(ANNOTATION_SCM_ORGANIZATION); + if (deleteSecretIfMisconfigured(secret)) { + continue; + } + + if (isSecretMatchesSearchCriteria(cheUser, oAuthProviderName, scmServerUrl, secret)) { + PersonalAccessTokenParams personalAccessTokenParams = + this.secret2PersonalAccessTokenParams(secret); Optional scmUsername = - scmPersonalAccessTokenFetcher.getScmUsername( - new PersonalAccessTokenParams( - trimmedUrl, providerName, tokenId, token, organization)); + scmPersonalAccessTokenFetcher.getScmUsername(personalAccessTokenParams); + if (scmUsername.isPresent()) { + Map secretAnnotations = secret.getMetadata().getAnnotations(); + PersonalAccessToken personalAccessToken = new PersonalAccessToken( - trimmedUrl, - annotations.get(ANNOTATION_CHE_USERID), - organization, + personalAccessTokenParams.getScmProviderUrl(), + secretAnnotations.get(ANNOTATION_CHE_USERID), + personalAccessTokenParams.getOrganization(), scmUsername.get(), - providerName, - tokenId, - token); + secretAnnotations.get(ANNOTATION_SCM_PERSONAL_ACCESS_TOKEN_NAME), + personalAccessTokenParams.getScmTokenId(), + personalAccessTokenParams.getToken()); return Optional.of(personalAccessToken); } + // Removing token that is no longer valid. If several tokens exist the next one could // be valid. If no valid token can be found, the caller should react in the same way // as it reacts if no token exists. Usually, that means that process of new token @@ -230,6 +225,64 @@ public class KubernetesPersonalAccessTokenManager implements PersonalAccessToken return Optional.empty(); } + private boolean deleteSecretIfMisconfigured(Secret secret) throws InfrastructureException { + Map secretAnnotations = secret.getMetadata().getAnnotations(); + + String configuredScmServerUrl = secretAnnotations.get(ANNOTATION_SCM_URL); + String configuredCheUserId = secretAnnotations.get(ANNOTATION_CHE_USERID); + String configuredOAuthProviderName = + secretAnnotations.get(ANNOTATION_SCM_PERSONAL_ACCESS_TOKEN_NAME); + + // if any of the required annotations is missing, the secret is not valid + if (isNullOrEmpty(configuredScmServerUrl) + || isNullOrEmpty(configuredCheUserId) + || isNullOrEmpty(configuredOAuthProviderName)) { + cheServerKubernetesClientFactory + .create() + .secrets() + .inNamespace(secret.getMetadata().getNamespace()) + .delete(secret); + return true; + } + + return false; + } + + private PersonalAccessTokenParams secret2PersonalAccessTokenParams(Secret secret) { + Map secretAnnotations = secret.getMetadata().getAnnotations(); + + String token = new String(Base64.getDecoder().decode(secret.getData().get("token"))).trim(); + String configuredOAuthProviderName = + secretAnnotations.get(ANNOTATION_SCM_PERSONAL_ACCESS_TOKEN_NAME); + String configuredTokenId = secretAnnotations.get(ANNOTATION_SCM_PERSONAL_ACCESS_TOKEN_ID); + String configuredScmOrganization = secretAnnotations.get(ANNOTATION_SCM_ORGANIZATION); + String configuredScmServerUrl = secretAnnotations.get(ANNOTATION_SCM_URL); + + return new PersonalAccessTokenParams( + trimEnd(configuredScmServerUrl, '/'), + configuredOAuthProviderName, + configuredTokenId, + token, + configuredScmOrganization); + } + + private boolean isSecretMatchesSearchCriteria( + Subject cheUser, + @Nullable String oAuthProviderName, + @Nullable String scmServerUrl, + Secret secret) { + Map secretAnnotations = secret.getMetadata().getAnnotations(); + String configuredScmServerUrl = secretAnnotations.get(ANNOTATION_SCM_URL); + String configuredCheUserId = secretAnnotations.get(ANNOTATION_CHE_USERID); + String configuredOAuthProviderName = + secretAnnotations.get(ANNOTATION_SCM_PERSONAL_ACCESS_TOKEN_NAME); + + return (configuredCheUserId.equals(cheUser.getUserId())) + && (oAuthProviderName == null || oAuthProviderName.equals(configuredOAuthProviderName)) + && (scmServerUrl == null + || trimEnd(configuredScmServerUrl, '/').equals(trimEnd(scmServerUrl, '/'))); + } + @Override public PersonalAccessToken getAndStore(String scmServerUrl) throws ScmCommunicationException, ScmConfigurationPersistenceException, diff --git a/infrastructures/infrastructure-factory/src/test/java/org/eclipse/che/api/factory/server/scm/kubernetes/KubernetesPersonalAccessTokenManagerTest.java b/infrastructures/infrastructure-factory/src/test/java/org/eclipse/che/api/factory/server/scm/kubernetes/KubernetesPersonalAccessTokenManagerTest.java index 5920601db6..47cfdb423f 100644 --- a/infrastructures/infrastructure-factory/src/test/java/org/eclipse/che/api/factory/server/scm/kubernetes/KubernetesPersonalAccessTokenManagerTest.java +++ b/infrastructures/infrastructure-factory/src/test/java/org/eclipse/che/api/factory/server/scm/kubernetes/KubernetesPersonalAccessTokenManagerTest.java @@ -13,9 +13,7 @@ package org.eclipse.che.api.factory.server.scm.kubernetes; import static java.nio.charset.StandardCharsets.UTF_8; import static java.util.Collections.singletonList; -import static org.eclipse.che.api.factory.server.scm.kubernetes.KubernetesPersonalAccessTokenManager.ANNOTATION_CHE_USERID; -import static org.eclipse.che.api.factory.server.scm.kubernetes.KubernetesPersonalAccessTokenManager.ANNOTATION_SCM_PERSONAL_ACCESS_TOKEN_ID; -import static org.eclipse.che.api.factory.server.scm.kubernetes.KubernetesPersonalAccessTokenManager.ANNOTATION_SCM_URL; +import static org.eclipse.che.api.factory.server.scm.kubernetes.KubernetesPersonalAccessTokenManager.*; import static org.mockito.ArgumentMatchers.any; import static org.mockito.ArgumentMatchers.eq; import static org.mockito.Mockito.times; @@ -105,7 +103,13 @@ public class KubernetesPersonalAccessTokenManagerTest { ObjectMeta meta1 = new ObjectMetaBuilder() .withAnnotations( - Map.of(ANNOTATION_CHE_USERID, "user", ANNOTATION_SCM_URL, "http://host1")) + Map.of( + ANNOTATION_SCM_PERSONAL_ACCESS_TOKEN_NAME, + "github", + ANNOTATION_CHE_USERID, + "user", + ANNOTATION_SCM_URL, + "http://host1")) .build(); Secret secret = new SecretBuilder().withMetadata(meta1).withData(data).build(); @@ -176,17 +180,35 @@ public class KubernetesPersonalAccessTokenManagerTest { ObjectMeta meta1 = new ObjectMetaBuilder() .withAnnotations( - Map.of(ANNOTATION_CHE_USERID, "user1", ANNOTATION_SCM_URL, "http://host1")) + Map.of( + ANNOTATION_SCM_PERSONAL_ACCESS_TOKEN_NAME, + "github", + ANNOTATION_CHE_USERID, + "user1", + ANNOTATION_SCM_URL, + "http://host1")) .build(); ObjectMeta meta2 = new ObjectMetaBuilder() .withAnnotations( - Map.of(ANNOTATION_CHE_USERID, "user1", ANNOTATION_SCM_URL, "http://host2")) + Map.of( + ANNOTATION_SCM_PERSONAL_ACCESS_TOKEN_NAME, + "github", + ANNOTATION_CHE_USERID, + "user1", + ANNOTATION_SCM_URL, + "http://host2")) .build(); ObjectMeta meta3 = new ObjectMetaBuilder() .withAnnotations( - Map.of(ANNOTATION_CHE_USERID, "user2", ANNOTATION_SCM_URL, "http://host3")) + Map.of( + ANNOTATION_SCM_PERSONAL_ACCESS_TOKEN_NAME, + "github", + ANNOTATION_CHE_USERID, + "user2", + ANNOTATION_SCM_URL, + "http://host3")) .build(); Secret secret1 = new SecretBuilder().withMetadata(meta1).withData(data1).build(); @@ -227,6 +249,8 @@ public class KubernetesPersonalAccessTokenManagerTest { new ObjectMetaBuilder() .withAnnotations( Map.of( + ANNOTATION_SCM_PERSONAL_ACCESS_TOKEN_NAME, + "github", ANNOTATION_CHE_USERID, "user1", ANNOTATION_SCM_URL, @@ -269,7 +293,13 @@ public class KubernetesPersonalAccessTokenManagerTest { ObjectMeta metaData = new ObjectMetaBuilder() .withAnnotations( - Map.of(ANNOTATION_CHE_USERID, "user1", ANNOTATION_SCM_URL, "http://host1")) + Map.of( + ANNOTATION_SCM_PERSONAL_ACCESS_TOKEN_NAME, + "github", + ANNOTATION_CHE_USERID, + "user1", + ANNOTATION_SCM_URL, + "http://host1")) .build(); Secret secret = new SecretBuilder().withMetadata(metaData).withData(data).build(); @@ -308,12 +338,24 @@ public class KubernetesPersonalAccessTokenManagerTest { ObjectMeta meta1 = new ObjectMetaBuilder() .withAnnotations( - Map.of(ANNOTATION_CHE_USERID, "user1", ANNOTATION_SCM_URL, "http://host1.com/")) + Map.of( + ANNOTATION_SCM_PERSONAL_ACCESS_TOKEN_NAME, + "github", + ANNOTATION_CHE_USERID, + "user1", + ANNOTATION_SCM_URL, + "http://host1.com/")) .build(); ObjectMeta meta2 = new ObjectMetaBuilder() .withAnnotations( - Map.of(ANNOTATION_CHE_USERID, "user1", ANNOTATION_SCM_URL, "http://host2.com")) + Map.of( + ANNOTATION_SCM_PERSONAL_ACCESS_TOKEN_NAME, + "github", + ANNOTATION_CHE_USERID, + "user1", + ANNOTATION_SCM_URL, + "http://host2.com")) .build(); Secret secret1 = new SecretBuilder().withMetadata(meta1).withData(data1).build(); @@ -336,6 +378,41 @@ public class KubernetesPersonalAccessTokenManagerTest { assertNotNull(token2); } + @Test + public void shouldDeleteMisconfiguredTokensOnGet() throws Exception { + // given + KubernetesNamespaceMeta meta = new KubernetesNamespaceMetaImpl("test"); + when(namespaceFactory.list()).thenReturn(singletonList(meta)); + KubernetesNamespace kubernetesnamespace = Mockito.mock(KubernetesNamespace.class); + KubernetesSecrets secrets = Mockito.mock(KubernetesSecrets.class); + when(namespaceFactory.access(eq(null), eq(meta.getName()))).thenReturn(kubernetesnamespace); + when(kubernetesnamespace.secrets()).thenReturn(secrets); + when(cheServerKubernetesClientFactory.create()).thenReturn(kubeClient); + when(kubeClient.secrets()).thenReturn(secretsMixedOperation); + when(secretsMixedOperation.inNamespace(eq(meta.getName()))).thenReturn(nonNamespaceOperation); + Map data1 = + Map.of("token", Base64.getEncoder().encodeToString("token1".getBytes(UTF_8))); + ObjectMeta meta1 = + new ObjectMetaBuilder() + .withNamespace("test") + .withAnnotations( + Map.of( + ANNOTATION_SCM_PERSONAL_ACCESS_TOKEN_NAME, + "github", + ANNOTATION_CHE_USERID, + "user1")) + .build(); + Secret secret1 = new SecretBuilder().withMetadata(meta1).withData(data1).build(); + when(secrets.get(any(LabelSelector.class))).thenReturn(Arrays.asList(secret1)); + // when + Optional token = + personalAccessTokenManager.get( + new SubjectImpl("user", "user1", "t1", false), "http://host1"); + // then + assertFalse(token.isPresent()); + verify(nonNamespaceOperation, times(1)).delete(eq(secret1)); + } + @Test public void shouldDeleteInvalidTokensOnGet() throws Exception { // given @@ -355,7 +432,13 @@ public class KubernetesPersonalAccessTokenManagerTest { ObjectMeta meta1 = new ObjectMetaBuilder() .withAnnotations( - Map.of(ANNOTATION_CHE_USERID, "user1", ANNOTATION_SCM_URL, "http://host1")) + Map.of( + ANNOTATION_SCM_PERSONAL_ACCESS_TOKEN_NAME, + "github", + ANNOTATION_CHE_USERID, + "user1", + ANNOTATION_SCM_URL, + "http://host1")) .build(); Secret secret1 = new SecretBuilder().withMetadata(meta1).withData(data1).build(); when(secrets.get(any(LabelSelector.class))).thenReturn(Arrays.asList(secret1)); @@ -397,6 +480,8 @@ public class KubernetesPersonalAccessTokenManagerTest { new ObjectMetaBuilder() .withAnnotations( Map.of( + ANNOTATION_SCM_PERSONAL_ACCESS_TOKEN_NAME, + "github", ANNOTATION_CHE_USERID, "user1", ANNOTATION_SCM_URL, @@ -408,6 +493,8 @@ public class KubernetesPersonalAccessTokenManagerTest { new ObjectMetaBuilder() .withAnnotations( Map.of( + ANNOTATION_SCM_PERSONAL_ACCESS_TOKEN_NAME, + "github", ANNOTATION_CHE_USERID, "user1", ANNOTATION_SCM_URL,