From 889187e4e1218ee769e0327e8fb27d1aa45109be Mon Sep 17 00:00:00 2001 From: Max Shaposhnik Date: Wed, 17 Feb 2021 19:46:20 +0200 Subject: [PATCH] Correct response codes and messages related to private factory flow --- .../org/eclipse/che/api/core/ErrorCodes.java | 26 ++---- .../resource/api/workspace/ErrorCodes.java | 25 ------ .../api/workspace/LimitExceededException.java | 1 + .../BitbucketServerOAuthAuthenticator.java | 10 +++ .../oauth1/NoopOAuthAuthenticator.java | 6 ++ .../security/oauth1/OAuthAuthenticator.java | 8 ++ ...rAuthorizingFactoryParametersResolver.java | 3 +- .../server/BitbucketServerApiClient.java | 3 +- .../server/HttpBitbucketServerApiClient.java | 12 +-- ...rverOAuth1AuthorizationHeaderSupplier.java | 5 +- ...tServerPersonalAccessTokenFetcherTest.java | 2 +- .../GithubFactoryParametersResolver.java | 4 +- .../DefaultFactoryParameterResolver.java | 4 +- .../server/FactoryParametersResolver.java | 5 +- .../api/factory/server/FactoryService.java | 3 +- .../exception/ScmUnauthorizedException.java | 32 +++++++- .../server/urlfactory/URLFactoryBuilder.java | 51 +++++++++--- .../urlfactory/URLFactoryBuilderTest.java | 82 ++++++++++++++++++- 18 files changed, 199 insertions(+), 83 deletions(-) delete mode 100644 multiuser/api/che-multiuser-api-resource/src/main/java/org/eclipse/che/multiuser/resource/api/workspace/ErrorCodes.java diff --git a/core/che-core-api-core/src/main/java/org/eclipse/che/api/core/ErrorCodes.java b/core/che-core-api-core/src/main/java/org/eclipse/che/api/core/ErrorCodes.java index f306ef201c..d1997f1bac 100644 --- a/core/che-core-api-core/src/main/java/org/eclipse/che/api/core/ErrorCodes.java +++ b/core/che-core-api-core/src/main/java/org/eclipse/che/api/core/ErrorCodes.java @@ -12,27 +12,15 @@ package org.eclipse.che.api.core; /** - * Error codes that are used in exceptions. Defined error codes MUST BE in range 15000-32999 - * inclusive. + * Defines error codes that are used in exceptions, defined codes MUST NOT conflict, error codes + * must be in range 10000-14999 inclusive. * * @author Igor Vinokur + * @author Yevhenii Voevodin */ -public class ErrorCodes { +public final class ErrorCodes { + + public static final int LIMIT_EXCEEDED = 10000; + private ErrorCodes() {} - - public static final int NO_COMMITTER_NAME_OR_EMAIL_DEFINED = 15216; - public static final int UNABLE_GET_PRIVATE_SSH_KEY = 32068; - public static final int UNAUTHORIZED_GIT_OPERATION = 32080; - public static final int UNAUTHORIZED_SVN_OPERATION = 32090; - public static final int MERGE_CONFLICT = 32062; - public static final int FAILED_CHECKOUT = 32063; - public static final int FAILED_CHECKOUT_WITH_START_POINT = 32064; - public static final int INIT_COMMIT_WAS_NOT_PERFORMED = 32082; - - public static final int NO_PROJECT_ON_FILE_SYSTEM = 10; - public static final int NO_PROJECT_CONFIGURED_IN_WS = 11; - public static final int PROJECT_TYPE_IS_NOT_REGISTERED = 12; - public static final int ATTRIBUTE_NAME_PROBLEM = 13; - public static final int NOT_UPDATED_PROJECT = 14; - public static final int ITEM_NOT_FOUND = 15; } diff --git a/multiuser/api/che-multiuser-api-resource/src/main/java/org/eclipse/che/multiuser/resource/api/workspace/ErrorCodes.java b/multiuser/api/che-multiuser-api-resource/src/main/java/org/eclipse/che/multiuser/resource/api/workspace/ErrorCodes.java deleted file mode 100644 index b057eccbb7..0000000000 --- a/multiuser/api/che-multiuser-api-resource/src/main/java/org/eclipse/che/multiuser/resource/api/workspace/ErrorCodes.java +++ /dev/null @@ -1,25 +0,0 @@ -/* - * Copyright (c) 2012-2018 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/ - * - * SPDX-License-Identifier: EPL-2.0 - * - * Contributors: - * Red Hat, Inc. - initial API and implementation - */ -package org.eclipse.che.multiuser.resource.api.workspace; - -/** - * Defines error codes, defined codes MUST NOT conflict with existing {@link - * org.eclipse.che.api.core.ErrorCodes}, error codes must be in range 10000-14999 inclusive. - * - * @author Yevhenii Voevodin - */ -public final class ErrorCodes { - - public static final int LIMIT_EXCEEDED = 10000; - - private ErrorCodes() {} -} diff --git a/multiuser/api/che-multiuser-api-resource/src/main/java/org/eclipse/che/multiuser/resource/api/workspace/LimitExceededException.java b/multiuser/api/che-multiuser-api-resource/src/main/java/org/eclipse/che/multiuser/resource/api/workspace/LimitExceededException.java index f4163c3419..1c53245f02 100644 --- a/multiuser/api/che-multiuser-api-resource/src/main/java/org/eclipse/che/multiuser/resource/api/workspace/LimitExceededException.java +++ b/multiuser/api/che-multiuser-api-resource/src/main/java/org/eclipse/che/multiuser/resource/api/workspace/LimitExceededException.java @@ -12,6 +12,7 @@ package org.eclipse.che.multiuser.resource.api.workspace; import java.util.Map; +import org.eclipse.che.api.core.ErrorCodes; import org.eclipse.che.api.core.ServerException; import org.eclipse.che.api.core.rest.shared.dto.ExtendedError; import org.eclipse.che.dto.server.DtoFactory; diff --git a/wsmaster/che-core-api-auth-bitbucket/src/main/java/org/eclipse/che/security/oauth1/BitbucketServerOAuthAuthenticator.java b/wsmaster/che-core-api-auth-bitbucket/src/main/java/org/eclipse/che/security/oauth1/BitbucketServerOAuthAuthenticator.java index 9366b084df..f6ca7ade15 100644 --- a/wsmaster/che-core-api-auth-bitbucket/src/main/java/org/eclipse/che/security/oauth1/BitbucketServerOAuthAuthenticator.java +++ b/wsmaster/che-core-api-auth-bitbucket/src/main/java/org/eclipse/che/security/oauth1/BitbucketServerOAuthAuthenticator.java @@ -21,6 +21,7 @@ import com.google.inject.Singleton; @Singleton public class BitbucketServerOAuthAuthenticator extends OAuthAuthenticator { public static final String AUTHENTICATOR_NAME = "bitbucket-server"; + private final String apiEndpoint; public BitbucketServerOAuthAuthenticator( String consumerKey, String privateKey, String bitbucketEndpoint, String apiEndpoint) { @@ -32,10 +33,19 @@ public class BitbucketServerOAuthAuthenticator extends OAuthAuthenticator { apiEndpoint + "/oauth/1.0/callback", null, privateKey); + this.apiEndpoint = apiEndpoint; } @Override public final String getOAuthProvider() { return AUTHENTICATOR_NAME; } + + @Override + public String getLocalAuthenticateUrl() { + return apiEndpoint + + "/oauth/1.0/authenticate?oauth_provider=" + + AUTHENTICATOR_NAME + + "&request_method=POST&signature_method=rsa"; + } } diff --git a/wsmaster/che-core-api-auth-bitbucket/src/main/java/org/eclipse/che/security/oauth1/NoopOAuthAuthenticator.java b/wsmaster/che-core-api-auth-bitbucket/src/main/java/org/eclipse/che/security/oauth1/NoopOAuthAuthenticator.java index b3e1ffadbc..c4d7997556 100644 --- a/wsmaster/che-core-api-auth-bitbucket/src/main/java/org/eclipse/che/security/oauth1/NoopOAuthAuthenticator.java +++ b/wsmaster/che-core-api-auth-bitbucket/src/main/java/org/eclipse/che/security/oauth1/NoopOAuthAuthenticator.java @@ -46,4 +46,10 @@ public class NoopOAuthAuthenticator extends OAuthAuthenticator { throw new RuntimeException( "The fallback noop authenticator cannot be used for authentication. Make sure OAuth is properly configured."); } + + @Override + public String getLocalAuthenticateUrl() { + throw new RuntimeException( + "The fallback noop authenticator cannot be used for authentication. Make sure OAuth is properly configured."); + } } diff --git a/wsmaster/che-core-api-auth/src/main/java/org/eclipse/che/security/oauth1/OAuthAuthenticator.java b/wsmaster/che-core-api-auth/src/main/java/org/eclipse/che/security/oauth1/OAuthAuthenticator.java index 9af159fdf2..9323be4b36 100644 --- a/wsmaster/che-core-api-auth/src/main/java/org/eclipse/che/security/oauth1/OAuthAuthenticator.java +++ b/wsmaster/che-core-api-auth/src/main/java/org/eclipse/che/security/oauth1/OAuthAuthenticator.java @@ -229,6 +229,14 @@ public abstract class OAuthAuthenticator { */ abstract String getOAuthProvider(); + /** + * Returns URL to initiate authentication process using given authenticator. Typically points to + * {@code /api/oauth/} or {@code /api/oauth/1.0} endpoint with necessary request params. + * + * @return URL to initiate authentication process + */ + public abstract String getLocalAuthenticateUrl(); + /** * Compute the Authorization header to sign the OAuth 1 request. * 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 f653330d3f..99de4ee9bb 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 @@ -19,6 +19,7 @@ import java.util.Map; import javax.inject.Inject; import javax.inject.Singleton; import javax.validation.constraints.NotNull; +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.GitCredentialManager; @@ -81,7 +82,7 @@ public class BitbucketServerAuthorizingFactoryParametersResolver */ @Override public FactoryMetaDto createFactory(@NotNull final Map factoryParameters) - throws BadRequestException { + throws ApiException { // no need to check null value of url parameter as accept() method has performed the check final BitbucketUrl bitbucketUrl = diff --git a/wsmaster/che-core-api-factory-bitbucket-server/src/main/java/org/eclipse/che/api/factory/server/bitbucket/server/BitbucketServerApiClient.java b/wsmaster/che-core-api-factory-bitbucket-server/src/main/java/org/eclipse/che/api/factory/server/bitbucket/server/BitbucketServerApiClient.java index 87ee733379..e1f315ae78 100644 --- a/wsmaster/che-core-api-factory-bitbucket-server/src/main/java/org/eclipse/che/api/factory/server/bitbucket/server/BitbucketServerApiClient.java +++ b/wsmaster/che-core-api-factory-bitbucket-server/src/main/java/org/eclipse/che/api/factory/server/bitbucket/server/BitbucketServerApiClient.java @@ -32,7 +32,8 @@ public interface BitbucketServerApiClient { * @throws ScmUnauthorizedException - in case if {@link Subject} is not linked to any {@link * BitbucketUser} */ - BitbucketUser getUser(Subject cheUser) throws ScmUnauthorizedException, ScmCommunicationException; + BitbucketUser getUser(Subject cheUser) + throws ScmUnauthorizedException, ScmCommunicationException, ScmItemNotFoundException; /** * @param slug diff --git a/wsmaster/che-core-api-factory-bitbucket-server/src/main/java/org/eclipse/che/api/factory/server/bitbucket/server/HttpBitbucketServerApiClient.java b/wsmaster/che-core-api-factory-bitbucket-server/src/main/java/org/eclipse/che/api/factory/server/bitbucket/server/HttpBitbucketServerApiClient.java index d8c58225cf..1a0a56ed12 100644 --- a/wsmaster/che-core-api-factory-bitbucket-server/src/main/java/org/eclipse/che/api/factory/server/bitbucket/server/HttpBitbucketServerApiClient.java +++ b/wsmaster/che-core-api-factory-bitbucket-server/src/main/java/org/eclipse/che/api/factory/server/bitbucket/server/HttpBitbucketServerApiClient.java @@ -13,7 +13,6 @@ package org.eclipse.che.api.factory.server.bitbucket.server; import static java.net.HttpURLConnection.HTTP_BAD_REQUEST; import static java.net.HttpURLConnection.HTTP_NOT_FOUND; -import static java.net.HttpURLConnection.HTTP_UNAUTHORIZED; import static java.time.Duration.ofSeconds; import com.fasterxml.jackson.core.JsonProcessingException; @@ -89,7 +88,7 @@ public class HttpBitbucketServerApiClient implements BitbucketServerApiClient { @Override public BitbucketUser getUser(Subject cheUser) - throws ScmUnauthorizedException, ScmCommunicationException { + throws ScmUnauthorizedException, ScmCommunicationException, ScmItemNotFoundException { try { // Since Bitbucket server API doesn't provide a way to get an account profile currently // authenticated user we will try to find it and by iterating over the list available to the @@ -118,11 +117,10 @@ public class HttpBitbucketServerApiClient implements BitbucketServerApiClient { if (currentUser.isPresent()) { return currentUser.get(); } - } catch (ScmBadRequestException | ScmItemNotFoundException scmBadRequestException) { - throw new ScmCommunicationException( - scmBadRequestException.getMessage(), scmBadRequestException); + } catch (ScmBadRequestException | ScmItemNotFoundException scmException) { + throw new ScmCommunicationException(scmException.getMessage(), scmException); } - throw new ScmUnauthorizedException( + throw new ScmItemNotFoundException( "Current user not found. That is possible only if user are not authorized against " + serverUri); } @@ -349,8 +347,6 @@ public class HttpBitbucketServerApiClient implements BitbucketServerApiClient { switch (response.statusCode()) { case HTTP_BAD_REQUEST: throw new ScmBadRequestException(body); - case HTTP_UNAUTHORIZED: - throw new ScmUnauthorizedException(body); case HTTP_NOT_FOUND: throw new ScmItemNotFoundException(body); default: diff --git a/wsmaster/che-core-api-factory-bitbucket-server/src/main/java/org/eclipse/che/security/oauth1/BitbucketServerOAuth1AuthorizationHeaderSupplier.java b/wsmaster/che-core-api-factory-bitbucket-server/src/main/java/org/eclipse/che/security/oauth1/BitbucketServerOAuth1AuthorizationHeaderSupplier.java index fd6897f0a2..ef1f76ef7d 100644 --- a/wsmaster/che-core-api-factory-bitbucket-server/src/main/java/org/eclipse/che/security/oauth1/BitbucketServerOAuth1AuthorizationHeaderSupplier.java +++ b/wsmaster/che-core-api-factory-bitbucket-server/src/main/java/org/eclipse/che/security/oauth1/BitbucketServerOAuth1AuthorizationHeaderSupplier.java @@ -45,7 +45,10 @@ public class BitbucketServerOAuth1AuthorizationHeaderSupplier subject.getUserName() + " is not authorized in " + authenticator.getOAuthProvider() - + " OAuth1 provider"); + + " OAuth1 provider", + authenticator.getOAuthProvider(), + "1.0", + authenticator.getLocalAuthenticateUrl()); } return authorizationHeader; } catch (OAuthAuthenticationException e) { diff --git a/wsmaster/che-core-api-factory-bitbucket-server/src/test/java/org/eclipse/che/api/factory/server/bitbucket/BitbucketServerPersonalAccessTokenFetcherTest.java b/wsmaster/che-core-api-factory-bitbucket-server/src/test/java/org/eclipse/che/api/factory/server/bitbucket/BitbucketServerPersonalAccessTokenFetcherTest.java index e601216a51..a470b8f70e 100644 --- a/wsmaster/che-core-api-factory-bitbucket-server/src/test/java/org/eclipse/che/api/factory/server/bitbucket/BitbucketServerPersonalAccessTokenFetcherTest.java +++ b/wsmaster/che-core-api-factory-bitbucket-server/src/test/java/org/eclipse/che/api/factory/server/bitbucket/BitbucketServerPersonalAccessTokenFetcherTest.java @@ -107,7 +107,7 @@ public class BitbucketServerPersonalAccessTokenFetcherTest { dataProvider = "expectedExceptions", expectedExceptions = {ScmUnauthorizedException.class, ScmCommunicationException.class}) public void shouldRethrowBasicExceptionsOnGetUserStep(Class exception) - throws ScmUnauthorizedException, ScmCommunicationException { + throws ScmUnauthorizedException, ScmCommunicationException, ScmItemNotFoundException { // given when(bitbucketServerApiClient.isConnected(eq(someNotBitbucketURL))).thenReturn(true); doThrow(exception).when(bitbucketServerApiClient).getUser(eq(subject)); 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 8296b1d648..cd0feb5e2e 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 @@ -19,6 +19,7 @@ import java.util.Map; import javax.inject.Inject; import javax.inject.Singleton; import javax.validation.constraints.NotNull; +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.urlfactory.ProjectConfigDtoMerger; @@ -82,8 +83,7 @@ public class GithubFactoryParametersResolver extends DefaultFactoryParameterReso */ @Override public FactoryMetaDto createFactory(@NotNull final Map factoryParameters) - throws BadRequestException { - + throws ApiException { // no need to check null value of url parameter as accept() method has performed the check final GithubUrl githubUrl = githubUrlParser.parse(factoryParameters.get(URL_PARAMETER_NAME)); 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 68300b6430..fdb88c5635 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 @@ -28,8 +28,8 @@ import java.util.function.Supplier; import javax.inject.Inject; import javax.inject.Singleton; import javax.validation.constraints.NotNull; +import org.eclipse.che.api.core.ApiException; import org.eclipse.che.api.core.BadRequestException; -import org.eclipse.che.api.core.ServerException; import org.eclipse.che.api.factory.server.urlfactory.DefaultFactoryUrl; import org.eclipse.che.api.factory.server.urlfactory.URLFactoryBuilder; import org.eclipse.che.api.factory.shared.dto.FactoryMetaDto; @@ -72,7 +72,7 @@ public class DefaultFactoryParameterResolver implements FactoryParametersResolve */ @Override public FactoryMetaDto createFactory(@NotNull final Map factoryParameters) - throws BadRequestException, ServerException { + throws ApiException { // This should never be null, because our contract in #accept prohibits that String devfileLocation = factoryParameters.get(URL_PARAMETER_NAME); 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 68cc80989d..3a63e09013 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 @@ -13,8 +13,8 @@ package org.eclipse.che.api.factory.server; import java.util.Map; import javax.validation.constraints.NotNull; +import org.eclipse.che.api.core.ApiException; import org.eclipse.che.api.core.BadRequestException; -import org.eclipse.che.api.core.ServerException; import org.eclipse.che.api.factory.shared.dto.FactoryMetaDto; /** @@ -39,6 +39,5 @@ public interface FactoryParametersResolver { * @param factoryParameters map containing factory data parameters provided through URL * @throws BadRequestException when data are invalid */ - FactoryMetaDto createFactory(@NotNull Map factoryParameters) - throws BadRequestException, ServerException; + FactoryMetaDto createFactory(@NotNull Map factoryParameters) 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 d1e964fea8..9de90d0a07 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 @@ -30,7 +30,6 @@ import javax.ws.rs.Produces; import javax.ws.rs.QueryParam; import org.eclipse.che.api.core.ApiException; import org.eclipse.che.api.core.BadRequestException; -import org.eclipse.che.api.core.ServerException; import org.eclipse.che.api.core.rest.Service; import org.eclipse.che.api.factory.shared.dto.FactoryMetaDto; import org.eclipse.che.api.user.server.UserManager; @@ -87,7 +86,7 @@ public class FactoryService extends Service { @DefaultValue("false") @QueryParam(VALIDATE_QUERY_PARAMETER) Boolean validate) - throws BadRequestException, ServerException { + throws ApiException { // check parameter requiredNotNull(parameters, "Factory build parameters"); diff --git a/wsmaster/che-core-api-factory/src/main/java/org/eclipse/che/api/factory/server/scm/exception/ScmUnauthorizedException.java b/wsmaster/che-core-api-factory/src/main/java/org/eclipse/che/api/factory/server/scm/exception/ScmUnauthorizedException.java index 0b74adec18..62c6cfe74a 100644 --- a/wsmaster/che-core-api-factory/src/main/java/org/eclipse/che/api/factory/server/scm/exception/ScmUnauthorizedException.java +++ b/wsmaster/che-core-api-factory/src/main/java/org/eclipse/che/api/factory/server/scm/exception/ScmUnauthorizedException.java @@ -14,11 +14,39 @@ package org.eclipse.che.api.factory.server.scm.exception; /** In case if OAuth1 or Oauth2 token is missing and we cant make any authorised calls */ public class ScmUnauthorizedException extends Exception { - public ScmUnauthorizedException(String message) { + private final String oauthProvider; + private final String oauthVersion; + private final String authenticateUrl; + + public ScmUnauthorizedException( + String message, String oauthProvider, String oauthVersion, String authenticateUrl) { super(message); + this.oauthProvider = oauthProvider; + this.oauthVersion = oauthVersion; + this.authenticateUrl = authenticateUrl; } - public ScmUnauthorizedException(String message, Throwable cause) { + public ScmUnauthorizedException( + String message, + String oauthProvider, + Throwable cause, + String oauthVersion, + String authenticateUrl) { super(message, cause); + this.oauthProvider = oauthProvider; + this.oauthVersion = oauthVersion; + this.authenticateUrl = authenticateUrl; + } + + public String getOauthProvider() { + return oauthProvider; + } + + public String getAuthenticateUrl() { + return authenticateUrl; + } + + public String getOauthVersion() { + return oauthVersion; } } diff --git a/wsmaster/che-core-api-factory/src/main/java/org/eclipse/che/api/factory/server/urlfactory/URLFactoryBuilder.java b/wsmaster/che-core-api-factory/src/main/java/org/eclipse/che/api/factory/server/urlfactory/URLFactoryBuilder.java index 46524b0e50..cd8bfa7176 100644 --- a/wsmaster/che-core-api-factory/src/main/java/org/eclipse/che/api/factory/server/urlfactory/URLFactoryBuilder.java +++ b/wsmaster/che-core-api-factory/src/main/java/org/eclipse/che/api/factory/server/urlfactory/URLFactoryBuilder.java @@ -12,7 +12,6 @@ package org.eclipse.che.api.factory.server.urlfactory; import static com.google.common.base.Strings.isNullOrEmpty; -import static java.lang.String.format; import static org.eclipse.che.api.factory.shared.Constants.CURRENT_VERSION; import static org.eclipse.che.api.workspace.server.devfile.Constants.CURRENT_API_VERSION; import static org.eclipse.che.api.workspace.shared.Constants.WORKSPACE_TOOLING_EDITOR_ATTRIBUTE; @@ -27,7 +26,13 @@ import java.util.Optional; import javax.inject.Inject; import javax.inject.Named; import javax.inject.Singleton; +import org.eclipse.che.api.core.ApiException; import org.eclipse.che.api.core.BadRequestException; +import org.eclipse.che.api.core.ServerException; +import org.eclipse.che.api.core.UnauthorizedException; +import org.eclipse.che.api.factory.server.scm.exception.ScmCommunicationException; +import org.eclipse.che.api.factory.server.scm.exception.ScmUnauthorizedException; +import org.eclipse.che.api.factory.server.scm.exception.UnknownScmProviderException; import org.eclipse.che.api.factory.server.urlfactory.RemoteFactoryUrl.DevfileLocation; import org.eclipse.che.api.factory.shared.dto.FactoryDevfileV2Dto; import org.eclipse.che.api.factory.shared.dto.FactoryDto; @@ -94,7 +99,7 @@ public class URLFactoryBuilder { RemoteFactoryUrl remoteFactoryUrl, FileContentProvider fileContentProvider, Map overrideProperties) - throws BadRequestException { + throws ApiException { String devfileYamlContent; for (DevfileLocation location : remoteFactoryUrl.devfileFileLocations()) { try { @@ -108,10 +113,7 @@ public class URLFactoryBuilder { continue; } catch (DevfileException e) { LOG.debug("Unexpected devfile exception: {}", e.getMessage()); - throw new BadRequestException( - format( - "There is an error resolving defvile. Error: %s. URL is %s", - e.getMessage(), location.location())); + throw toApiException(e, location); } if (isNullOrEmpty(devfileYamlContent)) { return Optional.empty(); @@ -121,17 +123,42 @@ public class URLFactoryBuilder { JsonNode parsedDevfile = devfileParser.parseYamlRaw(devfileYamlContent); return Optional.of( createFactory(parsedDevfile, overrideProperties, fileContentProvider, location)); - } catch (DevfileException | OverrideParameterException e) { - throw new BadRequestException( - "Error occurred during creation a workspace from devfile located at `" - + location.location() - + "`. Cause: " - + e.getMessage()); + } catch (OverrideParameterException e) { + throw new BadRequestException("Error processing override parameter(s): " + e.getMessage()); + } catch (DevfileException e) { + throw toApiException(e, location); } } return Optional.empty(); } + private ApiException toApiException(DevfileException devfileException, DevfileLocation location) { + Throwable cause = devfileException.getCause(); + if (cause instanceof ScmUnauthorizedException) { + ScmUnauthorizedException scmCause = (ScmUnauthorizedException) cause; + return new UnauthorizedException( + "SCM Authentication required", + 401, + Map.of( + "oauth_version", scmCause.getOauthVersion(), + "oauth_provider", scmCause.getOauthProvider(), + "oauth_authentication_url", scmCause.getAuthenticateUrl())); + } else if (cause instanceof UnknownScmProviderException) { + return new ServerException( + "Provided location is unknown or misconfigured on the server side. Error message:" + + cause.getMessage()); + } else if (cause instanceof ScmCommunicationException) { + return new ServerException( + "There is an error happened when communicate with SCM server. Error message:" + + cause.getMessage()); + } + return new BadRequestException( + "Error occurred during creation a workspace from devfile located at `" + + location.location() + + "`. Cause: " + + devfileException.getMessage()); + } + /** * Converts given devfile json into factory based on the devfile version. * diff --git a/wsmaster/che-core-api-factory/src/test/java/org/eclipse/che/api/factory/server/urlfactory/URLFactoryBuilderTest.java b/wsmaster/che-core-api-factory/src/test/java/org/eclipse/che/api/factory/server/urlfactory/URLFactoryBuilderTest.java index 13b70db273..895afb3833 100644 --- a/wsmaster/che-core-api-factory/src/test/java/org/eclipse/che/api/factory/server/urlfactory/URLFactoryBuilderTest.java +++ b/wsmaster/che-core-api-factory/src/test/java/org/eclipse/che/api/factory/server/urlfactory/URLFactoryBuilderTest.java @@ -36,7 +36,13 @@ import java.util.Collections; import java.util.HashMap; import java.util.Map; import java.util.Optional; -import org.eclipse.che.api.core.BadRequestException; +import org.eclipse.che.api.core.ApiException; +import org.eclipse.che.api.core.ServerException; +import org.eclipse.che.api.core.UnauthorizedException; +import org.eclipse.che.api.core.rest.shared.dto.ExtendedError; +import org.eclipse.che.api.factory.server.scm.exception.ScmCommunicationException; +import org.eclipse.che.api.factory.server.scm.exception.ScmUnauthorizedException; +import org.eclipse.che.api.factory.server.scm.exception.UnknownScmProviderException; import org.eclipse.che.api.factory.server.urlfactory.RemoteFactoryUrl.DevfileLocation; import org.eclipse.che.api.factory.shared.dto.FactoryDevfileV2Dto; import org.eclipse.che.api.factory.shared.dto.FactoryDto; @@ -133,7 +139,7 @@ public class URLFactoryBuilderTest { } @Test - public void testDevfileV2() throws BadRequestException, DevfileException { + public void testDevfileV2() throws ApiException, DevfileException { String myLocation = "http://foo-location/"; Map devfileAsMap = Map.of("hello", "there", "how", "are", "you", "?"); @@ -157,7 +163,7 @@ public class URLFactoryBuilderTest { } @Test - public void testDevfileV2WithFilename() throws BadRequestException, DevfileException { + public void testDevfileV2WithFilename() throws ApiException, DevfileException { String myLocation = "http://foo-location/"; Map devfileAsMap = Map.of("hello", "there", "how", "are", "you", "?"); @@ -220,7 +226,7 @@ public class URLFactoryBuilderTest { @Test(dataProvider = "devfiles") public void checkThatDtoHasCorrectNames(DevfileImpl devfile, String expectedGenerateName) - throws BadRequestException, DevfileException, IOException, OverrideParameterException { + throws ApiException, IOException, OverrideParameterException, DevfileException { DefaultFactoryUrl defaultFactoryUrl = mock(DefaultFactoryUrl.class); FileContentProvider fileContentProvider = mock(FileContentProvider.class); when(defaultFactoryUrl.devfileFileLocations()) @@ -251,4 +257,72 @@ public class URLFactoryBuilderTest { assertNull(factory.getDevfile().getMetadata().getName()); assertEquals(factory.getDevfile().getMetadata().getGenerateName(), expectedGenerateName); } + + @Test(dataProvider = "devfileExceptions") + public void checkCorrectExceptionThrownDependingOnCause( + Throwable cause, + Class expectedClass, + String expectedMessage, + Map expectedAttributes) + throws IOException, DevfileException { + DefaultFactoryUrl defaultFactoryUrl = mock(DefaultFactoryUrl.class); + FileContentProvider fileContentProvider = mock(FileContentProvider.class); + when(defaultFactoryUrl.devfileFileLocations()) + .thenReturn( + singletonList( + new DevfileLocation() { + @Override + public Optional filename() { + return Optional.empty(); + } + + @Override + public String location() { + return "http://foo.bar/anything"; + } + })); + + when(fileContentProvider.fetchContent(anyString())).thenThrow(new DevfileException("", cause)); + + // when + try { + urlFactoryBuilder.createFactoryFromDevfile( + defaultFactoryUrl, fileContentProvider, emptyMap()); + } catch (ApiException e) { + assertTrue(e.getClass().isAssignableFrom(expectedClass)); + assertEquals(e.getMessage(), expectedMessage); + if (e.getServiceError() instanceof ExtendedError) + assertEquals(((ExtendedError) e.getServiceError()).getAttributes(), expectedAttributes); + } + } + + @DataProvider + public static Object[][] devfileExceptions() { + return new Object[][] { + { + new ScmCommunicationException("foo"), + ServerException.class, + "There is an error happened when communicate with SCM server. Error message:foo", + null + }, + { + new UnknownScmProviderException("foo", "bar"), + ServerException.class, + "Provided location is unknown or misconfigured on the server side. Error message:foo", + null + }, + { + new ScmUnauthorizedException("foo", "bitbucket", "1.0", "http://foo.bar"), + UnauthorizedException.class, + "SCM Authentication required", + Map.of( + "oauth_version", + "1.0", + "oauth_authentication_url", + "http://foo.bar", + "oauth_provider", + "bitbucket") + } + }; + } }