resolve suggestions

wso2-extensions · Aug 7, 2023 · 4dc01d6 · 4dc01d6
1 parent 81fa5f4
commit 4dc01d6
Show file tree

Hide file tree

Showing 13 changed files with 95 additions and 68 deletions.
diff --git a/...tity.oauth.common/src/main/java/org/wso2/carbon/identity/oauth/common/OAuthConstants.java b/...tity.oauth.common/src/main/java/org/wso2/carbon/identity/oauth/common/OAuthConstants.java
@@ -574,6 +574,7 @@ 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";
         }
 
         /**
@@ -587,6 +588,7 @@ 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";
         }
     }
 

diff --git a/...oint/src/main/java/org/wso2/carbon/identity/oauth/endpoint/authz/OAuth2AuthzEndpoint.java b/...oint/src/main/java/org/wso2/carbon/identity/oauth/endpoint/authz/OAuth2AuthzEndpoint.java
@@ -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;
@@ -284,7 +285,7 @@ public Response authorize(@Context HttpServletRequest request, @Context HttpServ
         } catch (InvalidRequestParentException e) {
             EndpointUtil.triggerOnAuthzRequestException(e, request);
             throw e;
-        } catch (OAuthProblemException e) {
+        } catch (IdentityException e) {
             EndpointUtil.triggerOnAuthzRequestException(e, request);
             throw new InvalidRequestException(e.getMessage(), OAuth2ErrorCodes.INVALID_REQUEST, e);
         }

diff --git a/...endpoint/src/main/java/org/wso2/carbon/identity/oauth/endpoint/par/OAuth2ParEndpoint.java b/...endpoint/src/main/java/org/wso2/carbon/identity/oauth/endpoint/par/OAuth2ParEndpoint.java
@@ -167,8 +167,7 @@ private void checkClientAuthentication(HttpServletRequest request) throws ParCor
                         oAuthClientAuthnContext.getErrorMessage());
             } else if (OAuth2ErrorCodes.INVALID_CLIENT.equals(oAuthClientAuthnContext.getErrorCode())) {
                 throw new ParClientException(oAuthClientAuthnContext.getErrorCode(),
-                        "A valid OAuth client could not be found for client_id: " +
-                                oAuthClientAuthnContext.getClientId());
+                        ParConstants.INVALID_CLIENT_ERROR + oAuthClientAuthnContext.getClientId());
             }
             throw new ParClientException(oAuthClientAuthnContext.getErrorCode(),
                     oAuthClientAuthnContext.getErrorMessage());
@@ -223,9 +222,9 @@ private void validateAuthzRequest(HttpServletRequest request) throws ParCoreExce
         try {
             EndpointUtil.getOAuthAuthzRequest(request);
         } catch (OAuthProblemException e) {
-            throw new ParClientException(e.getError(), e.getDescription());
+            throw new ParClientException(e.getError(), e.getDescription(), e);
         } catch (OAuthSystemException e) {
-            throw new ParCoreException(OAuth2ErrorCodes.SERVER_ERROR, e.getMessage());
+            throw new ParCoreException(OAuth2ErrorCodes.SERVER_ERROR, e.getMessage(), e);
         }
     }
 
@@ -234,7 +233,7 @@ private void validateInputParameters(HttpServletRequest request) throws ParClien
         try {
             getOAuth2Service().validateInputParameters(request);
         } catch (InvalidOAuthRequestException e) {
-            throw new ParClientException(e.getErrorCode(), e.getMessage());
+            throw new ParClientException(e.getErrorCode(), e.getMessage(), e);
         }
     }
 }
diff --git a/...uth.endpoint/src/main/java/org/wso2/carbon/identity/oauth/endpoint/util/EndpointUtil.java b/...uth.endpoint/src/main/java/org/wso2/carbon/identity/oauth/endpoint/util/EndpointUtil.java
@@ -1845,7 +1845,6 @@ public static OAuthAuthzRequest getOAuthAuthzRequest(HttpServletRequest request)
         }
         log.debug("Initiating the default OAuthAuthzRequest implementation");
         return new CarbonOAuthAuthzRequest(request);
-
     }
 
     /**
@@ -1873,7 +1872,6 @@ private static Class<? extends OAuthAuthzRequest> getOAuthAuthzRequestClass() th
                     OAuthServerConfiguration.getInstance().getOAuthAuthzRequestClassName();
             oAuthAuthzRequestClass = (Class<? extends OAuthAuthzRequest>) Thread.currentThread()
                     .getContextClassLoader().loadClass(oauthAuthzRequestClassName);
-
         }
         return oAuthAuthzRequestClass;
     }

diff --git a/...ntity.oauth.par/src/main/java/org/wso2/carbon/identity/oauth/par/common/ParConstants.java b/...ntity.oauth.par/src/main/java/org/wso2/carbon/identity/oauth/par/common/ParConstants.java
@@ -41,6 +41,7 @@ public class ParConstants {
     public static final String CLIENT_AUTH_REQUIRED_ERROR = "Client authentication required.";
     public static final String INTERNAL_SERVER_ERROR = "Internal Server Error.";
     public static final String INVALID_REQUEST_URI_FORMAT = "Invalid request_uri format";
+    public static final String INVALID_CLIENT_ERROR = "A valid OAuth client could not be found for client_id: ";
 
     private ParConstants() {
 

diff --git a/...uth/par/model/OAuthParRequestWrapper.java → ...auth/par/core/OAuthParRequestWrapper.java b/...uth/par/model/OAuthParRequestWrapper.java → ...auth/par/core/OAuthParRequestWrapper.java
@@ -15,9 +15,9 @@
  * specific language governing permissions and limitations
  * under the License.
  */
