Commit 35243641 authored by Matthias Piepkorn's avatar Matthias Piepkorn
Browse files

Improve serviceValidate error handling

parent 37a97a22
...@@ -84,11 +84,6 @@ public class AuthorizationEndpoint extends AuthorizationEndpointBase { ...@@ -84,11 +84,6 @@ public class AuthorizationEndpoint extends AuthorizationEndpointBase {
throw new ErrorPageException(session, Messages.CLIENT_DISABLED); throw new ErrorPageException(session, Messages.CLIENT_DISABLED);
} }
if (client.isBearerOnly()) {
event.error(Errors.NOT_ALLOWED);
throw new ErrorPageException(session, Messages.BEARER_ONLY);
}
redirectUri = RedirectUtils.verifyRedirectUri(uriInfo, service, realm, client); redirectUri = RedirectUtils.verifyRedirectUri(uriInfo, service, realm, client);
event.client(client.getClientId()); event.client(client.getClientId());
......
...@@ -8,9 +8,9 @@ import org.keycloak.models.UserSessionModel; ...@@ -8,9 +8,9 @@ import org.keycloak.models.UserSessionModel;
import org.keycloak.protocol.ProtocolMapper; import org.keycloak.protocol.ProtocolMapper;
import org.keycloak.protocol.cas.mappers.CASAttributeMapper; import org.keycloak.protocol.cas.mappers.CASAttributeMapper;
import org.keycloak.protocol.cas.representations.CasServiceResponse; import org.keycloak.protocol.cas.representations.CasServiceResponse;
import org.keycloak.protocol.cas.utils.CASValidationException;
import org.keycloak.protocol.cas.utils.ContentTypeHelper; import org.keycloak.protocol.cas.utils.ContentTypeHelper;
import org.keycloak.protocol.cas.utils.ServiceResponseHelper; import org.keycloak.protocol.cas.utils.ServiceResponseHelper;
import org.keycloak.services.ErrorResponseException;
import org.keycloak.services.managers.ClientSessionCode; import org.keycloak.services.managers.ClientSessionCode;
import javax.ws.rs.core.*; import javax.ws.rs.core.*;
...@@ -45,9 +45,9 @@ public class ServiceValidateEndpoint extends ValidateEndpoint { ...@@ -45,9 +45,9 @@ public class ServiceValidateEndpoint extends ValidateEndpoint {
} }
@Override @Override
protected Response errorResponse(ErrorResponseException e) { protected Response errorResponse(CASValidationException e) {
CasServiceResponse serviceResponse = ServiceResponseHelper.createFailure("CODE", "Description"); CasServiceResponse serviceResponse = ServiceResponseHelper.createFailure(e.getError(), e.getErrorDescription());
return prepare(Response.Status.FORBIDDEN, serviceResponse); return prepare(e.getStatus(), serviceResponse);
} }
private Response prepare(Response.Status status, CasServiceResponse serviceResponse) { private Response prepare(Response.Status status, CasServiceResponse serviceResponse) {
......
...@@ -3,7 +3,6 @@ package org.keycloak.protocol.cas.endpoints; ...@@ -3,7 +3,6 @@ package org.keycloak.protocol.cas.endpoints;
import org.jboss.logging.Logger; import org.jboss.logging.Logger;
import org.jboss.resteasy.annotations.cache.NoCache; import org.jboss.resteasy.annotations.cache.NoCache;
import org.jboss.resteasy.spi.HttpRequest; import org.jboss.resteasy.spi.HttpRequest;
import org.keycloak.OAuthErrorException;
import org.keycloak.common.ClientConnection; import org.keycloak.common.ClientConnection;
import org.keycloak.events.Details; import org.keycloak.events.Details;
import org.keycloak.events.Errors; import org.keycloak.events.Errors;
...@@ -11,12 +10,11 @@ import org.keycloak.events.EventBuilder; ...@@ -11,12 +10,11 @@ import org.keycloak.events.EventBuilder;
import org.keycloak.events.EventType; import org.keycloak.events.EventType;
import org.keycloak.models.*; import org.keycloak.models.*;
import org.keycloak.protocol.cas.CASLoginProtocol; import org.keycloak.protocol.cas.CASLoginProtocol;
import org.keycloak.protocol.cas.representations.CASErrorCode;
import org.keycloak.protocol.cas.utils.CASValidationException;
import org.keycloak.protocol.oidc.utils.RedirectUtils; import org.keycloak.protocol.oidc.utils.RedirectUtils;
import org.keycloak.services.ErrorPageException;
import org.keycloak.services.ErrorResponseException;
import org.keycloak.services.managers.AuthenticationManager; import org.keycloak.services.managers.AuthenticationManager;
import org.keycloak.services.managers.ClientSessionCode; import org.keycloak.services.managers.ClientSessionCode;
import org.keycloak.services.messages.Messages;
import javax.ws.rs.GET; import javax.ws.rs.GET;
import javax.ws.rs.core.*; import javax.ws.rs.core.*;
...@@ -71,7 +69,7 @@ public class ValidateEndpoint { ...@@ -71,7 +69,7 @@ public class ValidateEndpoint {
event.success(); event.success();
return successResponse(); return successResponse();
} catch (ErrorResponseException e) { } catch (CASValidationException e) {
return errorResponse(e); return errorResponse(e);
} }
} }
...@@ -80,26 +78,26 @@ public class ValidateEndpoint { ...@@ -80,26 +78,26 @@ public class ValidateEndpoint {
return Response.ok(RESPONSE_OK).type(MediaType.TEXT_PLAIN).build(); return Response.ok(RESPONSE_OK).type(MediaType.TEXT_PLAIN).build();
} }
protected Response errorResponse(ErrorResponseException e) { protected Response errorResponse(CASValidationException e) {
return Response.status(Response.Status.UNAUTHORIZED).entity(RESPONSE_FAILED).type(MediaType.TEXT_PLAIN).build(); return Response.status(e.getStatus()).entity(RESPONSE_FAILED).type(MediaType.TEXT_PLAIN).build();
} }
private void checkSsl() { private void checkSsl() {
if (!uriInfo.getBaseUri().getScheme().equals("https") && realm.getSslRequired().isRequired(clientConnection)) { if (!uriInfo.getBaseUri().getScheme().equals("https") && realm.getSslRequired().isRequired(clientConnection)) {
throw new ErrorResponseException(OAuthErrorException.INVALID_REQUEST, "HTTPS required", Response.Status.FORBIDDEN); throw new CASValidationException(CASErrorCode.INVALID_REQUEST, "HTTPS required", Response.Status.FORBIDDEN);
} }
} }
private void checkRealm() { private void checkRealm() {
if (!realm.isEnabled()) { if (!realm.isEnabled()) {
throw new ErrorResponseException("access_denied", "Realm not enabled", Response.Status.FORBIDDEN); throw new CASValidationException(CASErrorCode.INTERNAL_ERROR, "Realm not enabled", Response.Status.FORBIDDEN);
} }
} }
private void checkClient(String service) { private void checkClient(String service) {
if (service == null) { if (service == null) {
event.error(Errors.INVALID_REQUEST); event.error(Errors.INVALID_REQUEST);
throw new ErrorPageException(session, Messages.MISSING_PARAMETER, CASLoginProtocol.SERVICE_PARAM); throw new CASValidationException(CASErrorCode.INVALID_REQUEST, "Missing parameter: " + CASLoginProtocol.SERVICE_PARAM, Response.Status.BAD_REQUEST);
} }
client = realm.getClients().stream() client = realm.getClients().stream()
...@@ -108,17 +106,12 @@ public class ValidateEndpoint { ...@@ -108,17 +106,12 @@ public class ValidateEndpoint {
.findFirst().orElse(null); .findFirst().orElse(null);
if (client == null) { if (client == null) {
event.error(Errors.CLIENT_NOT_FOUND); event.error(Errors.CLIENT_NOT_FOUND);
throw new ErrorPageException(session, Messages.CLIENT_NOT_FOUND); throw new CASValidationException(CASErrorCode.INVALID_SERVICE, "Client not found", Response.Status.BAD_REQUEST);
} }
if (!client.isEnabled()) { if (!client.isEnabled()) {
event.error(Errors.CLIENT_DISABLED); event.error(Errors.CLIENT_DISABLED);
throw new ErrorPageException(session, Messages.CLIENT_DISABLED); throw new CASValidationException(CASErrorCode.INVALID_SERVICE, "Client disabled", Response.Status.BAD_REQUEST);
}
if (client.isBearerOnly()) {
event.error(Errors.NOT_ALLOWED);
throw new ErrorPageException(session, Messages.BEARER_ONLY);
} }
event.client(client.getClientId()); event.client(client.getClientId());
...@@ -127,9 +120,13 @@ public class ValidateEndpoint { ...@@ -127,9 +120,13 @@ public class ValidateEndpoint {
} }
private void checkTicket(String ticket, boolean requireReauth) { private void checkTicket(String ticket, boolean requireReauth) {
if (ticket == null || !ticket.startsWith(CASLoginProtocol.SERVICE_TICKET_PREFIX)) { if (ticket == null) {
event.error(Errors.INVALID_CODE);
throw new CASValidationException(CASErrorCode.INVALID_REQUEST, "Missing parameter: " + CASLoginProtocol.TICKET_PARAM, Response.Status.BAD_REQUEST);
}
if (!ticket.startsWith(CASLoginProtocol.SERVICE_TICKET_PREFIX)) {
event.error(Errors.INVALID_CODE); event.error(Errors.INVALID_CODE);
throw new ErrorResponseException(OAuthErrorException.INVALID_REQUEST, "Missing or invalid parameter: " + CASLoginProtocol.TICKET_PARAM, Response.Status.BAD_REQUEST); throw new CASValidationException(CASErrorCode.INVALID_TICKET_SPEC, "Malformed service ticket", Response.Status.BAD_REQUEST);
} }
String code = ticket.substring(CASLoginProtocol.SERVICE_TICKET_PREFIX.length()); String code = ticket.substring(CASLoginProtocol.SERVICE_TICKET_PREFIX.length());
...@@ -144,7 +141,7 @@ public class ValidateEndpoint { ...@@ -144,7 +141,7 @@ public class ValidateEndpoint {
if (parseResult.getClientSession() != null) { if (parseResult.getClientSession() != null) {
session.sessions().removeClientSession(realm, parseResult.getClientSession()); session.sessions().removeClientSession(realm, parseResult.getClientSession());
} }
throw new ErrorResponseException(OAuthErrorException.INVALID_GRANT, "Code not valid", Response.Status.BAD_REQUEST); throw new CASValidationException(CASErrorCode.INVALID_TICKET, "Code not valid", Response.Status.BAD_REQUEST);
} }
clientSession = parseResult.getClientSession(); clientSession = parseResult.getClientSession();
...@@ -152,7 +149,7 @@ public class ValidateEndpoint { ...@@ -152,7 +149,7 @@ public class ValidateEndpoint {
if (!parseResult.getCode().isValid(ClientSessionModel.Action.CODE_TO_TOKEN.name(), ClientSessionCode.ActionType.CLIENT)) { if (!parseResult.getCode().isValid(ClientSessionModel.Action.CODE_TO_TOKEN.name(), ClientSessionCode.ActionType.CLIENT)) {
event.error(Errors.INVALID_CODE); event.error(Errors.INVALID_CODE);
throw new ErrorResponseException(OAuthErrorException.INVALID_GRANT, "Code is expired", Response.Status.BAD_REQUEST); throw new CASValidationException(CASErrorCode.INVALID_TICKET, "Code is expired", Response.Status.BAD_REQUEST);
} }
parseResult.getCode().setAction(null); parseResult.getCode().setAction(null);
...@@ -161,17 +158,17 @@ public class ValidateEndpoint { ...@@ -161,17 +158,17 @@ public class ValidateEndpoint {
if (userSession == null) { if (userSession == null) {
event.error(Errors.USER_SESSION_NOT_FOUND); event.error(Errors.USER_SESSION_NOT_FOUND);
throw new ErrorResponseException(OAuthErrorException.INVALID_GRANT, "User session not found", Response.Status.BAD_REQUEST); throw new CASValidationException(CASErrorCode.INVALID_TICKET, "User session not found", Response.Status.BAD_REQUEST);
} }
UserModel user = userSession.getUser(); UserModel user = userSession.getUser();
if (user == null) { if (user == null) {
event.error(Errors.USER_NOT_FOUND); event.error(Errors.USER_NOT_FOUND);
throw new ErrorResponseException(OAuthErrorException.INVALID_GRANT, "User not found", Response.Status.BAD_REQUEST); throw new CASValidationException(CASErrorCode.INVALID_TICKET, "User not found", Response.Status.BAD_REQUEST);
} }
if (!user.isEnabled()) { if (!user.isEnabled()) {
event.error(Errors.USER_DISABLED); event.error(Errors.USER_DISABLED);
throw new ErrorResponseException(OAuthErrorException.INVALID_GRANT, "User disabled", Response.Status.BAD_REQUEST); throw new CASValidationException(CASErrorCode.INVALID_TICKET, "User disabled", Response.Status.BAD_REQUEST);
} }
event.user(userSession.getUser()); event.user(userSession.getUser());
...@@ -179,12 +176,12 @@ public class ValidateEndpoint { ...@@ -179,12 +176,12 @@ public class ValidateEndpoint {
if (!client.getClientId().equals(clientSession.getClient().getClientId())) { if (!client.getClientId().equals(clientSession.getClient().getClientId())) {
event.error(Errors.INVALID_CODE); event.error(Errors.INVALID_CODE);
throw new ErrorResponseException(OAuthErrorException.INVALID_GRANT, "Auth error", Response.Status.BAD_REQUEST); throw new CASValidationException(CASErrorCode.INVALID_SERVICE, "Auth error", Response.Status.BAD_REQUEST);
} }
if (!AuthenticationManager.isSessionValid(realm, userSession)) { if (!AuthenticationManager.isSessionValid(realm, userSession)) {
event.error(Errors.USER_SESSION_NOT_FOUND); event.error(Errors.USER_SESSION_NOT_FOUND);
throw new ErrorResponseException(OAuthErrorException.INVALID_GRANT, "Session not active", Response.Status.BAD_REQUEST); throw new CASValidationException(CASErrorCode.INVALID_TICKET, "Session not active", Response.Status.BAD_REQUEST);
} }
} }
......
package org.keycloak.protocol.cas.representations;
public enum CASErrorCode {
/** not all of the required request parameters were present */
INVALID_REQUEST,
/** failure to meet the requirements of validation specification */
INVALID_TICKET_SPEC,
/** the service is not authorized to perform proxy authentication */
UNAUTHORIZED_SERVICE_PROXY,
/** The proxy callback specified is invalid. The credentials specified for proxy authentication do not meet the security requirements */
INVALID_PROXY_CALLBACK,
/** the ticket provided was not valid, or the ticket did not come from an initial login and renew was set on validation. */
INVALID_TICKET,
/** the ticket provided was valid, but the service specified did not match the service associated with the ticket. */
INVALID_SERVICE,
/** an internal error occurred during ticket validation */
INTERNAL_ERROR
}
package org.keycloak.protocol.cas.utils;
import org.keycloak.protocol.cas.representations.CASErrorCode;
import javax.ws.rs.WebApplicationException;
import javax.ws.rs.core.Response;
public class CASValidationException extends WebApplicationException {
private final CASErrorCode error;
private final String errorDescription;
private final Response.Status status;
public CASValidationException(CASErrorCode error, String errorDescription, Response.Status status) {
this.error = error;
this.errorDescription = errorDescription;
this.status = status;
}
public CASErrorCode getError() {
return error;
}
public String getErrorDescription() {
return errorDescription;
}
public Response.Status getStatus() {
return status;
}
}
package org.keycloak.protocol.cas.utils; package org.keycloak.protocol.cas.utils;
import org.keycloak.protocol.cas.representations.CASErrorCode;
import org.keycloak.protocol.cas.representations.CasServiceResponse; import org.keycloak.protocol.cas.representations.CasServiceResponse;
import org.keycloak.protocol.cas.representations.CasServiceResponseAuthenticationFailure; import org.keycloak.protocol.cas.representations.CasServiceResponseAuthenticationFailure;
import org.keycloak.protocol.cas.representations.CasServiceResponseAuthenticationSuccess; import org.keycloak.protocol.cas.representations.CasServiceResponseAuthenticationSuccess;
...@@ -32,10 +33,10 @@ public final class ServiceResponseHelper { ...@@ -32,10 +33,10 @@ public final class ServiceResponseHelper {
return response; return response;
} }
public static CasServiceResponse createFailure(String errorCode, String errorDescription) { public static CasServiceResponse createFailure(CASErrorCode errorCode, String errorDescription) {
CasServiceResponse response = new CasServiceResponse(); CasServiceResponse response = new CasServiceResponse();
CasServiceResponseAuthenticationFailure failure = new CasServiceResponseAuthenticationFailure(); CasServiceResponseAuthenticationFailure failure = new CasServiceResponseAuthenticationFailure();
failure.setCode(errorCode); failure.setCode(errorCode == null ? CASErrorCode.INTERNAL_ERROR.name() : errorCode.name());
failure.setDescription(errorDescription); failure.setDescription(errorDescription);
response.setAuthenticationFailure(failure); response.setAuthenticationFailure(failure);
......
package org.keycloak.protocol.cas; package org.keycloak.protocol.cas;
import org.junit.Test; import org.junit.Test;
import org.keycloak.protocol.cas.representations.CASErrorCode;
import org.keycloak.protocol.cas.representations.CasServiceResponse; import org.keycloak.protocol.cas.representations.CasServiceResponse;
import org.keycloak.protocol.cas.utils.ServiceResponseHelper; import org.keycloak.protocol.cas.utils.ServiceResponseHelper;
import org.keycloak.protocol.cas.utils.ServiceResponseMarshaller; import org.keycloak.protocol.cas.utils.ServiceResponseMarshaller;
...@@ -45,13 +46,13 @@ public class ServiceResponseTest { ...@@ -45,13 +46,13 @@ public class ServiceResponseTest {
private static final String EXPECTED_JSON_FAILURE = "{\n" + private static final String EXPECTED_JSON_FAILURE = "{\n" +
" \"serviceResponse\" : {\n" + " \"serviceResponse\" : {\n" +
" \"authenticationFailure\" : {\n" + " \"authenticationFailure\" : {\n" +
" \"code\" : \"ERROR_CODE\",\n" + " \"code\" : \"INVALID_REQUEST\",\n" +
" \"description\" : \"Error description\"\n" + " \"description\" : \"Error description\"\n" +
" }\n" + " }\n" +
" }\n" + " }\n" +
"}"; "}";
private static final String EXPECTED_XML_FAILURE = "<cas:serviceResponse xmlns:cas=\"http://www.yale.edu/tp/cas\">\n" + private static final String EXPECTED_XML_FAILURE = "<cas:serviceResponse xmlns:cas=\"http://www.yale.edu/tp/cas\">\n" +
" <cas:authenticationFailure code=\"ERROR_CODE\">Error description</cas:authenticationFailure>\n" + " <cas:authenticationFailure code=\"INVALID_REQUEST\">Error description</cas:authenticationFailure>\n" +
"</cas:serviceResponse>"; "</cas:serviceResponse>";
@Test @Test
...@@ -70,7 +71,7 @@ public class ServiceResponseTest { ...@@ -70,7 +71,7 @@ public class ServiceResponseTest {
@Test @Test
public void testErrorResponse() throws Exception { public void testErrorResponse() throws Exception {
CasServiceResponse response = ServiceResponseHelper.createFailure("ERROR_CODE", "Error description"); CasServiceResponse response = ServiceResponseHelper.createFailure(CASErrorCode.INVALID_REQUEST, "Error description");
assertEquals(EXPECTED_JSON_FAILURE, ServiceResponseMarshaller.marshalJson(response)); assertEquals(EXPECTED_JSON_FAILURE, ServiceResponseMarshaller.marshalJson(response));
assertEquals(EXPECTED_XML_FAILURE, ServiceResponseMarshaller.marshalXml(response)); assertEquals(EXPECTED_XML_FAILURE, ServiceResponseMarshaller.marshalXml(response));
......
Markdown is supported
0% or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment