Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Introduce extension point for building requests to authorize EP #2127

Merged
merged 15 commits into from
Aug 15, 2023
Merged

Introduce extension point for building requests to authorize EP #2127

Show file tree
Hide file tree
Changes from 5 commits
Commits
Show all changes
15 commits
Select commit Hold shift + click to select a range
2ca830e
add validations and extension point
Yoshani Aug 2, 2023
81fa5f4
resolve suggestions
Yoshani Aug 4, 2023
aa33241
resolve suggestions
Yoshani Aug 7, 2023
5f279cd
resolve suggestions
Yoshani Aug 9, 2023
447a085
resolve suggestions
Yoshani Aug 10, 2023
6269edc
add diagnostic logs
Yoshani Aug 10, 2023
e77bbe0
remove extra catch clause
Yoshani Aug 14, 2023
2831ab8
Merge branch 'wso2-extensions:master' into par-improvements
Yoshani Aug 14, 2023
1f67d50
change log level
Yoshani Aug 14, 2023
9650bfd
add log dependency
Yoshani Aug 14, 2023
a10086d
add null check for request
Yoshani Aug 14, 2023
2247054
convert parcache to eager loading
Yoshani Aug 14, 2023
e4d80d0
resolve comments
Yoshani Aug 14, 2023
c7b82f8
resolve comments
Yoshani Aug 15, 2023
62c80dc
resolve comments
Yoshani Aug 15, 2023
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
5 changes: 5 additions & 0 deletions ...tity.oauth.common/src/main/java/org/wso2/carbon/identity/oauth/common/OAuthConstants.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -574,6 +574,9 @@ public static class ActionIDs {
public static final String HAND_OVER_TO_FRAMEWORK = "hand-over-to-framework";
public static final String PERSIST_OAUTH_SCOPE_CONSENT = "persist-oauth-scope-consent";
public static final String GENERATE_CONSENT_CLAIMS = "generate-consent-claims";
public static final String HANDLE_REQUEST = "handle-request";
public static final String BUILD_REQUEST = "build-request";
public static final String RETRIEVE_PARAMETERS = "retrieve-parameters";
}