-package org.wso2.carbon.identity.oauth.par.model;
+package org.wso2.carbon.identity.oauth.par.core;
 
-import org.apache.oltu.oauth2.common.exception.OAuthProblemException;
+import org.wso2.carbon.identity.base.IdentityException;
 import org.wso2.carbon.identity.oauth.common.OAuthConstants;
 import org.wso2.carbon.identity.oauth.par.common.ParConstants;
 import org.wso2.carbon.identity.oauth.par.exceptions.ParAuthFailureException;
@@ -43,9 +43,9 @@ public class OAuthParRequestWrapper extends HttpServletRequestWrapper {
      * Wraps the request with parameters obtained from the PAR endpoint.
      *
      * @param request HttpServletRequest.
-     * @throws OAuthProblemException OAuthProblemException.
+     * @throws IdentityException IdentityException.
      */
-    public OAuthParRequestWrapper(HttpServletRequest request) throws OAuthProblemException {
+    public OAuthParRequestWrapper(HttpServletRequest request) throws IdentityException {
 
         super(request);
 
@@ -60,12 +60,12 @@ public OAuthParRequestWrapper(HttpServletRequest request) throws OAuthProblemExc
             params = ParUtil.getParAuthService()
                     .retrieveParams(uuid, request.getParameter(OAuthConstants.OAuth20Params.CLIENT_ID));
             params.put(OAuthConstants.ALLOW_REQUEST_URI_AND_REQUEST_OBJECT_IN_REQUEST, "true");
-            // Set request_uri to empty string to avoid conflicting with OIDC flow.
+            // Set request_uri to empty string to avoid conflicting with OIDC requests passed by reference.
             params.put(OAuthConstants.OAuth20Params.REQUEST_URI, "");
         } catch (ParClientException e) {
-            throw new ParAuthFailureException(e.getMessage());
+            throw new ParAuthFailureException(e.getMessage(), e);
         } catch (ParCoreException e) {
-            throw new ParAuthFailureException("Error occurred while retrieving params from PAR request");
+            throw new ParAuthFailureException("Error occurred while retrieving params from PAR request", e);
         }
     }
 

diff --git a/...ntity.oauth.par/src/main/java/org/wso2/carbon/identity/oauth/par/core/ParAuthService.java b/...ntity.oauth.par/src/main/java/org/wso2/carbon/identity/oauth/par/core/ParAuthService.java
@@ -1,4 +1,4 @@
-/**
+/*
  * Copyright (c) 2023, WSO2 LLC. (https://www.wso2.com).
  *
  * WSO2 LLC. licenses this file to you under the Apache License,

diff --git a/...y.oauth.par/src/main/java/org/wso2/carbon/identity/oauth/par/core/ParAuthServiceImpl.java b/...y.oauth.par/src/main/java/org/wso2/carbon/identity/oauth/par/core/ParAuthServiceImpl.java
@@ -1,4 +1,4 @@
-/**
+/*
  * Copyright (c) 2023, WSO2 LLC. (https://www.wso2.com).
  *
  * WSO2 LLC. licenses this file to you under the Apache License,

diff --git a/...ty.oauth.par/src/main/java/org/wso2/carbon/identity/oauth/par/core/ParRequestBuilder.java b/...ty.oauth.par/src/main/java/org/wso2/carbon/identity/oauth/par/core/ParRequestBuilder.java
@@ -18,39 +18,67 @@
 package org.wso2.carbon.identity.oauth.par.core;
 
 import org.apache.commons.lang.StringUtils;
-import org.apache.oltu.oauth2.common.exception.OAuthProblemException;
+import org.apache.commons.logging.Log;
+import org.apache.commons.logging.LogFactory;
 import org.apache.oltu.oauth2.common.utils.OAuthUtils;
+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.oauth.common.OAuthConstants;
-import org.wso2.carbon.identity.oauth.par.model.OAuthParRequestWrapper;
 import org.wso2.carbon.identity.oauth2.AbstractRequestBuilder;
 import org.wso2.carbon.identity.oauth2.util.OAuth2Util;
+import org.wso2.carbon.utils.DiagnosticLog;
 
 import javax.servlet.http.HttpServletRequest;
 
+import static org.wso2.carbon.identity.oauth.common.OAuthConstants.LogConstants.InputKeys.REQUEST_BUILDER;
+
 /**
  * This builds the PAR request if the incoming request satisfies the PAR conditions.
  */
 public class ParRequestBuilder implements AbstractRequestBuilder {
 
+    private static final Log log = LogFactory.getLog(ParRequestBuilder.class);
+
     private static final String REQUEST_BUILDER_NAME = "Pushed authorization request builder";
 
     @Override
-    public HttpServletRequest buildRequest(HttpServletRequest request) throws OAuthProblemException {
+    public HttpServletRequest buildRequest(HttpServletRequest request) throws IdentityException {
 
         return new OAuthParRequestWrapper(request);
     }
 
     @Override
     public boolean canHandle(HttpServletRequest request) {
 
-        boolean isOIDCRequest = OAuth2Util.isOIDCAuthzRequest(OAuthUtils.decodeScopes(request.getParameter("scope")));
-        return StringUtils.isNotBlank(request.getParameter(OAuthConstants.OAuth20Params.REQUEST_URI)) && !isOIDCRequest;
+        // Requests that separately contain the scope `openid` will not be handled in the PAR flow since they are
+        // considered as OIDC requests passed by reference. Refer section 6.2.2 of the OIDC Core spec.
+        boolean isParRequest = !OAuth2Util.isOIDCAuthzRequest(OAuthUtils.decodeScopes(request.getParameter("scope")));
+        boolean canHandle = StringUtils.isNotBlank(request.getParameter(OAuthConstants.OAuth20Params.REQUEST_URI))
+                && isParRequest;
+
+        if (!isParRequest) {
+            log.debug("Request is an OIDC request. Therefore, PAR request builder cannot handle the request.");
+        }
+
+        if (canHandle && LoggerUtils.isDiagnosticLogsEnabled()) {
+            DiagnosticLog.DiagnosticLogBuilder diagnosticLogBuilder = new DiagnosticLog.DiagnosticLogBuilder(
+                    OAuthConstants.LogConstants.OAUTH_INBOUND_SERVICE,
+                    OAuthConstants.LogConstants.ActionIDs.HANDLE_REQUEST);
+            diagnosticLogBuilder
+                    .inputParam(LogConstants.InputKeys.CLIENT_ID,
+                            request.getParameter(OAuthConstants.OAuth20Params.CLIENT_ID))
+                    .inputParam(REQUEST_BUILDER, getName())
+                    .resultMessage("PAR request builder handling the request")
+                    .logDetailLevel(DiagnosticLog.LogDetailLevel.APPLICATION);
+            LoggerUtils.triggerDiagnosticLogEvent(diagnosticLogBuilder);
+        }
+        return canHandle;
     }
 
     @Override
     public String getName() {
 
         return REQUEST_BUILDER_NAME;
     }
-
 }
diff --git a/.../src/main/java/org/wso2/carbon/identity/oauth/par/exceptions/ParAuthFailureException.java b/.../src/main/java/org/wso2/carbon/identity/oauth/par/exceptions/ParAuthFailureException.java
@@ -1,4 +1,4 @@
-/**
+/*
  * Copyright (c) 2023, WSO2 LLC. (https://www.wso2.com).
  *
  * WSO2 LLC. licenses this file to you under the Apache License,
@@ -18,15 +18,12 @@
 
 package org.wso2.carbon.identity.oauth.par.exceptions;
 
-import org.apache.oltu.oauth2.common.exception.OAuthProblemException;
+import org.wso2.carbon.identity.base.IdentityException;
 
 /**
  * PAR Authorization flow failure.
  */
-public class ParAuthFailureException extends OAuthProblemException {
-
-    private String errorCode;
-    private String errorMsg;
+public class ParAuthFailureException extends IdentityException {
 
     /**
      * Constructor with error message.
@@ -39,42 +36,13 @@ public ParAuthFailureException(String errorMsg) {
     }
 
     /**
-     * Get error code.
-     *
-     * @return Error code.
-     */
-    public String getErrorCode() {
-
-        return errorCode;
-    }
-
-    /**
-     * Get error message.
-     *
-     * @return Error message.
-     */
-    public String getErrorMsg() {
-
-        return errorMsg;
-    }
-
-    /**
-     * Set error code.
-     *
-     * @param errorCode Error code.
-     */
-    public void setErrorCode(String errorCode) {
-
-        this.errorCode = errorCode;
-    }
-
-    /**
-     * Set error message.
+     * Constructor with error message and throwable.
      *
-     * @param  errorMsg Error message.
+     * @param message Error message.
+     * @param cause Throwable.
      */
-    public void setErrorMsg(String errorMsg) {
+    public ParAuthFailureException(String message, Throwable cause) {
 
-        this.errorMsg = errorMsg;
+        super(message, cause);
     }
 }
diff --git a/...h.par/src/main/java/org/wso2/carbon/identity/oauth/par/exceptions/ParClientException.java b/...h.par/src/main/java/org/wso2/carbon/identity/oauth/par/exceptions/ParClientException.java
@@ -1,4 +1,4 @@
-/**
+/*
  * Copyright (c) 2023, WSO2 LLC. (https://www.wso2.com).
  *
  * WSO2 LLC. licenses this file to you under the Apache License,
@@ -33,4 +33,16 @@ public ParClientException(String errorCode, String message) {
 
         super(errorCode, message);
     }
+
+    /**
+     * Constructor with error code, error message and throwable.
+     *
+     * @param errorCode Error code.
+     * @param message Error message.
+     * @param cause Throwable.
+     */
+    public ParClientException(String errorCode, String message, Throwable cause) {
+
+        super(errorCode, message, cause);
+    }
 }
diff --git a/...uth.par/src/main/java/org/wso2/carbon/identity/oauth/par/exceptions/ParCoreException.java b/...uth.par/src/main/java/org/wso2/carbon/identity/oauth/par/exceptions/ParCoreException.java
@@ -1,4 +1,4 @@
-/**
+/*
  * Copyright (c) 2023, WSO2 LLC. (https://www.wso2.com).
  *
  * WSO2 LLC. licenses this file to you under the Apache License,
@@ -46,8 +46,26 @@ public ParCoreException(String errorCode, String message) {
         super(errorCode, message);
     }
 
+    /**
+     * Constructor with error message and throwable.
+     *
+     * @param message Error message.
+     * @param cause Throwable.
+     */
     public ParCoreException(String message, Throwable cause) {
 
         super(message, cause);
     }
+
+    /**
+     * Constructor with error code, error message and throwable.
+     *
+     * @param errorCode Error code.
+     * @param message Error message.
+     * @param cause Throwable.
+     */
+    public ParCoreException(String errorCode, String message, Throwable cause) {
+
+        super(errorCode, message, cause);
+    }
 }
diff --git a/....identity.oauth/src/main/java/org/wso2/carbon/identity/oauth2/AbstractRequestBuilder.java b/....identity.oauth/src/main/java/org/wso2/carbon/identity/oauth2/AbstractRequestBuilder.java
@@ -17,7 +17,7 @@
  */
 package org.wso2.carbon.identity.oauth2;
 
-import org.apache.oltu.oauth2.common.exception.OAuthProblemException;
+import org.wso2.carbon.identity.base.IdentityException;
 
 import javax.servlet.http.HttpServletRequest;
 
@@ -31,9 +31,9 @@ public interface AbstractRequestBuilder {
      *
      * @param request Incoming HttpServletRequest.
      * @return Built HttpServletRequest.
-     * @throws OAuthProblemException OAuthProblemException.
+     * @throws IdentityException IdentityException.
      */
-    HttpServletRequest buildRequest(HttpServletRequest request) throws OAuthProblemException;
+    HttpServletRequest buildRequest(HttpServletRequest request) throws IdentityException;
 
     /**
      * Check whether the request builder can handle the incoming request.