Skip to content

Commit

Permalink
Add error message for credentials not being sent over HTTP (#613)
Browse files Browse the repository at this point in the history
  • Loading branch information
TadCordle authored Jul 13, 2018
1 parent db524dd commit cfdc014
Show file tree
Hide file tree
Showing 7 changed files with 131 additions and 10 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@
import com.google.cloud.tools.jib.configuration.CacheConfiguration;
import com.google.cloud.tools.jib.registry.InsecureRegistryException;
import com.google.cloud.tools.jib.registry.RegistryAuthenticationFailedException;
import com.google.cloud.tools.jib.registry.RegistryCredentialsNotSentException;
import com.google.cloud.tools.jib.registry.RegistryUnauthorizedException;
import com.google.common.annotations.VisibleForTesting;
import java.io.IOException;
Expand Down Expand Up @@ -232,6 +233,10 @@ public void build(HelpfulSuggestions helpfulSuggestions) throws BuildStepsExecut
buildConfiguration,
helpfulSuggestions);

} else if (exceptionDuringBuildSteps instanceof RegistryCredentialsNotSentException) {
throw new BuildStepsExecutionException(
helpfulSuggestions.forCredentialsNotSent(), exceptionDuringBuildSteps);

} else if (exceptionDuringBuildSteps instanceof RegistryAuthenticationFailedException
&& exceptionDuringBuildSteps.getCause() instanceof HttpResponseException) {
handleRegistryUnauthorizedException(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -96,6 +96,10 @@ public String forCredentialsNotCorrect(String registry) {
return suggest("make sure your credentials for '" + registry + "' are set up correctly");
}

public String forCredentialsNotSent() {
return suggest("use a registry that supports HTTPS so credentials can be sent safely");
}

public String forDockerContextInsecureRecursiveDelete(String directory) {
return suggest("clear " + directory + " manually before creating the Docker context");
}
Expand All @@ -108,6 +112,11 @@ public String forDockerNotInstalled() {
return suggest("make sure Docker is installed and you have correct privileges to run it");
}

public String forInsecureRegistry() {
return suggest(
"use a registry that supports HTTPS or set the configuration parameter 'allowInsecureRegistries'");
}

/**
* @param parameter the parameter name (e.g. 'to.image' or {@literal <to><image>})
* @param buildConfigFilename the name of the build config (build.gradle or pom.xml)
Expand Down Expand Up @@ -145,9 +154,4 @@ private String forNoCredentialHelpersDefined(
+ "' or "
+ authConfiguration);
}

public String forInsecureRegistry() {
return suggest(
"use a registry that supports HTTPS or set the configuration parameter 'allowInsecureRegistries'");
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,36 @@
/*
* Copyright 2018 Google LLC. All rights reserved.
*
* Licensed under the Apache License, Version 2.0 (the "License"); you may not
* use this file except in compliance with the License. You may obtain a copy of
* the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
* WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
* License for the specific language governing permissions and limitations under
* the License.
*/

package com.google.cloud.tools.jib.registry;

/** Thrown when registry request was unauthorized because credentials weren't sent. */
public class RegistryCredentialsNotSentException extends RegistryException {

/**
* Identifies the image registry and repository that denied access.
*
* @param registry the image registry
* @param repository the image repository
*/
RegistryCredentialsNotSentException(String registry, String repository) {
super(
"Required credentials for "
+ registry
+ "/"
+ repository
+ " were not sent because the connection was over HTTP");
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,8 @@ class RegistryEndpointCaller<T> {
private static final String DEFAULT_PROTOCOL = "https";

/** Maintains the state of a request. This is used to retry requests with different parameters. */
private static class RequestState {
@VisibleForTesting
static class RequestState {

@Nullable private final Authorization authorization;
private final URL url;
Expand All @@ -61,7 +62,8 @@ private static class RequestState {
* @param authorization authentication credentials
* @param url the endpoint URL to call
*/
private RequestState(@Nullable Authorization authorization, URL url) {
@VisibleForTesting
RequestState(@Nullable Authorization authorization, URL url) {
this.authorization = authorization;
this.url = url;
}
Expand Down Expand Up @@ -148,8 +150,9 @@ T call() throws IOException, RegistryException {
* @throws IOException for most I/O exceptions when making the request
* @throws RegistryException for known exceptions when interacting with the registry
*/
@VisibleForTesting
@Nullable
private T call(RequestState requestState) throws IOException, RegistryException {
T call(RequestState requestState) throws IOException, RegistryException {
boolean isHttpProtocol = "http".equals(requestState.url.getProtocol());
if (!allowHttp && isHttpProtocol) {
throw new InsecureRegistryException(requestState.url);
Expand Down Expand Up @@ -193,13 +196,28 @@ private T call(RequestState requestState) throws IOException, RegistryException

throw registryErrorExceptionBuilder.build();

} else if (httpResponseException.getStatusCode() == HttpStatusCodes.STATUS_CODE_UNAUTHORIZED
|| httpResponseException.getStatusCode() == HttpStatusCodes.STATUS_CODE_FORBIDDEN) {
} else if (httpResponseException.getStatusCode() == HttpStatusCodes.STATUS_CODE_FORBIDDEN) {
throw new RegistryUnauthorizedException(
registryEndpointRequestProperties.getServerUrl(),
registryEndpointRequestProperties.getImageName(),
httpResponseException);

} else if (httpResponseException.getStatusCode()
== HttpStatusCodes.STATUS_CODE_UNAUTHORIZED) {
if (isHttpProtocol) {
// Using HTTP, so credentials weren't sent.
throw new RegistryCredentialsNotSentException(
registryEndpointRequestProperties.getServerUrl(),
registryEndpointRequestProperties.getImageName());

} else {
// Using HTTPS, so credentials are missing.
throw new RegistryUnauthorizedException(
registryEndpointRequestProperties.getServerUrl(),
registryEndpointRequestProperties.getImageName(),
httpResponseException);
}

} else if (httpResponseException.getStatusCode()
== HttpStatusCodes.STATUS_CODE_TEMPORARY_REDIRECT
|| httpResponseException.getStatusCode()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@
import com.google.cloud.tools.jib.cache.CacheMetadataCorruptedException;
import com.google.cloud.tools.jib.configuration.CacheConfiguration;
import com.google.cloud.tools.jib.registry.InsecureRegistryException;
import com.google.cloud.tools.jib.registry.RegistryCredentialsNotSentException;
import com.google.cloud.tools.jib.registry.RegistryUnauthorizedException;
import com.google.common.collect.ImmutableList;
import java.io.IOException;
Expand Down Expand Up @@ -64,6 +65,7 @@ public class BuildStepsRunnerTest {
@Mock private SourceFilesConfiguration mockSourceFilesConfiguration;
@Mock private BuildLogger mockBuildLogger;
@Mock private RegistryUnauthorizedException mockRegistryUnauthorizedException;
@Mock private RegistryCredentialsNotSentException mockRegistryCredentialsNotSentException;
@Mock private HttpResponseException mockHttpResponseException;
@Mock private ExecutionException mockExecutionException;
@Mock private BuildConfiguration mockBuildConfiguration;
Expand Down Expand Up @@ -215,6 +217,24 @@ public void testBuildImage_executionException_registryUnauthorizedException_noCr
}
}

@Test
public void testBuildImage_executionException_registryCredentialsNotSentException()
throws InterruptedException, ExecutionException, CacheMetadataCorruptedException, IOException,
CacheDirectoryNotOwnedException, CacheDirectoryCreationException {
Mockito.when(mockExecutionException.getCause())
.thenReturn(mockRegistryCredentialsNotSentException);
Mockito.doThrow(mockExecutionException).when(mockBuildSteps).run();

try {
testBuildImageStepsRunner.build(TEST_HELPFUL_SUGGESTIONS);
Assert.fail("buildImage should have thrown an exception");

} catch (BuildStepsExecutionException ex) {
Assert.assertEquals(TEST_HELPFUL_SUGGESTIONS.forCredentialsNotSent(), ex.getMessage());
Assert.assertEquals(mockRegistryCredentialsNotSentException, ex.getCause());
}
}

@Test
public void testBuildImage_executionException_registryUnauthorizedException_other()
throws InterruptedException, ExecutionException, CacheMetadataCorruptedException, IOException,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -68,5 +68,8 @@ public void testSuggestions_smoke() {
"messagePrefix, perhaps you should add a parameter configuration parameter to your buildFile or set the parameter via the commandline (e.g. 'command').",
TEST_HELPFUL_SUGGESTIONS.forToNotConfigured("parameter", "buildFile", "command"));
Assert.assertEquals("messagePrefix", TEST_HELPFUL_SUGGESTIONS.none());
Assert.assertEquals(
"messagePrefix, perhaps you should use a registry that supports HTTPS so credentials can be sent safely",
TEST_HELPFUL_SUGGESTIONS.forCredentialsNotSent());
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@
import com.google.cloud.tools.jib.http.Connection;
import com.google.cloud.tools.jib.http.Response;
import com.google.cloud.tools.jib.json.JsonTemplateMapper;
import com.google.cloud.tools.jib.registry.RegistryEndpointCaller.RequestState;
import com.google.cloud.tools.jib.registry.json.ErrorEntryTemplate;
import com.google.cloud.tools.jib.registry.json.ErrorResponseTemplate;
import java.io.IOException;
Expand Down Expand Up @@ -143,6 +144,40 @@ public void testCall_unauthorized() throws IOException, RegistryException {
verifyThrowsRegistryUnauthorizedException(HttpStatusCodes.STATUS_CODE_UNAUTHORIZED);
}

@Test
public void testCall_credentialsNotSent() throws IOException, RegistryException {
// Mocks a response for temporary redirect to a new location.
Mockito.when(mockHttpResponse.getStatusCode())
.thenReturn(HttpStatusCodes.STATUS_CODE_UNAUTHORIZED);
Mockito.when(mockHttpResponse.getHeaders())
.thenReturn(new HttpHeaders().setLocation("http://location"));

HttpResponseException httpResponseException = new HttpResponseException(mockHttpResponse);
Mockito.when(mockConnection.send(Mockito.eq("httpMethod"), Mockito.any()))
.thenThrow(httpResponseException)
.thenReturn(mockResponse);

RegistryEndpointCaller<String> testRegistryEndpointCallerInsecure =
new RegistryEndpointCaller<>(
"userAgent",
"apiRouteBase",
new TestRegistryEndpointProvider(),
Authorizations.withBasicToken("token"),
new RegistryEndpointRequestProperties("serverUrl", "imageName"),
true,
mockConnectionFactory);
try {
testRegistryEndpointCallerInsecure.call(
new RequestState(Authorizations.withBasicToken("token"), new URL("http://location")));
Assert.fail("Call should have failed");

} catch (RegistryCredentialsNotSentException ex) {
Assert.assertEquals(
"Required credentials for serverUrl/imageName were not sent because the connection was over HTTP",
ex.getMessage());
}
}

@Test
public void testCall_forbidden() throws IOException, RegistryException {
verifyThrowsRegistryUnauthorizedException(HttpStatusCodes.STATUS_CODE_FORBIDDEN);
Expand Down

0 comments on commit cfdc014

Please sign in to comment.