Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Omit scm-username annotation from the PAT secret #533

Merged
merged 12 commits into from
Jul 25, 2023
Merged

Omit scm-username annotation from the PAT secret #533

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
12 commits
Select commit Hold shift + click to select a range
1657ebb
Omit scm-username annotation from the PAT secret
vinokurig Jul 19, 2023
d4f2698
fixup! Omit scm-username annotation from the PAT secret
vinokurig Jul 20, 2023
6a1daa4
fixup! Omit scm-username annotation from the PAT secret
vinokurig Jul 20, 2023
ea736d1
fixup! Omit scm-username annotation from the PAT secret
vinokurig Jul 20, 2023
75bf468
fixup! Omit scm-username annotation from the PAT secret
vinokurig Jul 21, 2023
3ea02a0
fixup! Omit scm-username annotation from the PAT secret
vinokurig Jul 25, 2023
1edc87c
fixup! Omit scm-username annotation from the PAT secret
vinokurig Jul 25, 2023
9c245e1
fixup! Omit scm-username annotation from the PAT secret
vinokurig Jul 25, 2023
640a76a
fixup! Omit scm-username annotation from the PAT secret
vinokurig Jul 25, 2023
22a034d
Update PAT_info.json
vinokurig Jul 25, 2023
2dc2f42
fixup! Omit scm-username annotation from the PAT secret
vinokurig Jul 25, 2023
df64e40
fixup! Omit scm-username annotation from the PAT secret
vinokurig Jul 25, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 1 addition & 6 deletions ...ava/org/eclipse/che/api/factory/server/scm/kubernetes/KubernetesGitCredentialManager.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -126,19 +126,14 @@ public void createOrReplace(PersonalAccessToken personalAccessToken)
'/'))
&& personalAccessToken
.getCheUserId()
.equals(s.getMetadata().getAnnotations().get(ANNOTATION_CHE_USERID))
&& personalAccessToken
.getScmUserName()
.equals(
s.getMetadata().getAnnotations().get(ANNOTATION_SCM_USERNAME)))
.equals(s.getMetadata().getAnnotations().get(ANNOTATION_CHE_USERID)))
.findFirst();

