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

fix(frontend): update cookie module #8862

Merged
merged 13 commits into from
Oct 17, 2023
4 changes: 2 additions & 2 deletions datahub-frontend/app/auth/AuthModule.java
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,7 @@ public class AuthModule extends AbstractModule {
* Pac4j Stores Session State in a browser-side cookie in encrypted fashion. This configuration
* value provides a stable encryption base from which to derive the encryption key.
*
* We hash this value (SHA1), then take the first 16 bytes as the AES key.
* We hash this value (SHA256), then take the first 16 bytes as the AES key.
*/
private static final String PAC4J_AES_KEY_BASE_CONF = "play.http.secret.key";
private static final String PAC4J_SESSIONSTORE_PROVIDER_CONF = "pac4j.sessionStore.provider";
Expand Down Expand Up @@ -93,7 +93,7 @@ protected void configure() {
// it to hex and slice the first 16 bytes, because AES key length must strictly
// have a specific length.
final String aesKeyBase = _configs.getString(PAC4J_AES_KEY_BASE_CONF);
final String aesKeyHash = DigestUtils.sha1Hex(aesKeyBase.getBytes(StandardCharsets.UTF_8));
final String aesKeyHash = DigestUtils.sha256Hex(aesKeyBase.getBytes(StandardCharsets.UTF_8));
final String aesEncryptionKey = aesKeyHash.substring(0, 16);
david-leifker marked this conversation as resolved.
Show resolved Hide resolved
playCacheCookieStore = new PlayCookieSessionStore(
new ShiroAesDataEncrypter(aesEncryptionKey.getBytes()));
Expand Down
9 changes: 8 additions & 1 deletion datahub-frontend/app/auth/AuthUtils.java
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,11 @@ public class AuthUtils {
*/
public static final String SYSTEM_CLIENT_SECRET_CONFIG_PATH = "systemClientSecret";

/**
* Cookie name for redirect url that is manually separated from the session to reduce size
*/
public static final String REDIRECT_URL_COOKIE_NAME = "REDIRECT_URL";

public static final CorpuserUrn DEFAULT_ACTOR_URN = new CorpuserUrn("datahub");

public static final String LOGIN_ROUTE = "/login";
Expand Down Expand Up @@ -77,7 +82,9 @@ public static boolean isEligibleForForwarding(Http.Request req) {
* as well as their agreement to determine authentication status.
*/
public static boolean hasValidSessionCookie(final Http.Request req) {
return req.session().data().containsKey(ACTOR)
Map<String, String> sessionCookie = req.session().data();
return sessionCookie.containsKey(ACCESS_TOKEN)
&& sessionCookie.containsKey(ACTOR)
&& req.getCookie(ACTOR).isPresent()
&& req.session().data().get(ACTOR).equals(req.getCookie(ACTOR).get().value());
}
Expand Down
22 changes: 22 additions & 0 deletions datahub-frontend/app/auth/cookie/CustomCookiesModule.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
package auth.cookie;

import com.google.inject.AbstractModule;
import play.api.libs.crypto.CookieSigner;
import play.api.libs.crypto.CookieSignerProvider;
import play.api.mvc.DefaultFlashCookieBaker;
import play.api.mvc.FlashCookieBaker;
import play.api.mvc.SessionCookieBaker;


public class CustomCookiesModule extends AbstractModule {

@Override
public void configure() {
bind(CookieSigner.class).toProvider(CookieSignerProvider.class);
// We override the session cookie baker to not use a fallback, this prevents using an old URL Encoded cookie
bind(SessionCookieBaker.class).to(CustomSessionCookieBaker.class);
// We don't care about flash cookies, we don't use them
bind(FlashCookieBaker.class).to(DefaultFlashCookieBaker.class);
}

}
25 changes: 25 additions & 0 deletions datahub-frontend/app/auth/cookie/CustomSessionCookieBaker.scala
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
package auth.cookie

import com.google.inject.Inject
import play.api.http.{SecretConfiguration, SessionConfiguration}
import play.api.libs.crypto.CookieSigner
import play.api.mvc.DefaultSessionCookieBaker

import scala.collection.immutable.Map

/**
* Overrides default fallback to URL Encoding behavior, prevents usage of old URL encoded session cookies
* @param config
* @param secretConfiguration
* @param cookieSigner
*/
class CustomSessionCookieBaker @Inject() (
override val config: SessionConfiguration,
override val secretConfiguration: SecretConfiguration,
cookieSigner: CookieSigner
) extends DefaultSessionCookieBaker(config, secretConfiguration, cookieSigner) {
// Has to be a Scala class because it extends a trait with concrete implementations, Scala does compilation tricks

// Forces use of jwt encoding and disallows fallback to legacy url encoding
override def decode(encodedData: String): Map[String, String] = jwtCodec.decode(encodedData)
}
37 changes: 0 additions & 37 deletions datahub-frontend/app/auth/sso/oidc/OidcAuthorizationGenerator.java
Original file line number Diff line number Diff line change
@@ -1,19 +1,9 @@
package auth.sso.oidc;

import java.text.ParseException;
import java.util.Map.Entry;
import java.util.Optional;

import com.nimbusds.jose.Algorithm;
import com.nimbusds.jose.Header;
import com.nimbusds.jose.JWEAlgorithm;
import com.nimbusds.jose.JWSAlgorithm;
import com.nimbusds.jose.util.Base64URL;
import com.nimbusds.jose.util.JSONObjectUtils;
import com.nimbusds.jwt.EncryptedJWT;
import com.nimbusds.jwt.JWTParser;
import com.nimbusds.jwt.SignedJWT;
import net.minidev.json.JSONObject;
import org.pac4j.core.authorization.generator.AuthorizationGenerator;
import org.pac4j.core.context.WebContext;
import org.pac4j.core.profile.AttributeLocation;
Expand Down Expand Up @@ -63,32 +53,5 @@ public Optional<UserProfile> generate(WebContext context, UserProfile profile) {

return Optional.ofNullable(profile);
}

private static JWT parse(final String s) throws ParseException {
RyanHolstien marked this conversation as resolved.
Show resolved Hide resolved
final int firstDotPos = s.indexOf(".");

if (firstDotPos == -1) {
throw new ParseException("Invalid JWT serialization: Missing dot delimiter(s)", 0);
}

Base64URL header = new Base64URL(s.substring(0, firstDotPos));
JSONObject jsonObject;

try {
jsonObject = JSONObjectUtils.parse(header.decodeToString());
} catch (ParseException e) {
throw new ParseException("Invalid unsecured/JWS/JWE header: " + e.getMessage(), 0);
}

Algorithm alg = Header.parseAlgorithm(jsonObject);

if (alg instanceof JWSAlgorithm) {
return SignedJWT.parse(s);
} else if (alg instanceof JWEAlgorithm) {
return EncryptedJWT.parse(s);
} else {
throw new AssertionError("Unexpected algorithm type: " + alg);
}
}

}
19 changes: 17 additions & 2 deletions datahub-frontend/app/auth/sso/oidc/OidcCallbackLogic.java
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@
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;
Expand All @@ -49,19 +50,21 @@
import java.util.stream.Collectors;
import lombok.extern.slf4j.Slf4j;
import org.pac4j.core.config.Config;
import org.pac4j.core.context.Cookie;
import org.pac4j.core.engine.DefaultCallbackLogic;
import org.pac4j.core.http.adapter.HttpActionAdapter;
import org.pac4j.core.profile.CommonProfile;
import org.pac4j.core.profile.ProfileManager;
import org.pac4j.core.profile.UserProfile;
import org.pac4j.core.util.Pac4jConstants;
import org.pac4j.play.PlayWebContext;
import play.mvc.Result;
import auth.sso.SsoManager;

import static auth.AuthUtils.createActorCookie;
import static auth.AuthUtils.createSessionMap;
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;


Expand Down Expand Up @@ -97,6 +100,9 @@ public OidcCallbackLogic(final SsoManager ssoManager, final Authentication syste
public Result perform(PlayWebContext context, Config config,
HttpActionAdapter<Result, PlayWebContext> httpActionAdapter, String defaultUrl, Boolean saveInSession,
Boolean multiProfile, Boolean renewSession, String defaultClient) {

setContextRedirectUrl(context);

final Result result =
super.perform(context, config, httpActionAdapter, defaultUrl, saveInSession, multiProfile, renewSession,
defaultClient);
Expand All @@ -111,6 +117,15 @@ public Result perform(PlayWebContext context, Config config,
return handleOidcCallback(oidcConfigs, result, context, getProfileManager(context));
}

@SuppressWarnings("unchecked")
private void setContextRedirectUrl(PlayWebContext context) {
Optional<Cookie> redirectUrl = context.getRequestCookies().stream()
.filter(cookie -> REDIRECT_URL_COOKIE_NAME.equals(cookie.getName())).findFirst();
redirectUrl.ifPresent(
cookie -> context.getSessionStore().set(context, Pac4jConstants.REQUESTED_URL,
JAVA_SER_HELPER.deserializeFromBytes(uncompressBytes(Base64.getDecoder().decode(cookie.getValue())))));
}

private Result handleOidcCallback(final OidcConfigs oidcConfigs, final Result result, final PlayWebContext context,
final ProfileManager<UserProfile> profileManager) {

Expand Down
24 changes: 10 additions & 14 deletions datahub-frontend/app/controllers/AuthenticationController.java
Original file line number Diff line number Diff line change
Expand Up @@ -13,14 +13,15 @@
import com.typesafe.config.Config;
import java.net.URLEncoder;
import java.nio.charset.StandardCharsets;
import java.util.Base64;
import java.util.Optional;
import javax.annotation.Nonnull;
import javax.inject.Inject;
import org.apache.commons.lang3.StringUtils;
import org.pac4j.core.client.Client;
import org.pac4j.core.context.Cookie;
import org.pac4j.core.exception.http.FoundAction;
import org.pac4j.core.exception.http.RedirectionAction;
import org.pac4j.core.util.Pac4jConstants;
import org.pac4j.play.PlayWebContext;
import org.pac4j.play.http.PlayHttpActionAdapter;
import org.pac4j.play.store.PlaySessionStore;
Expand All @@ -33,18 +34,9 @@
import play.mvc.Results;
import security.AuthenticationManager;

import static auth.AuthUtils.DEFAULT_ACTOR_URN;
import static auth.AuthUtils.EMAIL;
import static auth.AuthUtils.FULL_NAME;
import static auth.AuthUtils.INVITE_TOKEN;
import static auth.AuthUtils.LOGIN_ROUTE;
import static auth.AuthUtils.PASSWORD;
import static auth.AuthUtils.RESET_TOKEN;
import static auth.AuthUtils.TITLE;
import static auth.AuthUtils.USER_NAME;
import static auth.AuthUtils.createActorCookie;
import static auth.AuthUtils.createSessionMap;
import static auth.AuthUtils.*;
import static org.pac4j.core.client.IndirectClient.ATTEMPTED_AUTHENTICATION_SUFFIX;
import static org.pac4j.play.store.PlayCookieSessionStore.*;


// TODO add logging.
Expand Down Expand Up @@ -297,8 +289,12 @@ private Optional<Result> redirectToIdentityProvider(Http.RequestHeader request,
}

private void configurePac4jSessionStore(PlayWebContext context, Client client, String redirectPath) {
// Set the originally requested path for post-auth redirection.
_playSessionStore.set(context, Pac4jConstants.REQUESTED_URL, new FoundAction(redirectPath));
// Set the originally requested path for post-auth redirection. We split off into a separate cookie from the session
// to reduce size of the session cookie
FoundAction foundAction = new FoundAction(redirectPath);
byte[] javaSerBytes = JAVA_SER_HELPER.serializeToBytes(foundAction);
String serialized = Base64.getEncoder().encodeToString(compressBytes(javaSerBytes));
context.addResponseCookie(new Cookie(REDIRECT_URL_COOKIE_NAME, serialized));
// This is to prevent previous login attempts from being cached.
// We replicate the logic here, which is buried in the Pac4j client.
if (_playSessionStore.get(context, client.getName() + ATTEMPTED_AUTHENTICATION_SUFFIX) != null) {
Expand Down
15 changes: 12 additions & 3 deletions datahub-frontend/conf/application.conf
Original file line number Diff line number Diff line change
Expand Up @@ -22,11 +22,16 @@ play.application.loader = play.inject.guice.GuiceApplicationLoader
play.http.parser.maxMemoryBuffer = 10MB
play.http.parser.maxMemoryBuffer = ${?DATAHUB_PLAY_MEM_BUFFER_SIZE}

# TODO: Disable legacy URL encoding eventually
play.modules.disabled += "play.api.mvc.LegacyCookiesModule"
play.modules.disabled += "play.api.mvc.CookiesModule"
play.modules.enabled += "play.api.mvc.LegacyCookiesModule"
play.modules.enabled += "auth.cookie.CustomCookiesModule"
play.modules.enabled += "auth.AuthModule"

jwt {
# 'alg' https://tools.ietf.org/html/rfc7515#section-4.1.1
signatureAlgorithm = "HS256"
}

# We override the Akka server provider to allow setting the max header count to a higher value
# This is useful while using proxies like Envoy that result in the frontend server rejecting GMS
# responses as there's more than the max of 64 allowed headers
Expand Down Expand Up @@ -199,10 +204,14 @@ auth.native.enabled = ${?AUTH_NATIVE_ENABLED}
# auth.native.enabled = false
# auth.oidc.enabled = false # (or simply omit oidc configurations)

# Login session expiration time
# Login session expiration time, controls when the actor cookie is expired on the browser side
auth.session.ttlInHours = 24
auth.session.ttlInHours = ${?AUTH_SESSION_TTL_HOURS}

# Control the length of time a session token is valid
play.http.session.maxAge = 24h
play.http.session.maxAge = ${?MAX_SESSION_TOKEN_AGE}

analytics.enabled = true
analytics.enabled = ${?DATAHUB_ANALYTICS_ENABLED}

Expand Down
28 changes: 23 additions & 5 deletions datahub-frontend/test/app/ApplicationTest.java
Original file line number Diff line number Diff line change
@@ -1,6 +1,11 @@
package app;

import com.nimbusds.jwt.JWT;
import com.nimbusds.jwt.JWTClaimsSet;
import com.nimbusds.jwt.JWTParser;
import controllers.routes;
import java.text.ParseException;
import java.util.Date;
import no.nav.security.mock.oauth2.MockOAuth2Server;
import no.nav.security.mock.oauth2.token.DefaultOAuth2TokenCallback;
import okhttp3.mockwebserver.MockResponse;
Expand All @@ -27,8 +32,6 @@

import java.io.IOException;
import java.net.InetAddress;
import java.net.URLEncoder;
import java.nio.charset.StandardCharsets;
import java.util.List;
import java.util.Map;

Expand Down Expand Up @@ -149,16 +152,31 @@ public void testOpenIdConfig() {
}

@Test
public void testHappyPathOidc() throws InterruptedException {
public void testHappyPathOidc() throws ParseException {
browser.goTo("/authenticate");
assertEquals("", browser.url());

Cookie actorCookie = browser.getCookie("actor");
assertEquals(TEST_USER, actorCookie.getValue());

Cookie sessionCookie = browser.getCookie("PLAY_SESSION");
assertTrue(sessionCookie.getValue().contains("token=" + TEST_TOKEN));
assertTrue(sessionCookie.getValue().contains("actor=" + URLEncoder.encode(TEST_USER, StandardCharsets.UTF_8)));
String jwtStr = sessionCookie.getValue();
JWT jwt = JWTParser.parse(jwtStr);
JWTClaimsSet claims = jwt.getJWTClaimsSet();
Map<String, String> data = (Map<String, String>) claims.getClaim("data");
assertEquals(TEST_TOKEN, data.get("token"));
assertEquals(TEST_USER, data.get("actor"));
// Default expiration is 24h, so should always be less than current time + 1 day since it stamps the time before this executes
assertTrue(claims.getExpirationTime().compareTo(new Date(System.currentTimeMillis() + (24 * 60 * 60 * 1000))) < 0);
}

@Test
public void testAPI() throws ParseException {
testHappyPathOidc();
int requestCount = _gmsServer.getRequestCount();

browser.goTo("/api/v2/graphql/");
assertEquals(++requestCount, _gmsServer.getRequestCount());
}

@Test
Expand Down
5 changes: 3 additions & 2 deletions docs/authentication/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -31,8 +31,9 @@ When a user makes a request for Data within DataHub, the request is authenticate
and programmatic calls to DataHub APIs. There are two types of tokens that are important:

1. **Session Tokens**: Generated for users of the DataHub web application. By default, having a duration of 24 hours.
These tokens are encoded and stored inside browser-side session cookies. The duration a session token is valid for is configurable via the `AUTH_SESSION_TTL_HOURS` environment variable
on the datahub-frontend deployment.
These tokens are encoded and stored inside browser-side session cookies. The duration a session token is valid for is configurable via the `MAX_SESSION_TOKEN_AGE` environment variable
on the datahub-frontend deployment. Additionally, the `AUTH_SESSION_TTL_HOURS` configures the expiration time of the actor cookie on the user's browser which will also prompt a user login. The difference between these is that the actor cookie expiration only affects the browser session and can still be used programmatically,
but when the session expires it can no longer be used programmatically either as it is created as a JWT with an expiration claim.
2. **Personal Access Tokens**: These are tokens generated via the DataHub settings panel useful for interacting
with DataHub APIs. They can be used to automate processes like enriching documentation, ownership, tags, and more on DataHub. Learn
more about Personal Access Tokens [here](personal-access-tokens.md).
Expand Down
3 changes: 2 additions & 1 deletion docs/authentication/guides/sso/configure-oidc-react.md
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,8 @@ AUTH_OIDC_BASE_URL=your-datahub-url
- `AUTH_OIDC_CLIENT_SECRET`: Unique client secret received from identity provider
- `AUTH_OIDC_DISCOVERY_URI`: Location of the identity provider OIDC discovery API. Suffixed with `.well-known/openid-configuration`
- `AUTH_OIDC_BASE_URL`: The base URL of your DataHub deployment, e.g. https://yourorgdatahub.com (prod) or http://localhost:9002 (testing)
- `AUTH_SESSION_TTL_HOURS`: The length of time in hours before a user will be prompted to login again. Session tokens are stateless so this determines at what time a session token may no longer be used and a valid session token can be used until this time has passed.
- `AUTH_SESSION_TTL_HOURS`: The length of time in hours before a user will be prompted to login again. Controls the actor cookie expiration time in the browser. Numeric value converted to hours, default 24.
- `MAX_SESSION_TOKEN_AGE`: Determines the expiration time of a session token. Session tokens are stateless so this determines at what time a session token may no longer be used and a valid session token can be used until this time has passed. Accepts a valid relative Java date style String, default 24h.

Providing these configs will cause DataHub to delegate authentication to your identity
provider, requesting the "oidc email profile" scopes and parsing the "preferred_username" claim from
Expand Down
Loading
Loading