diff --git a/components/authentication-framework/org.wso2.carbon.identity.application.authentication.framework/src/main/java/org/wso2/carbon/identity/application/authentication/framework/AbstractApplicationAuthenticator.java b/components/authentication-framework/org.wso2.carbon.identity.application.authentication.framework/src/main/java/org/wso2/carbon/identity/application/authentication/framework/AbstractApplicationAuthenticator.java index 098c86a7245d..cf8dcdda75cd 100644 --- a/components/authentication-framework/org.wso2.carbon.identity.application.authentication.framework/src/main/java/org/wso2/carbon/identity/application/authentication/framework/AbstractApplicationAuthenticator.java +++ b/components/authentication-framework/org.wso2.carbon.identity.application.authentication.framework/src/main/java/org/wso2/carbon/identity/application/authentication/framework/AbstractApplicationAuthenticator.java @@ -84,6 +84,7 @@ public abstract class AbstractApplicationAuthenticator implements ApplicationAut private static final long serialVersionUID = -4406878411547612129L; private static final Log log = LogFactory.getLog(AbstractApplicationAuthenticator.class); public static final String ENABLE_RETRY_FROM_AUTHENTICATOR = "enableRetryFromAuthenticator"; + public static final String SKIP_RETRY_FROM_AUTHENTICATOR = "skipRetryFromAuthenticator"; @Override public AuthenticatorFlowStatus process(HttpServletRequest request, @@ -133,8 +134,17 @@ public AuthenticatorFlowStatus process(HttpServletRequest request, boolean sendToMultiOptionPage = isStepHasMultiOption(context) && isRedirectToMultiOptionPageOnFailure(); context.setSendToMultiOptionPage(sendToMultiOptionPage); - context.setRetrying(retryAuthenticationEnabled() && !skipPrompt); - if (retryAuthenticationEnabled(context) && !sendToMultiOptionPage) { + /* + Certain authenticators require to fail the flow when there is an error or when the user + aborts the authentication flow. When retry is enabled for the authenticator, the flow won't fail + hence it retries and shows the error in it. SKIP_RETRY_FROM_AUTHENTICATOR is introduced to + forcefully skip the retry and fail the flow which ends up in the client application. This logic + can be further improved to handle the retry logic with user abort scenarios. + */ + boolean skipRetryFromAuthenticator = context.getProperty(SKIP_RETRY_FROM_AUTHENTICATOR) != null + && (Boolean) context.getProperty(SKIP_RETRY_FROM_AUTHENTICATOR); + context.setRetrying(retryAuthenticationEnabled() && !skipPrompt && !skipRetryFromAuthenticator); + if (retryAuthenticationEnabled(context) && !sendToMultiOptionPage && !skipRetryFromAuthenticator) { if (log.isDebugEnabled()) { log.debug("Error occurred during the authentication process, hence retrying.", e); } diff --git a/components/authentication-framework/org.wso2.carbon.identity.application.authentication.framework/src/test/java/org/wso2/carbon/identity/application/authentication/framework/AbstractAppAuthSkipRetryTest.java b/components/authentication-framework/org.wso2.carbon.identity.application.authentication.framework/src/test/java/org/wso2/carbon/identity/application/authentication/framework/AbstractAppAuthSkipRetryTest.java new file mode 100644 index 000000000000..cf14aea131b1 --- /dev/null +++ b/components/authentication-framework/org.wso2.carbon.identity.application.authentication.framework/src/test/java/org/wso2/carbon/identity/application/authentication/framework/AbstractAppAuthSkipRetryTest.java @@ -0,0 +1,134 @@ +/* + * Copyright (c) 2017, WSO2 Inc. (http://www.wso2.org) All Rights Reserved. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.wso2.carbon.identity.application.authentication.framework; + +import org.mockito.Mock; +import org.mockito.Spy; +import org.testng.annotations.BeforeTest; +import org.testng.annotations.Test; +import org.wso2.carbon.identity.application.authentication.framework.config.model.AuthenticatorConfig; +import org.wso2.carbon.identity.application.authentication.framework.config.model.SequenceConfig; +import org.wso2.carbon.identity.application.authentication.framework.config.model.graph.AuthenticationGraph; +import org.wso2.carbon.identity.application.authentication.framework.context.AuthenticationContext; +import org.wso2.carbon.identity.application.authentication.framework.exception.AuthenticationFailedException; +import org.wso2.carbon.identity.application.authentication.framework.internal.FrameworkServiceDataHolder; +import org.wso2.carbon.identity.application.authentication.framework.util.FrameworkConstants; + +import java.util.HashMap; +import java.util.Map; + +import javax.servlet.http.HttpServletRequest; +import javax.servlet.http.HttpServletResponse; + +import static org.mockito.ArgumentMatchers.any; +import static org.mockito.ArgumentMatchers.anyString; +import static org.mockito.Mockito.doCallRealMethod; +import static org.mockito.Mockito.doReturn; +import static org.mockito.Mockito.doThrow; +import static org.mockito.Mockito.when; +import static org.mockito.MockitoAnnotations.initMocks; +import static org.testng.Assert.assertFalse; +import static org.testng.Assert.assertThrows; + +public class AbstractAppAuthSkipRetryTest { + + @Mock + AbstractApplicationAuthenticator abstractApplicationAuthenticator; + + @Mock + HttpServletRequest request; + + @Mock + HttpServletResponse response; + + @Mock + FrameworkServiceDataHolder frameworkServiceDataHolder; + + @Mock + AuthenticationDataPublisher authenticationDataPublisherProxy; + + @Spy + AuthenticationContext context; + + @Mock + SequenceConfig sequenceConfig; + + private static final String AUTHENTICATOR = "AbstractAuthenticator"; + private static final String USER_NAME = "DummyUser"; + private static final String USER_STORE_NAME = "TEST.COM"; + private static final String TENANT_DOMAIN = "ABC.COM"; + + abstract class TestApplicationAuthenticator extends AbstractApplicationAuthenticator + implements FederatedApplicationAuthenticator { + } + + @Mock + TestApplicationAuthenticator testApplicationAuthenticator; + + @BeforeTest + public void setUp() throws Exception { + initMocks(this); + when(abstractApplicationAuthenticator.retryAuthenticationEnabled()).thenCallRealMethod(); + when(abstractApplicationAuthenticator.retryAuthenticationEnabled(any())).thenCallRealMethod(); + when(abstractApplicationAuthenticator.getName()).thenReturn(AUTHENTICATOR); + context.initializeAnalyticsData(); + when(context.getTenantDomain()).thenReturn(TENANT_DOMAIN); + doCallRealMethod().when(abstractApplicationAuthenticator).getUserStoreAppendedName(anyString()); + doCallRealMethod().when(abstractApplicationAuthenticator).process(request, response, context); + } + + @Test + public void testProcessRetryLogic() throws Exception { + + // Mock necessary behavior + when(context.isLogoutRequest()).thenReturn(false); + when(abstractApplicationAuthenticator.retryAuthenticationEnabled()).thenReturn(true); + when(context.getSequenceConfig()).thenReturn(sequenceConfig); + when(context.getCurrentAuthenticator()).thenReturn("TestAuthenticator"); + AuthenticatorConfig authenticatorConfig = new AuthenticatorConfig(); + Map parameterMap = new HashMap<>(); + authenticatorConfig.setParameterMap(parameterMap); + when(abstractApplicationAuthenticator.getAuthenticatorConfig()).thenReturn(authenticatorConfig); + Map authParams = new HashMap<>(); + authParams.put(AbstractApplicationAuthenticator.ENABLE_RETRY_FROM_AUTHENTICATOR, "true"); + when(context.getAuthenticatorParams("TestAuthenticator")).thenReturn(authParams); + AuthenticationGraph graph = new AuthenticationGraph(); + graph.setEnabled(true); + when(sequenceConfig.getAuthenticationGraph()).thenReturn(graph); + + when(context.getProperty(AbstractApplicationAuthenticator.SKIP_RETRY_FROM_AUTHENTICATOR)).thenReturn(true); + when(request.getAttribute(FrameworkConstants.REQ_ATTR_HANDLED)).thenReturn(false); + doReturn(false).when(abstractApplicationAuthenticator).isRedirectToMultiOptionPageOnFailure(); + doReturn(true).when(abstractApplicationAuthenticator).canHandle(request); + + // Setting context properties + context.setProperty(AbstractApplicationAuthenticator.SKIP_RETRY_FROM_AUTHENTICATOR, true); + + // Mocking the behavior of processAuthenticationResponse + doThrow(new AuthenticationFailedException("")).when(abstractApplicationAuthenticator) + .processAuthenticationResponse(request, response, context); + + // Mock authenticator name + doReturn("AbstractApplicationAuthenticator").when(abstractApplicationAuthenticator).getName(); + + // since retry is skipped, expect an exception. + assertThrows(AuthenticationFailedException.class, () -> { + abstractApplicationAuthenticator.process(request, response, context); + }); + assertFalse(context.isRetrying()); + } +} diff --git a/components/authentication-framework/org.wso2.carbon.identity.application.authentication.framework/src/test/resources/testng.xml b/components/authentication-framework/org.wso2.carbon.identity.application.authentication.framework/src/test/resources/testng.xml index 6a92baeb22da..326c8e3100eb 100644 --- a/components/authentication-framework/org.wso2.carbon.identity.application.authentication.framework/src/test/resources/testng.xml +++ b/components/authentication-framework/org.wso2.carbon.identity.application.authentication.framework/src/test/resources/testng.xml @@ -21,6 +21,7 @@ +