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 @@ -52,7 +52,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 @@ -89,7 +89,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
42 changes: 41 additions & 1 deletion datahub-frontend/app/auth/AuthUtils.java
Original file line number Diff line number Diff line change
@@ -1,7 +1,18 @@
package auth;

import com.linkedin.common.urn.CorpuserUrn;
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.JWT;
import com.nimbusds.jwt.SignedJWT;
import java.text.ParseException;
import lombok.extern.slf4j.Slf4j;
import net.minidev.json.JSONObject;
import play.mvc.Http;

import javax.annotation.Nonnull;
Expand Down Expand Up @@ -77,7 +88,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 Expand Up @@ -127,4 +140,31 @@ private static Http.Cookie.SameSite convertSameSiteValue(@Nonnull final String s
}
}

public 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);
}
}

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

}
16 changes: 12 additions & 4 deletions datahub-frontend/conf/application.conf
Original file line number Diff line number Diff line change
Expand Up @@ -22,11 +22,15 @@ 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.CookiesModule"
play.modules.enabled += "play.api.mvc.LegacyCookiesModule"
play.modules.disabled += "play.api.mvc.LegacyCookiesModule"
play.modules.enabled += "play.api.mvc.CookiesModule"
RyanHolstien marked this conversation as resolved.
Show resolved Hide resolved
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 +203,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
27 changes: 22 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 auth.AuthUtils;
import com.nimbusds.jwt.JWT;
import com.nimbusds.jwt.JWTClaimsSet;
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,30 @@ 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 = AuthUtils.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();

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

@Test
Expand Down
9 changes: 6 additions & 3 deletions docker/build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,8 @@ task quickstart(type: Exec, dependsOn: ':metadata-ingestion:install') {
'--no-pull-images',
'--standalone_consumers',
'--version', "v${version}",
'--dump-logs-on-failure'
'--dump-logs-on-failure',
'-f ../docker/quickstart/docker-compose-without-neo4j-m1.quickstart.yml'
]

commandLine 'bash', '-c', cmd.join(" ")
Expand All @@ -67,7 +68,8 @@ task quickstartSlim(type: Exec, dependsOn: ':metadata-ingestion:install') {
'--no-pull-images',
'--standalone_consumers',
'--version', "v${version}",
'--dump-logs-on-failure'
'--dump-logs-on-failure',
'-f ../docker/quickstart/docker-compose-without-neo4j-m1.quickstart.yml'
]

commandLine 'bash', '-c', cmd.join(" ")
Expand Down Expand Up @@ -96,7 +98,8 @@ task quickstartDebug(type: Exec, dependsOn: ':metadata-ingestion:install') {
'datahub docker quickstart',
'--no-pull-images',
'--version', "debug",
'--dump-logs-on-failure'
'--dump-logs-on-failure',
'-f ../docker/quickstart/docker-compose-without-neo4j-m1.quickstart.yml'
RyanHolstien marked this conversation as resolved.
Show resolved Hide resolved
] + debug_compose_args
commandLine 'bash', '-c', cmd.join(" ")
}
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
13 changes: 7 additions & 6 deletions docs/deploy/environment-vars.md
Original file line number Diff line number Diff line change
Expand Up @@ -79,9 +79,10 @@ Simply replace the dot, `.`, with an underscore, `_`, and convert to uppercase.

## Frontend

| Variable | Default | Unit/Type | Components | Description |
|------------------------------------|----------|-----------|--------------|-----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------|
| `AUTH_VERBOSE_LOGGING` | `false` | boolean | [`Frontend`] | Enable verbose authentication logging. Enabling this will leak sensisitve information in the logs. Disable when finished debugging. |
| `AUTH_OIDC_GROUPS_CLAIM` | `groups` | string | [`Frontend`] | Claim to use as the user's group. |
| `AUTH_OIDC_EXTRACT_GROUPS_ENABLED` | `false` | boolean | [`Frontend`] | Auto-provision the group from the user's group claim. |
| `AUTH_SESSION_TTL_HOURS` | `24` | string | [`Frontend`] | The number of hours a user session is valid. [User session tokens are stateless and will become invalid after this time](https://www.playframework.com/documentation/2.8.x/SettingsSession#Session-Timeout-/-Expiration) requiring a user to login again. |
| Variable | Default | Unit/Type | Components | Description |
|------------------------------------|----------|-----------|---------------|----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------|
| `AUTH_VERBOSE_LOGGING` | `false` | boolean | [`Frontend`] | Enable verbose authentication logging. Enabling this will leak sensisitve information in the logs. Disable when finished debugging. |
| `AUTH_OIDC_GROUPS_CLAIM` | `groups` | string | [`Frontend`] | Claim to use as the user's group. |
| `AUTH_OIDC_EXTRACT_GROUPS_ENABLED` | `false` | boolean | [`Frontend`] | Auto-provision the group from the user's group claim. |
| `AUTH_SESSION_TTL_HOURS` | `24` | string | [`Frontend`] | The number of hours a user session is valid. After this many hours the actor cookie will be expired by the browser and the user will be prompted to login again. |
| `MAX_SESSION_TOKEN_AGE` | `24h` | string | [`Frontend`] | The maximum age of the session token. [User session tokens are stateless and will become invalid after this time](https://www.playframework.com/documentation/2.8.x/SettingsSession#Session-Timeout-/-Expiration) requiring a user to login again. |
2 changes: 2 additions & 0 deletions docs/how/updating-datahub.md
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,8 @@ This file documents any backwards-incompatible changes in DataHub and assists pe
### Deprecations

### Other Notable Changes
- Session token configuration has changed, all previously created session tokens will be invalid and users will be prompted to log in. Expiration time has also been shortened which may result in more login prompts with the default settings.
There should be no other interruption due to this change.

## 0.11.0

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -29,4 +29,6 @@ public class AuthenticationConfiguration {
* The lifespan of a UI session token.
*/
private long sessionTokenDurationMs;

private TokenServiceConfiguration tokenService;
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
package com.datahub.authentication;

import lombok.Data;


@Data
/**
* Configurations for DataHub token service
*/
public class TokenServiceConfiguration {
private String signingKey;
private String salt;
private String issuer;
private String signingAlgorithm;
}
2 changes: 2 additions & 0 deletions metadata-service/auth-filter/build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -14,4 +14,6 @@ dependencies {

annotationProcessor externalDependency.lombok
testImplementation externalDependency.mockito
testImplementation externalDependency.testng
testImplementation externalDependency.springBootTest
}
Loading
Loading