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(PlayCookie) PLAY_TOKEN cookie rejected because userprofile exceeds 4096 chars #5114

Merged
merged 6 commits into from
Jul 14, 2022
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
1 change: 1 addition & 0 deletions build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -116,6 +116,7 @@ project.ext.externalDependency = [
'parquet': 'org.apache.parquet:parquet-avro:1.12.2',
'picocli': 'info.picocli:picocli:4.5.0',
'playCache': 'com.typesafe.play:play-cache_2.12:2.7.6',
'playEhcache': 'com.typesafe.play:play-ehcache_2.12:2.7.6',
'playWs': 'com.typesafe.play:play-ahc-ws-standalone_2.12:2.0.8',
'playDocs': 'com.typesafe.play:play-docs_2.12:2.7.6',
'playGuice': 'com.typesafe.play:play-guice_2.12:2.7.6',
Expand Down
50 changes: 35 additions & 15 deletions datahub-frontend/app/auth/AuthModule.java
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@
import org.pac4j.core.context.session.SessionStore;
import org.pac4j.play.LogoutController;
import org.pac4j.play.http.PlayHttpActionAdapter;
import org.pac4j.play.store.PlayCacheSessionStore;
import org.pac4j.play.store.PlayCookieSessionStore;
import org.pac4j.play.store.PlaySessionStore;
import org.pac4j.play.store.ShiroAesDataEncrypter;
Expand All @@ -32,6 +33,7 @@
import auth.sso.SsoConfigs;
import auth.sso.SsoManager;
import controllers.SsoCallbackController;
import play.cache.SyncCacheApi;
import utils.ConfigUtil;

import static auth.AuthUtils.*;
Expand All @@ -51,6 +53,8 @@ public class AuthModule extends AbstractModule {
* We hash this value (SHA1), 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";

private final com.typesafe.config.Config _configs;

public AuthModule(final Environment environment, final com.typesafe.config.Config configs) {
Expand All @@ -59,22 +63,38 @@ public AuthModule(final Environment environment, final com.typesafe.config.Confi

@Override
protected void configure() {
PlayCookieSessionStore playCacheCookieStore;
try {
// To generate a valid encryption key from an input value, we first
// hash the input to generate a fixed-length string. Then, we convert
// 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 aesEncryptionKey = aesKeyHash.substring(0, 16);
playCacheCookieStore = new PlayCookieSessionStore(
new ShiroAesDataEncrypter(aesEncryptionKey));
} catch (Exception e) {
throw new RuntimeException("Failed to instantiate Pac4j cookie session store!", e);
/**
* In Pac4J, you are given the option to store the profiles of authenticated users in either
* (i) PlayCacheSessionStore - saves your data in the Play cache or
* (ii) PlayCookieSessionStore saves your data in the Play session cookie
* However there is problem (https://github.com/datahub-project/datahub/issues/4448) observed when storing the Pac4j profile in cookie.
* Whenever the profile returned by Pac4j is greater than 4096 characters, the response will be rejected by the browser.
* Default to PlayCacheCookieStore so that datahub-frontend container remains as a stateless service
*/
String sessionStoreProvider = _configs.getString(PAC4J_SESSIONSTORE_PROVIDER_CONF);

if (sessionStoreProvider.equals("PlayCacheSessionStore")) {
final PlayCacheSessionStore playCacheSessionStore = new PlayCacheSessionStore(getProvider(SyncCacheApi.class));
bind(SessionStore.class).toInstance(playCacheSessionStore);
bind(PlaySessionStore.class).toInstance(playCacheSessionStore);
} else {
PlayCookieSessionStore playCacheCookieStore;
try {
// To generate a valid encryption key from an input value, we first
// hash the input to generate a fixed-length string. Then, we convert
// 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 aesEncryptionKey = aesKeyHash.substring(0, 16);
playCacheCookieStore = new PlayCookieSessionStore(
new ShiroAesDataEncrypter(aesEncryptionKey));
} catch (Exception e) {
throw new RuntimeException("Failed to instantiate Pac4j cookie session store!", e);
}
bind(SessionStore.class).toInstance(playCacheCookieStore);
bind(PlaySessionStore.class).toInstance(playCacheCookieStore);
}
bind(SessionStore.class).toInstance(playCacheCookieStore);
bind(PlaySessionStore.class).toInstance(playCacheCookieStore);

try {
bind(SsoCallbackController.class).toConstructor(SsoCallbackController.class.getConstructor(
Expand Down
5 changes: 5 additions & 0 deletions datahub-frontend/conf/application.conf
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,11 @@ play.http.server.akka.max-header-count = ${?DATAHUB_AKKA_MAX_HEADER_COUNT}
play.server.akka.max-header-value-length = 8k
play.server.akka.max-header-value-length = ${?DATAHUB_AKKA_MAX_HEADER_VALUE_LENGTH}

# pac4j configuration
# default to PlayCookieSessionStore to keep datahub-frontend's statelessness
pac4j.sessionStore.provider= "PlayCookieSessionStore"
pac4j.sessionStore.provider= ${?PAC4J_SESSIONSTORE_PROVIDER}
neojunjie marked this conversation as resolved.
Show resolved Hide resolved

# Database configuration
# ~~~~~
# You can declare as many datasources as you want.
Expand Down
1 change: 1 addition & 0 deletions datahub-frontend/play.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,7 @@ dependencies {
implementation externalDependency.playPac4j
implementation externalDependency.shiroCore
implementation externalDependency.playCache
implementation externalDependency.playEhcache
implementation externalDependency.playWs
implementation externalDependency.playServer
implementation externalDependency.playAkkaHttpServer
Expand Down
15 changes: 15 additions & 0 deletions docs/debugging.md
Original file line number Diff line number Diff line change
Expand Up @@ -205,3 +205,18 @@ You'll need to ingest some metadata of the following form to see it inside the D
"proposedDelta": null
}
```

## I've configured OIDC, but I cannot login. I get continuously redirected. What do I do?

Sorry to hear that!

This phenomena may be due to the size of a Cookie DataHub uses to authenticate its users. If it's too large ( > 4096), then you'll see this behavior. The cookie embeds an encoded version of the information returned by your OIDC Identity Provider - if they return a lot of information, this can be the root cause.

One solution is to use Play Cache to persist this session information for a user. This means the attributes about the user (and their session info) will be stored in an in-memory store in the `datahub-frontend` service, instead of a browser-side cookie.

To configure the Play Cache session store, you can set the env variable "PAC4J_SESSIONSTORE_PROVIDER" as "PlayCacheSessionStore" for the `datahub-frontend` container.

Do note that there are downsides to using the Play Cache. Specifically, it will make `datahub-frontend` a stateful server. If you have multiple instances of `datahub-frontend` deployed, you'll need to ensure that the same user is deterministically routed to the same service container (since the sessions are stored in memory). If you're using a single instance of `datahub-frontend` (the default), then things should "just work".

For more details, please refer to https://github.com/datahub-project/datahub/pull/5114