Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
Original file line number Diff line number Diff line change
@@ -1,8 +1,11 @@
package org.keycloak.protocol.docker;

import java.util.Arrays;
import java.util.Collections;
import java.util.List;
import java.util.Locale;
import java.util.Optional;
import java.util.stream.Collectors;

import jakarta.ws.rs.core.HttpHeaders;
import jakarta.ws.rs.core.MediaType;
Expand Down Expand Up @@ -42,8 +45,7 @@ protected void nullUserAction(final AuthenticationFlowContext context, final Rea
protected void userDisabledAction(AuthenticationFlowContext context, RealmModel realm, UserModel user, String eventError) {
context.getEvent().user(user);
context.getEvent().error(eventError);
final DockerError error = new DockerError("UNAUTHORIZED","Invalid username or password.",
Collections.singletonList(new DockerAccess(context.getAuthenticationSession().getClientNote(DockerAuthV2Protocol.SCOPE_PARAM))));
final DockerError error = new DockerError("UNAUTHORIZED","Invalid username or password.", requestedAccessItems(context));
context.failure(AuthenticationFlowError.USER_DISABLED, new ResponseBuilderImpl()
.status(Response.Status.UNAUTHORIZED)
.header(HttpHeaders.CONTENT_TYPE, MediaType.APPLICATION_JSON)
Expand All @@ -59,8 +61,7 @@ private void invalidUserAction(final AuthenticationFlowContext context, final Re
context.getEvent().user(userId);
context.getEvent().error(Errors.INVALID_USER_CREDENTIALS);

final DockerError error = new DockerError("UNAUTHORIZED","Invalid username or password.",
Collections.singletonList(new DockerAccess(context.getAuthenticationSession().getClientNote(DockerAuthV2Protocol.SCOPE_PARAM))));
final DockerError error = new DockerError("UNAUTHORIZED","Invalid username or password.", requestedAccessItems(context));

context.failure(AuthenticationFlowError.INVALID_USER, new ResponseBuilderImpl()
.status(Response.Status.UNAUTHORIZED)
Expand All @@ -73,4 +74,16 @@ private void invalidUserAction(final AuthenticationFlowContext context, final Re
public boolean configuredFor(KeycloakSession session, RealmModel realm, UserModel user) {
return true;
}

private static List<DockerAccess> requestedAccessItems(final AuthenticationFlowContext context) {
final String requestedScopes = context.getAuthenticationSession().getClientNote(DockerAuthV2Protocol.SCOPE_PARAM);
if (requestedScopes == null || requestedScopes.isBlank()) {
return Collections.singletonList(new DockerAccess(null));
}

return Arrays.stream(requestedScopes.split(" "))
.filter(scope -> !scope.isBlank())
.map(DockerAccess::new)
.collect(Collectors.toList());
}
}
Original file line number Diff line number Diff line change
@@ -1,6 +1,9 @@
package org.keycloak.protocol.docker;

import java.util.List;

import jakarta.ws.rs.GET;
import jakarta.ws.rs.core.MultivaluedHashMap;
import jakarta.ws.rs.core.MultivaluedMap;
import jakarta.ws.rs.core.Response;

Expand All @@ -16,6 +19,7 @@
import org.keycloak.protocol.AuthorizationEndpointBase;
import org.keycloak.protocol.oidc.endpoints.request.AuthorizationEndpointRequest;
import org.keycloak.protocol.oidc.endpoints.request.AuthorizationEndpointRequestParserProcessor;
import org.keycloak.representations.docker.DockerAccess;
import org.keycloak.services.ErrorResponseException;
import org.keycloak.services.Urls;
import org.keycloak.services.managers.AuthenticationManager;
Expand Down Expand Up @@ -49,21 +53,40 @@ public DockerEndpoint(KeycloakSession session, final EventBuilder event, final E
public Response build() {
ProfileHelper.requireFeature(Profile.Feature.DOCKER);

final MultivaluedMap<String, String> params = session.getContext().getUri().getQueryParameters();
final MultivaluedMap<String, String> rawParams = session.getContext().getUri().getQueryParameters();

account = params.getFirst(DockerAuthV2Protocol.ACCOUNT_PARAM);
account = rawParams.getFirst(DockerAuthV2Protocol.ACCOUNT_PARAM);
if (account == null) {
logger.debug("Account parameter not provided by docker auth. This is techincally required, but not actually used since " +
"username is provided by Basic auth header.");
}
service = params.getFirst(DockerAuthV2Protocol.SERVICE_PARAM);
scope = params.getFirst(DockerAuthV2Protocol.SCOPE_PARAM);
service = rawParams.getFirst(DockerAuthV2Protocol.SERVICE_PARAM);

List<String> scopeValues = rawParams.get(DockerAuthV2Protocol.SCOPE_PARAM);
scope = (scopeValues == null || scopeValues.isEmpty()) ? null : String.join(" ", scopeValues);
Comment thread
jimmychakkalakal marked this conversation as resolved.
Resolved
Comment thread
Copilot marked this conversation as resolved.
Resolved

final MultivaluedMap<String, String> params;
if (scopeValues != null && scopeValues.size() > 1) {
params = new MultivaluedHashMap<>(rawParams);
params.putSingle(DockerAuthV2Protocol.SCOPE_PARAM, scope);
} else {
params = rawParams;
}

checkSsl();
checkRealm();
checkService();
checkScopes(scopeValues);

final AuthorizationEndpointRequest authRequest = AuthorizationEndpointRequestParserProcessor.parseRequest(event, session, client, params, AuthorizationEndpointRequestParserProcessor.EndpointType.DOCKER_ENDPOINT);

if (authRequest.getInvalidRequestMessage() != null && !authRequest.getInvalidRequestMessage().isBlank()) {
event.detail(Details.REASON, authRequest.getInvalidRequestMessage());
event.error(Errors.INVALID_REQUEST);

throw new ErrorResponseException("invalid_request", authRequest.getInvalidRequestMessage(), Response.Status.BAD_REQUEST);
}

authenticationSession = createAuthenticationSession(client, authRequest.getState());

updateAuthenticationSession();
Expand All @@ -74,6 +97,25 @@ public Response build() {
return handleBrowserAuthenticationRequest(authenticationSession, new DockerAuthV2Protocol(session, realm, session.getContext().getUri(), headers, event), false, false);
}

private void checkScopes(List<String> scopeValues) {
if (scopeValues == null) {
return;
}

for (final String scopeValue : scopeValues) {
for (final String requestedScope : scopeValue.split(" ")) {
try {
// DockerAccess is the single source of truth for the scope format, so let it do the parsing
new DockerAccess(requestedScope);
} catch (final IllegalArgumentException e) {
event.detail(Details.REASON, "Invalid parameter: " + DockerAuthV2Protocol.SCOPE_PARAM);
event.error(Errors.INVALID_REQUEST);
throw new ErrorResponseException("invalid_request", "scope parameter is malformed", Response.Status.BAD_REQUEST);
}
}
}
}

private void updateAuthenticationSession() {
authenticationSession.setProtocol(DockerAuthV2Protocol.LOGIN_PROTOCOL);
authenticationSession.setAction(CommonClientSessionModel.Action.AUTHENTICATE.name());
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -180,7 +180,8 @@ public void shouldPerformDockerAuthAgainstRegistry() throws Exception {
result = dockerClientContainer.execInContainer("docker", "logout");
printCommandResult(result);
assertThat("Error performing logout", result.getExitCode(), is(0));
assertLogin(dockerUser);

events.skipAll();

ClientResource client = AdminApiUtil.findClientByClientId(managedRealm.admin(), CLIENT_ID);
ClientRepresentation clientRep = client.toRepresentation();
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,202 @@
package org.keycloak.tests.docker;

import java.io.IOException;
import java.nio.charset.StandardCharsets;
import java.util.Base64;
import java.util.Map;

import org.keycloak.common.Profile;
import org.keycloak.events.Details;
import org.keycloak.events.Errors;
import org.keycloak.events.EventType;
import org.keycloak.protocol.docker.DockerAuthV2Protocol;
import org.keycloak.protocol.oidc.OIDCConfigAttributes;
import org.keycloak.representations.idm.RealmRepresentation;
import org.keycloak.testframework.annotations.InjectEvents;
import org.keycloak.testframework.annotations.InjectHttpClient;
import org.keycloak.testframework.annotations.InjectRealm;
import org.keycloak.testframework.annotations.KeycloakIntegrationTest;
import org.keycloak.testframework.events.EventAssertion;
import org.keycloak.testframework.events.Events;
import org.keycloak.testframework.realm.ManagedRealm;
import org.keycloak.testframework.realm.RealmBuilder;
import org.keycloak.testframework.realm.RealmConfig;
import org.keycloak.testframework.server.KeycloakServerConfig;
import org.keycloak.testframework.server.KeycloakServerConfigBuilder;

import org.apache.http.HttpHeaders;
import org.apache.http.client.methods.CloseableHttpResponse;
import org.apache.http.client.methods.HttpGet;
import org.apache.http.impl.client.CloseableHttpClient;
import org.apache.http.util.EntityUtils;
import org.junit.jupiter.api.Test;

import static org.hamcrest.MatcherAssert.assertThat;
import static org.hamcrest.Matchers.containsString;
import static org.hamcrest.Matchers.is;

@KeycloakIntegrationTest(config = DockerEndpointInvalidRequestTest.DockerServerConfig.class)
public class DockerEndpointInvalidRequestTest {

private static final String REALM_ID = "docker-invalid-request-realm";
private static final String CLIENT_ID = "docker-test-client";
private static final String REQUEST_URI_CLIENT_ID = "docker-request-uri-client";
private static final String DOCKER_USER = "docker-user";
private static final String DOCKER_USER_PASSWORD = "password";

@InjectRealm(config = DockerRealmConfig.class)
ManagedRealm managedRealm;

@InjectHttpClient
CloseableHttpClient httpClient;

@InjectEvents
Events events;

@Test
public void shouldRejectDuplicatedParameter() throws IOException {
// 'state' is sent twice, so the request cannot be parsed unambiguously
try (CloseableHttpResponse response = get(authUrl() + "&scope=repository:foo:pull&state=one&state=two")) {
assertThat(response.getStatusLine().getStatusCode(), is(400));

String body = EntityUtils.toString(response.getEntity());
assertThat(body, containsString("invalid_request"));
assertThat(body, containsString("duplicated parameter"));
}

EventAssertion.assertError(events.poll())
.type(EventType.LOGIN_ERROR)
.clientId(CLIENT_ID)
.error(Errors.INVALID_REQUEST)
.details(Details.REASON, "duplicated parameter");
}

@Test
public void shouldNotRejectWellFormedRequest() throws IOException {
// The same parameter sent only once must still authenticate and return a docker token
try (CloseableHttpResponse response = get(authUrl() + "&scope=repository:foo:pull")) {
assertThat("A well formed request must not be rejected as malformed",
response.getStatusLine().getStatusCode(), is(200));
assertThat(EntityUtils.toString(response.getEntity()), containsString("token"));
}

EventAssertion.assertSuccess(events.poll())
.type(EventType.LOGIN)
.clientId(CLIENT_ID)
.details(Details.AUTH_METHOD, DockerAuthV2Protocol.LOGIN_PROTOCOL);
}

@Test
public void shouldSupportMultipleScopeParametersOnDockerPush() throws IOException {
// Docker registry v2 token specification sends multiple 'scope' query parameters during push operations
try (CloseableHttpResponse response = get(authUrl() + "&scope=repository:empty:pull&scope=repository:empty:pull,push")) {
Comment thread
jimmychakkalakal marked this conversation as resolved.
Resolved
assertThat("Multiple scope parameters must be accepted for Docker push",
response.getStatusLine().getStatusCode(), is(200));
assertThat(EntityUtils.toString(response.getEntity()), containsString("token"));
}

EventAssertion.assertSuccess(events.poll())
.type(EventType.LOGIN)
.clientId(CLIENT_ID)
.details(Details.AUTH_METHOD, DockerAuthV2Protocol.LOGIN_PROTOCOL);
}

@Test
public void shouldReturnDockerErrorForMultipleScopesWithInvalidCredentials() throws IOException {
// Multiple scopes are stored as one space delimited note, which must still map to one access item each
try (CloseableHttpResponse response = get(authUrl() + "&scope=repository:empty:pull&scope=repository:other:pull,push", "wrong-password")) {
assertThat("Invalid credentials must produce a docker 401, not a server error",
response.getStatusLine().getStatusCode(), is(401));

String body = EntityUtils.toString(response.getEntity());
assertThat(body, containsString("UNAUTHORIZED"));
assertThat(body, containsString("empty"));
assertThat(body, containsString("other"));
}

EventAssertion.assertError(events.poll())
.type(EventType.LOGIN_ERROR)
.clientId(CLIENT_ID)
.error(Errors.INVALID_USER_CREDENTIALS);
}

@Test
public void shouldEnforceRequestUriPolicyForSingleScope() throws IOException {
// Baseline: a client restricted to 'request_uri only' must not authenticate without a request object
try (CloseableHttpResponse response = get(authUrl(REQUEST_URI_CLIENT_ID) + "&scope=repository:foo:pull")) {
assertThat(response.getStatusLine().getStatusCode(), is(400));
}

EventAssertion.assertError(events.poll())
.type(EventType.LOGIN_ERROR)
.clientId(REQUEST_URI_CLIENT_ID)
.error(Errors.INVALID_REQUEST)
.details(Details.REASON, "Client is required to use 'request_uri' parameter.");
}

@Test
public void shouldEnforceRequestUriPolicyForRepeatedScopes() throws IOException {
// Repeating 'scope' used to skip the request object policy entirely and return a docker token
try (CloseableHttpResponse response = get(authUrl(REQUEST_URI_CLIENT_ID)
+ "&scope=repository:foo:pull&scope=repository:foo:pull,push")) {
assertThat("Repeated scopes must not bypass the request object policy",
response.getStatusLine().getStatusCode(), is(400));
}

EventAssertion.assertError(events.poll())
.type(EventType.LOGIN_ERROR)
.clientId(REQUEST_URI_CLIENT_ID)
.error(Errors.INVALID_REQUEST)
.details(Details.REASON, "Client is required to use 'request_uri' parameter.");
}

private CloseableHttpResponse get(String url) throws IOException {
HttpGet get = new HttpGet(url);
String credentials = DOCKER_USER + ":" + DOCKER_USER_PASSWORD;
get.setHeader(HttpHeaders.AUTHORIZATION,
"Basic " + Base64.getEncoder().encodeToString(credentials.getBytes(StandardCharsets.UTF_8)));
return httpClient.execute(get);
}

private CloseableHttpResponse get(String url, String password) throws IOException {
HttpGet get = new HttpGet(url);
String credentials = DOCKER_USER + ":" + password;
get.setHeader(HttpHeaders.AUTHORIZATION,
"Basic " + Base64.getEncoder().encodeToString(credentials.getBytes(StandardCharsets.UTF_8)));
return httpClient.execute(get);
}

private String authUrl() {
return authUrl(CLIENT_ID);
}

private String authUrl(String clientId) {
return managedRealm.getBaseUrl() + "/protocol/docker-v2/auth"
+ "?service=" + clientId
+ "&account=" + DOCKER_USER
+ "&client_id=docker"
+ "&offline_token=true";
}

public static class DockerServerConfig implements KeycloakServerConfig {
@Override
public KeycloakServerConfigBuilder configure(KeycloakServerConfigBuilder config) {
return config.features(Profile.Feature.DOCKER);
}
}

public static class DockerRealmConfig implements RealmConfig {
@Override
public RealmBuilder configure(RealmBuilder realmBuilder) {
RealmRepresentation dockerRealm = DockerTestRealmSetup.createRealm(REALM_ID);
DockerTestRealmSetup.configureDockerRegistryClient(dockerRealm, CLIENT_ID);
// A client that may only authenticate through a request object, used to verify the policy is enforced
DockerTestRealmSetup.configureDockerRegistryClient(dockerRealm, REQUEST_URI_CLIENT_ID);
dockerRealm.getClients().stream()
.filter(client -> REQUEST_URI_CLIENT_ID.equals(client.getClientId()))
.forEach(client -> client.setAttributes(Map.of(OIDCConfigAttributes.REQUEST_OBJECT_REQUIRED, OIDCConfigAttributes.REQUEST_OBJECT_REQUIRED_REQUEST_URI)));
DockerTestRealmSetup.configureUser(dockerRealm, DOCKER_USER, DOCKER_USER_PASSWORD);
return RealmBuilder.update(dockerRealm);
}
}
}
Loading