/**
Expand All @@ -587,6 +590,8 @@ public static class InputKeys {
public static final String AUTHORIZED_SCOPES = "authorized scopes";
public static final String GRANT_TYPE = "grant type";
public static final String AUTHORIZATION_CODE = "authorization code";
public static final String REQUEST_BUILDER = "request builder";
public static final String REQUEST_URI_REF = "request uri reference";
}
}

Expand Down
2 changes: 1 addition & 1 deletion components/org.wso2.carbon.identity.oauth.endpoint/pom.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -332,7 +332,7 @@
<limit implementation="org.jacoco.report.check.Limit">
<counter>COMPLEXITY</counter>
<value>COVEREDRATIO</value>
<minimum>0.53</minimum>
<minimum>0.52</minimum>
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What is the reason to reduce the code cover ratio?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unit tests have not been added covering the PAR implementation. Will be addressed in a new PR.

</limit>
</limits>
</rule>
Expand Down
84 changes: 7 additions & 77 deletions ...oint/src/main/java/org/wso2/carbon/identity/oauth/endpoint/authz/OAuth2AuthzEndpoint.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,7 @@
import org.wso2.carbon.identity.application.common.model.ServiceProvider;
import org.wso2.carbon.identity.application.common.model.ServiceProviderProperty;
import org.wso2.carbon.identity.base.IdentityConstants;
import org.wso2.carbon.identity.base.IdentityException;
import org.wso2.carbon.identity.central.log.mgt.utils.LogConstants;
import org.wso2.carbon.identity.central.log.mgt.utils.LoggerUtils;
import org.wso2.carbon.identity.claim.metadata.mgt.ClaimMetadataHandler;
Expand Down Expand Up @@ -104,14 +105,14 @@
import org.wso2.carbon.identity.oauth2.dto.OAuth2ClientValidationResponseDTO;
import org.wso2.carbon.identity.oauth2.internal.OAuth2ServiceComponentHolder;
import org.wso2.carbon.identity.oauth2.model.AccessTokenExtendedAttributes;
import org.wso2.carbon.identity.oauth2.model.CarbonOAuthAuthzRequest;
import org.wso2.carbon.identity.oauth2.model.HttpRequestHeaderHandler;
import org.wso2.carbon.identity.oauth2.model.OAuth2Parameters;
import org.wso2.carbon.identity.oauth2.responsemode.provider.AuthorizationResponseDTO;
import org.wso2.carbon.identity.oauth2.responsemode.provider.ResponseModeProvider;
import org.wso2.carbon.identity.oauth2.scopeservice.ScopeMetadataService;
import org.wso2.carbon.identity.oauth2.token.bindings.TokenBinder;
import org.wso2.carbon.identity.oauth2.util.OAuth2Util;
import org.wso2.carbon.identity.oauth2.util.RequestUtil;
import org.wso2.carbon.identity.oidc.session.OIDCSessionState;
import org.wso2.carbon.identity.oidc.session.util.OIDCSessionManagementUtil;
import org.wso2.carbon.identity.openidconnect.OIDCConstants;
Expand All @@ -124,8 +125,6 @@

import java.io.IOException;
import java.io.UnsupportedEncodingException;
import java.lang.reflect.Constructor;
import java.lang.reflect.InvocationTargetException;
import java.net.URI;
import java.net.URISyntaxException;
import java.net.URLEncoder;
Expand Down Expand Up @@ -183,6 +182,7 @@
import static org.wso2.carbon.identity.oauth.endpoint.util.EndpointUtil.getErrorPageURL;
import static org.wso2.carbon.identity.oauth.endpoint.util.EndpointUtil.getLoginPageURL;
import static org.wso2.carbon.identity.oauth.endpoint.util.EndpointUtil.getOAuth2Service;
import static org.wso2.carbon.identity.oauth.endpoint.util.EndpointUtil.getOAuthAuthzRequest;
import static org.wso2.carbon.identity.oauth.endpoint.util.EndpointUtil.getOAuthServerConfiguration;
import static org.wso2.carbon.identity.oauth.endpoint.util.EndpointUtil.getSSOConsentService;
import static org.wso2.carbon.identity.oauth.endpoint.util.EndpointUtil.retrieveStateForErrorURL;
Expand Down Expand Up @@ -280,11 +280,15 @@ public Response authorize(@Context HttpServletRequest request, @Context HttpServ

// Using a separate try-catch block as this next try block has operations in the final block.
try {
request = RequestUtil.buildRequest(request);
oAuthMessage = buildOAuthMessage(request, response);

} catch (InvalidRequestParentException e) {
EndpointUtil.triggerOnAuthzRequestException(e, request);
throw e;
} catch (IdentityException e) {
EndpointUtil.triggerOnAuthzRequestException(e, request);
throw new InvalidRequestException(e.getMessage(), OAuth2ErrorCodes.INVALID_REQUEST, e);
}

try {
Expand Down Expand Up @@ -1937,7 +1941,6 @@ private String handleOAuthAuthorizationRequest(OAuthMessage oAuthMessage)

OAuth2ClientValidationResponseDTO validationResponse = validateClient(oAuthMessage);


if (!validationResponse.isValidClient()) {
EndpointUtil.triggerOnRequestValidationFailure(oAuthMessage, validationResponse);
return getErrorPageURL(oAuthMessage.getRequest(), validationResponse.getErrorCode(), OAuth2ErrorCodes
Expand Down Expand Up @@ -2014,79 +2017,6 @@ private void populateValidationResponseWithAppDetail(OAuthMessage oAuthMessage,
}
}

/**
* Method to check whether the configured OAuthAuthzRequestImplementation is the default implementation.
*
* @return boolean whether the default class name is configured.
*/
private boolean isDefaultOAuthAuthzRequestClassConfigured() {

String oauthAuthzRequestClassName = OAuthServerConfiguration.getInstance().getOAuthAuthzRequestClassName();
return OAuthServerConfiguration.DEFAULT_OAUTH_AUTHZ_REQUEST_CLASSNAME.equals(oauthAuthzRequestClassName);
}

/**
* Load OAuthAuthzRequest class.
*
* @return OAuthAuthzRequest Class.
* @throws ClassNotFoundException when configured class name is invalid.
*/
private static Class<? extends OAuthAuthzRequest> getOAuthAuthzRequestClass() throws ClassNotFoundException {

if (oAuthAuthzRequestClass == null) {

String oauthAuthzRequestClassName =
OAuthServerConfiguration.getInstance().getOAuthAuthzRequestClassName();
oAuthAuthzRequestClass = (Class<? extends OAuthAuthzRequest>) Thread.currentThread()
.getContextClassLoader().loadClass(oauthAuthzRequestClassName);

}
return oAuthAuthzRequestClass;
}

