Incorrect error message OIDC client authentication (#14656)

closes #12162


Co-authored-by: Pedro Hos <pedro-hos@outlook.com>
This commit is contained in:
Marcelo Daniel Silva Sales
2022-09-30 09:40:05 +02:00
committed by GitHub
parent 581def56d6
commit 22713bc144
13 changed files with 48 additions and 27 deletions

View File

@@ -836,16 +836,16 @@ public class AuthenticationProcessor {
ServicesLogger.LOGGER.failedClientAuthentication(e);
if (e.getError() == AuthenticationFlowError.CLIENT_NOT_FOUND) {
event.error(Errors.CLIENT_NOT_FOUND);
return ClientAuthUtil.errorResponse(Response.Status.BAD_REQUEST.getStatusCode(), "unauthorized_client", "Invalid client credentials");
return ClientAuthUtil.errorResponse(Response.Status.UNAUTHORIZED.getStatusCode(), "invalid_client", "Invalid client or Invalid client credentials");
} else if (e.getError() == AuthenticationFlowError.CLIENT_DISABLED) {
event.error(Errors.CLIENT_DISABLED);
return ClientAuthUtil.errorResponse(Response.Status.BAD_REQUEST.getStatusCode(), "unauthorized_client", "Invalid client credentials");
return ClientAuthUtil.errorResponse(Response.Status.UNAUTHORIZED.getStatusCode(), "invalid_client", "Invalid client or Invalid client credentials");
} else if (e.getError() == AuthenticationFlowError.CLIENT_CREDENTIALS_SETUP_REQUIRED) {
event.error(Errors.INVALID_CLIENT_CREDENTIALS);
return ClientAuthUtil.errorResponse(Response.Status.BAD_REQUEST.getStatusCode(), "unauthorized_client", "Client credentials setup required");
} else {
event.error(Errors.INVALID_CLIENT_CREDENTIALS);
return ClientAuthUtil.errorResponse(Response.Status.BAD_REQUEST.getStatusCode(), "invalid_client", "Invalid client credentials");
return ClientAuthUtil.errorResponse(Response.Status.UNAUTHORIZED.getStatusCode(), "invalid_client", "Invalid client or Invalid client credentials");
}
} else {
ServicesLogger.LOGGER.errorAuthenticatingClient(failure);

View File

@@ -107,7 +107,8 @@ public class ClientAuthenticationFlow implements AuthenticationFlow {
processor.getEvent().error(Errors.INVALID_CLIENT);
return alternativeChallenge;
}
throw new AuthenticationFlowException("Invalid client credentials", AuthenticationFlowError.INVALID_CREDENTIALS);
throw new AuthenticationFlowException("Invalid client or Invalid client credentials", AuthenticationFlowError.CLIENT_NOT_FOUND);
}
protected List<AuthenticationExecutionModel> findExecutionsToRun() {

View File

@@ -122,7 +122,7 @@ public class ClientIdAndSecretAuthenticator extends AbstractClientAuthenticator
}
if (clientSecret == null) {
Response challengeResponse = ClientAuthUtil.errorResponse(Response.Status.UNAUTHORIZED.getStatusCode(), "unauthorized_client", "Client secret not provided in request");
Response challengeResponse = ClientAuthUtil.errorResponse(Response.Status.UNAUTHORIZED.getStatusCode(), "unauthorized_client", "Invalid client or Invalid client credentials");
context.challenge(challengeResponse);
return;
}
@@ -210,7 +210,7 @@ public class ClientIdAndSecretAuthenticator extends AbstractClientAuthenticator
}
private void reportFailedAuth(ClientAuthenticationFlowContext context) {
Response challengeResponse = ClientAuthUtil.errorResponse(Response.Status.UNAUTHORIZED.getStatusCode(), "unauthorized_client", "Invalid client secret");
Response challengeResponse = ClientAuthUtil.errorResponse(Response.Status.UNAUTHORIZED.getStatusCode(), "unauthorized_client", "Invalid client or Invalid client credentials");
context.failure(AuthenticationFlowError.INVALID_CLIENT_CREDENTIALS, challengeResponse);
}
}

