Details
diff --git a/services/src/main/java/org/keycloak/authentication/authenticators/client/ClientIdAndSecretAuthenticator.java b/services/src/main/java/org/keycloak/authentication/authenticators/client/ClientIdAndSecretAuthenticator.java
index e86e68e..b96e101 100644
--- a/services/src/main/java/org/keycloak/authentication/authenticators/client/ClientIdAndSecretAuthenticator.java
+++ b/services/src/main/java/org/keycloak/authentication/authenticators/client/ClientIdAndSecretAuthenticator.java
@@ -86,13 +86,13 @@ public class ClientIdAndSecretAuthenticator extends AbstractClientAuthenticator
return;
}
+ context.setClient(client);
+
if (!client.isEnabled()) {
context.failure(AuthenticationFlowError.CLIENT_DISABLED, null);
return;
}
- context.setClient(client);
-
// Skip client_secret validation for public client
if (client.isPublicClient()) {
context.success();
@@ -106,8 +106,8 @@ public class ClientIdAndSecretAuthenticator extends AbstractClientAuthenticator
}
if (client.getSecret() == null) {
- Response challengeResponse = ClientAuthUtil.errorResponse(Response.Status.BAD_REQUEST.getStatusCode(), "unauthorized_client", "Client secret setup required for client " + client.getClientId());
- context.challenge(challengeResponse);
+ Response challengeResponse = ClientAuthUtil.errorResponse(Response.Status.BAD_REQUEST.getStatusCode(), "unauthorized_client", "Invalid client secret");
+ context.failure(AuthenticationFlowError.INVALID_CLIENT_CREDENTIALS, challengeResponse);
return;
}
diff --git a/services/src/main/java/org/keycloak/authentication/ClientAuthenticationFlow.java b/services/src/main/java/org/keycloak/authentication/ClientAuthenticationFlow.java
index 2065de0..225491f 100644
--- a/services/src/main/java/org/keycloak/authentication/ClientAuthenticationFlow.java
+++ b/services/src/main/java/org/keycloak/authentication/ClientAuthenticationFlow.java
@@ -48,12 +48,9 @@ public class ClientAuthenticationFlow implements AuthenticationFlow {
AuthenticationProcessor.Result context = processor.createClientAuthenticatorContext(model, authenticator, executions);
authenticator.authenticateClient(context);
- Response response = processResult(context);
- if (response != null) return response;
ClientModel client = processor.getClient();
if (client != null) {
-
String expectedClientAuthType = client.getClientAuthenticatorType();
// Fallback to secret just in case (for backwards compatibility)
@@ -64,12 +61,16 @@ public class ClientAuthenticationFlow implements AuthenticationFlow {
// Check if client authentication matches
if (factory.getId().equals(expectedClientAuthType)) {
+ Response response = processResult(context);
+ if (response != null) return response;
+
+ if (!context.getStatus().equals(FlowStatus.SUCCESS)) {
+ throw new AuthenticationFlowException("Expected success, but for an unknown reason the status was " + context.getStatus(), AuthenticationFlowError.INTERNAL_ERROR);
+ }
+
AuthenticationProcessor.logger.debugv("Client {0} authenticated by {1}", client.getClientId(), factory.getId());
processor.getEvent().detail(Details.CLIENT_AUTH_METHOD, factory.getId());
return null;
- } else {
- throw new AuthenticationFlowException("Client " + client.getClientId() + " was authenticated by incorrect method " + factory.getId(),
- AuthenticationFlowError.INVALID_CLIENT_CREDENTIALS);
}
}
}
@@ -116,7 +117,9 @@ public class ClientAuthenticationFlow implements AuthenticationFlow {
if (status == FlowStatus.SUCCESS) {
return null;
- } else if (status == FlowStatus.FAILED) {
+ }
+
+ if (status == FlowStatus.FAILED) {
if (result.getChallenge() != null) {
return sendChallenge(result, execution);
} else {
@@ -130,11 +133,9 @@ public class ClientAuthenticationFlow implements AuthenticationFlow {
if (alternativeChallenge == null) {
alternativeChallenge = result.getChallenge();
}
- return null;
+ return sendChallenge(result, execution);
} else if (status == FlowStatus.FAILURE_CHALLENGE) {
return sendChallenge(result, execution);
- } else if (status == FlowStatus.ATTEMPTED) {
- return null;
} else {
AuthenticationProcessor.logger.error("Unknown result status");
throw new AuthenticationFlowException(AuthenticationFlowError.INTERNAL_ERROR);
diff --git a/testsuite/integration/src/test/java/org/keycloak/testsuite/oauth/AccessTokenTest.java b/testsuite/integration/src/test/java/org/keycloak/testsuite/oauth/AccessTokenTest.java
index cbfc76f..7d9c2b2 100755
--- a/testsuite/integration/src/test/java/org/keycloak/testsuite/oauth/AccessTokenTest.java
+++ b/testsuite/integration/src/test/java/org/keycloak/testsuite/oauth/AccessTokenTest.java
@@ -165,6 +165,21 @@ public class AccessTokenTest {
}
@Test
+ public void accessTokenMissingClientCredentials() throws Exception {
+ oauth.doLogin("test-user@localhost", "password");
+
+ Event loginEvent = events.expectLogin().assertEvent();
+ String codeId = loginEvent.getDetails().get(Details.CODE_ID);
+
+ String code = oauth.getCurrentQuery().get(OAuth2Constants.CODE);
+ AccessTokenResponse response = oauth.doAccessTokenRequest(code, null);
+ Assert.assertEquals(400, response.getStatusCode());
+
+ AssertEvents.ExpectedEvent expectedEvent = events.expectCodeToToken(codeId, loginEvent.getSessionId()).error("invalid_client_credentials").clearDetails().user((String) null).session((String) null);
+ expectedEvent.assertEvent();
+ }
+
+ @Test
public void accessTokenInvalidRedirectUri() throws Exception {
oauth.doLogin("test-user@localhost", "password");
diff --git a/testsuite/integration/src/test/java/org/keycloak/testsuite/oauth/ResourceOwnerPasswordCredentialsGrantTest.java b/testsuite/integration/src/test/java/org/keycloak/testsuite/oauth/ResourceOwnerPasswordCredentialsGrantTest.java
index 2529091..23cb7f2 100755
--- a/testsuite/integration/src/test/java/org/keycloak/testsuite/oauth/ResourceOwnerPasswordCredentialsGrantTest.java
+++ b/testsuite/integration/src/test/java/org/keycloak/testsuite/oauth/ResourceOwnerPasswordCredentialsGrantTest.java
@@ -40,6 +40,9 @@ public class ResourceOwnerPasswordCredentialsGrantTest {
ClientModel app = new ClientManager(manager).createClient(appRealm, "resource-owner");
app.setSecret("secret");
+ ClientModel app2 = new ClientManager(manager).createClient(appRealm, "resource-owner-public");
+ app2.setPublicClient(true);
+
UserModel user = session.users().addUser(appRealm, "direct-login");
user.setEmail("direct-login@localhost");
user.setEnabled(true);
@@ -66,16 +69,22 @@ public class ResourceOwnerPasswordCredentialsGrantTest {
@Test
public void grantAccessTokenUsername() throws Exception {
- grantAccessToken("direct-login");
+ grantAccessToken("direct-login", "resource-owner");
}
@Test
public void grantAccessTokenEmail() throws Exception {
- grantAccessToken("direct-login@localhost");
+ grantAccessToken("direct-login@localhost", "resource-owner");
}
- private void grantAccessToken(String login) throws Exception {
- oauth.clientId("resource-owner");
+ @Test
+ public void grantAccessTokenPublic() throws Exception {
+ grantAccessToken("direct-login", "resource-owner-public");
+ }
+
+
+ private void grantAccessToken(String login, String clientId) throws Exception {
+ oauth.clientId(clientId);
OAuthClient.AccessTokenResponse response = oauth.doGrantAccessTokenRequest("secret", login, "password");
@@ -85,7 +94,7 @@ public class ResourceOwnerPasswordCredentialsGrantTest {
RefreshToken refreshToken = oauth.verifyRefreshToken(response.getRefreshToken());
events.expectLogin()
- .client("resource-owner")
+ .client(clientId)
.user(userId)
.session(accessToken.getSessionState())
.detail(Details.RESPONSE_TYPE, "token")
@@ -107,7 +116,7 @@ public class ResourceOwnerPasswordCredentialsGrantTest {
assertEquals(accessToken.getSessionState(), refreshedAccessToken.getSessionState());
assertEquals(accessToken.getSessionState(), refreshedRefreshToken.getSessionState());
- events.expectRefresh(refreshToken.getId(), refreshToken.getSessionState()).user(userId).client("resource-owner").assertEvent();
+ events.expectRefresh(refreshToken.getId(), refreshToken.getSessionState()).user(userId).client(clientId).assertEvent();
}
@Test
@@ -147,8 +156,6 @@ public class ResourceOwnerPasswordCredentialsGrantTest {
.error(Errors.INVALID_TOKEN).assertEvent();
}
-
-
@Test
public void grantAccessTokenInvalidClientCredentials() throws Exception {
oauth.clientId("resource-owner");
@@ -169,6 +176,25 @@ public class ResourceOwnerPasswordCredentialsGrantTest {
}
@Test
+ public void grantAccessTokenMissingClientCredentials() throws Exception {
+ oauth.clientId("resource-owner");
+
+ OAuthClient.AccessTokenResponse response = oauth.doGrantAccessTokenRequest(null, "test-user@localhost", "password");
+
+ assertEquals(400, response.getStatusCode());
+
+ assertEquals("unauthorized_client", response.getError());
+
+ events.expectLogin()
+ .client("resource-owner")
+ .session((String) null)
+ .clearDetails()
+ .error(Errors.INVALID_CLIENT_CREDENTIALS)
+ .user((String) null)
+ .assertEvent();
+ }
+
+ @Test
public void grantAccessTokenVerifyEmail() throws Exception {
keycloakRule.update(new KeycloakRule.KeycloakSetup() {