From a8623992b60785e4ba3c4aaac531bc6431a26a30 Mon Sep 17 00:00:00 2001 From: Igor Vinokur Date: Mon, 26 Sep 2022 09:05:04 +0300 Subject: [PATCH] chore: Allign the /token/refresh/ factory API method (#359) Fix the /token/refresh/ factory API method by picking out the hostname from the url parameter and passing it to personalAccessTokenManager.getAndStore(hostname) instead of the entire factory url --- ...tServerAuthorizingFactoryParametersResolver.java | 6 ++++++ .../BitbucketFactoryParametersResolver.java | 6 ++++++ .../github/GithubFactoryParametersResolver.java | 6 ++++++ .../gitlab/GitlabFactoryParametersResolver.java | 6 ++++++ .../server/DefaultFactoryParameterResolver.java | 6 ++++++ .../factory/server/FactoryParametersResolver.java | 12 +++++++++++- .../che/api/factory/server/FactoryService.java | 13 ++++++++++--- .../che/api/factory/server/FactoryServiceTest.java | 12 ++++++++++-- 8 files changed, 61 insertions(+), 6 deletions(-) diff --git a/wsmaster/che-core-api-factory-bitbucket-server/src/main/java/org/eclipse/che/api/factory/server/bitbucket/BitbucketServerAuthorizingFactoryParametersResolver.java b/wsmaster/che-core-api-factory-bitbucket-server/src/main/java/org/eclipse/che/api/factory/server/bitbucket/BitbucketServerAuthorizingFactoryParametersResolver.java index c76461d6e3..708adfe567 100644 --- a/wsmaster/che-core-api-factory-bitbucket-server/src/main/java/org/eclipse/che/api/factory/server/bitbucket/BitbucketServerAuthorizingFactoryParametersResolver.java +++ b/wsmaster/che-core-api-factory-bitbucket-server/src/main/java/org/eclipse/che/api/factory/server/bitbucket/BitbucketServerAuthorizingFactoryParametersResolver.java @@ -23,6 +23,7 @@ import org.eclipse.che.api.core.ApiException; import org.eclipse.che.api.core.BadRequestException; import org.eclipse.che.api.factory.server.DefaultFactoryParameterResolver; import org.eclipse.che.api.factory.server.scm.PersonalAccessTokenManager; +import org.eclipse.che.api.factory.server.urlfactory.RemoteFactoryUrl; import org.eclipse.che.api.factory.server.urlfactory.URLFactoryBuilder; import org.eclipse.che.api.factory.shared.dto.FactoryDevfileV2Dto; import org.eclipse.che.api.factory.shared.dto.FactoryDto; @@ -153,4 +154,9 @@ public class BitbucketServerAuthorizingFactoryParametersResolver return factory; } } + + @Override + public RemoteFactoryUrl parseFactoryUrl(String factoryUrl) throws ApiException { + return bitbucketURLParser.parse(factoryUrl); + } } diff --git a/wsmaster/che-core-api-factory-bitbucket/src/main/java/org/eclipse/che/api/factory/server/bitbucket/BitbucketFactoryParametersResolver.java b/wsmaster/che-core-api-factory-bitbucket/src/main/java/org/eclipse/che/api/factory/server/bitbucket/BitbucketFactoryParametersResolver.java index 55a791568b..fe83b2430b 100644 --- a/wsmaster/che-core-api-factory-bitbucket/src/main/java/org/eclipse/che/api/factory/server/bitbucket/BitbucketFactoryParametersResolver.java +++ b/wsmaster/che-core-api-factory-bitbucket/src/main/java/org/eclipse/che/api/factory/server/bitbucket/BitbucketFactoryParametersResolver.java @@ -24,6 +24,7 @@ import org.eclipse.che.api.core.BadRequestException; import org.eclipse.che.api.factory.server.DefaultFactoryParameterResolver; import org.eclipse.che.api.factory.server.scm.PersonalAccessTokenManager; import org.eclipse.che.api.factory.server.urlfactory.ProjectConfigDtoMerger; +import org.eclipse.che.api.factory.server.urlfactory.RemoteFactoryUrl; import org.eclipse.che.api.factory.server.urlfactory.URLFactoryBuilder; import org.eclipse.che.api.factory.shared.dto.FactoryDevfileV2Dto; import org.eclipse.che.api.factory.shared.dto.FactoryDto; @@ -162,4 +163,9 @@ public class BitbucketFactoryParametersResolver extends DefaultFactoryParameterR return factory; } } + + @Override + public RemoteFactoryUrl parseFactoryUrl(String factoryUrl) { + return bitbucketURLParser.parse(factoryUrl); + } } diff --git a/wsmaster/che-core-api-factory-github/src/main/java/org/eclipse/che/api/factory/server/github/GithubFactoryParametersResolver.java b/wsmaster/che-core-api-factory-github/src/main/java/org/eclipse/che/api/factory/server/github/GithubFactoryParametersResolver.java index fe13c8d9b4..2f2fa3a50f 100644 --- a/wsmaster/che-core-api-factory-github/src/main/java/org/eclipse/che/api/factory/server/github/GithubFactoryParametersResolver.java +++ b/wsmaster/che-core-api-factory-github/src/main/java/org/eclipse/che/api/factory/server/github/GithubFactoryParametersResolver.java @@ -25,6 +25,7 @@ import org.eclipse.che.api.core.BadRequestException; import org.eclipse.che.api.factory.server.DefaultFactoryParameterResolver; import org.eclipse.che.api.factory.server.scm.PersonalAccessTokenManager; import org.eclipse.che.api.factory.server.urlfactory.ProjectConfigDtoMerger; +import org.eclipse.che.api.factory.server.urlfactory.RemoteFactoryUrl; import org.eclipse.che.api.factory.server.urlfactory.URLFactoryBuilder; import org.eclipse.che.api.factory.shared.dto.FactoryDevfileV2Dto; import org.eclipse.che.api.factory.shared.dto.FactoryDto; @@ -173,4 +174,9 @@ public class GithubFactoryParametersResolver extends DefaultFactoryParameterReso return factory; } } + + @Override + public RemoteFactoryUrl parseFactoryUrl(String factoryUrl) throws ApiException { + return githubUrlParser.parse(factoryUrl); + } } diff --git a/wsmaster/che-core-api-factory-gitlab/src/main/java/org/eclipse/che/api/factory/server/gitlab/GitlabFactoryParametersResolver.java b/wsmaster/che-core-api-factory-gitlab/src/main/java/org/eclipse/che/api/factory/server/gitlab/GitlabFactoryParametersResolver.java index 170f054716..e18979640e 100644 --- a/wsmaster/che-core-api-factory-gitlab/src/main/java/org/eclipse/che/api/factory/server/gitlab/GitlabFactoryParametersResolver.java +++ b/wsmaster/che-core-api-factory-gitlab/src/main/java/org/eclipse/che/api/factory/server/gitlab/GitlabFactoryParametersResolver.java @@ -23,6 +23,7 @@ import org.eclipse.che.api.core.ApiException; import org.eclipse.che.api.core.BadRequestException; import org.eclipse.che.api.factory.server.DefaultFactoryParameterResolver; import org.eclipse.che.api.factory.server.scm.PersonalAccessTokenManager; +import org.eclipse.che.api.factory.server.urlfactory.RemoteFactoryUrl; import org.eclipse.che.api.factory.server.urlfactory.URLFactoryBuilder; import org.eclipse.che.api.factory.shared.dto.FactoryDevfileV2Dto; import org.eclipse.che.api.factory.shared.dto.FactoryDto; @@ -145,4 +146,9 @@ public class GitlabFactoryParametersResolver extends DefaultFactoryParameterReso return factory; } } + + @Override + public RemoteFactoryUrl parseFactoryUrl(String factoryUrl) throws ApiException { + return gitlabURLParser.parse(factoryUrl); + } } diff --git a/wsmaster/che-core-api-factory/src/main/java/org/eclipse/che/api/factory/server/DefaultFactoryParameterResolver.java b/wsmaster/che-core-api-factory/src/main/java/org/eclipse/che/api/factory/server/DefaultFactoryParameterResolver.java index f765820b61..2ee79b6c2e 100644 --- a/wsmaster/che-core-api-factory/src/main/java/org/eclipse/che/api/factory/server/DefaultFactoryParameterResolver.java +++ b/wsmaster/che-core-api-factory/src/main/java/org/eclipse/che/api/factory/server/DefaultFactoryParameterResolver.java @@ -31,6 +31,7 @@ import javax.inject.Singleton; import org.eclipse.che.api.core.ApiException; import org.eclipse.che.api.core.BadRequestException; import org.eclipse.che.api.factory.server.urlfactory.DefaultFactoryUrl; +import org.eclipse.che.api.factory.server.urlfactory.RemoteFactoryUrl; import org.eclipse.che.api.factory.server.urlfactory.URLFactoryBuilder; import org.eclipse.che.api.factory.shared.dto.FactoryMetaDto; import org.eclipse.che.api.workspace.server.devfile.URLFetcher; @@ -94,6 +95,11 @@ public class DefaultFactoryParameterResolver implements FactoryParametersResolve .orElse(null); } + @Override + public RemoteFactoryUrl parseFactoryUrl(String factoryUrl) throws ApiException { + throw new ApiException("Operation is not supported"); + } + /** * Finds and returns devfile override parameters in general factory parameters map. * diff --git a/wsmaster/che-core-api-factory/src/main/java/org/eclipse/che/api/factory/server/FactoryParametersResolver.java b/wsmaster/che-core-api-factory/src/main/java/org/eclipse/che/api/factory/server/FactoryParametersResolver.java index 06fc0d39ff..b700d9bddc 100644 --- a/wsmaster/che-core-api-factory/src/main/java/org/eclipse/che/api/factory/server/FactoryParametersResolver.java +++ b/wsmaster/che-core-api-factory/src/main/java/org/eclipse/che/api/factory/server/FactoryParametersResolver.java @@ -1,5 +1,5 @@ /* - * Copyright (c) 2012-2021 Red Hat, Inc. + * Copyright (c) 2012-2022 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/ @@ -15,6 +15,7 @@ import jakarta.validation.constraints.NotNull; import java.util.Map; import org.eclipse.che.api.core.ApiException; import org.eclipse.che.api.core.BadRequestException; +import org.eclipse.che.api.factory.server.urlfactory.RemoteFactoryUrl; import org.eclipse.che.api.factory.shared.dto.FactoryMetaDto; /** @@ -40,4 +41,13 @@ public interface FactoryParametersResolver { * @throws BadRequestException when data are invalid */ FactoryMetaDto createFactory(@NotNull Map factoryParameters) throws ApiException; + + /** + * Parses a factory Url String to a {@link RemoteFactoryUrl} object + * + * @param factoryUrl the factory Url string + * @return {@link RemoteFactoryUrl} representation of the factory URL + * @throws ApiException when authentication required operations fail + */ + RemoteFactoryUrl parseFactoryUrl(String factoryUrl) throws ApiException; } diff --git a/wsmaster/che-core-api-factory/src/main/java/org/eclipse/che/api/factory/server/FactoryService.java b/wsmaster/che-core-api-factory/src/main/java/org/eclipse/che/api/factory/server/FactoryService.java index 796b369eea..52d8cfc4f5 100644 --- a/wsmaster/che-core-api-factory/src/main/java/org/eclipse/che/api/factory/server/FactoryService.java +++ b/wsmaster/che-core-api-factory/src/main/java/org/eclipse/che/api/factory/server/FactoryService.java @@ -12,6 +12,8 @@ package org.eclipse.che.api.factory.server; import static jakarta.ws.rs.core.MediaType.APPLICATION_JSON; +import static java.util.Collections.singletonMap; +import static org.eclipse.che.api.factory.server.ApiExceptionMapper.toApiException; import static org.eclipse.che.api.factory.server.FactoryLinksHelper.createLinks; import static org.eclipse.che.api.factory.shared.Constants.URL_PARAMETER_NAME; @@ -144,13 +146,18 @@ public class FactoryService extends Service { requiredNotNull(url, "Factory url"); try { - personalAccessTokenManager.getAndStore(url); + FactoryParametersResolver factoryParametersResolver = + factoryParametersResolverHolder.getFactoryParametersResolver( + singletonMap(URL_PARAMETER_NAME, url)); + personalAccessTokenManager.getAndStore( + factoryParametersResolver.parseFactoryUrl(url).getHostName()); } catch (ScmCommunicationException | ScmConfigurationPersistenceException | UnknownScmProviderException - | UnsatisfiedScmPreconditionException - | ScmUnauthorizedException e) { + | UnsatisfiedScmPreconditionException e) { throw new ApiException(e); + } catch (ScmUnauthorizedException e) { + throw toApiException(e); } } diff --git a/wsmaster/che-core-api-factory/src/test/java/org/eclipse/che/api/factory/server/FactoryServiceTest.java b/wsmaster/che-core-api-factory/src/test/java/org/eclipse/che/api/factory/server/FactoryServiceTest.java index 5a4ccb718b..e2cf5bb169 100644 --- a/wsmaster/che-core-api-factory/src/test/java/org/eclipse/che/api/factory/server/FactoryServiceTest.java +++ b/wsmaster/che-core-api-factory/src/test/java/org/eclipse/che/api/factory/server/FactoryServiceTest.java @@ -28,6 +28,7 @@ import static org.mockito.ArgumentMatchers.eq; import static org.mockito.Mockito.doReturn; import static org.mockito.Mockito.doThrow; import static org.mockito.Mockito.lenient; +import static org.mockito.Mockito.mock; import static org.mockito.Mockito.spy; import static org.mockito.Mockito.verify; import static org.mockito.Mockito.when; @@ -55,6 +56,7 @@ import org.eclipse.che.api.factory.server.impl.SourceStorageParametersValidator; import org.eclipse.che.api.factory.server.model.impl.AuthorImpl; import org.eclipse.che.api.factory.server.model.impl.FactoryImpl; import org.eclipse.che.api.factory.server.scm.PersonalAccessTokenManager; +import org.eclipse.che.api.factory.server.urlfactory.RemoteFactoryUrl; import org.eclipse.che.api.factory.shared.dto.FactoryDto; import org.eclipse.che.api.user.server.PreferenceManager; import org.eclipse.che.api.user.server.UserManager; @@ -234,11 +236,17 @@ public class FactoryServiceTest { @Test public void checkRefreshToken() throws Exception { // given + final FactoryParametersResolverHolder dummyHolder = spy(factoryParametersResolverHolder); + FactoryParametersResolver factoryParametersResolver = mock(FactoryParametersResolver.class); + RemoteFactoryUrl remoteFactoryUrl = mock(RemoteFactoryUrl.class); + when(factoryParametersResolver.parseFactoryUrl(eq("someUrl"))).thenReturn(remoteFactoryUrl); + when(remoteFactoryUrl.getHostName()).thenReturn("hostName"); + doReturn(factoryParametersResolver).when(dummyHolder).getFactoryParametersResolver(anyMap()); service = new FactoryService( userManager, acceptValidator, - factoryParametersResolverHolder, + dummyHolder, additionalFilenamesProvider, personalAccessTokenManager); @@ -250,7 +258,7 @@ public class FactoryServiceTest { .post(SERVICE_PATH + "/token/refresh"); // then - verify(personalAccessTokenManager).getAndStore(eq("someUrl")); + verify(personalAccessTokenManager).getAndStore(eq("hostName")); } @Test