View File

@@ -62,6 +62,11 @@ public class AuthorizeClientUtil {
if (client == null) {
throwErrorResponseException(Errors.INVALID_CLIENT, "Client authentication ended, but client is null", Response.Status.BAD_REQUEST, cors.allowAllOrigins());
}
if(!client.isEnabled()) {
event.error(Errors.CLIENT_DISABLED);
throwErrorResponseException(Errors.INVALID_CLIENT, "Invalid client or Invalid client credentials", Response.Status.UNAUTHORIZED, cors.allowAllOrigins());
}
if (cors != null) {
cors.allowedOrigins(session, client);

View File

@@ -492,7 +492,7 @@ public class KcAdmTest extends AbstractAdmCliTest {
assertExitCodeAndStreamSizes(exe, 1, 0, 2);
Assert.assertEquals("login message", "Logging into " + serverUrl + " as user user1 of realm test", exe.stderrLines().get(0));
Assert.assertEquals("error message", "Invalid client secret [unauthorized_client]", exe.stderrLines().get(1));
Assert.assertEquals("error message", "Invalid client or Invalid client credentials [unauthorized_client]", exe.stderrLines().get(1));
// try whole CRUD

View File

@@ -493,7 +493,7 @@ public class KcRegTest extends AbstractRegCliTest {
assertExitCodeAndStreamSizes(exe, 1, 0, 2);
Assert.assertEquals("login message", "Logging into " + serverUrl + " as user user1 of realm test", exe.stderrLines().get(0));
Assert.assertEquals("error message", "Invalid client secret [unauthorized_client]", exe.stderrLines().get(1));
Assert.assertEquals("error message", "Invalid client or Invalid client credentials [unauthorized_client]", exe.stderrLines().get(1));
// try whole CRUD

View File

@@ -237,7 +237,7 @@ public class MutualTLSClientTest extends AbstractTestRealmKeycloakTest {
}
private void assertTokenNotObtained(OAuthClient.AccessTokenResponse token) {
Assert.assertEquals(400, token.getStatusCode());
Assert.assertEquals(401, token.getStatusCode());
Assert.assertNull(token.getAccessToken());
}

View File

@@ -337,7 +337,7 @@ public class CustomFlowTest extends AbstractFlowTest {
testingClient.testing().updateAuthenticator(state);
OAuthClient.AccessTokenResponse response = oauth.doGrantAccessTokenRequest("password", "test-user", "password");
assertEquals(400, response.getStatusCode());
assertEquals(401, response.getStatusCode());
assertEquals("invalid_client", response.getError());
events.expectLogin()
@@ -347,7 +347,7 @@ public class CustomFlowTest extends AbstractFlowTest {
.removeDetail(Details.CODE_ID)
.removeDetail(Details.REDIRECT_URI)
.removeDetail(Details.CONSENT)
.error(Errors.INVALID_CLIENT_CREDENTIALS)
.error(Errors.CLIENT_NOT_FOUND)
.assertEvent();
state.setClientId("test-app");

View File

@@ -656,7 +656,7 @@ public class AccessTokenTest extends AbstractKeycloakTest {
Response response = executeGrantAccessTokenRequest(grantTarget);
assertEquals(400, response.getStatus());
assertEquals(401, response.getStatus());
response.close();
{

View File

@@ -147,7 +147,7 @@ public class ClientAuthSecretSignedJWTTest extends AbstractKeycloakTest {
OAuthClient.AccessTokenResponse response = doAccessTokenRequest(code,
jwt);
assertEquals(400, response.getStatusCode());
assertEquals(401, response.getStatusCode());
assertEquals("invalid_client", response.getError());
}

View File

@@ -90,6 +90,8 @@ import org.keycloak.testsuite.util.RealmBuilder;
import org.keycloak.testsuite.util.UserBuilder;
import org.keycloak.util.JsonSerialization;
import com.sun.jna.StringArray;
import javax.ws.rs.core.Response;
import java.io.ByteArrayInputStream;
import java.io.File;
@@ -760,7 +762,7 @@ public class ClientAuthSignedJWTTest extends AbstractKeycloakTest {
CloseableHttpResponse resp = sendRequest(oauth.getServiceAccountUrl(), parameters);
OAuthClient.AccessTokenResponse response = new OAuthClient.AccessTokenResponse(resp);
assertError(response, null, "invalid_client", Errors.INVALID_CLIENT_CREDENTIALS);
assertError(response, 401, null, "invalid_client", Errors.CLIENT_NOT_FOUND);
}
@Test
@@ -772,7 +774,8 @@ public class ClientAuthSignedJWTTest extends AbstractKeycloakTest {
CloseableHttpResponse resp = sendRequest(oauth.getServiceAccountUrl(), parameters);
OAuthClient.AccessTokenResponse response = new OAuthClient.AccessTokenResponse(resp);
assertError(response, null, "invalid_client", Errors.INVALID_CLIENT_CREDENTIALS);
assertError(response,401, null, "invalid_client", Errors.CLIENT_NOT_FOUND);
}
@Test
@@ -784,7 +787,7 @@ public class ClientAuthSignedJWTTest extends AbstractKeycloakTest {
CloseableHttpResponse resp = sendRequest(oauth.getServiceAccountUrl(), parameters);
OAuthClient.AccessTokenResponse response = new OAuthClient.AccessTokenResponse(resp);
assertError(response, null, "invalid_client", Errors.INVALID_CLIENT_CREDENTIALS);
assertError(response, 401,null, "invalid_client", Errors.CLIENT_NOT_FOUND);
}
@Test
@@ -799,7 +802,7 @@ public class ClientAuthSignedJWTTest extends AbstractKeycloakTest {
CloseableHttpResponse resp = sendRequest(oauth.getServiceAccountUrl(), parameters);
OAuthClient.AccessTokenResponse response = new OAuthClient.AccessTokenResponse(resp);
assertError(response, null, "invalid_client", Errors.INVALID_CLIENT_CREDENTIALS);
assertError(response,401, null, "invalid_client", Errors.CLIENT_NOT_FOUND);
}
@Test
@@ -814,7 +817,7 @@ public class ClientAuthSignedJWTTest extends AbstractKeycloakTest {
CloseableHttpResponse resp = sendRequest(oauth.getServiceAccountUrl(), parameters);
OAuthClient.AccessTokenResponse response = new OAuthClient.AccessTokenResponse(resp);
assertError(response, "unknown-client", "invalid_client", Errors.INVALID_CLIENT_CREDENTIALS);
assertError(response,401, "unknown-client", "invalid_client", Errors.CLIENT_NOT_FOUND);
}
@Test
@@ -832,7 +835,7 @@ public class ClientAuthSignedJWTTest extends AbstractKeycloakTest {
CloseableHttpResponse resp = sendRequest(oauth.getServiceAccountUrl(), parameters);
OAuthClient.AccessTokenResponse response = new OAuthClient.AccessTokenResponse(resp);
assertError(response, "client1", "unauthorized_client", Errors.CLIENT_DISABLED);
assertError(response,401, "client1", "invalid_client", Errors.CLIENT_DISABLED);
ClientManager.realm(adminClient.realm("test")).clientId("client1").enabled(true);
}
@@ -861,7 +864,7 @@ public class ClientAuthSignedJWTTest extends AbstractKeycloakTest {
CloseableHttpResponse resp = sendRequest(oauth.getServiceAccountUrl(), parameters);
OAuthClient.AccessTokenResponse response = new OAuthClient.AccessTokenResponse(resp);
assertError(response, "client1", OAuthErrorException.INVALID_CLIENT, "client_credentials_setup_required");
assertError(response,400, "client1", OAuthErrorException.INVALID_CLIENT, "client_credentials_setup_required");
ClientManager.realm(adminClient.realm("test")).clientId("client1").updateAttribute(JWTClientAuthenticator.CERTIFICATE_ATTR, backupClient1Cert.certificate);
}
@@ -987,6 +990,8 @@ public class ClientAuthSignedJWTTest extends AbstractKeycloakTest {
setTimeOffset(0);
assertError(response, "client1", OAuthErrorException.INVALID_CLIENT, Errors.INVALID_CLIENT_CREDENTIALS);
}
@@ -1018,19 +1023,19 @@ public class ClientAuthSignedJWTTest extends AbstractKeycloakTest {
@Test
public void testMissingIssuerClaim() throws Exception {
OAuthClient.AccessTokenResponse response = testMissingClaim("issuer");
assertError(response, null, OAuthErrorException.INVALID_CLIENT, Errors.INVALID_CLIENT_CREDENTIALS);
assertError(response,401, null, OAuthErrorException.INVALID_CLIENT, Errors.CLIENT_NOT_FOUND);
}
@Test
public void testMissingSubjectClaim() throws Exception {
OAuthClient.AccessTokenResponse response = testMissingClaim("subject");
assertError(response, null, "invalid_client", Errors.INVALID_CLIENT_CREDENTIALS);
assertError(response,401, null, "invalid_client", Errors.CLIENT_NOT_FOUND);
}
@Test
public void testMissingAudienceClaim() throws Exception {
OAuthClient.AccessTokenResponse response = testMissingClaim("audience");
assertError(response, app1.getClientId(), OAuthErrorException.INVALID_CLIENT, Errors.INVALID_CLIENT_CREDENTIALS);
assertError(response,400, app1.getClientId(), OAuthErrorException.INVALID_CLIENT, Errors.INVALID_CLIENT_CREDENTIALS);
}
@Test
@@ -1086,6 +1091,15 @@ public class ClientAuthSignedJWTTest extends AbstractKeycloakTest {
private void assertError(OAuthClient.AccessTokenResponse response, String clientId, String responseError, String eventError) {
assertEquals(400, response.getStatusCode());
assertMessageError(response,clientId,responseError,eventError);
}
private void assertError(OAuthClient.AccessTokenResponse response, int erroCode, String clientId, String responseError, String eventError) {
assertEquals(erroCode, response.getStatusCode());
assertMessageError(response, clientId, responseError, eventError);
}
private void assertMessageError(OAuthClient.AccessTokenResponse response, String clientId, String responseError, String eventError) {
assertEquals(responseError, response.getError());
events.expectClientLogin()
@@ -1097,6 +1111,7 @@ public class ClientAuthSignedJWTTest extends AbstractKeycloakTest {
.assertEvent();
}
private void assertSuccess(OAuthClient.AccessTokenResponse response, String clientId, String userId, String userName) {
assertEquals(200, response.getStatusCode());

View File

@@ -857,9 +857,9 @@ public class OAuth2DeviceAuthorizationGrantTest extends AbstractKeycloakTest {
oauth.clientId("test-device-public2");
OAuthClient.DeviceAuthorizationResponse response = oauth.doDeviceAuthorizationRequest("test-device-public2", null);
Assert.assertEquals(400, response.getStatusCode());
Assert.assertEquals(401, response.getStatusCode());
Assert.assertEquals(Errors.INVALID_CLIENT, response.getError());
Assert.assertEquals("Invalid client credentials", response.getErrorDescription());
Assert.assertEquals("Invalid client or Invalid client credentials", response.getErrorDescription());
}
@Test
public void testClientWithErrors() throws Exception {

View File

@@ -701,8 +701,8 @@ public class RefreshTokenTest extends AbstractKeycloakTest {
setTimeOffset(2);
response = oauth.doRefreshTokenRequest(refreshTokenString, "password");
assertEquals(400, response.getStatusCode());
assertEquals("unauthorized_client", response.getError());
assertEquals(401, response.getStatusCode());
assertEquals("invalid_client", response.getError());
events.expectRefresh(refreshToken.getId(), sessionId).user((String) null).session((String) null).clearDetails().error(Errors.CLIENT_DISABLED).assertEvent();
} finally {