/**
* Returns an instance of OAuthAuthzRequest. If the configured classname is invalid the default implementation
* will be returned.
*
* @param request http servlet request.
* @return instance of OAuthAuthzRequest.
* @throws OAuthProblemException thrown when initializing the OAuthAuthzRequestClass instance.
* @throws OAuthSystemException thrown when initializing the OAuthAuthzRequestClass instance.
*/
private OAuthAuthzRequest getOAuthAuthzRequest(HttpServletRequest request)
throws OAuthProblemException, OAuthSystemException {

OAuthAuthzRequest oAuthAuthzRequest;

if (isDefaultOAuthAuthzRequestClassConfigured()) {
oAuthAuthzRequest = new CarbonOAuthAuthzRequest(request);
} else {
try {
Class<? extends OAuthAuthzRequest> clazz = getOAuthAuthzRequestClass();
// Validations will be performed when initializing the class instance.
Constructor<?> constructor = clazz.getConstructor(HttpServletRequest.class);
oAuthAuthzRequest = (OAuthAuthzRequest) constructor.newInstance(request);
} catch (InvocationTargetException e) {
// Handle OAuthProblemException & OAuthSystemException thrown from extended class.
if (e.getTargetException() instanceof OAuthProblemException) {
throw (OAuthProblemException) e.getTargetException();
} else if (e.getTargetException() instanceof OAuthSystemException) {
throw (OAuthSystemException) e.getTargetException();
} else {
log.warn("Failed to initiate OAuthAuthzRequest from identity.xml. " +
"Hence initiating the default implementation");
oAuthAuthzRequest = new CarbonOAuthAuthzRequest(request);
}
} catch (ClassNotFoundException | InstantiationException | IllegalAccessException |
NoSuchMethodException e) {
log.warn("Failed to initiate OAuthAuthzRequest from identity.xml. " +
"Hence initiating the default implementation");
oAuthAuthzRequest = new CarbonOAuthAuthzRequest(request);
}
}
return oAuthAuthzRequest;
}

