Skip to content

Commit

Permalink
fix: evaluate session state before FORM rendering and submission
Browse files Browse the repository at this point in the history
 a new attribute has been added to the session to determine if the session is ongoing or not

 if it is actions related to a form are executed as usual

 if it is not a redirect to authorization endpoint is perform to follow the auth flow

 this is to avoid issues when a user is fully authenticated but use the back arraow on the browser

 this solution is not perfect as a back click before the end of the auth flow

 will not skip the page rendering

fixes AM-4342
  • Loading branch information
leleueri committed Nov 22, 2024
1 parent 3484823 commit 3616f30
Show file tree
Hide file tree
Showing 35 changed files with 385 additions and 137 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -276,4 +276,10 @@ public interface ConstantKeys {
String PROTOCOL_VALUE_SAML_REDIRECT = "SAML/HTTP-Redirect";
String PROTOCOL_VALUE_SAML_POST = "SAML/HTTP-POST";
String IDP_CODE_VERIFIER = "idp_code_verifier";

// This const is used by the authorization endpoint to flag an ongoing authentication
String SESSION_KEY_AUTH_FLOW_STATE = "auth_state";
// note: Create an enum if another value becomes useful
String SESSION_KEY_AUTH_FLOW_STATE_ONGOING = "ongoing";

}
Original file line number Diff line number Diff line change
@@ -0,0 +1,58 @@
/**
* Copyright (C) 2015 The Gravitee team (http://gravitee.io)
*
* 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 io.gravitee.am.gateway.core;


import org.springframework.core.env.Environment;

import static java.util.Objects.isNull;

/**
* @author Eric LELEU (eric.leleu at graviteesource.com)
* @author GraviteeSource Team
*/
public enum LegacySettingsKeys {

HANDLER_SKIP_BYPASS_DIRECT_REQUEST_HDL("legacy.handler.skipBypassDirectRequestHandler", Boolean.FALSE),
OIDC_FILTER_CUSTOM_PROMPT("legacy.openid.filterCustomPrompt", Boolean.FALSE),
OIDC_SCOPE_FULL_PROFILE("legacy.openid.openid_scope_full_profile", Boolean.FALSE),
OIDC_ALWAYS_ENHANCE_SCOPE("legacy.openid.always_enhance_scopes", Boolean.FALSE),
HANDLER_ALWAYS_APPLY_BODY_HDL("legacy.handler.alwaysApplyBodyHandler", Boolean.FALSE),
REGISTRATION_KEEP_PARAMS("legacy.registration.keepParams", Boolean.TRUE),
RESET_PWD_KEEP_PARAMS("legacy.resetPassword.keepParam", Boolean.TRUE),
OIDC_SANITIZE_PARAM_ENCODING("legacy.openid.sanitizeParametersEncoding", Boolean.TRUE);

private String key;
private Boolean defaultValue;

LegacySettingsKeys(String key, Boolean defaultValue) {
this.key = key;
this.defaultValue = defaultValue;
}

public String getKey() {
return key;
}

public Boolean getDefaultValue() {
return defaultValue;
}

public boolean from(Environment environment) {
return isNull(environment) ? defaultValue : environment.getProperty(key, Boolean.class, defaultValue);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,8 @@
import lombok.NoArgsConstructor;
import org.springframework.core.env.Environment;

import static io.gravitee.am.gateway.core.LegacySettingsKeys.OIDC_SANITIZE_PARAM_ENCODING;

@NoArgsConstructor(access = AccessLevel.PRIVATE)
public class StaticEnvironmentProvider {

Expand All @@ -33,7 +35,7 @@ public static void setEnvironment(Environment environment) {

public static boolean sanitizeParametersEncoding() {
if (sanitizeParametersEncoding == null) {
sanitizeParametersEncoding = getEnvironmentProperty("legacy.openid.sanitizeParametersEncoding", boolean.class, true);
sanitizeParametersEncoding = OIDC_SANITIZE_PARAM_ENCODING.from(env);
}
return sanitizeParametersEncoding;
}
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,74 @@
/**
* Copyright (C) 2015 The Gravitee team (http://gravitee.io)
*
* 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 io.gravitee.am.gateway.handler.common.vertx.utils;


import io.gravitee.am.common.utils.ConstantKeys;
import io.gravitee.am.service.utils.vertx.RequestUtils;
import io.gravitee.common.http.HttpHeaders;
import io.vertx.rxjava3.core.MultiMap;
import io.vertx.rxjava3.core.http.HttpServerResponse;
import io.vertx.rxjava3.ext.web.RoutingContext;

import static io.gravitee.am.gateway.handler.common.vertx.utils.UriBuilderRequest.CONTEXT_PATH;

/**
* @author Eric LELEU (eric.leleu at graviteesource.com)
* @author GraviteeSource Team
*/
public class RedirectHelper {

/**
* Compute the URL on which a user has to be redirected by looking into the
* user session, then the request and finally if no returnUrl is found, forge
* an URL to authorization endpoint.
*
* @param context
* @param queryParams
* @return
*/
public static String getReturnUrl(RoutingContext context, MultiMap queryParams) {
// look into the session
if (context.session().get(ConstantKeys.RETURN_URL_KEY) != null) {
return context.session().get(ConstantKeys.RETURN_URL_KEY);
}
// look into the request parameters
if (context.request().getParam(ConstantKeys.RETURN_URL_KEY) != null) {
return context.request().getParam(ConstantKeys.RETURN_URL_KEY);
}
// fallback to the OAuth 2.0 authorize endpoint
return UriBuilderRequest.resolveProxyRequest(context.request(), context.get(CONTEXT_PATH) + "/oauth/authorize", queryParams, true);
}

public static void doRedirect(HttpServerResponse response, String url) {
response.putHeader(HttpHeaders.LOCATION, url)
.setStatusCode(302)
.end();
}

/**
* Retrieve the redirectUrl from the routing context using the {@link #getReturnUrl(RoutingContext, MultiMap)} method
* and apply the redirect (302 status) on the HttpResponse.
*
* @param context
*/
public static void doRedirect(RoutingContext context) {
final MultiMap queryParams = RequestUtils.getCleanedQueryParams(context.request());
final String redirectUri = getReturnUrl(context, queryParams);
doRedirect(context.response(), redirectUri);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -15,11 +15,14 @@
*/
package io.gravitee.am.gateway.handler.common.vertx.web.handler.impl.internal;

import io.gravitee.am.common.utils.ConstantKeys;
import io.vertx.core.Handler;
import io.vertx.rxjava3.ext.web.RoutingContext;

import java.util.List;

import static org.springframework.util.StringUtils.hasText;

/**
* @author Titouan COMPIEGNE (titouan.compiegne at graviteesource.com)
* @author GraviteeSource Team
Expand All @@ -34,6 +37,10 @@ public AuthenticationFlowChainHandler(List<AuthenticationFlowStep> steps) {

@Override
public void handle(RoutingContext routingContext) {
if (routingContext.session() != null && !hasText(routingContext.session().get(ConstantKeys.SESSION_KEY_AUTH_FLOW_STATE))) {
routingContext.session().put(ConstantKeys.SESSION_KEY_AUTH_FLOW_STATE, ConstantKeys.SESSION_KEY_AUTH_FLOW_STATE_ONGOING);
}

new AuthenticationFlowChain(steps)
.exitHandler(stepHandler -> stepHandler.handle(routingContext))
.handle(routingContext);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@

import org.junit.Test;

import static io.gravitee.am.gateway.core.LegacySettingsKeys.OIDC_SANITIZE_PARAM_ENCODING;
import static org.junit.Assert.assertFalse;
import static org.mockito.Mockito.*;

Expand All @@ -29,7 +30,7 @@ public class StaticEnvironmentProviderTest {

@BeforeClass
public static void setup() {
when(env.getProperty("legacy.openid.sanitizeParametersEncoding", boolean.class, true))
when(env.getProperty(OIDC_SANITIZE_PARAM_ENCODING.getKey(), Boolean.class, OIDC_SANITIZE_PARAM_ENCODING.getDefaultValue()))
.thenReturn(false)
.thenReturn(true);
StaticEnvironmentProvider.setEnvironment(env);
Expand All @@ -41,6 +42,6 @@ public void sanitizeParametersEncoding_environment_returns_cached_value() {

// Call method twice to test cached value is used
assertFalse(StaticEnvironmentProvider.sanitizeParametersEncoding());
verify(env).getProperty("legacy.openid.sanitizeParametersEncoding", boolean.class, true);
verify(env).getProperty(OIDC_SANITIZE_PARAM_ENCODING.getKey(), Boolean.class, OIDC_SANITIZE_PARAM_ENCODING.getDefaultValue());
}
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,61 @@
/**
* Copyright (C) 2015 The Gravitee team (http://gravitee.io)
*
* 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 io.gravitee.am.gateway.handler.manager.session;


import io.gravitee.am.common.utils.ConstantKeys;
import io.vertx.rxjava3.ext.web.RoutingContext;

/**
* @author Eric LELEU (eric.leleu at graviteesource.com)
* @author GraviteeSource Team
*/
public class SessionManager {



public void cleanSessionAfterAuth(RoutingContext context) {
cleanSessionOnMfaChallenge(context);
if (context.session() != null) {
context.session().remove(ConstantKeys.TRANSACTION_ID_KEY);
context.session().remove(ConstantKeys.USER_CONSENT_COMPLETED_KEY);
context.session().remove(ConstantKeys.WEBAUTHN_CREDENTIAL_ID_CONTEXT_KEY);
context.session().remove(ConstantKeys.WEBAUTHN_CREDENTIAL_INTERNAL_ID_CONTEXT_KEY);
context.session().remove(ConstantKeys.PASSWORDLESS_AUTH_ACTION_KEY);
context.session().remove(ConstantKeys.MFA_FACTOR_ID_CONTEXT_KEY);
context.session().remove(ConstantKeys.MFA_ENROLLMENT_COMPLETED_KEY);
context.session().remove(ConstantKeys.MFA_CHALLENGE_COMPLETED_KEY);
context.session().remove(ConstantKeys.USER_LOGIN_COMPLETED_KEY);
context.session().remove(ConstantKeys.MFA_ENROLL_CONDITIONAL_SKIPPED_KEY);

context.session().remove(ConstantKeys.SESSION_KEY_AUTH_FLOW_STATE);
}
}

public void cleanSessionOnMfaChallenge(RoutingContext context) {
if (context.session() != null) {
context.session().remove(ConstantKeys.PASSWORDLESS_CHALLENGE_KEY);
context.session().remove(ConstantKeys.PASSWORDLESS_CHALLENGE_USERNAME_KEY);

context.session().remove(ConstantKeys.ENROLLED_FACTOR_ID_KEY);
context.session().remove(ConstantKeys.ENROLLED_FACTOR_SECURITY_VALUE_KEY);
context.session().remove(ConstantKeys.ENROLLED_FACTOR_PHONE_NUMBER);
context.session().remove(ConstantKeys.ENROLLED_FACTOR_EXTENSION_PHONE_NUMBER);
context.session().remove(ConstantKeys.ENROLLED_FACTOR_EMAIL_ADDRESS);
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -72,6 +72,7 @@
import io.gravitee.am.gateway.handler.root.resources.endpoint.webauthn.WebAuthnRegisterPostEndpoint;
import io.gravitee.am.gateway.handler.root.resources.endpoint.webauthn.WebAuthnRegisterSuccessEndpoint;
import io.gravitee.am.gateway.handler.root.resources.endpoint.webauthn.WebAuthnResponseEndpoint;
import io.gravitee.am.gateway.handler.root.resources.handler.BypassDirectRequestHandler;
import io.gravitee.am.gateway.handler.root.resources.handler.login.LoginAuthenticationHandler;
import io.gravitee.am.gateway.handler.root.resources.handler.transactionid.TransactionIdHandler;
import io.gravitee.am.gateway.handler.root.resources.handler.ConditionalBodyHandler;
Expand Down Expand Up @@ -152,6 +153,7 @@
import org.springframework.core.env.Environment;

import static io.gravitee.am.common.utils.ConstantKeys.DEFAULT_REMEMBER_ME_COOKIE_NAME;
import static io.gravitee.am.gateway.core.LegacySettingsKeys.HANDLER_SKIP_BYPASS_DIRECT_REQUEST_HDL;
import static io.vertx.core.http.HttpMethod.GET;

/**
Expand Down Expand Up @@ -367,6 +369,7 @@ protected void doStart() throws Exception {
Handler<RoutingContext> loginPostWebAuthnHandler = new LoginPostWebAuthnHandler(webAuthnCookieService);
Handler<RoutingContext> userRememberMeHandler = new UserRememberMeRequestHandler(jwtService, domain, rememberMeCookieName);
Handler<RoutingContext> redirectUriValidationHandler = new RedirectUriValidationHandler(domain, userService);
Handler<RoutingContext> bypassHandler = new BypassDirectRequestHandler(HANDLER_SKIP_BYPASS_DIRECT_REQUEST_HDL.from(environment));

// Root policy chain handler
rootRouter.route()
Expand All @@ -382,6 +385,7 @@ protected void doStart() throws Exception {
// Identifier First Login route
rootRouter.get(PATH_IDENTIFIER_FIRST_LOGIN)
.handler(clientRequestParseHandler)
.handler(bypassHandler)
.handler(redirectUriValidationHandler)
.handler(new LoginAuthenticationHandler(identityProviderManager, jwtService, certificateManager))
.handler(policyChainHandler.create(ExtensionPoint.PRE_LOGIN_IDENTIFIER))
Expand All @@ -390,6 +394,7 @@ protected void doStart() throws Exception {

rootRouter.post(PATH_IDENTIFIER_FIRST_LOGIN)
.handler(clientRequestParseHandler)
.handler(bypassHandler)
.handler(redirectUriValidationHandler)
.handler(botDetectionHandler)
.handler(new LoginAuthenticationHandler(identityProviderManager, jwtService, certificateManager))
Expand All @@ -401,6 +406,7 @@ protected void doStart() throws Exception {
// login route
rootRouter.get(PATH_LOGIN)
.handler(clientRequestParseHandler)
.handler(bypassHandler)
.handler(new LoginAuthenticationHandler(identityProviderManager, jwtService, certificateManager))
.handler(redirectUriValidationHandler)
.handler(policyChainHandler.create(ExtensionPoint.PRE_LOGIN))
Expand All @@ -411,6 +417,7 @@ protected void doStart() throws Exception {

rootRouter.post(PATH_LOGIN)
.handler(clientRequestParseHandler)
.handler(bypassHandler)
.handler(redirectUriValidationHandler)
.handler(botDetectionHandler)
.handler(loginAttemptHandler)
Expand Down Expand Up @@ -474,11 +481,13 @@ protected void doStart() throws Exception {
Handler<RoutingContext> mfaChallengeUserHandler = new MFAChallengeUserHandler(userService);
rootRouter.route(PATH_MFA_ENROLL)
.handler(clientRequestParseHandler)
.handler(bypassHandler)
.handler(redirectUriValidationHandler)
.handler(localeHandler)
.handler(new MFAEnrollEndpoint(factorManager, thymeleafTemplateEngine, userService, domain, applicationContext, ruleEngine));
rootRouter.route(PATH_MFA_CHALLENGE)
.handler(clientRequestParseHandler)
.handler(bypassHandler)
.handler(redirectUriValidationHandler)
.handler(rememberDeviceSettingsHandler)
.handler(localeHandler)
Expand All @@ -488,12 +497,14 @@ protected void doStart() throws Exception {
.failureHandler(new MFAChallengeFailureHandler(authenticationFlowContextService));
rootRouter.route(PATH_MFA_CHALLENGE_ALTERNATIVES)
.handler(clientRequestParseHandler)
.handler(bypassHandler)
.handler(redirectUriValidationHandler)
.handler(localeHandler)
.handler(mfaChallengeUserHandler)
.handler(new MFAChallengeAlternativesEndpoint(thymeleafTemplateEngine, factorManager, domain));
rootRouter.route(PATH_MFA_RECOVERY_CODE)
.handler(clientRequestParseHandler)
.handler(bypassHandler)
.handler(redirectUriValidationHandler)
.handler(localeHandler)
.handler(new MFARecoveryCodeEndpoint(thymeleafTemplateEngine, domain, userService, factorManager, applicationContext));
Expand All @@ -503,13 +514,15 @@ protected void doStart() throws Exception {
Handler<RoutingContext> webAuthnRememberDeviceHandler = new WebAuthnRememberDeviceHandler(webAuthnCookieService, domain);
rootRouter.get(PATH_WEBAUTHN_REGISTER)
.handler(clientRequestParseHandler)
.handler(bypassHandler)
.handler(redirectUriValidationHandler)
.handler(webAuthnAccessHandler)
.handler(localeHandler)
.handler(policyChainHandler.create(ExtensionPoint.PRE_WEBAUTHN_REGISTER))
.handler(new WebAuthnRegisterEndpoint(thymeleafTemplateEngine, domain, factorManager));
rootRouter.post(PATH_WEBAUTHN_REGISTER)
.handler(clientRequestParseHandler)
.handler(bypassHandler)
.handler(redirectUriValidationHandler)
.handler(webAuthnAccessHandler)
.handler(new WebAuthnRegisterHandler(userService, factorManager, domain, webAuthn, credentialService))
Expand All @@ -525,6 +538,7 @@ protected void doStart() throws Exception {
.handler(new WebAuthnRegisterSuccessEndpoint(thymeleafTemplateEngine, credentialService, domain));
rootRouter.get(PATH_WEBAUTHN_LOGIN)
.handler(clientRequestParseHandler)
.handler(bypassHandler)
.handler(redirectUriValidationHandler)
.handler(webAuthnAccessHandler)
.handler(new LoginAuthenticationHandler(identityProviderManager, jwtService, certificateManager))
Expand All @@ -533,6 +547,7 @@ protected void doStart() throws Exception {
.handler(new WebAuthnLoginEndpoint(thymeleafTemplateEngine, domain, deviceIdentifierManager, userActivityService));
rootRouter.post(PATH_WEBAUTHN_LOGIN)
.handler(clientRequestParseHandler)
.handler(bypassHandler)
.handler(redirectUriValidationHandler)
.handler(webAuthnAccessHandler)
.handler(new WebAuthnLoginHandler(userService, factorManager, domain, webAuthn, credentialService, userAuthenticationManager))
Expand Down
Loading

0 comments on commit 3616f30

Please sign in to comment.