Declining terms and conditions in account-console results in error

Closes #28328

Signed-off-by: vramik <vramik@redhat.com>
This commit is contained in:
vramik 2024-10-10 14:20:26 +02:00 committed by Pedro Igor
parent 8c2bc39418
commit 3d91df42d8
10 changed files with 89 additions and 17 deletions

View file

@ -120,6 +120,8 @@ public interface RequiredActionContext {
Status getStatus(); Status getStatus();
String getErrorMessage();
/** /**
* Send a challenge Response back to user * Send a challenge Response back to user
* *
@ -127,11 +129,19 @@ public interface RequiredActionContext {
*/ */
void challenge(Response response); void challenge(Response response);
/**
* Abort the authentication with an error, optionally with an erroMessage.
*
*/
void failure(String errorMessage);
/** /**
* Abort the authentication with an error * Abort the authentication with an error
* *
*/ */
void failure(); default void failure() {
failure(null);
}
/** /**
* Mark this required action as successful. The required action will be removed from the UserModel * Mark this required action as successful. The required action will be removed from the UserModel

View file

@ -86,6 +86,10 @@ public interface LoginProtocol extends Provider {
Response sendError(AuthenticationSessionModel authSession, Error error); Response sendError(AuthenticationSessionModel authSession, Error error);
default Response sendError(AuthenticationSessionModel authSession, Error error, String errorMessage) {
return sendError(authSession, error);
}
/** /**
* Returns client data, which will be wrapped in the "clientData" parameter sent within "authentication flow" requests. The purpose of clientData is to be able to send HTTP error * Returns client data, which will be wrapped in the "clientData" parameter sent within "authentication flow" requests. The purpose of clientData is to be able to send HTTP error
* response back to the client if authentication fails due some error and authenticationSession is not available anymore (was either expired or removed). So clientData need to contain * response back to the client if authentication fails due some error and authenticationSession is not available anymore (was either expired or removed). So clientData need to contain

View file

@ -46,6 +46,7 @@ public class RequiredActionContextResult implements RequiredActionContext {
protected EventBuilder eventBuilder; protected EventBuilder eventBuilder;
protected KeycloakSession session; protected KeycloakSession session;
protected Status status; protected Status status;
protected String errorMessage;
protected Response challenge; protected Response challenge;
protected HttpRequest httpRequest; protected HttpRequest httpRequest;
protected UserModel user; protected UserModel user;
@ -66,6 +67,7 @@ public class RequiredActionContextResult implements RequiredActionContext {
this.config = realm.getRequiredActionConfigByAlias(factory.getId()); this.config = realm.getRequiredActionConfigByAlias(factory.getId());
} }
@Override
public RequiredActionConfigModel getConfig() { public RequiredActionConfigModel getConfig() {
return config; return config;
} }
@ -119,6 +121,11 @@ public class RequiredActionContextResult implements RequiredActionContext {
return status; return status;
} }
@Override
public String getErrorMessage() {
return errorMessage;
}
@Override @Override
public void challenge(Response response) { public void challenge(Response response) {
status = Status.CHALLENGE; status = Status.CHALLENGE;
@ -127,7 +134,8 @@ public class RequiredActionContextResult implements RequiredActionContext {
} }
@Override @Override
public void failure() { public void failure(String errorMessage) {
this.errorMessage = errorMessage;
status = Status.FAILURE; status = Status.FAILURE;
} }

View file

@ -23,6 +23,7 @@ import org.keycloak.common.util.Time;
import org.keycloak.models.KeycloakSession; import org.keycloak.models.KeycloakSession;
import org.keycloak.models.KeycloakSessionFactory; import org.keycloak.models.KeycloakSessionFactory;
import org.keycloak.models.UserModel; import org.keycloak.models.UserModel;
import org.keycloak.services.messages.Messages;
import jakarta.ws.rs.core.Response; import jakarta.ws.rs.core.Response;
import java.util.Arrays; import java.util.Arrays;
@ -80,7 +81,7 @@ public class TermsAndConditions implements RequiredActionProvider, RequiredActio
if (context.getHttpRequest().getDecodedFormParameters().containsKey("cancel")) { if (context.getHttpRequest().getDecodedFormParameters().containsKey("cancel")) {
context.getUser().removeAttribute(USER_ATTRIBUTE); context.getUser().removeAttribute(USER_ATTRIBUTE);
context.failure(); context.failure(Messages.TERMS_AND_CONDITIONS_DECLINED);
return; return;
} }

View file

@ -342,6 +342,11 @@ public class OIDCLoginProtocol implements LoginProtocol {
@Override @Override
public Response sendError(AuthenticationSessionModel authSession, Error error) { public Response sendError(AuthenticationSessionModel authSession, Error error) {
return sendError(authSession, error, null);
}
@Override
public Response sendError(AuthenticationSessionModel authSession, Error error, String errorMessage) {
if (isOAuth2DeviceVerificationFlow(authSession)) { if (isOAuth2DeviceVerificationFlow(authSession)) {
return denyOAuth2DeviceAuthorization(authSession, error, session); return denyOAuth2DeviceAuthorization(authSession, error, session);
} }
@ -352,7 +357,7 @@ public class OIDCLoginProtocol implements LoginProtocol {
String redirect = authSession.getRedirectUri(); String redirect = authSession.getRedirectUri();
String state = authSession.getClientNote(OIDCLoginProtocol.STATE_PARAM); String state = authSession.getClientNote(OIDCLoginProtocol.STATE_PARAM);
OIDCRedirectUriBuilder redirectUri = buildErrorRedirectUri(redirect, state, error); OIDCRedirectUriBuilder redirectUri = buildErrorRedirectUri(redirect, state, error, errorMessage);
// Remove authenticationSession from current tab // Remove authenticationSession from current tab
new AuthenticationSessionManager(session).removeTabIdInAuthenticationSession(realm, authSession); new AuthenticationSessionManager(session).removeTabIdInAuthenticationSession(realm, authSession);
@ -360,10 +365,10 @@ public class OIDCLoginProtocol implements LoginProtocol {
return buildRedirectUri(redirectUri, authSession, null, null, null, error); return buildRedirectUri(redirectUri, authSession, null, null, null, error);
} }
private OIDCRedirectUriBuilder buildErrorRedirectUri(String redirect, String state, Error error) { private OIDCRedirectUriBuilder buildErrorRedirectUri(String redirect, String state, Error error, String errorMessage) {
OIDCRedirectUriBuilder redirectUri = OIDCRedirectUriBuilder.fromUri(redirect, responseMode, session, null); OIDCRedirectUriBuilder redirectUri = OIDCRedirectUriBuilder.fromUri(redirect, responseMode, session, null);
OAuth2ErrorRepresentation oauthError = translateError(error); OAuth2ErrorRepresentation oauthError = translateError(error, errorMessage);
if (oauthError.getError() != null) { if (oauthError.getError() != null) {
redirectUri.addParam(OAuth2Constants.ERROR, oauthError.getError()); redirectUri.addParam(OAuth2Constants.ERROR, oauthError.getError());
} }
@ -411,11 +416,11 @@ public class OIDCLoginProtocol implements LoginProtocol {
} }
setupResponseTypeAndMode(clientData.getResponseType(), clientData.getResponseMode()); setupResponseTypeAndMode(clientData.getResponseType(), clientData.getResponseMode());
OIDCRedirectUriBuilder redirectUri = buildErrorRedirectUri(clientData.getRedirectUri(), clientData.getState(), error); OIDCRedirectUriBuilder redirectUri = buildErrorRedirectUri(clientData.getRedirectUri(), clientData.getState(), error, null);
return buildRedirectUri(redirectUri, null, null, null, null, error); return buildRedirectUri(redirectUri, null, null, null, null, error);
} }
private OAuth2ErrorRepresentation translateError(Error error) { private OAuth2ErrorRepresentation translateError(Error error, String errorMessage) {
switch (error) { switch (error) {
case CANCELLED_AIA_SILENT: case CANCELLED_AIA_SILENT:
return new OAuth2ErrorRepresentation(null, null); return new OAuth2ErrorRepresentation(null, null);
@ -423,7 +428,7 @@ public class OIDCLoginProtocol implements LoginProtocol {
return new OAuth2ErrorRepresentation(OAuthErrorException.ACCESS_DENIED, "User cancelled application-initiated action."); return new OAuth2ErrorRepresentation(OAuthErrorException.ACCESS_DENIED, "User cancelled application-initiated action.");
case CANCELLED_BY_USER: case CANCELLED_BY_USER:
case CONSENT_DENIED: case CONSENT_DENIED:
return new OAuth2ErrorRepresentation(OAuthErrorException.ACCESS_DENIED, "User denied consent"); return new OAuth2ErrorRepresentation(OAuthErrorException.ACCESS_DENIED, errorMessage);
case PASSIVE_INTERACTION_REQUIRED: case PASSIVE_INTERACTION_REQUIRED:
return new OAuth2ErrorRepresentation(OAuthErrorException.INTERACTION_REQUIRED, null); return new OAuth2ErrorRepresentation(OAuthErrorException.INTERACTION_REQUIRED, null);
case PASSIVE_LOGIN_REQUIRED: case PASSIVE_LOGIN_REQUIRED:

View file

@ -271,6 +271,8 @@ public class Messages {
public static final String CONSENT_DENIED="consentDenied"; public static final String CONSENT_DENIED="consentDenied";
public static final String TERMS_AND_CONDITIONS_DECLINED="termsAndConditionsDeclined";
public static final String ALREADY_LOGGED_IN="alreadyLoggedIn"; public static final String ALREADY_LOGGED_IN="alreadyLoggedIn";
public static final String DIFFERENT_USER_AUTHENTICATED = "differentUserAuthenticated"; public static final String DIFFERENT_USER_AUTHENTICATED = "differentUserAuthenticated";

View file

@ -1203,7 +1203,7 @@ public class LoginActionsService {
event.detail(Details.CUSTOM_REQUIRED_ACTION, action); event.detail(Details.CUSTOM_REQUIRED_ACTION, action);
event.error(Errors.REJECTED_BY_USER); event.error(Errors.REJECTED_BY_USER);
return protocol.sendError(authSession, error); return protocol.sendError(authSession, error, context.getErrorMessage());
} }
private boolean isCancelAppInitiatedAction(String providerId, AuthenticationSessionModel authSession, RequiredActionContextResult context) { private boolean isCancelAppInitiatedAction(String providerId, AuthenticationSessionModel authSession, RequiredActionContextResult context) {

View file

@ -17,6 +17,7 @@
package org.keycloak.testsuite.pages; package org.keycloak.testsuite.pages;
import org.keycloak.testsuite.util.DroneUtils;
import org.keycloak.testsuite.util.OAuthClient; import org.keycloak.testsuite.util.OAuthClient;
import org.openqa.selenium.WebElement; import org.openqa.selenium.WebElement;
import org.openqa.selenium.support.FindBy; import org.openqa.selenium.support.FindBy;
@ -34,16 +35,16 @@ public class AppPage extends AbstractPage {
@Override @Override
public void open() { public void open() {
driver.navigate().to(OAuthClient.APP_AUTH_ROOT); DroneUtils.getCurrentDriver().navigate().to(OAuthClient.APP_AUTH_ROOT);
} }
@Override @Override
public boolean isCurrent() { public boolean isCurrent() {
return removeDefaultPorts(driver.getCurrentUrl()).startsWith(OAuthClient.APP_AUTH_ROOT); return removeDefaultPorts(DroneUtils.getCurrentDriver().getCurrentUrl()).startsWith(OAuthClient.APP_AUTH_ROOT);
} }
public RequestType getRequestType() { public RequestType getRequestType() {
return RequestType.valueOf(driver.getTitle()); return RequestType.valueOf(DroneUtils.getCurrentDriver().getTitle());
} }
public void openAccount() { public void openAccount() {

View file

@ -199,11 +199,13 @@ public class LoginPage extends LanguageComboboxAwarePage {
} }
} }
@Override
public boolean isCurrent() { public boolean isCurrent() {
String realm = "test"; String realm = "test";
return isCurrent(realm); return isCurrent(realm);
} }
@Override
public boolean isCurrent(String realm) { public boolean isCurrent(String realm) {
return DroneUtils.getCurrentDriver().getTitle().equals("Sign in to " + realm) || DroneUtils.getCurrentDriver().getTitle().equals("Anmeldung bei " + realm); return DroneUtils.getCurrentDriver().getTitle().equals("Sign in to " + realm) || DroneUtils.getCurrentDriver().getTitle().equals("Anmeldung bei " + realm);
} }

View file

@ -17,6 +17,7 @@
package org.keycloak.testsuite.actions; package org.keycloak.testsuite.actions;
import org.hamcrest.Matchers; import org.hamcrest.Matchers;
import org.jboss.arquillian.drone.api.annotation.Drone;
import org.jboss.arquillian.graphene.page.Page; import org.jboss.arquillian.graphene.page.Page;
import org.junit.Assert; import org.junit.Assert;
import org.junit.Before; import org.junit.Before;
@ -26,6 +27,7 @@ import org.keycloak.authentication.requiredactions.TermsAndConditions;
import org.keycloak.events.Details; import org.keycloak.events.Details;
import org.keycloak.events.Errors; import org.keycloak.events.Errors;
import org.keycloak.events.EventType; import org.keycloak.events.EventType;
import org.keycloak.models.Constants;
import org.keycloak.models.UserModel; import org.keycloak.models.UserModel;
import org.keycloak.representations.idm.RealmRepresentation; import org.keycloak.representations.idm.RealmRepresentation;
import org.keycloak.representations.idm.RequiredActionProviderRepresentation; import org.keycloak.representations.idm.RequiredActionProviderRepresentation;
@ -36,15 +38,24 @@ import org.keycloak.testsuite.pages.AppPage;
import org.keycloak.testsuite.pages.AppPage.RequestType; import org.keycloak.testsuite.pages.AppPage.RequestType;
import org.keycloak.testsuite.pages.LoginPage; import org.keycloak.testsuite.pages.LoginPage;
import org.keycloak.testsuite.pages.TermsAndConditionsPage; import org.keycloak.testsuite.pages.TermsAndConditionsPage;
import org.keycloak.testsuite.util.DroneUtils;
import org.keycloak.testsuite.util.JavascriptBrowser;
import org.keycloak.testsuite.util.UIUtils;
import org.keycloak.testsuite.util.UserBuilder; import org.keycloak.testsuite.util.UserBuilder;
import org.openqa.selenium.By;
import org.openqa.selenium.WebDriver;
import org.openqa.selenium.WebElement;
import java.util.List; import java.util.List;
import java.util.Map; import java.util.Map;
import static org.hamcrest.MatcherAssert.assertThat;
import static org.hamcrest.Matchers.equalTo;
import static org.junit.Assert.assertNotNull; import static org.junit.Assert.assertNotNull;
import static org.junit.Assert.assertNull; import static org.junit.Assert.assertNull;
import static org.junit.Assert.assertTrue; import static org.junit.Assert.assertTrue;
import static org.junit.Assert.fail; import static org.junit.Assert.fail;
import static org.keycloak.testsuite.util.ServerURLs.getAuthServerContextRoot;
/** /**
* @author <a href="mailto:sthorger@redhat.com">Stian Thorgersen</a> * @author <a href="mailto:sthorger@redhat.com">Stian Thorgersen</a>
@ -54,19 +65,34 @@ public class TermsAndConditionsTest extends AbstractTestRealmKeycloakTest {
@Rule @Rule
public AssertEvents events = new AssertEvents(this); public AssertEvents events = new AssertEvents(this);
@Drone
@JavascriptBrowser
private WebDriver jsDriver;
@Page @Page
@JavascriptBrowser
protected AppPage appPage; protected AppPage appPage;
@Page @Page
@JavascriptBrowser
protected LoginPage loginPage; protected LoginPage loginPage;
@Page @Page
@JavascriptBrowser
protected TermsAndConditionsPage termsPage; protected TermsAndConditionsPage termsPage;
@Override @Override
public void configureTestRealm(RealmRepresentation testRealm) { public void configureTestRealm(RealmRepresentation testRealm) {
} }
@Before
public void driver() {
appPage.setDriver(jsDriver);
termsPage.setDriver(jsDriver);
loginPage.setDriver(jsDriver);
DroneUtils.addWebDriver(jsDriver);
}
@Before @Before
public void addTermsAndConditionRequiredAction() { public void addTermsAndConditionRequiredAction() {
UserRepresentation user = ActionUtil.findUserWithAdminClient(adminClient, "test-user@localhost"); UserRepresentation user = ActionUtil.findUserWithAdminClient(adminClient, "test-user@localhost");
@ -105,16 +131,18 @@ public class TermsAndConditionsTest extends AbstractTestRealmKeycloakTest {
assertNotNull("expected non-null timestamp for terms acceptance in user attribute " assertNotNull("expected non-null timestamp for terms acceptance in user attribute "
+ TermsAndConditions.USER_ATTRIBUTE, timestamp); + TermsAndConditions.USER_ATTRIBUTE, timestamp);
try { try {
Integer.parseInt(timestamp); Integer.valueOf(timestamp);
} } catch (NumberFormatException e) {
catch (NumberFormatException e) {
fail("timestamp for terms acceptance is not a valid integer: '" + timestamp + "'"); fail("timestamp for terms acceptance is not a valid integer: '" + timestamp + "'");
} }
} }
@Test @Test
public void termsDeclined() { public void termsDeclined() {
loginPage.open(); appPage.open();
appPage.openAccount();
loginPage.assertCurrent();
loginPage.login("test-user@localhost", "password"); loginPage.login("test-user@localhost", "password");
@ -126,6 +154,8 @@ public class TermsAndConditionsTest extends AbstractTestRealmKeycloakTest {
.error(Errors.REJECTED_BY_USER) .error(Errors.REJECTED_BY_USER)
.removeDetail(Details.CONSENT) .removeDetail(Details.CONSENT)
.session(Matchers.nullValue(String.class)) .session(Matchers.nullValue(String.class))
.client(Constants.ACCOUNT_CONSOLE_CLIENT_ID)
.detail(Details.REDIRECT_URI, getAuthServerContextRoot() + "/auth/realms/" + TEST_REALM_NAME + "/account")
.assertEvent(); .assertEvent();
@ -136,6 +166,15 @@ public class TermsAndConditionsTest extends AbstractTestRealmKeycloakTest {
assertNull("expected null for terms acceptance user attribute " + TermsAndConditions.USER_ATTRIBUTE, assertNull("expected null for terms acceptance user attribute " + TermsAndConditions.USER_ATTRIBUTE,
attributes.get(TermsAndConditions.USER_ATTRIBUTE)); attributes.get(TermsAndConditions.USER_ATTRIBUTE));
} }
assertThat(DroneUtils.getCurrentDriver().getTitle(), equalTo("Account Management"));
Assert.assertTrue(DroneUtils.getCurrentDriver().getPageSource().contains("You need to agree to Terms and Conditions"));
Assert.assertFalse(DroneUtils.getCurrentDriver().getPageSource().contains("An unexpected error occurred"));
WebElement tryAgainButton = DroneUtils.getCurrentDriver().findElement(By.tagName("button"));
assertThat(tryAgainButton.getText(), equalTo("Try again"));
UIUtils.click(tryAgainButton);
loginPage.assertCurrent();
} }