/**
* Checks whether the given authentication flow requires {@code nonce} as a mandatory parameter.
*
Expand Down
83 changes: 63 additions & 20 deletions ...endpoint/src/main/java/org/wso2/carbon/identity/oauth/endpoint/par/OAuth2ParEndpoint.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
/**
/*
* Copyright (c) 2023, WSO2 LLC. (https://www.wso2.com).
*
* WSO2 LLC. licenses this file to you under the Apache License,
Expand All @@ -22,12 +22,13 @@
import org.apache.commons.logging.Log;
import org.apache.commons.logging.LogFactory;
import org.apache.cxf.interceptor.InInterceptors;
import org.apache.oltu.oauth2.common.error.OAuthError;
import org.apache.oltu.oauth2.common.exception.OAuthProblemException;
import org.apache.oltu.oauth2.common.exception.OAuthSystemException;
import org.json.JSONObject;
import org.wso2.carbon.identity.oauth.client.authn.filter.OAuthClientAuthenticatorProxy;
import org.wso2.carbon.identity.oauth.common.OAuth2ErrorCodes;
import org.wso2.carbon.identity.oauth.common.OAuthConstants;
import org.wso2.carbon.identity.oauth.endpoint.util.EndpointUtil;
import org.wso2.carbon.identity.oauth.common.exception.InvalidOAuthRequestException;
import org.wso2.carbon.identity.oauth.par.common.ParConstants;
import org.wso2.carbon.identity.oauth.par.exceptions.ParClientException;
import org.wso2.carbon.identity.oauth.par.exceptions.ParCoreException;
Expand All @@ -51,6 +52,9 @@
import javax.ws.rs.core.Response;

import static org.wso2.carbon.identity.oauth.endpoint.util.EndpointUtil.getOAuth2Service;
import static org.wso2.carbon.identity.oauth.endpoint.util.EndpointUtil.getOAuthAuthzRequest;
import static org.wso2.carbon.identity.oauth.endpoint.util.EndpointUtil.getParAuthService;
import static org.wso2.carbon.identity.oauth.endpoint.util.EndpointUtil.validateParams;

/**
* REST implementation for OAuth2 PAR endpoint.
Expand All @@ -63,8 +67,6 @@ public class OAuth2ParEndpoint {

private static final Log log = LogFactory.getLog(OAuth2ParEndpoint.class);

private static final String PAR_CLIENT_AUTH_ERROR = "Client Authentication Failed";

@POST
@Path("/")
@Consumes("application/x-www-form-urlencoded")
Expand All @@ -77,7 +79,7 @@ public Response par(@Context HttpServletRequest request, @Context HttpServletRes
handleValidation(request, params);
Map<String, String> parameters = transformParams(params);
ParAuthData parAuthData =
EndpointUtil.getParAuthService().handleParAuthRequest(parameters);
getParAuthService().handleParAuthRequest(parameters);
return createAuthResponse(response, parAuthData);
} catch (ParClientException e) {
return handleParClientException(e);
Expand Down Expand Up @@ -133,25 +135,20 @@ private Response handleParCoreException(ParCoreException parCoreException) {

JSONObject parErrorResponse = new JSONObject();
parErrorResponse.put(OAuthConstants.OAUTH_ERROR, OAuth2ErrorCodes.SERVER_ERROR);
parErrorResponse.put(OAuthConstants.OAUTH_ERROR_DESCRIPTION, "Internal Server Error.");
parErrorResponse.put(OAuthConstants.OAUTH_ERROR_DESCRIPTION, ParConstants.INTERNAL_SERVER_ERROR);

Response.ResponseBuilder respBuilder = Response.status(HttpServletResponse.SC_INTERNAL_SERVER_ERROR);
log.debug("Exception occurred when handling the request: ", parCoreException);
return respBuilder.entity(parErrorResponse.toString()).build();
}

private void handleValidation(HttpServletRequest request, MultivaluedMap<String, String> params)
throws ParClientException {
throws ParCoreException {

OAuth2ClientValidationResponseDTO validationResponse = getOAuth2Service().validateClientInfo(request);

if (!validationResponse.isValidClient()) {
throw new ParClientException(validationResponse.getErrorCode(), validationResponse.getErrorMsg());
}
if (isRequestUriProvided(params)) {
throw new ParClientException(OAuth2ErrorCodes.INVALID_REQUEST,
ParConstants.REQUEST_URI_IN_REQUEST_BODY_ERROR);
}
validateInputParameters(request);
validateClient(request, params);
validateRepeatedParams(request, params);
validateAuthzRequest(request);
}

private boolean isRequestUriProvided(MultivaluedMap<String, String> params) {
Expand All @@ -169,12 +166,15 @@ private void checkClientAuthentication(HttpServletRequest request) throws ParCor
if (OAuth2ErrorCodes.SERVER_ERROR.equals(oAuthClientAuthnContext.getErrorCode())) {
throw new ParCoreException(oAuthClientAuthnContext.getErrorCode(),
oAuthClientAuthnContext.getErrorMessage());
} else if (OAuth2ErrorCodes.INVALID_CLIENT.equals(oAuthClientAuthnContext.getErrorCode())) {
throw new ParClientException(oAuthClientAuthnContext.getErrorCode(),
ParConstants.INVALID_CLIENT_ERROR + oAuthClientAuthnContext.getClientId());
}
throw new ParClientException(oAuthClientAuthnContext.getErrorCode(),
oAuthClientAuthnContext.getErrorMessage());
}

throw new ParClientException(OAuth2ErrorCodes.UNAUTHORIZED_CLIENT, "Client authentication required");
throw new ParClientException(OAuth2ErrorCodes.UNAUTHORIZED_CLIENT, ParConstants.CLIENT_AUTH_REQUIRED_ERROR);
}

private OAuthClientAuthnContext getClientAuthnContext(HttpServletRequest request) {
Expand All @@ -190,8 +190,51 @@ private OAuthClientAuthnContext createNewOAuthClientAuthnContext() {

OAuthClientAuthnContext oAuthClientAuthnContext = new OAuthClientAuthnContext();
oAuthClientAuthnContext.setAuthenticated(false);
oAuthClientAuthnContext.setErrorMessage(PAR_CLIENT_AUTH_ERROR);
oAuthClientAuthnContext.setErrorCode(OAuthError.TokenResponse.INVALID_REQUEST);
oAuthClientAuthnContext.setErrorMessage(ParConstants.PAR_CLIENT_AUTH_ERROR);
oAuthClientAuthnContext.setErrorCode(OAuth2ErrorCodes.INVALID_REQUEST);
return oAuthClientAuthnContext;
}

private void validateClient(HttpServletRequest request, MultivaluedMap<String, String> params)
throws ParClientException {

OAuth2ClientValidationResponseDTO validationResponse = getOAuth2Service().validateClientInfo(request);

if (!validationResponse.isValidClient()) {
throw new ParClientException(validationResponse.getErrorCode(), validationResponse.getErrorMsg());
}
if (isRequestUriProvided(params)) {
throw new ParClientException(OAuth2ErrorCodes.INVALID_REQUEST,
ParConstants.REQUEST_URI_IN_REQUEST_BODY_ERROR);
}
Yoshani marked this conversation as resolved.
Show resolved Hide resolved
}

private void validateRepeatedParams(HttpServletRequest request, Map<String, List<String>> paramMap)
throws ParClientException {

if (!validateParams(request, paramMap)) {
throw new ParClientException(OAuth2ErrorCodes.INVALID_REQUEST,
ParConstants.REPEATED_PARAMS_IN_REQUEST_ERROR);
}
}

private void validateAuthzRequest(HttpServletRequest request) throws ParCoreException {

try {
getOAuthAuthzRequest(request);
} catch (OAuthProblemException e) {
throw new ParClientException(e.getError(), e.getDescription(), e);
} catch (OAuthSystemException e) {
throw new ParCoreException(OAuth2ErrorCodes.SERVER_ERROR, e.getMessage(), e);
}
}

private void validateInputParameters(HttpServletRequest request) throws ParClientException {

try {
getOAuth2Service().validateInputParameters(request);
} catch (InvalidOAuthRequestException e) {
throw new ParClientException(e.getErrorCode(), e.getMessage(), e);
}
}
}
Loading
Loading