From 8d0d9ac5d82350901d77c2ff00f60abc78833d2e Mon Sep 17 00:00:00 2001 From: Igor Vinokur Date: Wed, 19 Apr 2023 09:37:29 +0300 Subject: [PATCH] Withdraw scm-userid value in PAT secrets and use scm-username instead (#496) Withdraw the scm-userid annotation from the PAT secret structure. Rework the PAT validation to check scm-username instead scm-userid. --- .../KubernetesPersonalAccessTokenManager.java | 3 --- .../KubernetesGitCredentialManagerTest.java | 12 ++-------- ...ernetesPersonalAccessTokenManagerTest.java | 8 +------ ...AzureDevOpsPersonalAccessTokenFetcher.java | 3 +-- ...eDevOpsPersonalAccessTokenFetcherTest.java | 2 +- .../BitbucketPersonalAccessTokenFetcher.java | 1 - ...tbucketPersonalAccessTokenFetcherTest.java | 4 ---- .../GithubPersonalAccessTokenFetcher.java | 3 +-- .../GithubPersonalAccessTokenFetcherTest.java | 6 +---- .../gitlab/GitlabOAuthTokenFetcher.java | 3 +-- .../gitlab/GitlabOAuthTokenFetcherTest.java | 5 ++-- .../server/scm/PersonalAccessToken.java | 24 ++----------------- 12 files changed, 12 insertions(+), 62 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 2175d19b2b..dca15f0951 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 @@ -59,7 +59,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_USERID = "che.eclipse.org/scm-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 = @@ -97,7 +96,6 @@ public class KubernetesPersonalAccessTokenManager implements PersonalAccessToken .withAnnotations( new ImmutableMap.Builder() .put(ANNOTATION_CHE_USERID, personalAccessToken.getCheUserId()) - .put(ANNOTATION_SCM_USERID, personalAccessToken.getScmUserId()) .put(ANNOTATION_SCM_USERNAME, personalAccessToken.getScmUserName()) .put(ANNOTATION_SCM_URL, personalAccessToken.getScmProviderUrl()) .put( @@ -180,7 +178,6 @@ public class KubernetesPersonalAccessTokenManager implements PersonalAccessToken annotations.get(ANNOTATION_CHE_USERID), annotations.get(ANNOTATION_SCM_ORGANIZATION), annotations.get(ANNOTATION_SCM_USERNAME), - annotations.get(ANNOTATION_SCM_USERID), annotations.get(ANNOTATION_SCM_PERSONAL_ACCESS_TOKEN_NAME), annotations.get(ANNOTATION_SCM_PERSONAL_ACCESS_TOKEN_ID), token); diff --git a/infrastructures/infrastructure-factory/src/test/java/org/eclipse/che/api/factory/server/scm/kubernetes/KubernetesGitCredentialManagerTest.java b/infrastructures/infrastructure-factory/src/test/java/org/eclipse/che/api/factory/server/scm/kubernetes/KubernetesGitCredentialManagerTest.java index eb6d39a681..3c8d6b1e78 100644 --- a/infrastructures/infrastructure-factory/src/test/java/org/eclipse/che/api/factory/server/scm/kubernetes/KubernetesGitCredentialManagerTest.java +++ b/infrastructures/infrastructure-factory/src/test/java/org/eclipse/che/api/factory/server/scm/kubernetes/KubernetesGitCredentialManagerTest.java @@ -1,5 +1,5 @@ /* - * Copyright (c) 2012-2021 Red Hat, Inc. + * Copyright (c) 2012-2023 Red Hat, Inc. * This program and the accompanying materials are made * available under the terms of the Eclipse Public License 2.0 * which is available at https://www.eclipse.org/legal/epl-2.0/ @@ -93,13 +93,7 @@ public class KubernetesGitCredentialManagerTest { PersonalAccessToken token = new PersonalAccessToken( - "https://bitbucket.com", - "cheUser", - "username", - "userId", - "token-name", - "tid-23434", - "token123"); + "https://bitbucket.com", "cheUser", "username", "token-name", "tid-23434", "token123"); // when kubernetesGitCredentialManager.createOrReplace(token); @@ -132,7 +126,6 @@ public class KubernetesGitCredentialManagerTest { "https://bitbucket.com", "cheUser", "username", - "userId", "oauth2-token-name", "tid-23434", "token123"); @@ -158,7 +151,6 @@ public class KubernetesGitCredentialManagerTest { "https://bitbucket.com:5648", "cheUser", "username", - "userId", "token-name", "tid-23434", "token123"); 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 9457c1e08c..f24bdc5288 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 @@ -130,13 +130,7 @@ public class KubernetesPersonalAccessTokenManagerTest { PersonalAccessToken token = new PersonalAccessToken( - "https://bitbucket.com", - "cheUser", - "username", - "userId", - "token-name", - "tid-24", - "token123"); + "https://bitbucket.com", "cheUser", "username", "token-name", "tid-24", "token123"); // when personalAccessTokenManager.save(token); diff --git a/wsmaster/che-core-api-factory-azure-devops/src/main/java/org/eclipse/che/api/factory/server/azure/devops/AzureDevOpsPersonalAccessTokenFetcher.java b/wsmaster/che-core-api-factory-azure-devops/src/main/java/org/eclipse/che/api/factory/server/azure/devops/AzureDevOpsPersonalAccessTokenFetcher.java index 8d19308d95..b028ff77e0 100644 --- a/wsmaster/che-core-api-factory-azure-devops/src/main/java/org/eclipse/che/api/factory/server/azure/devops/AzureDevOpsPersonalAccessTokenFetcher.java +++ b/wsmaster/che-core-api-factory-azure-devops/src/main/java/org/eclipse/che/api/factory/server/azure/devops/AzureDevOpsPersonalAccessTokenFetcher.java @@ -87,7 +87,6 @@ public class AzureDevOpsPersonalAccessTokenFetcher implements PersonalAccessToke scmServerUrl, cheSubject.getUserId(), user.getEmailAddress(), - user.getId(), NameGenerator.generate(OAUTH_2_PREFIX, 5), NameGenerator.generate("id-", 5), oAuthToken.getToken()); @@ -144,7 +143,7 @@ public class AzureDevOpsPersonalAccessTokenFetcher implements PersonalAccessToke azureDevOpsApiClient.getUserWithPAT( personalAccessToken.getToken(), personalAccessToken.getScmOrganization()); } - return Optional.of(personalAccessToken.getScmUserId().equals(user.getId())); + return Optional.of(personalAccessToken.getScmUserName().equals(user.getEmailAddress())); } catch (ScmItemNotFoundException | ScmCommunicationException | ScmBadRequestException e) { return Optional.of(Boolean.FALSE); } diff --git a/wsmaster/che-core-api-factory-azure-devops/src/test/java/org/eclipse/che/api/factory/server/azure/devops/AzureDevOpsPersonalAccessTokenFetcherTest.java b/wsmaster/che-core-api-factory-azure-devops/src/test/java/org/eclipse/che/api/factory/server/azure/devops/AzureDevOpsPersonalAccessTokenFetcherTest.java index aac1a6e44a..5068f24504 100644 --- a/wsmaster/che-core-api-factory-azure-devops/src/test/java/org/eclipse/che/api/factory/server/azure/devops/AzureDevOpsPersonalAccessTokenFetcherTest.java +++ b/wsmaster/che-core-api-factory-azure-devops/src/test/java/org/eclipse/che/api/factory/server/azure/devops/AzureDevOpsPersonalAccessTokenFetcherTest.java @@ -57,7 +57,7 @@ public class AzureDevOpsPersonalAccessTokenFetcherTest { public void fetchPersonalAccessTokenShouldReturnToken() throws Exception { when(oAuthAPI.getToken(AzureDevOps.PROVIDER_NAME)).thenReturn(oAuthToken); when(azureDevOpsApiClient.getUserWithOAuthToken(any())).thenReturn(azureDevOpsUser); - when(azureDevOpsUser.getId()).thenReturn("user-id"); + when(azureDevOpsUser.getEmailAddress()).thenReturn("user-email"); PersonalAccessToken personalAccessToken = personalAccessTokenFetcher.fetchPersonalAccessToken( diff --git a/wsmaster/che-core-api-factory-bitbucket/src/main/java/org/eclipse/che/api/factory/server/bitbucket/BitbucketPersonalAccessTokenFetcher.java b/wsmaster/che-core-api-factory-bitbucket/src/main/java/org/eclipse/che/api/factory/server/bitbucket/BitbucketPersonalAccessTokenFetcher.java index fa7a1687a6..046bce6a7d 100644 --- a/wsmaster/che-core-api-factory-bitbucket/src/main/java/org/eclipse/che/api/factory/server/bitbucket/BitbucketPersonalAccessTokenFetcher.java +++ b/wsmaster/che-core-api-factory-bitbucket/src/main/java/org/eclipse/che/api/factory/server/bitbucket/BitbucketPersonalAccessTokenFetcher.java @@ -90,7 +90,6 @@ public class BitbucketPersonalAccessTokenFetcher implements PersonalAccessTokenF scmServerUrl, cheSubject.getUserId(), user.getName(), - user.getId(), NameGenerator.generate(OAUTH_PROVIDER_NAME, 5), NameGenerator.generate("id-", 5), oAuthToken.getToken()); diff --git a/wsmaster/che-core-api-factory-bitbucket/src/test/java/org/eclipse/che/api/factory/server/bitbucket/BitbucketPersonalAccessTokenFetcherTest.java b/wsmaster/che-core-api-factory-bitbucket/src/test/java/org/eclipse/che/api/factory/server/bitbucket/BitbucketPersonalAccessTokenFetcherTest.java index 81b5b815be..8cd3a1b382 100644 --- a/wsmaster/che-core-api-factory-bitbucket/src/test/java/org/eclipse/che/api/factory/server/bitbucket/BitbucketPersonalAccessTokenFetcherTest.java +++ b/wsmaster/che-core-api-factory-bitbucket/src/test/java/org/eclipse/che/api/factory/server/bitbucket/BitbucketPersonalAccessTokenFetcherTest.java @@ -87,7 +87,6 @@ public class BitbucketPersonalAccessTokenFetcherTest { "https://bitbucket.org/", "cheUserId", "scmUserName", - "scmUserId", "scmTokenName", "scmTokenId", bitbucketOauthToken); @@ -169,7 +168,6 @@ public class BitbucketPersonalAccessTokenFetcherTest { "https://bitbucket.org", "cheUser", "username", - "123456789", "token-name", "tid-23434", bitbucketOauthToken); @@ -194,7 +192,6 @@ public class BitbucketPersonalAccessTokenFetcherTest { "https://bitbucket.org", "cheUser", "username", - "123456789", OAUTH_2_PREFIX + "-token-name", "tid-23434", bitbucketOauthToken); @@ -211,7 +208,6 @@ public class BitbucketPersonalAccessTokenFetcherTest { "https://bitbucket.org", "cheUser", "username", - "123456789", OAUTH_2_PREFIX + "-token-name", "tid-23434", bitbucketOauthToken); diff --git a/wsmaster/che-core-api-factory-github/src/main/java/org/eclipse/che/api/factory/server/github/GithubPersonalAccessTokenFetcher.java b/wsmaster/che-core-api-factory-github/src/main/java/org/eclipse/che/api/factory/server/github/GithubPersonalAccessTokenFetcher.java index defb1d98b6..a5dd1ba535 100644 --- a/wsmaster/che-core-api-factory-github/src/main/java/org/eclipse/che/api/factory/server/github/GithubPersonalAccessTokenFetcher.java +++ b/wsmaster/che-core-api-factory-github/src/main/java/org/eclipse/che/api/factory/server/github/GithubPersonalAccessTokenFetcher.java @@ -157,7 +157,6 @@ public class GithubPersonalAccessTokenFetcher implements PersonalAccessTokenFetc scmServerUrl, cheSubject.getUserId(), user.getLogin(), - Long.toString(user.getId()), NameGenerator.generate(OAUTH_2_PREFIX, 5), NameGenerator.generate("id-", 5), oAuthToken.getToken()); @@ -212,7 +211,7 @@ public class GithubPersonalAccessTokenFetcher implements PersonalAccessTokenFetc } else { // No REST API for PAT-s in Github found yet. Just try to do some action. GithubUser user = githubApiClient.getUser(personalAccessToken.getToken()); - if (personalAccessToken.getScmUserId().equals(Long.toString(user.getId()))) { + if (personalAccessToken.getScmUserName().equals(user.getLogin())) { return Optional.of(Boolean.TRUE); } else { return Optional.of(Boolean.FALSE); diff --git a/wsmaster/che-core-api-factory-github/src/test/java/org/eclipse/che/api/factory/server/github/GithubPersonalAccessTokenFetcherTest.java b/wsmaster/che-core-api-factory-github/src/test/java/org/eclipse/che/api/factory/server/github/GithubPersonalAccessTokenFetcherTest.java index b2d6166ac7..9084bc40d2 100644 --- a/wsmaster/che-core-api-factory-github/src/test/java/org/eclipse/che/api/factory/server/github/GithubPersonalAccessTokenFetcherTest.java +++ b/wsmaster/che-core-api-factory-github/src/test/java/org/eclipse/che/api/factory/server/github/GithubPersonalAccessTokenFetcherTest.java @@ -91,7 +91,6 @@ public class GithubPersonalAccessTokenFetcherTest { "https://github.com/", "cheUserId", "scmUserName", - "scmUserId", "scmTokenName", "scmTokenId", githubOauthToken); @@ -203,8 +202,7 @@ public class GithubPersonalAccessTokenFetcherTest { new PersonalAccessToken( wireMockServer.url("/"), "cheUser", - "username", - "123456789", + "github-user", "token-name", "tid-23434", githubOauthToken); @@ -230,7 +228,6 @@ public class GithubPersonalAccessTokenFetcherTest { wireMockServer.url("/"), "cheUser", "username", - "123456789", OAUTH_2_PREFIX + "-token-name", "tid-23434", githubOauthToken); @@ -247,7 +244,6 @@ public class GithubPersonalAccessTokenFetcherTest { wireMockServer.url("/"), "cheUser", "username", - "123456789", OAUTH_2_PREFIX + "-token-name", "tid-23434", githubOauthToken); diff --git a/wsmaster/che-core-api-factory-gitlab/src/main/java/org/eclipse/che/api/factory/server/gitlab/GitlabOAuthTokenFetcher.java b/wsmaster/che-core-api-factory-gitlab/src/main/java/org/eclipse/che/api/factory/server/gitlab/GitlabOAuthTokenFetcher.java index bfbdf42322..091146a7bd 100644 --- a/wsmaster/che-core-api-factory-gitlab/src/main/java/org/eclipse/che/api/factory/server/gitlab/GitlabOAuthTokenFetcher.java +++ b/wsmaster/che-core-api-factory-gitlab/src/main/java/org/eclipse/che/api/factory/server/gitlab/GitlabOAuthTokenFetcher.java @@ -111,7 +111,6 @@ public class GitlabOAuthTokenFetcher implements PersonalAccessTokenFetcher { scmServerUrl, cheSubject.getUserId(), user.getUsername(), - Long.toString(user.getId()), NameGenerator.generate(OAUTH_2_PREFIX, 5), NameGenerator.generate("id-", 5), oAuthToken.getToken()); @@ -171,7 +170,7 @@ public class GitlabOAuthTokenFetcher implements PersonalAccessTokenFetcher { // latest GitLab version, we just perform check by accessing something from API. try { GitlabUser user = gitlabApiClient.getUser(personalAccessToken.getToken()); - if (personalAccessToken.getScmUserId().equals(Long.toString(user.getId()))) { + if (personalAccessToken.getScmUserName().equals(user.getUsername())) { return Optional.of(Boolean.TRUE); } else { return Optional.of(Boolean.FALSE); diff --git a/wsmaster/che-core-api-factory-gitlab/src/test/java/org/eclipse/che/api/factory/server/gitlab/GitlabOAuthTokenFetcherTest.java b/wsmaster/che-core-api-factory-gitlab/src/test/java/org/eclipse/che/api/factory/server/gitlab/GitlabOAuthTokenFetcherTest.java index c99ec8eff7..b61650c490 100644 --- a/wsmaster/che-core-api-factory-gitlab/src/test/java/org/eclipse/che/api/factory/server/gitlab/GitlabOAuthTokenFetcherTest.java +++ b/wsmaster/che-core-api-factory-gitlab/src/test/java/org/eclipse/che/api/factory/server/gitlab/GitlabOAuthTokenFetcherTest.java @@ -1,5 +1,5 @@ /* - * Copyright (c) 2012-2021 Red Hat, Inc. + * Copyright (c) 2012-2023 Red Hat, Inc. * This program and the accompanying materials are made * available under the terms of the Eclipse Public License 2.0 * which is available at https://www.eclipse.org/legal/epl-2.0/ @@ -170,8 +170,7 @@ public class GitlabOAuthTokenFetcherTest { new PersonalAccessToken( wireMockServer.baseUrl(), "cheUser", - "username", - "1", + "john_smith", "token-name", "tid-23434", "token123"); diff --git a/wsmaster/che-core-api-factory/src/main/java/org/eclipse/che/api/factory/server/scm/PersonalAccessToken.java b/wsmaster/che-core-api-factory/src/main/java/org/eclipse/che/api/factory/server/scm/PersonalAccessToken.java index 0d9654703e..acc065d686 100644 --- a/wsmaster/che-core-api-factory/src/main/java/org/eclipse/che/api/factory/server/scm/PersonalAccessToken.java +++ b/wsmaster/che-core-api-factory/src/main/java/org/eclipse/che/api/factory/server/scm/PersonalAccessToken.java @@ -26,7 +26,6 @@ public class PersonalAccessToken { /** Organization that user belongs to. Can be null if user is not a member of any organization. */ @Nullable private final String scmOrganization; - private final String scmUserId; private final String scmTokenName; private final String scmTokenId; private final String token; @@ -37,14 +36,12 @@ public class PersonalAccessToken { String cheUserId, String scmOrganization, String scmUserName, - String scmUserId, String scmTokenName, String scmTokenId, String token) { this.scmProviderUrl = scmProviderUrl; this.scmOrganization = scmOrganization; this.scmUserName = scmUserName; - this.scmUserId = scmUserId; this.scmTokenName = scmTokenName; this.scmTokenId = scmTokenId; this.token = token; @@ -55,11 +52,10 @@ public class PersonalAccessToken { String scmProviderUrl, String cheUserId, String scmUserName, - String scmUserId, String scmTokenName, String scmTokenId, String token) { - this(scmProviderUrl, cheUserId, null, scmUserName, scmUserId, scmTokenName, scmTokenId, token); + this(scmProviderUrl, cheUserId, null, scmUserName, scmTokenName, scmTokenId, token); } public PersonalAccessToken(String scmProviderUrl, String scmUserName, String token) { @@ -70,7 +66,6 @@ public class PersonalAccessToken { scmUserName, null, null, - null, token); } @@ -90,10 +85,6 @@ public class PersonalAccessToken { return scmUserName; } - public String getScmUserId() { - return scmUserId; - } - public String getToken() { return token; } @@ -115,7 +106,6 @@ public class PersonalAccessToken { return Objects.equal(scmProviderUrl, that.scmProviderUrl) && Objects.equal(scmUserName, that.scmUserName) && Objects.equal(scmOrganization, that.scmOrganization) - && Objects.equal(scmUserId, that.scmUserId) && Objects.equal(scmTokenName, that.scmTokenName) && Objects.equal(scmTokenId, that.scmTokenId) && Objects.equal(token, that.token) @@ -125,14 +115,7 @@ public class PersonalAccessToken { @Override public int hashCode() { return Objects.hashCode( - scmProviderUrl, - scmUserName, - scmOrganization, - scmUserId, - scmTokenName, - scmTokenId, - token, - cheUserId); + scmProviderUrl, scmUserName, scmOrganization, scmTokenName, scmTokenId, token, cheUserId); } @Override @@ -147,9 +130,6 @@ public class PersonalAccessToken { + ", scmOrganization='" + scmOrganization + '\'' - + ", scmUserId='" - + scmUserId - + '\'' + ", scmTokenName='" + scmTokenName + '\''