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.

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

Improve framework to skip retry from authenticator #6334

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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);
}
Expand Down
Original file line number Diff line number Diff line change
@@ -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<String, String> parameterMap = new HashMap<>();
authenticatorConfig.setParameterMap(parameterMap);
when(abstractApplicationAuthenticator.getAuthenticatorConfig()).thenReturn(authenticatorConfig);
Map<String, String> 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());
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@
<!--<parameter name="log-level" value="debug"/>-->
<classes>
<class name="org.wso2.carbon.identity.application.authentication.framework.ApplicationAuthenticationServiceTest"/>
<class name="org.wso2.carbon.identity.application.authentication.framework.AbstractAppAuthSkipRetryTest"/>

<class name="org.wso2.carbon.identity.application.authentication.framework.handler.claims.impl.DefaultClaimHandlerTest"/>
<class name="org.wso2.carbon.identity.application.authentication.framework.handler.hrd.impl.DefaultHomeRealmDiscovererTest"/>
Expand Down
Loading