Remove redundan PATs on refresh mode (#703)

Iterate over personal access token secrets on refresh function. Remove outdated token secrets except the latest refreshed token secret.
eclipse-che · Jul 23, 2024 · 0227c54 · 0227c54
1 parent a0bd836
commit 0227c54
Show file tree

Hide file tree

Showing 2 changed files with 37 additions and 10 deletions.
diff --git a/...g/eclipse/che/api/factory/server/scm/kubernetes/KubernetesPersonalAccessTokenManager.java b/...g/eclipse/che/api/factory/server/scm/kubernetes/KubernetesPersonalAccessTokenManager.java
@@ -23,6 +23,7 @@
 import io.fabric8.kubernetes.api.model.SecretBuilder;
 import io.fabric8.kubernetes.client.KubernetesClientException;
 import java.nio.charset.StandardCharsets;
+import java.util.ArrayList;
 import java.util.Base64;
 import java.util.Collections;
 import java.util.Comparator;
@@ -152,7 +153,7 @@ public PersonalAccessToken fetchAndSave(Subject cheUser, String scmServerUrl)
   @Override
   public Optional<PersonalAccessToken> get(Subject cheUser, String scmServerUrl)
       throws ScmConfigurationPersistenceException {
-    return doGetPersonalAccessToken(cheUser, null, scmServerUrl);
+    return doGetPersonalAccessTokens(cheUser, null, scmServerUrl).stream().findFirst();
   }
 
   @Override
@@ -174,12 +175,13 @@ public PersonalAccessToken get(String scmServerUrl)
   public Optional<PersonalAccessToken> get(
       Subject cheUser, String oAuthProviderName, @Nullable String scmServerUrl)
       throws ScmConfigurationPersistenceException {
-    return doGetPersonalAccessToken(cheUser, oAuthProviderName, scmServerUrl);
+    return doGetPersonalAccessTokens(cheUser, oAuthProviderName, scmServerUrl).stream().findFirst();
   }
 
-  private Optional<PersonalAccessToken> doGetPersonalAccessToken(
+  private List<PersonalAccessToken> doGetPersonalAccessTokens(
       Subject cheUser, @Nullable String oAuthProviderName, @Nullable String scmServerUrl)
       throws ScmConfigurationPersistenceException {
+    List<PersonalAccessToken> result = new ArrayList<>();
     try {
       LOG.debug(
           "Fetching personal access token for user {} and OAuth provider {}",
@@ -231,7 +233,8 @@ private Optional<PersonalAccessToken> doGetPersonalAccessToken(
                       personalAccessTokenParams.getScmTokenName(),
                       personalAccessTokenParams.getScmTokenId(),
                       personalAccessTokenParams.getToken());
-              return Optional.of(personalAccessToken);
+              result.add(personalAccessToken);
+              continue;
             }
 
             // Removing token that is no longer valid. If several tokens exist the next one could
@@ -251,7 +254,7 @@ private Optional<PersonalAccessToken> doGetPersonalAccessToken(
       LOG.debug("Failed to get personal access token", e);
       throw new ScmConfigurationPersistenceException(e.getMessage(), e);
     }
-    return Optional.empty();
+    return result;
   }
 
   /**
@@ -351,6 +354,29 @@ public void forceRefreshPersonalAccessToken(String scmServerUrl)
         scmPersonalAccessTokenFetcher.refreshPersonalAccessToken(subject, scmServerUrl);
     gitCredentialManager.createOrReplace(personalAccessToken);
     store(personalAccessToken);
+    removePreviousTokensIfPresent(subject, scmServerUrl);
+  }
+
+  private void removePreviousTokensIfPresent(Subject subject, String scmServerUrl)
+      throws ScmConfigurationPersistenceException, UnsatisfiedScmPreconditionException {
+    List<PersonalAccessToken> personalAccessTokens =
+        doGetPersonalAccessTokens(subject, null, scmServerUrl);
+    for (int i = 1; i < personalAccessTokens.size(); i++) {
+      PersonalAccessToken token = personalAccessTokens.get(i);
+      if (token.getScmProviderUrl().equals(scmServerUrl)) {
+        try {
+          String namespace = getFirstNamespace();
+          cheServerKubernetesClientFactory
+              .create()
+              .secrets()
+              .inNamespace(namespace)
+              .withName(token.getScmTokenId())
+              .delete();
+        } catch (InfrastructureException e) {
+          throw new ScmConfigurationPersistenceException(e.getMessage(), e);
+        }
+      }
+    }
   }
 
   @Override

diff --git a/...lipse/che/api/factory/server/scm/kubernetes/KubernetesPersonalAccessTokenManagerTest.java b/...lipse/che/api/factory/server/scm/kubernetes/KubernetesPersonalAccessTokenManagerTest.java
@@ -464,7 +464,7 @@ public void shouldDeleteInvalidTokensOnGet() throws Exception {
   }
 
   @Test(dependsOnMethods = "shouldDeleteInvalidTokensOnGet")
-  public void shouldReturnFirstValidToken() throws Exception {
+  public void shouldReturnFirstValidTokenAndDeleteTheOlderOne() throws Exception {
     // given
     KubernetesNamespaceMeta meta = new KubernetesNamespaceMetaImpl("test");
     when(namespaceFactory.list()).thenReturn(singletonList(meta));
@@ -481,10 +481,10 @@ public void shouldReturnFirstValidToken() throws Exception {
                       ? Optional.of("user")
                       : Optional.empty();
                 });
-    //    when(cheServerKubernetesClientFactory.create()).thenReturn(kubeClient);
-    //    when(kubeClient.secrets()).thenReturn(secretsMixedOperation);
-    //
-    // when(secretsMixedOperation.inNamespace(eq(meta.getName()))).thenReturn(nonNamespaceOperation);
+    when(cheServerKubernetesClientFactory.create()).thenReturn(kubeClient);
+    when(kubeClient.secrets()).thenReturn(secretsMixedOperation);
+
+    when(secretsMixedOperation.inNamespace(eq(meta.getName()))).thenReturn(nonNamespaceOperation);
     Map<String, String> data1 =
         Map.of("token", Base64.getEncoder().encodeToString("token1".getBytes(UTF_8)));
     Map<String, String> data2 =
@@ -527,5 +527,6 @@ public void shouldReturnFirstValidToken() throws Exception {
     // then
     assertTrue(token.isPresent());
     assertEquals(token.get().getScmTokenId(), "id2");
+    verify(nonNamespaceOperation, times(1)).delete(eq(secret1));
   }
 }