Merge branch 'master' into master

build.gradle

@@ -27,7 +27,7 @@ buildscript {
   dependencies {
     classpath 'com.linkedin.pegasus:gradle-plugins:' + pegasusVersion
     classpath 'com.github.node-gradle:gradle-node-plugin:2.2.4'
-    classpath 'io.acryl.gradle.plugin:gradle-avro-plugin:0.8.1'
+    classpath 'io.acryl.gradle.plugin:gradle-avro-plugin:0.2.0'
     classpath 'org.springframework.boot:spring-boot-gradle-plugin:' + springBootVersion
     classpath "io.codearte.gradle.nexus:gradle-nexus-staging-plugin:0.30.0"
     classpath "com.palantir.gradle.gitversion:gradle-git-version:3.0.0"
@@ -67,8 +67,8 @@ project.ext.externalDependency = [
     'antlr4Runtime': 'org.antlr:antlr4-runtime:4.7.2',
     'antlr4': 'org.antlr:antlr4:4.7.2',
     'assertJ': 'org.assertj:assertj-core:3.11.1',
-    'avro_1_7': 'org.apache.avro:avro:1.7.7',
-    'avroCompiler_1_7': 'org.apache.avro:avro-compiler:1.7.7',
+    'avro': 'org.apache.avro:avro:1.11.3',
+    'avroCompiler': 'org.apache.avro:avro-compiler:1.11.3',
     'awsGlueSchemaRegistrySerde': 'software.amazon.glue:schema-registry-serde:1.1.10',
     'awsMskIamAuth': 'software.amazon.msk:aws-msk-iam-auth:1.1.1',
     'awsSecretsManagerJdbc': 'com.amazonaws.secretsmanager:aws-secretsmanager-jdbc:1.0.8',
@@ -127,7 +127,6 @@ project.ext.externalDependency = [
     'jgrapht': 'org.jgrapht:jgrapht-core:1.5.1',
     'jna': 'net.java.dev.jna:jna:5.12.1',
     'jsonPatch': 'com.github.java-json-tools:json-patch:1.13',
-    'jsonSchemaAvro': 'com.github.fge:json-schema-avro:0.1.4',
     'jsonSimple': 'com.googlecode.json-simple:json-simple:1.1.1',
     'jsonSmart': 'net.minidev:json-smart:2.4.9',
     'json': 'org.json:json:20230227',

buildSrc/build.gradle

@@ -5,7 +5,14 @@ buildscript {
 }
 
 dependencies {
-  implementation('io.acryl:json-schema-avro:0.1.5') {
+  /**
+   * Forked version of abandoned repository: https://github.com/fge/json-schema-avro
+   * Maintainer last active 2014, we maintain an active fork of this repository to utilize mapping Avro schemas to Json Schemas,
+   * repository is as close to official library for this as you can get. Original maintainer is one of the authors of Json Schema spec.
+   * Other companies are also separately maintaining forks (like: https://github.com/java-json-tools/json-schema-avro).
+   * We have built several customizations on top of it for various bug fixes, especially around union scheams
+   */
+  implementation('io.acryl:json-schema-avro:0.2.2') {
     exclude group: 'com.fasterxml.jackson.core', module: 'jackson-databind'
     exclude group: 'com.google.guava', module: 'guava'
   }

datahub-frontend/app/auth/AuthModule.java

@@ -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";
@@ -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);
                 playCacheCookieStore = new PlayCookieSessionStore(
                         new ShiroAesDataEncrypter(aesEncryptionKey.getBytes()));

datahub-frontend/app/auth/AuthUtils.java

@@ -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";
@@ -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());
     }

datahub-frontend/app/auth/cookie/CustomCookiesModule.java

@@ -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);
+  }
+
+}

datahub-frontend/app/auth/cookie/CustomSessionCookieBaker.scala

@@ -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)
+}

datahub-frontend/app/auth/sso/oidc/OidcAuthorizationGenerator.java

@@ -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;
@@ -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 {
-        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);
-        }
-    }
 
 }

datahub-frontend/app/auth/sso/oidc/OidcCallbackLogic.java

@@ -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;
@@ -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;
 
 
@@ -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);
@@ -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) {
 

datahub-frontend/app/controllers/AuthenticationController.java

@@ -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;
@@ -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.
@@ -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) {

datahub-frontend/conf/application.conf

@@ -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
@@ -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}
 

datahub-frontend/test/app/ApplicationTest.java

@@ -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;
@@ -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;
 
@@ -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