fix(authentication): fix PlayCacheSessionStore (#14754)

Co-authored-by: trialiya <trialiya@gmail.com>
Co-authored-by: Deepak Garg <deepak.garg@datahub.com>
This commit is contained in:
trialiya 2025-10-11 17:29:34 +03:00 committed by GitHub
parent ef0b4a71ce
commit b45d606062
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
5 changed files with 50 additions and 35 deletions

View File

@ -3,7 +3,6 @@ package auth.sso.oidc;
import static auth.AuthUtils.*;
import static com.linkedin.metadata.Constants.CORP_USER_ENTITY_NAME;
import static com.linkedin.metadata.Constants.GROUP_MEMBERSHIP_ASPECT_NAME;
import static org.pac4j.play.store.PlayCookieSessionStore.*;
import static play.mvc.Results.internalServerError;
import static utils.FrontendConstants.SSO_LOGIN;
@ -50,7 +49,6 @@ import java.net.URLEncoder;
import java.nio.charset.StandardCharsets;
import java.util.ArrayList;
import java.util.Arrays;
import java.util.Base64;
import java.util.Collection;
import java.util.Collections;
import java.util.List;
@ -71,6 +69,7 @@ import org.pac4j.core.context.CallContext;
import org.pac4j.core.context.Cookie;
import org.pac4j.core.context.FrameworkParameters;
import org.pac4j.core.context.WebContext;
import org.pac4j.core.context.session.SessionStore;
import org.pac4j.core.credentials.Credentials;
import org.pac4j.core.engine.DefaultCallbackLogic;
import org.pac4j.core.exception.http.HttpAction;
@ -80,10 +79,10 @@ import org.pac4j.core.profile.ProfileManager;
import org.pac4j.core.profile.UserProfile;
import org.pac4j.core.util.CommonHelper;
import org.pac4j.core.util.Pac4jConstants;
import org.pac4j.play.store.PlayCookieSessionStore;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
import play.mvc.Result;
import utils.SerializationUtils;
/**
* This class contains the logic that is executed when an OpenID Connect Identity Provider redirects
@ -207,21 +206,15 @@ public class OidcCallbackLogic extends DefaultCallbackLogic {
private void setContextRedirectUrl(CallContext ctx) {
WebContext context = ctx.webContext();
PlayCookieSessionStore sessionStore = (PlayCookieSessionStore) ctx.sessionStore();
SessionStore sessionStore = ctx.sessionStore();
Optional<Cookie> redirectUrl =
context.getRequestCookies().stream()
.filter(cookie -> REDIRECT_URL_COOKIE_NAME.equals(cookie.getName()))
.findFirst();
redirectUrl.ifPresent(
cookie ->
sessionStore.set(
context,
Pac4jConstants.REQUESTED_URL,
sessionStore
.getSerializer()
.deserializeFromBytes(
uncompressBytes(Base64.getDecoder().decode(cookie.getValue())))));
.map(Cookie::getValue)
.map(SerializationUtils::deserializeFoundAction)
.findFirst()
.ifPresent(
foundAction -> sessionStore.set(context, Pac4jConstants.REQUESTED_URL, foundAction));
}
private Result handleOidcCallback(

View File

@ -2,7 +2,6 @@ package controllers;
import static auth.AuthUtils.*;
import static org.pac4j.core.client.IndirectClient.ATTEMPTED_AUTHENTICATION_SUFFIX;
import static org.pac4j.play.store.PlayCookieSessionStore.*;
import static utils.FrontendConstants.FALLBACK_LOGIN;
import static utils.FrontendConstants.GUEST_LOGIN;
import static utils.FrontendConstants.PASSWORD_LOGIN;
@ -28,7 +27,6 @@ import java.net.URISyntaxException;
import java.net.URLEncoder;
import java.nio.charset.StandardCharsets;
import java.time.Duration;
import java.util.Base64;
import java.util.Optional;
import javax.annotation.Nonnull;
import javax.inject.Inject;
@ -37,11 +35,11 @@ import org.apache.http.client.RedirectException;
import org.pac4j.core.client.Client;
import org.pac4j.core.context.CallContext;
import org.pac4j.core.context.WebContext;
import org.pac4j.core.context.session.SessionStore;
import org.pac4j.core.exception.http.FoundAction;
import org.pac4j.core.exception.http.RedirectionAction;
import org.pac4j.play.PlayWebContext;
import org.pac4j.play.http.PlayHttpActionAdapter;
import org.pac4j.play.store.PlayCookieSessionStore;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
import play.data.validation.Constraints;
@ -51,6 +49,7 @@ import play.mvc.Http;
import play.mvc.Result;
import play.mvc.Results;
import security.AuthenticationManager;
import utils.SerializationUtils;
public class AuthenticationController extends Controller {
public static final String AUTH_VERBOSE_LOGGING = "auth.verbose.logging";
@ -75,7 +74,7 @@ public class AuthenticationController extends Controller {
@Inject private org.pac4j.core.config.Config ssoConfig;
@VisibleForTesting @Inject protected PlayCookieSessionStore playCookieSessionStore;
@VisibleForTesting @Inject protected SessionStore sessionStore;
@VisibleForTesting @Inject protected SsoManager ssoManager;
@ -371,11 +370,9 @@ public class AuthenticationController extends Controller {
// to reduce size of the session cookie
FoundAction foundAction =
new FoundAction(BasePathUtils.addBasePath(redirectPath, this.basePath));
byte[] javaSerBytes =
((PlayCookieSessionStore) ctx.sessionStore()).getSerializer().serializeToBytes(foundAction);
String serialized = Base64.getEncoder().encodeToString(compressBytes(javaSerBytes));
Http.CookieBuilder redirectCookieBuilder =
Http.Cookie.builder(REDIRECT_URL_COOKIE_NAME, serialized);
Http.Cookie.builder(
REDIRECT_URL_COOKIE_NAME, SerializationUtils.serializeFoundAction(foundAction));
redirectCookieBuilder.withPath(BasePathUtils.addBasePath("/", this.basePath));
redirectCookieBuilder.withSecure(true);
redirectCookieBuilder.withHttpOnly(true);
@ -422,7 +419,7 @@ public class AuthenticationController extends Controller {
PlayWebContext webContext = new PlayWebContext(request);
// Then create CallContext using the web context and session store
return new CallContext(webContext, playCookieSessionStore);
return new CallContext(webContext, sessionStore);
}
private void configurePac4jSessionStore(CallContext ctx, Client client) {
@ -431,11 +428,11 @@ public class AuthenticationController extends Controller {
// This is to prevent previous login attempts from being cached.
// We replicate the logic here, which is buried in the Pac4j client.
Optional<Object> attempt =
playCookieSessionStore.get(context, client.getName() + ATTEMPTED_AUTHENTICATION_SUFFIX);
ctx.sessionStore().get(context, client.getName() + ATTEMPTED_AUTHENTICATION_SUFFIX);
if (attempt.isPresent() && !"".equals(attempt.get())) {
logger.debug(
"Found previous login attempt. Removing it manually to prevent unexpected errors.");
playCookieSessionStore.set(context, client.getName() + ATTEMPTED_AUTHENTICATION_SUFFIX, "");
ctx.sessionStore().set(context, client.getName() + ATTEMPTED_AUTHENTICATION_SUFFIX, "");
}
}

View File

@ -0,0 +1,27 @@
package utils;
import static org.pac4j.play.store.PlayCookieSessionStore.compressBytes;
import static org.pac4j.play.store.PlayCookieSessionStore.uncompressBytes;
import java.util.Base64;
import javax.annotation.Nonnull;
import org.pac4j.core.exception.http.FoundAction;
import org.pac4j.core.util.serializer.JavaSerializer;
public class SerializationUtils {
private static final JavaSerializer JAVA_SERIALIZER = new JavaSerializer();
private SerializationUtils() {}
public static String serializeFoundAction(@Nonnull final FoundAction foundAction) {
byte[] javaSerBytes = JAVA_SERIALIZER.serializeToBytes(foundAction);
return Base64.getEncoder().encodeToString(compressBytes(javaSerBytes));
}
public static FoundAction deserializeFoundAction(@Nonnull final String serialized) {
return (FoundAction)
JAVA_SERIALIZER.deserializeFromBytes(
uncompressBytes(Base64.getDecoder().decode(serialized)));
}
}

View File

@ -100,7 +100,7 @@ public class AuthenticationControllerTest {
// Create the controller
controller = new AuthenticationController(mockConfig);
controller.playCookieSessionStore = playCookieSessionStore;
controller.sessionStore = playCookieSessionStore;
controller.ssoManager = ssoManager;
controller.authClient = authClient;
}
@ -353,7 +353,7 @@ public class AuthenticationControllerTest {
AuthenticationController testController = new AuthenticationController(config);
testController.ssoManager = ssoManager;
testController.authClient = authClient;
testController.playCookieSessionStore = playCookieSessionStore;
testController.sessionStore = playCookieSessionStore;
// Configure SSO to be enabled
when(ssoManager.isSsoEnabled()).thenReturn(true);
@ -408,7 +408,7 @@ public class AuthenticationControllerTest {
AuthenticationController testController = new AuthenticationController(config);
testController.ssoManager = ssoManager;
testController.authClient = authClient;
testController.playCookieSessionStore = playCookieSessionStore;
testController.sessionStore = playCookieSessionStore;
// Configure SSO to be enabled
when(ssoManager.isSsoEnabled()).thenReturn(true);
@ -465,7 +465,7 @@ public class AuthenticationControllerTest {
Config config = ConfigFactory.parseMap(configMap);
AuthenticationController testController = new AuthenticationController(config);
testController.ssoManager = ssoManager;
testController.playCookieSessionStore = playCookieSessionStore;
testController.sessionStore = playCookieSessionStore;
// Configure SSO to be enabled
when(ssoManager.isSsoEnabled()).thenReturn(true);
@ -619,7 +619,7 @@ public class AuthenticationControllerTest {
Config config = ConfigFactory.parseMap(configMap);
AuthenticationController testController = new AuthenticationController(config);
testController.ssoManager = ssoManager;
testController.playCookieSessionStore = playCookieSessionStore;
testController.sessionStore = playCookieSessionStore;
// Configure SSO to be enabled
when(ssoManager.isSsoEnabled()).thenReturn(true);

View File

@ -49,8 +49,6 @@ export JAVA_OPTS="${JAVA_MEMORY_OPTS:-"-Xms512m -Xmx1024m"} \
-Djava.security.auth.login.config=datahub-frontend/conf/jaas.conf \
-Dlogback.configurationFile=datahub-frontend/conf/logback.xml \
-Dlogback.debug=false \
--add-opens java.base/java.lang=ALL-UNNAMED \
--add-opens=java.base/java.util=ALL-UNNAMED \
${PROMETHEUS_AGENT:-} ${OTEL_AGENT:-} \
${TRUSTSTORE_FILE:-} ${TRUSTSTORE_TYPE:-} ${TRUSTSTORE_PASSWORD:-} \
${HTTP_PROXY:-} ${HTTPS_PROXY:-} ${NO_PROXY:-} \