Merge pull request #1441 from kasundharmadasa/change-error-response-a…

…ccording-to-response-type Redirect to callback URL during error scenarios according to the response type
wso2-extensions · Sep 11, 2020 · b9ec616 · b9ec616
2 parents 81deeb5 + 7c4c36d
commit b9ec616
Show file tree

Hide file tree

Showing 3 changed files with 101 additions and 15 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
@@ -216,6 +216,7 @@ public static class OAuth20Params {
         public static final String CLIENT_ID = "client_id";
         public static final String REDIRECT_URI = "redirect_uri";
         public static final String STATE = "state";
+        public static final String RESPONSE_TYPE = "response_type";
         public static final String REQUEST = "request";
 
         private OAuth20Params() {

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
@@ -429,22 +429,13 @@ public static String getErrorPageURL(HttpServletRequest request, String errorCod
             return getErrorPageURL(request, errorCode, errorMessage, appName);
         } else {
             String redirectUri = request.getParameter(OAuthConstants.OAuth20Params.REDIRECT_URI);
-            String state = retrieveStateForErrorURL(oAuth2Parameters);
 
-            Map<String, String> params = new HashMap<>();
-            params.put(PROP_ERROR, errorCode);
-            params.put(PROP_ERROR_DESCRIPTION, errorMessage);
-            if (state != null) {
-                params.put(OAuthConstants.OAuth20Params.STATE, state);
-            }
-
-            try {
-                redirectUri = FrameworkUtils.buildURLWithQueryParams(redirectUri, params);
-            } catch (UnsupportedEncodingException e) {
-                //ignore
-                if (log.isDebugEnabled()) {
-                    log.debug("Error while encoding the error page url", e);
-                }
+            // If the redirect url is not set in the request, page is redirected to common OAuth error page.
+            if (StringUtils.isBlank(redirectUri)) {
+                redirectUri = getErrorPageURL(request, errorCode, errorMessage, appName);
+            } else {
+                String state = retrieveStateForErrorURL(oAuth2Parameters);
+                redirectUri = getUpdatedRedirectURL(request, redirectUri, errorCode, errorMessage, state, appName);
             }
             return redirectUri;
         }
@@ -966,6 +957,40 @@ private static String retrieveStateForErrorURL(OAuth2Parameters oAuth2Parameters
         return state;
     }
 
+    /**
+     * Return updated redirect URL.
+     *
+     * @param request       HttpServletRequest
+     * @param redirectUri   Redirect Uri
+     * @param errorCode     Error Code
+     * @param errorMessage  Message of the error
+     * @param state         State from the request
+     * @param appName       Application Name
+     * @return Updated Redirect URL
+     */
+    private static String getUpdatedRedirectURL(HttpServletRequest request, String redirectUri, String errorCode,
+                                                String errorMessage, String state, String appName) {
+
+        String updatedRedirectUri = redirectUri;
+        try {
+            OAuthProblemException ex = OAuthProblemException.error(errorCode).description(errorMessage);
+            if (OAuth2Util.isImplicitResponseType(request.getParameter(OAuthConstants.OAuth20Params.RESPONSE_TYPE))
+                    || OAuth2Util.isHybridResponseType(request.getParameter(OAuthConstants.OAuth20Params.
+                    RESPONSE_TYPE))) {
+                updatedRedirectUri = OAuthASResponse.errorResponse(HttpServletResponse.SC_FOUND)
+                        .error(ex).location(redirectUri).setState(state).setParam(OAuth.OAUTH_ACCESS_TOKEN, null)
+                        .buildQueryMessage().getLocationUri();
+            } else {
+                updatedRedirectUri = OAuthASResponse.errorResponse(HttpServletResponse.SC_FOUND)
+                        .error(ex).location(redirectUri).setState(state).buildQueryMessage().getLocationUri();
+            }
+
+        } catch (OAuthSystemException e) {
+            log.error("Server error occurred while building error redirect url for application: " + appName, e);
+        }
+        return updatedRedirectUri;
+    }
+
     /**
      * Method to retrieve the <AllowAdditionalParamsFromErrorUrl> config from the OAuth Configuration.
      * @return Retrieved config (true or false)

diff --git a/...endpoint/src/test/java/org/wso2/carbon/identity/oauth/endpoint/util/EndpointUtilTest.java b/...endpoint/src/test/java/org/wso2/carbon/identity/oauth/endpoint/util/EndpointUtilTest.java
@@ -428,6 +428,66 @@ public void testGetErrorRedirectURL(boolean isImplicitResponse, boolean isImplic
         Assert.assertTrue(url.contains(expected), "Expected error redirect url not returned");
     }
 
+    @DataProvider(name = "provideErrorPageData")
+    public Object[][] provideErrorPageData() {
+
+        OAuth2Parameters params1 = new OAuth2Parameters();
+        OAuth2Parameters params2 = new OAuth2Parameters();
+        String state = "active";
+        String responseType = "dummyResponceType";
+        String appName = "myApp";
+
+        params1.setState(state);
+        params1.setResponseType(responseType);
+        params1.setApplicationName(appName);
+        params1.setRedirectURI("http://localhost:8080/callback");
+
+        params2.setState(state);
+        params2.setResponseType(responseType);
+        params2.setApplicationName(appName);
+        params2.setRedirectURI(null);
+
+        return new Object[][]{
+                {true, true, true, params1, "http://localhost:8080/location", false},
+                {true, false, true, params1, "http://localhost:8080/location", false},
+                {false, true, true, params1, "http://localhost:8080/location", true},
+                {false, false, false, params1, ERROR_PAGE_URL, true},
+        };
+    }
+
+    @Test(dataProvider = "provideErrorPageData")
+    public void testGetErrorPageURL(boolean isImplicitResponse, boolean isHybridResponse,
+                                    boolean isRedirectToRedirectURI, Object oAuth2ParamObject, String expected,
+                                    boolean isDebugOn)
+            throws Exception {
+
+        setMockedLog(isDebugOn);
+
+        OAuth2Parameters parameters = (OAuth2Parameters) oAuth2ParamObject;
+
+        mockStatic(OAuthServerConfiguration.class);
+        when(OAuthServerConfiguration.getInstance()).thenReturn(mockedOAuthServerConfiguration);
+        when(mockedOAuthServerConfiguration.isRedirectToRequestedRedirectUriEnabled())
+                .thenReturn(isRedirectToRedirectURI);
+
+        mockStatic(OAuth2Util.class);
+        when(OAuth2Util.isImplicitResponseType(anyString())).thenReturn(isImplicitResponse);
+        when(OAuth2Util.isHybridResponseType(anyString())).thenReturn(isHybridResponse);
+
+
+        mockStatic(OAuth2Util.OAuthURL.class);
+        when(OAuth2Util.OAuthURL.getOAuth2ErrorPageUrl()).thenReturn(ERROR_PAGE_URL);
+
+        when(mockedOAuthResponse.getLocationUri()).thenReturn("http://localhost:8080/location");
+        when(mockedHttpServletRequest.getParameter(anyString())).thenReturn("http://localhost:8080/location");
+
+        String url = EndpointUtil.getErrorPageURL(mockedHttpServletRequest, "invalid request",
+                "invalid request object", "invalid request", "test", parameters);
+
+        Assert.assertTrue(url.contains(expected), "Expected error redirect url not returned");
+
+    }
+
     @DataProvider(name = "provideParams")
     public Object[][] provideParams() {