From 6255ebe6b537c9502cb5ae5163704c4681fbdee3 Mon Sep 17 00:00:00 2001 From: Juan Manuel Rodriguez Alvarado Date: Fri, 11 Dec 2020 19:39:01 -0600 Subject: [PATCH] [KEYCLOAK-16536] Implement Audit Events for Authorization Services requests --- .../java/org/keycloak/events/Details.java | 1 + .../main/java/org/keycloak/events/Errors.java | 3 + .../AuthorizationTokenService.java | 74 ++++++++++++++----- .../oidc/endpoints/TokenEndpoint.java | 16 +++- .../services/CorsErrorResponseException.java | 4 + .../authz/AbstractResourceServerTest.java | 1 + .../testsuite/authz/EntitlementAPITest.java | 15 ++++ .../authz/UserManagedAccessTest.java | 45 ++++++++++- 8 files changed, 137 insertions(+), 22 deletions(-) diff --git a/server-spi-private/src/main/java/org/keycloak/events/Details.java b/server-spi-private/src/main/java/org/keycloak/events/Details.java index 4e681cbbe1..7eaedddf80 100755 --- a/server-spi-private/src/main/java/org/keycloak/events/Details.java +++ b/server-spi-private/src/main/java/org/keycloak/events/Details.java @@ -47,6 +47,7 @@ public interface Details { String REASON = "reason"; String REVOKED_CLIENT = "revoked_client"; String AUDIENCE = "audience"; + String PERMISSION = "permission"; String SCOPE = "scope"; String REQUESTED_ISSUER = "requested_issuer"; String REQUESTED_SUBJECT = "requested_subject"; diff --git a/server-spi-private/src/main/java/org/keycloak/events/Errors.java b/server-spi-private/src/main/java/org/keycloak/events/Errors.java index a3acfc6a1b..a02d33e3b4 100755 --- a/server-spi-private/src/main/java/org/keycloak/events/Errors.java +++ b/server-spi-private/src/main/java/org/keycloak/events/Errors.java @@ -100,4 +100,7 @@ public interface Errors { String INVALID_SAML_DOCUMENT = "invalid_saml_document"; String UNSUPPORTED_NAMEID_FORMAT = "unsupported_nameid_format"; + String INVALID_PERMISSION_TICKET = "invalid_permission_ticket"; + String ACCESS_DENIED = "access_denied"; + } diff --git a/services/src/main/java/org/keycloak/authorization/authorization/AuthorizationTokenService.java b/services/src/main/java/org/keycloak/authorization/authorization/AuthorizationTokenService.java index c4523a276f..343a185614 100644 --- a/services/src/main/java/org/keycloak/authorization/authorization/AuthorizationTokenService.java +++ b/services/src/main/java/org/keycloak/authorization/authorization/AuthorizationTokenService.java @@ -174,19 +174,27 @@ public class AuthorizationTokenService { } private static void fireErrorEvent(EventBuilder event, String error, Exception cause) { - event.detail(Details.REASON, cause == null || cause.getMessage() == null ? "" : cause.getMessage()) - .error(error); + if (cause instanceof CorsErrorResponseException) { + // cast the exception to populate the event with a more descriptive reason + CorsErrorResponseException originalCause = (CorsErrorResponseException) cause; + event.detail(Details.REASON, originalCause.getErrorDescription() == null ? "" : originalCause.getErrorDescription()) + .error(error); + } else { + event.detail(Details.REASON, cause == null || cause.getMessage() == null ? "" : cause.getMessage()) + .error(error); + } + logger.debug(event.getEvent().getType(), cause); } - + public Response authorize(KeycloakAuthorizationRequest request) { - if (request == null) { - throw new CorsErrorResponseException(request.getCors(), OAuthErrorException.INVALID_GRANT, "Invalid authorization request.", Status.BAD_REQUEST); - } + EventBuilder event = request.getEvent(); // it is not secure to allow public clients to push arbitrary claims because message can be tampered if (isPublicClientRequestingEntitlementWithClaims(request)) { - throw new CorsErrorResponseException(request.getCors(), OAuthErrorException.INVALID_GRANT, "Public clients are not allowed to send claims", Status.FORBIDDEN); + CorsErrorResponseException forbiddenClientException = new CorsErrorResponseException(request.getCors(), OAuthErrorException.INVALID_GRANT, "Public clients are not allowed to send claims", Status.FORBIDDEN); + fireErrorEvent(event, Errors.INVALID_REQUEST, forbiddenClientException); + throw forbiddenClientException; } try { @@ -194,9 +202,15 @@ public class AuthorizationTokenService { request.setClaims(ticket.getClaims()); - ResourceServer resourceServer = getResourceServer(ticket, request); EvaluationContext evaluationContext = createEvaluationContext(request); KeycloakIdentity identity = KeycloakIdentity.class.cast(evaluationContext.getIdentity()); + + if (identity != null) { + event.user(identity.getId()); + } + + ResourceServer resourceServer = getResourceServer(ticket, request); + Collection permissions; if (request.getTicket() != null) { @@ -223,7 +237,9 @@ public class AuthorizationTokenService { } else if (RESPONSE_MODE_PERMISSIONS.equals(metadata.getResponseMode())) { return createSuccessfulResponse(permissions, request); } else { - throw new CorsErrorResponseException(request.getCors(), OAuthErrorException.INVALID_REQUEST, "Invalid response_mode", Status.BAD_REQUEST); + CorsErrorResponseException invalidResponseModeException = new CorsErrorResponseException(request.getCors(), OAuthErrorException.INVALID_REQUEST, "Invalid response_mode", Status.BAD_REQUEST); + fireErrorEvent(event, Errors.INVALID_REQUEST, invalidResponseModeException); + throw invalidResponseModeException; } } else { return createSuccessfulResponse(createAuthorizationResponse(identity, permissions, request, targetClient), request); @@ -231,9 +247,13 @@ public class AuthorizationTokenService { } if (request.isSubmitRequest()) { - throw new CorsErrorResponseException(request.getCors(), OAuthErrorException.ACCESS_DENIED, "request_submitted", Status.FORBIDDEN); + CorsErrorResponseException submittedRequestException = new CorsErrorResponseException(request.getCors(), OAuthErrorException.ACCESS_DENIED, "request_submitted", Status.FORBIDDEN); + fireErrorEvent(event, Errors.ACCESS_DENIED, submittedRequestException); + throw submittedRequestException; } else { - throw new CorsErrorResponseException(request.getCors(), OAuthErrorException.ACCESS_DENIED, "not_authorized", Status.FORBIDDEN); + CorsErrorResponseException notAuthorizedException = new CorsErrorResponseException(request.getCors(), OAuthErrorException.ACCESS_DENIED, "not_authorized", Status.FORBIDDEN); + fireErrorEvent(event, Errors.ACCESS_DENIED, notAuthorizedException); + throw notAuthorizedException; } } catch (ErrorResponseException | CorsErrorResponseException cause) { if (logger.isDebugEnabled()) { @@ -402,19 +422,25 @@ public class AuthorizationTokenService { String issuedFor = ticket.getIssuedFor(); if (issuedFor == null) { - throw new CorsErrorResponseException(request.getCors(), OAuthErrorException.INVALID_REQUEST, "You must provide the issuedFor", Status.BAD_REQUEST); + CorsErrorResponseException missingIssuedForException = new CorsErrorResponseException(request.getCors(), OAuthErrorException.INVALID_REQUEST, "You must provide the issuedFor", Status.BAD_REQUEST); + fireErrorEvent(request.getEvent(), Errors.INVALID_REQUEST, missingIssuedForException); + throw missingIssuedForException; } ClientModel clientModel = request.getRealm().getClientByClientId(issuedFor); if (clientModel == null) { - throw new CorsErrorResponseException(request.getCors(), OAuthErrorException.INVALID_REQUEST, "Unknown resource server id.", Status.BAD_REQUEST); + CorsErrorResponseException unknownServerIdException = new CorsErrorResponseException(request.getCors(), OAuthErrorException.INVALID_REQUEST, "Unknown resource server id: [" + issuedFor + "]", Status.BAD_REQUEST); + fireErrorEvent(request.getEvent(), Errors.INVALID_REQUEST, unknownServerIdException); + throw unknownServerIdException; } ResourceServer resourceServer = resourceServerStore.findById(clientModel.getId()); if (resourceServer == null) { - throw new CorsErrorResponseException(request.getCors(), OAuthErrorException.INVALID_REQUEST, "Client does not support permissions", Status.BAD_REQUEST); + CorsErrorResponseException unsupportedPermissionsException = new CorsErrorResponseException(request.getCors(), OAuthErrorException.INVALID_REQUEST, "Client does not support permissions", Status.BAD_REQUEST); + fireErrorEvent(request.getEvent(), Errors.INVALID_REQUEST, unsupportedPermissionsException); + throw unsupportedPermissionsException; } return resourceServer; @@ -430,7 +456,9 @@ public class AuthorizationTokenService { BiFunction evaluationContextProvider = SUPPORTED_CLAIM_TOKEN_FORMATS.get(claimTokenFormat); if (evaluationContextProvider == null) { - throw new CorsErrorResponseException(request.getCors(), OAuthErrorException.INVALID_REQUEST, "Claim token format [" + claimTokenFormat + "] not supported", Status.BAD_REQUEST); + CorsErrorResponseException unsupportedClaimTokenFormatException = new CorsErrorResponseException(request.getCors(), OAuthErrorException.INVALID_REQUEST, "Claim token format [" + claimTokenFormat + "] not supported", Status.BAD_REQUEST); + fireErrorEvent(request.getEvent(), Errors.INVALID_REQUEST, unsupportedClaimTokenFormatException); + throw unsupportedClaimTokenFormatException; } return evaluationContextProvider.apply(request, request.getAuthorization()); @@ -636,7 +664,9 @@ public class AuthorizationTokenService { } if (permissionsToEvaluate.isEmpty()) { - throw new CorsErrorResponseException(request.getCors(), "invalid_resource", "Resource with id [" + resourceId + "] does not exist.", Status.BAD_REQUEST); + CorsErrorResponseException invalidResourceException = new CorsErrorResponseException(request.getCors(), "invalid_resource", "Resource with id [" + resourceId + "] does not exist.", Status.BAD_REQUEST); + fireErrorEvent(request.getEvent(), Errors.INVALID_REQUEST, invalidResourceException); + throw invalidResourceException; } } @@ -657,7 +687,9 @@ public class AuthorizationTokenService { Objects::nonNull).collect(Collectors.toSet()); if (!requestedScopes.isEmpty() && requestedScopesModel.isEmpty()) { - throw new CorsErrorResponseException(request.getCors(), "invalid_scope", "One of the given scopes " + permission.getScopes() + " is invalid", Status.BAD_REQUEST); + CorsErrorResponseException invalidScopeException = new CorsErrorResponseException(request.getCors(), "invalid_scope", "One of the given scopes " + permission.getScopes() + " is invalid", Status.BAD_REQUEST); + fireErrorEvent(request.getEvent(), Errors.INVALID_REQUEST, invalidScopeException); + throw invalidScopeException; } return requestedScopesModel; } @@ -690,11 +722,15 @@ public class AuthorizationTokenService { PermissionTicketToken ticket = request.getKeycloakSession().tokens().decode(ticketString, PermissionTicketToken.class); if (ticket == null) { - throw new CorsErrorResponseException(request.getCors(), "invalid_ticket", "Ticket verification failed", Status.FORBIDDEN); + CorsErrorResponseException ticketVerificationException = new CorsErrorResponseException(request.getCors(), "invalid_ticket", "Ticket verification failed", Status.FORBIDDEN); + fireErrorEvent(request.getEvent(), Errors.INVALID_PERMISSION_TICKET, ticketVerificationException); + throw ticketVerificationException; } if (!ticket.isActive()) { - throw new CorsErrorResponseException(request.getCors(), "invalid_ticket", "Invalid permission ticket.", Status.FORBIDDEN); + CorsErrorResponseException invalidTicketException = new CorsErrorResponseException(request.getCors(), "invalid_ticket", "Invalid permission ticket.", Status.FORBIDDEN); + fireErrorEvent(request.getEvent(), Errors.INVALID_PERMISSION_TICKET, invalidTicketException); + throw invalidTicketException; } return ticket; diff --git a/services/src/main/java/org/keycloak/protocol/oidc/endpoints/TokenEndpoint.java b/services/src/main/java/org/keycloak/protocol/oidc/endpoints/TokenEndpoint.java index 17358f92b1..f0710af27e 100644 --- a/services/src/main/java/org/keycloak/protocol/oidc/endpoints/TokenEndpoint.java +++ b/services/src/main/java/org/keycloak/protocol/oidc/endpoints/TokenEndpoint.java @@ -1244,8 +1244,10 @@ public class TokenEndpoint { AccessToken invalidToken = new JWSInput(accessTokenString).readJsonContent(AccessToken.class); ClientModel client = realm.getClientByClientId(invalidToken.getIssuedFor()); cors.allowedOrigins(session, client); + event.client(client); } catch (JWSInputException ignore) { } + event.error(Errors.INVALID_TOKEN); throw new CorsErrorResponseException(cors, OAuthErrorException.INVALID_GRANT, "Invalid bearer token", Status.UNAUTHORIZED); } @@ -1254,6 +1256,7 @@ public class TokenEndpoint { session.getContext().setClient(client); cors.allowedOrigins(session, client); + event.client(client); } String claimToken = null; @@ -1300,6 +1303,7 @@ public class TokenEndpoint { if (rpt != null) { AccessToken accessToken = session.tokens().decode(rpt, AccessToken.class); if (accessToken == null) { + event.error(Errors.INVALID_REQUEST); throw new CorsErrorResponseException(cors, "invalid_rpt", "RPT signature is invalid", Status.FORBIDDEN); } @@ -1307,9 +1311,12 @@ public class TokenEndpoint { } authorizationRequest.setScope(formParams.getFirst("scope")); - authorizationRequest.setAudience(formParams.getFirst("audience")); + String audienceParam = formParams.getFirst("audience"); + authorizationRequest.setAudience(audienceParam); authorizationRequest.setSubjectToken(accessTokenString); + event.detail(Details.AUDIENCE, audienceParam); + String submitRequest = formParams.getFirst("submit_request"); authorizationRequest.setSubmitRequest(submitRequest == null ? true : Boolean.valueOf(submitRequest)); @@ -1318,6 +1325,7 @@ public class TokenEndpoint { List permissions = formParams.get("permission"); if (permissions != null) { + event.detail(Details.PERMISSION, String.join("|", permissions)); for (String permission : permissions) { String[] parts = permission.split("#"); String resource = parts[0]; @@ -1349,7 +1357,11 @@ public class TokenEndpoint { authorizationRequest.setMetadata(metadata); - return AuthorizationTokenService.instance().authorize(authorizationRequest); + Response authorizationResponse = AuthorizationTokenService.instance().authorize(authorizationRequest); + + event.success(); + + return authorizationResponse; } // https://tools.ietf.org/html/rfc7636#section-4.1 diff --git a/services/src/main/java/org/keycloak/services/CorsErrorResponseException.java b/services/src/main/java/org/keycloak/services/CorsErrorResponseException.java index d852bedd26..d9beba652d 100644 --- a/services/src/main/java/org/keycloak/services/CorsErrorResponseException.java +++ b/services/src/main/java/org/keycloak/services/CorsErrorResponseException.java @@ -41,6 +41,10 @@ public class CorsErrorResponseException extends WebApplicationException { this.status = status; } + public String getErrorDescription() { + return errorDescription; + } + @Override public Response getResponse() { OAuth2ErrorRepresentation errorRep = new OAuth2ErrorRepresentation(error, errorDescription); diff --git a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/authz/AbstractResourceServerTest.java b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/authz/AbstractResourceServerTest.java index 798c273f56..5288401491 100644 --- a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/authz/AbstractResourceServerTest.java +++ b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/authz/AbstractResourceServerTest.java @@ -77,6 +77,7 @@ public abstract class AbstractResourceServerTest extends AbstractAuthzTest { .client(ClientBuilder.create().clientId("test-app") .redirectUris("http://localhost:8180/auth/realms/master/app/auth", "https://localhost:8543/auth/realms/master/app/auth") .publicClient()) + .testEventListener() .build()); } diff --git a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/authz/EntitlementAPITest.java b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/authz/EntitlementAPITest.java index 192bd289b9..c2291d7086 100644 --- a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/authz/EntitlementAPITest.java +++ b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/authz/EntitlementAPITest.java @@ -24,6 +24,7 @@ import static org.junit.Assert.assertNull; import static org.junit.Assert.assertThat; import static org.junit.Assert.assertTrue; import static org.junit.Assert.fail; +import static org.keycloak.testsuite.AssertEvents.isUUID; import javax.ws.rs.core.Response; import java.io.IOException; @@ -90,6 +91,7 @@ import org.keycloak.representations.idm.authorization.ResourceServerRepresentati import org.keycloak.representations.idm.authorization.ScopePermissionRepresentation; import org.keycloak.representations.idm.authorization.ScopeRepresentation; import org.keycloak.representations.idm.authorization.UserPolicyRepresentation; +import org.keycloak.testsuite.AssertEvents; import org.keycloak.testsuite.arquillian.annotation.AuthServerContainerExclude; import org.keycloak.testsuite.arquillian.annotation.AuthServerContainerExclude.AuthServer; import org.keycloak.testsuite.admin.ApiUtil; @@ -125,6 +127,9 @@ public class EntitlementAPITest extends AbstractAuthzTest { @Rule public ExpectedException expectedException = ExpectedException.none(); + @Rule + public AssertEvents events = new AssertEvents(this); + @Override public void addTestRealms(List testRealms) { testRealms.add(RealmBuilder.create().name("authz-test") @@ -160,6 +165,7 @@ public class EntitlementAPITest extends AbstractAuthzTest { .secret("secret") .redirectUris("http://localhost:8180/auth/realms/master/app/auth/*", "https://localhost:8543/auth/realms/master/app/auth/*") .publicClient()) + .testEventListener() .build()); configureSectorIdentifierRedirectUris(); @@ -582,6 +588,8 @@ public class EntitlementAPITest extends AbstractAuthzTest { request.addPermission("Sensortest", "sensors:view"); + getTestContext().getTestingClient().testing().clearEventQueue(); + try { authzClient.authorization(accessToken).authorize(request); fail("resource is invalid"); @@ -589,6 +597,13 @@ public class EntitlementAPITest extends AbstractAuthzTest { assertEquals(400, HttpResponseException.class.cast(expected.getCause()).getStatusCode()); assertTrue(HttpResponseException.class.cast(expected.getCause()).toString().contains("invalid_resource")); } + + events.expect(EventType.PERMISSION_TOKEN_ERROR).realm(getRealm().toRepresentation().getId()).client(RESOURCE_SERVER_TEST) + .session((String) null) + .error("invalid_request") + .detail("reason", "Resource with id [Sensortest] does not exist.") + .user(isUUID()) + .assertEvent(); } @Test diff --git a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/authz/UserManagedAccessTest.java b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/authz/UserManagedAccessTest.java index 92633b0100..384d2e1f92 100644 --- a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/authz/UserManagedAccessTest.java +++ b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/authz/UserManagedAccessTest.java @@ -21,6 +21,7 @@ import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertNotNull; import static org.junit.Assert.assertTrue; import static org.junit.Assert.fail; +import static org.keycloak.testsuite.AssertEvents.isUUID; import java.util.Arrays; import java.util.ArrayList; @@ -28,11 +29,13 @@ import java.util.Collection; import java.util.List; import org.junit.Before; +import org.junit.Rule; import org.junit.Test; import org.keycloak.admin.client.resource.AuthorizationResource; import org.keycloak.admin.client.resource.ClientResource; import org.keycloak.authorization.client.AuthorizationDeniedException; import org.keycloak.authorization.client.resource.PermissionResource; +import org.keycloak.events.EventType; import org.keycloak.representations.AccessToken; import org.keycloak.representations.idm.authorization.AuthorizationRequest; import org.keycloak.representations.idm.authorization.AuthorizationResponse; @@ -44,6 +47,7 @@ import org.keycloak.representations.idm.authorization.ResourcePermissionRepresen import org.keycloak.representations.idm.authorization.ResourceRepresentation; import org.keycloak.representations.idm.authorization.ResourceServerRepresentation; import org.keycloak.representations.idm.authorization.ScopePermissionRepresentation; +import org.keycloak.testsuite.AssertEvents; import org.keycloak.testsuite.arquillian.annotation.AuthServerContainerExclude; import org.keycloak.testsuite.arquillian.annotation.AuthServerContainerExclude.AuthServer; @@ -55,6 +59,9 @@ public class UserManagedAccessTest extends AbstractResourceServerTest { private ResourceRepresentation resource; + @Rule + public AssertEvents events = new AssertEvents(this); + @Before public void configureAuthorization() throws Exception { ClientResource client = getClient(getRealm()); @@ -281,9 +288,12 @@ public class UserManagedAccessTest extends AbstractResourceServerTest { permission.addResource(resource.getId()); permission.addPolicy("Only Owner Policy"); - getClient(getRealm()).authorization().permissions().resource().create(permission).close(); + ClientResource client = getClient(getRealm()); + + client.authorization().permissions().resource().create(permission).close(); AuthorizationResponse response = authorize("marta", "password", "Resource A", new String[] {"ScopeA", "ScopeB"}); + String rpt = response.getToken(); assertNotNull(rpt); @@ -300,6 +310,8 @@ public class UserManagedAccessTest extends AbstractResourceServerTest { assertPermissions(permissions, "Resource A", "ScopeA", "ScopeB"); assertTrue(permissions.isEmpty()); + getTestContext().getTestingClient().testing().clearEventQueue(); + try { response = authorize("kolo", "password", resource.getId(), new String[] {}); fail("User should not have access to resource from another user"); @@ -307,6 +319,22 @@ public class UserManagedAccessTest extends AbstractResourceServerTest { } + String realmId = getRealm().toRepresentation().getId(); + String clientId = client.toRepresentation().getClientId(); + events.expectLogin().realm(realmId).client(clientId) + .user(isUUID()) + .clearDetails() + .assertEvent(); + events.expectLogin().realm(realmId).client(clientId) + .user(isUUID()) + .clearDetails() + .assertEvent(); + events.expect(EventType.PERMISSION_TOKEN_ERROR).realm(realmId).client(clientId).user(isUUID()) + .session((String) null) + .error("access_denied") + .detail("reason", "request_submitted") + .assertEvent(); + PermissionResource permissionResource = getAuthzClient().protection().permission(); List permissionTickets = permissionResource.findByResource(resource.getId()); @@ -330,6 +358,8 @@ public class UserManagedAccessTest extends AbstractResourceServerTest { assertTrue(ticket.isGranted()); } + getTestContext().getTestingClient().testing().clearEventQueue(); + response = authorize("kolo", "password", resource.getId(), new String[] {"ScopeA", "ScopeB"}); rpt = response.getToken(); @@ -346,6 +376,19 @@ public class UserManagedAccessTest extends AbstractResourceServerTest { assertNotNull(permissions); assertPermissions(permissions, resource.getName(), "ScopeA", "ScopeB"); assertTrue(permissions.isEmpty()); + + events.expectLogin().realm(realmId).client(clientId) + .user(isUUID()) + .clearDetails() + .assertEvent(); + events.expectLogin().realm(realmId).client(clientId) + .user(isUUID()) + .clearDetails() + .assertEvent(); + events.expect(EventType.PERMISSION_TOKEN).realm(realmId).client(clientId).user(isUUID()) + .session((String) null) + .clearDetails() + .assertEvent(); } @Test