Secret secret =
existing.orElseGet(
() -> {
Map<String, String> annotations = new HashMap<>(DEFAULT_SECRET_ANNOTATIONS);
annotations.put(ANNOTATION_SCM_URL, personalAccessToken.getScmProviderUrl());
annotations.put(ANNOTATION_SCM_USERNAME, personalAccessToken.getScmUserName());
annotations.put(ANNOTATION_CHE_USERID, personalAccessToken.getCheUserId());
ObjectMeta meta =
new ObjectMetaBuilder()
Expand Down
51 changes: 29 additions & 22 deletions ...g/eclipse/che/api/factory/server/scm/kubernetes/KubernetesPersonalAccessTokenManager.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,8 @@
*/
package org.eclipse.che.api.factory.server.scm.kubernetes;

import static com.google.common.base.Strings.isNullOrEmpty;

import com.google.common.annotations.VisibleForTesting;
import com.google.common.collect.ImmutableMap;
import io.fabric8.kubernetes.api.model.LabelSelector;
Expand All @@ -30,6 +32,7 @@
import org.eclipse.che.api.factory.server.scm.GitCredentialManager;
import org.eclipse.che.api.factory.server.scm.PersonalAccessToken;
import org.eclipse.che.api.factory.server.scm.PersonalAccessTokenManager;
import org.eclipse.che.api.factory.server.scm.PersonalAccessTokenParams;
import org.eclipse.che.api.factory.server.scm.ScmPersonalAccessTokenFetcher;
import org.eclipse.che.api.factory.server.scm.exception.ScmCommunicationException;
import org.eclipse.che.api.factory.server.scm.exception.ScmConfigurationPersistenceException;
Expand Down Expand Up @@ -59,7 +62,6 @@ public class KubernetesPersonalAccessTokenManager implements PersonalAccessToken
public static final String NAME_PATTERN = "personal-access-token-";

public static final String ANNOTATION_CHE_USERID = "che.eclipse.org/che-userid";
public static final String ANNOTATION_SCM_USERNAME = "che.eclipse.org/scm-username";
public static final String ANNOTATION_SCM_ORGANIZATION = "che.eclipse.org/scm-organization";
public static final String ANNOTATION_SCM_PERSONAL_ACCESS_TOKEN_ID =
"che.eclipse.org/scm-personal-access-token-id";
Expand Down Expand Up @@ -96,7 +98,6 @@ void save(PersonalAccessToken personalAccessToken)
.withAnnotations(
new ImmutableMap.Builder<String, String>()
.put(ANNOTATION_CHE_USERID, personalAccessToken.getCheUserId())
.put(ANNOTATION_SCM_USERNAME, personalAccessToken.getScmUserName())
.put(ANNOTATION_SCM_URL, personalAccessToken.getScmProviderUrl())
.put(
ANNOTATION_SCM_PERSONAL_ACCESS_TOKEN_ID,
Expand Down Expand Up @@ -191,28 +192,34 @@ private Optional<PersonalAccessToken> doGetPersonalAccessToken(
|| trimmedUrl.equals(StringUtils.trimEnd(scmServerUrl, '/')))) {
String token =
new String(Base64.getDecoder().decode(secret.getData().get("token"))).trim();
PersonalAccessToken personalAccessToken =
new PersonalAccessToken(
trimmedUrl,
annotations.get(ANNOTATION_CHE_USERID),
annotations.get(ANNOTATION_SCM_ORGANIZATION),
annotations.get(ANNOTATION_SCM_USERNAME),
annotations.get(ANNOTATION_SCM_PERSONAL_ACCESS_TOKEN_NAME),
annotations.get(ANNOTATION_SCM_PERSONAL_ACCESS_TOKEN_ID),
token);
if (scmPersonalAccessTokenFetcher.isValid(personalAccessToken)) {
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);
String scmUsername =
scmPersonalAccessTokenFetcher.isValid(
new PersonalAccessTokenParams(
trimmedUrl, providerName, tokenId, token, organization));
if (!isNullOrEmpty(scmUsername)) {
PersonalAccessToken personalAccessToken =
new PersonalAccessToken(
trimmedUrl,
annotations.get(ANNOTATION_CHE_USERID),
organization,
scmUsername,
providerName,
tokenId,
token);
return Optional.of(personalAccessToken);
} else {
// 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
// retrieval would be initiated.
cheServerKubernetesClientFactory
.create()
.secrets()
.inNamespace(namespaceMeta.getName())
.delete(secret);
}
// 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
// retrieval would be initiated.
cheServerKubernetesClientFactory
.create()
.secrets()
.inNamespace(namespaceMeta.getName())
.delete(secret);
}
}
}
Expand Down
3 changes: 0 additions & 3 deletions ...org/eclipse/che/api/factory/server/scm/kubernetes/KubernetesGitCredentialManagerTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,6 @@
import static java.util.Collections.singletonList;
import static org.eclipse.che.api.factory.server.scm.kubernetes.KubernetesGitCredentialManager.ANNOTATION_CHE_USERID;
import static org.eclipse.che.api.factory.server.scm.kubernetes.KubernetesGitCredentialManager.ANNOTATION_SCM_URL;
import static org.eclipse.che.api.factory.server.scm.kubernetes.KubernetesGitCredentialManager.ANNOTATION_SCM_USERNAME;
import static org.eclipse.che.api.factory.server.scm.kubernetes.KubernetesGitCredentialManager.DEFAULT_SECRET_ANNOTATIONS;
import static org.eclipse.che.api.factory.server.scm.kubernetes.KubernetesGitCredentialManager.NAME_PATTERN;
import static org.mockito.ArgumentMatchers.anyMap;
Expand Down Expand Up @@ -125,7 +124,6 @@ public void shouldUseHardcodedUsernameIfScmOrganizationIsDefined() throws Except
Map<String, String> annotations = new HashMap<>(DEFAULT_SECRET_ANNOTATIONS);

annotations.put(ANNOTATION_SCM_URL, token.getScmProviderUrl() + "/");
annotations.put(ANNOTATION_SCM_USERNAME, token.getScmUserName());
annotations.put(ANNOTATION_CHE_USERID, token.getCheUserId());
ObjectMeta objectMeta =
new ObjectMetaBuilder()
Expand Down Expand Up @@ -210,7 +208,6 @@ public void testUpdateTokenInExistingCredential() throws Exception {
Map<String, String> annotations = new HashMap<>(DEFAULT_SECRET_ANNOTATIONS);

annotations.put(ANNOTATION_SCM_URL, token.getScmProviderUrl() + "/");
annotations.put(ANNOTATION_SCM_USERNAME, token.getScmUserName());
annotations.put(ANNOTATION_CHE_USERID, token.getCheUserId());
ObjectMeta objectMeta =
new ObjectMetaBuilder()
Expand Down
115 changes: 100 additions & 15 deletions ...lipse/che/api/factory/server/scm/kubernetes/KubernetesPersonalAccessTokenManagerTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,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;
Expand All @@ -37,12 +38,12 @@
import io.fabric8.kubernetes.client.dsl.Resource;
import java.util.Arrays;
import java.util.Base64;
import java.util.Collections;
import java.util.List;
import java.util.Map;
import java.util.Optional;
import org.eclipse.che.api.factory.server.scm.GitCredentialManager;
import org.eclipse.che.api.factory.server.scm.PersonalAccessToken;
import org.eclipse.che.api.factory.server.scm.PersonalAccessTokenParams;
import org.eclipse.che.api.factory.server.scm.ScmPersonalAccessTokenFetcher;
import org.eclipse.che.commons.subject.SubjectImpl;
import org.eclipse.che.workspace.infrastructure.kubernetes.CheServerKubernetesClientFactory;
Expand Down Expand Up @@ -90,12 +91,13 @@ protected void init() {
@Test
public void shouldTrimBlankCharsInToken() throws Exception {
KubernetesNamespaceMeta meta = new KubernetesNamespaceMetaImpl("test");
when(namespaceFactory.list()).thenReturn(Collections.singletonList(meta));
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(scmPersonalAccessTokenFetcher.isValid(any(PersonalAccessToken.class))).thenReturn(true);
when(scmPersonalAccessTokenFetcher.isValid(any(PersonalAccessTokenParams.class)))
.thenReturn("user");

Map<String, String> data =
Map.of("token", Base64.getEncoder().encodeToString(" token_value \n".getBytes(UTF_8)));
Expand Down Expand Up @@ -124,7 +126,7 @@ public void shouldTrimBlankCharsInToken() throws Exception {
public void testSavingOfPersonalAccessToken() throws Exception {

KubernetesNamespaceMeta meta = new KubernetesNamespaceMetaImpl("test");
when(namespaceFactory.list()).thenReturn(Collections.singletonList(meta));
when(namespaceFactory.list()).thenReturn(singletonList(meta));

when(cheServerKubernetesClientFactory.create()).thenReturn(kubeClient);
when(kubeClient.secrets()).thenReturn(secretsMixedOperation);
Expand Down Expand Up @@ -156,12 +158,13 @@ public void testSavingOfPersonalAccessToken() throws Exception {
public void testGetTokenFromNamespace() throws Exception {

KubernetesNamespaceMeta meta = new KubernetesNamespaceMetaImpl("test");
when(namespaceFactory.list()).thenReturn(Collections.singletonList(meta));
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(scmPersonalAccessTokenFetcher.isValid(any(PersonalAccessToken.class))).thenReturn(true);
when(scmPersonalAccessTokenFetcher.isValid(any(PersonalAccessTokenParams.class)))
.thenReturn("user");

Map<String, String> data1 =
Map.of("token", Base64.getEncoder().encodeToString("token1".getBytes(UTF_8)));
Expand Down Expand Up @@ -205,16 +208,97 @@ public void testGetTokenFromNamespace() throws Exception {
assertEquals(token.getToken(), "token1");
}

@Test
public void shouldGetTokenFromASecretWithSCMUsername() throws Exception {

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(scmPersonalAccessTokenFetcher.isValid(any(PersonalAccessTokenParams.class)))
.thenReturn("user");

Map<String, String> data =
Map.of("token", Base64.getEncoder().encodeToString("token1".getBytes(UTF_8)));

ObjectMeta metaData =
new ObjectMetaBuilder()
.withAnnotations(
Map.of(
ANNOTATION_CHE_USERID,
"user1",
ANNOTATION_SCM_URL,
"http://host1",
"che.eclipse.org/scm-username",
"scm-username"))
.build();

Secret secret = new SecretBuilder().withMetadata(metaData).withData(data).build();

when(secrets.get(any(LabelSelector.class))).thenReturn(singletonList(secret));

// when
Optional<PersonalAccessToken> tokenOptional =
personalAccessTokenManager.get(
new SubjectImpl("user", "user1", "t1", false), "http://host1");

// then
assertTrue(tokenOptional.isPresent());
assertEquals(tokenOptional.get().getCheUserId(), "user1");
assertEquals(tokenOptional.get().getScmProviderUrl(), "http://host1");
assertEquals(tokenOptional.get().getToken(), "token1");
}

@Test
public void shouldGetTokenFromASecretWithoutSCMUsername() throws Exception {

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(scmPersonalAccessTokenFetcher.isValid(any(PersonalAccessTokenParams.class)))
.thenReturn("user");

Map<String, String> data =
Map.of("token", Base64.getEncoder().encodeToString("token1".getBytes(UTF_8)));

ObjectMeta metaData =
new ObjectMetaBuilder()
.withAnnotations(
Map.of(ANNOTATION_CHE_USERID, "user1", ANNOTATION_SCM_URL, "http://host1"))
.build();

Secret secret = new SecretBuilder().withMetadata(metaData).withData(data).build();

when(secrets.get(any(LabelSelector.class))).thenReturn(singletonList(secret));

// when
Optional<PersonalAccessToken> tokenOptional =
personalAccessTokenManager.get(
new SubjectImpl("user", "user1", "t1", false), "http://host1");

// then
assertTrue(tokenOptional.isPresent());
assertEquals(tokenOptional.get().getCheUserId(), "user1");
assertEquals(tokenOptional.get().getScmProviderUrl(), "http://host1");
assertEquals(tokenOptional.get().getToken(), "token1");
}

@Test
public void testGetTokenFromNamespaceWithTrailingSlashMismatch() throws Exception {

KubernetesNamespaceMeta meta = new KubernetesNamespaceMetaImpl("test");
when(namespaceFactory.list()).thenReturn(Collections.singletonList(meta));
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(scmPersonalAccessTokenFetcher.isValid(any(PersonalAccessToken.class))).thenReturn(true);
when(scmPersonalAccessTokenFetcher.isValid(any(PersonalAccessTokenParams.class)))
.thenReturn("user");

Map<String, String> data1 =
Map.of("token", Base64.getEncoder().encodeToString("token1".getBytes(UTF_8)));
Expand Down Expand Up @@ -256,12 +340,13 @@ public void testGetTokenFromNamespaceWithTrailingSlashMismatch() throws Exceptio
public void shouldDeleteInvalidTokensOnGet() throws Exception {
// given
KubernetesNamespaceMeta meta = new KubernetesNamespaceMetaImpl("test");
when(namespaceFactory.list()).thenReturn(Collections.singletonList(meta));
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(scmPersonalAccessTokenFetcher.isValid(any(PersonalAccessToken.class))).thenReturn(false);
when(scmPersonalAccessTokenFetcher.isValid(any(PersonalAccessTokenParams.class)))
.thenReturn(null);
when(cheServerKubernetesClientFactory.create()).thenReturn(kubeClient);
when(kubeClient.secrets()).thenReturn(secretsMixedOperation);
when(secretsMixedOperation.inNamespace(eq(meta.getName()))).thenReturn(nonNamespaceOperation);
Expand All @@ -287,17 +372,17 @@ public void shouldDeleteInvalidTokensOnGet() throws Exception {
public void shouldReturnFirstValidToken() throws Exception {
// given
KubernetesNamespaceMeta meta = new KubernetesNamespaceMetaImpl("test");
when(namespaceFactory.list()).thenReturn(Collections.singletonList(meta));
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(scmPersonalAccessTokenFetcher.isValid(any(PersonalAccessToken.class)))
when(scmPersonalAccessTokenFetcher.isValid(any(PersonalAccessTokenParams.class)))
.thenAnswer(
(Answer<Boolean>)
(Answer<String>)
invocation -> {
PersonalAccessToken token = invocation.getArgument(0);
return "id2".equals(token.getScmTokenId());
PersonalAccessTokenParams params = invocation.getArgument(0);
return "id2".equals(params.getScmTokenId()) ? "user" : null;
});
when(cheServerKubernetesClientFactory.create()).thenReturn(kubeClient);
when(kubeClient.secrets()).thenReturn(secretsMixedOperation);
Expand Down
Loading