Skip to content

Commit

Permalink
Merge branch 'master' into feat/elasticsearch-optimization-ext
Browse files Browse the repository at this point in the history
  • Loading branch information
david-leifker committed Jan 12, 2023
2 parents 33eef5e + 7c60659 commit 7283080
Show file tree
Hide file tree
Showing 89 changed files with 21,045 additions and 1,860 deletions.
3 changes: 1 addition & 2 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -3,9 +3,8 @@ import useBaseUrl from '@docusaurus/useBaseUrl';
export const Logo = (props) => {
return (
<div style={{ display: "flex", justifyContent: "center", padding: "20px" }}>
<div style={{ display: "flex", justifyContent: "center", padding: "20px", height: "190px" }}>
<img
height="150"
alt="DataHub Logo"
src={useBaseUrl("/img/datahub-logo-color-mark.svg")}
{...props}
Expand Down
30 changes: 26 additions & 4 deletions datahub-frontend/app/auth/AuthUtils.java
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
import lombok.extern.slf4j.Slf4j;
import play.mvc.Http;

import javax.annotation.Nonnull;
import java.time.Duration;
import java.time.temporal.ChronoUnit;
import java.util.HashMap;
Expand Down Expand Up @@ -44,6 +45,11 @@ public class AuthUtils {
public static final Integer DEFAULT_SESSION_TTL_HOURS = 720;
public static final CorpuserUrn DEFAULT_ACTOR_URN = new CorpuserUrn("datahub");

public static final String AUTH_COOKIE_SAME_SITE = "play.http.session.sameSite";
public static final String DEFAULT_AUTH_COOKIE_SAME_SITE = "LAX";
public static final String AUTH_COOKIE_SECURE = "play.http.session.secure";
public static final boolean DEFAULT_AUTH_COOKIE_SECURE = false;

public static final String LOGIN_ROUTE = "/login";
public static final String USER_NAME = "username";
public static final String PASSWORD = "password";
Expand Down Expand Up @@ -96,11 +102,18 @@ public static boolean hasAuthHeader(final Http.Request req) {
* @param actorUrn the urn of the authenticated actor, e.g. "urn:li:corpuser:datahub"
* @param ttlInHours the number of hours until the actor cookie expires after being set
*/
public static Http.Cookie createActorCookie(final String actorUrn, final Integer ttlInHours) {
public static Http.Cookie createActorCookie(
@Nonnull final String actorUrn,
@Nonnull final Integer ttlInHours,
@Nonnull final String sameSite,
final boolean isSecure
) {
return Http.Cookie.builder(ACTOR, actorUrn)
.withHttpOnly(false)
.withMaxAge(Duration.of(ttlInHours, ChronoUnit.HOURS))
.build();
.withHttpOnly(false)
.withMaxAge(Duration.of(ttlInHours, ChronoUnit.HOURS))
.withSameSite(convertSameSiteValue(sameSite))
.withSecure(isSecure)
.build();
}

public static Map<String, String> createSessionMap(final String userUrnStr, final String accessToken) {
Expand All @@ -112,4 +125,13 @@ public static Map<String, String> createSessionMap(final String userUrnStr, fina

private AuthUtils() { }

private static Http.Cookie.SameSite convertSameSiteValue(@Nonnull final String sameSiteValue) {
try {
return Http.Cookie.SameSite.valueOf(sameSiteValue);
} catch (IllegalArgumentException e) {
log.warn(String.format("Invalid AUTH_COOKIE_SAME_SITE value: %s. Using LAX instead.", sameSiteValue), e);
return Http.Cookie.SameSite.LAX;
}
}

}
18 changes: 18 additions & 0 deletions datahub-frontend/app/auth/sso/SsoConfigs.java
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,8 @@ public class SsoConfigs {
private final String _authSuccessRedirectPath;
private final Integer _sessionTtlInHours;
private final Boolean _oidcEnabled;
private final String _authCookieSameSite;
private final Boolean _authCookieSecure;

public SsoConfigs(final com.typesafe.config.Config configs) {
_authBaseUrl = getRequired(configs, AUTH_BASE_URL_CONFIG_PATH);
Expand All @@ -46,6 +48,14 @@ public SsoConfigs(final com.typesafe.config.Config configs) {
_oidcEnabled = configs.hasPath(OIDC_ENABLED_CONFIG_PATH)
&& Boolean.TRUE.equals(
Boolean.parseBoolean(configs.getString(OIDC_ENABLED_CONFIG_PATH)));
_authCookieSameSite = getOptional(
configs,
AUTH_COOKIE_SAME_SITE,
DEFAULT_AUTH_COOKIE_SAME_SITE);
_authCookieSecure = Boolean.parseBoolean(getOptional(
configs,
AUTH_COOKIE_SECURE,
String.valueOf(DEFAULT_AUTH_COOKIE_SECURE)));
}

public String getAuthBaseUrl() {
Expand All @@ -64,6 +74,14 @@ public Integer getSessionTtlInHours() {
return _sessionTtlInHours;
}

public String getAuthCookieSameSite() {
return _authCookieSameSite;
}

public boolean getAuthCookieSecure() {
return _authCookieSecure;
}

public Boolean isOidcEnabled() {
return _oidcEnabled;
}
Expand Down
9 changes: 8 additions & 1 deletion datahub-frontend/app/auth/sso/oidc/OidcCallbackLogic.java
Original file line number Diff line number Diff line change
Expand Up @@ -154,7 +154,14 @@ private Result handleOidcCallback(final OidcConfigs oidcConfigs, final Result re
final String accessToken = _authClient.generateSessionTokenForUser(corpUserUrn.getId());
return result
.withSession(createSessionMap(corpUserUrn.toString(), accessToken))
.withCookies(createActorCookie(corpUserUrn.toString(), oidcConfigs.getSessionTtlInHours()));
.withCookies(
createActorCookie(
corpUserUrn.toString(),
oidcConfigs.getSessionTtlInHours(),
oidcConfigs.getAuthCookieSameSite(),
oidcConfigs.getAuthCookieSecure()
)
);
}
return internalServerError(
"Failed to authenticate current user. Cannot find valid identity provider profile in session.");
Expand Down
50 changes: 35 additions & 15 deletions datahub-frontend/app/controllers/AuthenticationController.java
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,9 @@
import javax.inject.Inject;
import org.apache.commons.lang3.StringUtils;
import org.pac4j.core.client.Client;
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 @@ -30,7 +32,11 @@
import play.mvc.Results;
import security.AuthenticationManager;

import static auth.AuthUtils.AUTH_COOKIE_SAME_SITE;
import static auth.AuthUtils.AUTH_COOKIE_SECURE;
import static auth.AuthUtils.DEFAULT_ACTOR_URN;
import static auth.AuthUtils.DEFAULT_AUTH_COOKIE_SAME_SITE;
import static auth.AuthUtils.DEFAULT_AUTH_COOKIE_SECURE;
import static auth.AuthUtils.DEFAULT_SESSION_TTL_HOURS;
import static auth.AuthUtils.EMAIL;
import static auth.AuthUtils.FULL_NAME;
Expand Down Expand Up @@ -99,7 +105,7 @@ public Result authenticate(Http.Request request) {

// 1. If SSO is enabled, redirect to IdP if not authenticated.
if (_ssoManager.isSsoEnabled()) {
return redirectToIdentityProvider(request).orElse(
return redirectToIdentityProvider(request, redirectPath).orElse(
Results.redirect(LOGIN_ROUTE + String.format("?%s=%s", ERROR_MESSAGE_URI_PARAM, SSO_NO_REDIRECT_MESSAGE))
);
}
Expand All @@ -113,10 +119,15 @@ public Result authenticate(Http.Request request) {
// 3. If no auth enabled, fallback to using default user account & redirect.
// Generate GMS session token, TODO:
final String accessToken = _authClient.generateSessionTokenForUser(DEFAULT_ACTOR_URN.getId());
int ttlInHours = _configs.hasPath(SESSION_TTL_CONFIG_PATH) ? _configs.getInt(SESSION_TTL_CONFIG_PATH)
: DEFAULT_SESSION_TTL_HOURS;
String authCookieSameSite = _configs.hasPath(AUTH_COOKIE_SAME_SITE) ? _configs.getString(AUTH_COOKIE_SAME_SITE)
: DEFAULT_AUTH_COOKIE_SAME_SITE;
boolean authCookieSecure = _configs.hasPath(AUTH_COOKIE_SECURE) ? _configs.getBoolean(AUTH_COOKIE_SECURE)
: DEFAULT_AUTH_COOKIE_SECURE;

return Results.redirect(redirectPath).withSession(createSessionMap(DEFAULT_ACTOR_URN.toString(), accessToken))
.withCookies(createActorCookie(DEFAULT_ACTOR_URN.toString(),
_configs.hasPath(SESSION_TTL_CONFIG_PATH) ? _configs.getInt(SESSION_TTL_CONFIG_PATH)
: DEFAULT_SESSION_TTL_HOURS));
.withCookies(createActorCookie(DEFAULT_ACTOR_URN.toString(), ttlInHours, authCookieSameSite, authCookieSecure));
}

/**
Expand All @@ -125,7 +136,7 @@ public Result authenticate(Http.Request request) {
@Nonnull
public Result sso(Http.Request request) {
if (_ssoManager.isSsoEnabled()) {
return redirectToIdentityProvider(request).orElse(
return redirectToIdentityProvider(request, "/").orElse(
Results.redirect(LOGIN_ROUTE + String.format("?%s=%s", ERROR_MESSAGE_URI_PARAM, SSO_NO_REDIRECT_MESSAGE))
);
}
Expand Down Expand Up @@ -268,17 +279,10 @@ public Result resetNativeUserCredentials(Http.Request request) {
return createSession(userUrnString, accessToken);
}

private Optional<Result> redirectToIdentityProvider(Http.RequestHeader request) {
private Optional<Result> redirectToIdentityProvider(Http.RequestHeader request, String redirectPath) {
final PlayWebContext playWebContext = new PlayWebContext(request, _playSessionStore);
final Client client = _ssoManager.getSsoProvider().client();

// 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(playWebContext, client.getName() + ATTEMPTED_AUTHENTICATION_SUFFIX) != null) {
_logger.debug("Found previous login attempt. Removing it manually to prevent unexpected errors.");
_playSessionStore.set(playWebContext, client.getName() + ATTEMPTED_AUTHENTICATION_SUFFIX, "");
}

configurePac4jSessionStore(playWebContext, client, redirectPath);
try {
final Optional<RedirectionAction> action = client.getRedirectionAction(playWebContext);
return action.map(act -> new PlayHttpActionAdapter().adapt(act, playWebContext));
Expand All @@ -291,6 +295,17 @@ 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));
// 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) {
_logger.debug("Found previous login attempt. Removing it manually to prevent unexpected errors.");
_playSessionStore.set(context, client.getName() + ATTEMPTED_AUTHENTICATION_SUFFIX, "");
}
}

private String encodeRedirectUri(final String redirectUri) {
return URLEncoder.encode(redirectUri, StandardCharsets.UTF_8);
}
Expand Down Expand Up @@ -323,7 +338,12 @@ private boolean tryLogin(String username, String password) {
private Result createSession(String userUrnString, String accessToken) {
int ttlInHours = _configs.hasPath(SESSION_TTL_CONFIG_PATH) ? _configs.getInt(SESSION_TTL_CONFIG_PATH)
: DEFAULT_SESSION_TTL_HOURS;
String authCookieSameSite = _configs.hasPath(AUTH_COOKIE_SAME_SITE) ? _configs.getString(AUTH_COOKIE_SAME_SITE)
: DEFAULT_AUTH_COOKIE_SAME_SITE;
boolean authCookieSecure = _configs.hasPath(AUTH_COOKIE_SECURE) ? _configs.getBoolean(AUTH_COOKIE_SECURE)
: DEFAULT_AUTH_COOKIE_SECURE;

return Results.ok().withSession(createSessionMap(userUrnString, accessToken))
.withCookies(createActorCookie(userUrnString, ttlInHours));
.withCookies(createActorCookie(userUrnString, ttlInHours, authCookieSameSite, authCookieSecure));
}
}
12 changes: 11 additions & 1 deletion datahub-frontend/conf/application.conf
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,15 @@ play.http.server.akka.max-header-count = ${?DATAHUB_AKKA_MAX_HEADER_COUNT}
play.server.akka.max-header-size = 8k
play.server.akka.max-header-size = ${?DATAHUB_AKKA_MAX_HEADER_VALUE_LENGTH}

# Update AUTH_COOKIE_SAME_SITE and AUTH_COOKIE_SECURE in order to change how authentication cookies
# are configured. If you wish cookies to be sent in first and third party contexts, set
# AUTH_COOKIE_SAME_SITE = "NONE" and AUTH_COOKIE_SECURE = true. If AUTH_COOKIE_SAME_SITE is "NONE",
# AUTH_COOKIE_SECURE must be set to true.
play.http.session.sameSite = "LAX"
play.http.session.sameSite = ${?AUTH_COOKIE_SAME_SITE}
play.http.session.secure = false
play.http.session.secure = ${?AUTH_COOKIE_SECURE}

play.filters {
enabled = [
play.filters.gzip.GzipFilter
Expand Down Expand Up @@ -183,7 +192,8 @@ auth.native.enabled = ${?AUTH_NATIVE_ENABLED}
# auth.oidc.enabled = false # (or simply omit oidc configurations)

# Login session expiration time
# auth.session.ttlInHours = ${?AUTH_SESSION_TTL_HOURS}
auth.session.ttlInHours = 720
auth.session.ttlInHours = ${?AUTH_SESSION_TTL_HOURS}

analytics.enabled = ${?DATAHUB_ANALYTICS_ENABLED}

Expand Down
1 change: 1 addition & 0 deletions datahub-frontend/play.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,7 @@ dependencies {
implementation externalDependency.kafkaClients
implementation externalDependency.awsMskIamAuth

testImplementation 'org.seleniumhq.selenium:htmlunit-driver:2.67.0'
testImplementation externalDependency.mockito
testImplementation externalDependency.playTest
testImplementation 'org.awaitility:awaitility:4.2.0'
Expand Down
5 changes: 5 additions & 0 deletions datahub-frontend/test/app/ApplicationTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -161,4 +161,9 @@ public void testHappyPathOidc() throws InterruptedException {
assertTrue(sessionCookie.getValue().contains("actor=" + URLEncoder.encode(TEST_USER, StandardCharsets.UTF_8)));
}

@Test
public void testOidcRedirectToRequestedUrl() throws InterruptedException {
browser.goTo("/authenticate?redirect_uri=%2Fcontainer%2Furn%3Ali%3Acontainer%3ADATABASE");
assertEquals("container/urn:li:container:DATABASE", browser.url());
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,7 @@ private EntityPrivileges getGlossaryTermPrivileges(Urn termUrn, QueryContext con
}
Urn parentNodeUrn = GlossaryUtils.getParentUrn(termUrn, context, _entityClient);
if (parentNodeUrn != null) {
Boolean canManage = GlossaryUtils.canManageChildrenEntities(context, parentNodeUrn);
Boolean canManage = GlossaryUtils.canManageChildrenEntities(context, parentNodeUrn, _entityClient);
result.setCanManageEntity(canManage);
}
return result;
Expand All @@ -80,12 +80,12 @@ private EntityPrivileges getGlossaryNodePrivileges(Urn nodeUrn, QueryContext con
result.setCanManageChildren(true);
return result;
}
Boolean canManageChildren = GlossaryUtils.canManageChildrenEntities(context, nodeUrn);
Boolean canManageChildren = GlossaryUtils.canManageChildrenEntities(context, nodeUrn, _entityClient);
result.setCanManageChildren(canManageChildren);

Urn parentNodeUrn = GlossaryUtils.getParentUrn(nodeUrn, context, _entityClient);
if (parentNodeUrn != null) {
Boolean canManage = GlossaryUtils.canManageChildrenEntities(context, parentNodeUrn);
Boolean canManage = GlossaryUtils.canManageChildrenEntities(context, parentNodeUrn, _entityClient);
result.setCanManageEntity(canManage);
}
return result;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ public CompletableFuture<String> get(DataFetchingEnvironment environment) throws
final Urn parentNode = input.getParentNode() != null ? UrnUtils.getUrn(input.getParentNode()) : null;

return CompletableFuture.supplyAsync(() -> {
if (GlossaryUtils.canManageChildrenEntities(context, parentNode)) {
if (GlossaryUtils.canManageChildrenEntities(context, parentNode, _entityClient)) {
try {
final GlossaryNodeKey key = new GlossaryNodeKey();

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ public CompletableFuture<String> get(DataFetchingEnvironment environment) throws
final Urn parentNode = input.getParentNode() != null ? UrnUtils.getUrn(input.getParentNode()) : null;

return CompletableFuture.supplyAsync(() -> {
if (GlossaryUtils.canManageChildrenEntities(context, parentNode)) {
if (GlossaryUtils.canManageChildrenEntities(context, parentNode, _entityClient)) {
try {
final GlossaryTermKey key = new GlossaryTermKey();

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ public CompletableFuture<Boolean> get(final DataFetchingEnvironment environment)
final Urn parentNodeUrn = GlossaryUtils.getParentUrn(entityUrn, context, _entityClient);

return CompletableFuture.supplyAsync(() -> {
if (GlossaryUtils.canManageChildrenEntities(context, parentNodeUrn)) {
if (GlossaryUtils.canManageChildrenEntities(context, parentNodeUrn, _entityClient)) {
if (!_entityService.exists(entityUrn)) {
throw new RuntimeException(String.format("This urn does not exist: %s", entityUrn));
}
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
package com.linkedin.datahub.graphql.resolvers.ingest.execution;

import com.linkedin.common.urn.Urn;
import com.linkedin.data.template.StringMap;
import com.linkedin.datahub.graphql.QueryContext;
import com.linkedin.datahub.graphql.exception.AuthorizationException;
Expand All @@ -12,7 +13,9 @@
import com.linkedin.metadata.Constants;
import com.linkedin.metadata.config.IngestionConfiguration;
import com.linkedin.metadata.key.ExecutionRequestKey;
import com.linkedin.metadata.utils.EntityKeyUtils;
import com.linkedin.metadata.utils.GenericRecordUtils;
import com.linkedin.metadata.utils.IngestionUtils;
import com.linkedin.mxe.MetadataChangeProposal;
import graphql.schema.DataFetcher;
import graphql.schema.DataFetchingEnvironment;
Expand Down Expand Up @@ -64,6 +67,7 @@ public CompletableFuture<String> get(final DataFetchingEnvironment environment)
final UUID uuid = UUID.randomUUID();
final String uuidStr = uuid.toString();
key.setId(uuidStr);
final Urn executionRequestUrn = EntityKeyUtils.convertEntityKeyToUrn(key, Constants.EXECUTION_REQUEST_ENTITY_NAME);
proposal.setEntityKeyAspect(GenericRecordUtils.serializeAspect(key));

final ExecutionRequestInput execInput = new ExecutionRequestInput();
Expand All @@ -73,7 +77,7 @@ public CompletableFuture<String> get(final DataFetchingEnvironment environment)
execInput.setRequestedAt(System.currentTimeMillis());

Map<String, String> arguments = new HashMap<>();
arguments.put(RECIPE_ARG_NAME, input.getRecipe());
arguments.put(RECIPE_ARG_NAME, IngestionUtils.injectPipelineName(input.getRecipe(), executionRequestUrn.toString()));
if (input.getVersion() != null) {
arguments.put(VERSION_ARG_NAME, input.getVersion());
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,7 @@ private Boolean updateGlossaryTermName(
QueryContext context
) {
final Urn parentNodeUrn = GlossaryUtils.getParentUrn(targetUrn, context, _entityClient);
if (GlossaryUtils.canManageChildrenEntities(context, parentNodeUrn)) {
if (GlossaryUtils.canManageChildrenEntities(context, parentNodeUrn, _entityClient)) {
try {
GlossaryTermInfo glossaryTermInfo = (GlossaryTermInfo) getAspectFromEntity(
targetUrn.toString(), Constants.GLOSSARY_TERM_INFO_ASPECT_NAME, _entityService, null);
Expand All @@ -91,7 +91,7 @@ private Boolean updateGlossaryNodeName(
QueryContext context
) {
final Urn parentNodeUrn = GlossaryUtils.getParentUrn(targetUrn, context, _entityClient);
if (GlossaryUtils.canManageChildrenEntities(context, parentNodeUrn)) {
if (GlossaryUtils.canManageChildrenEntities(context, parentNodeUrn, _entityClient)) {
try {
GlossaryNodeInfo glossaryNodeInfo = (GlossaryNodeInfo) getAspectFromEntity(
targetUrn.toString(), Constants.GLOSSARY_NODE_INFO_ASPECT_NAME, _entityService, null);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,8 @@ public CompletableFuture<Boolean> get(DataFetchingEnvironment environment) throw
return CompletableFuture.supplyAsync(() -> {
Urn currentParentUrn = GlossaryUtils.getParentUrn(targetUrn, context, _entityClient);
// need to be able to manage current parent node and new parent node
if (GlossaryUtils.canManageChildrenEntities(context, currentParentUrn) && GlossaryUtils.canManageChildrenEntities(context, parentNodeUrn)) {
if (GlossaryUtils.canManageChildrenEntities(context, currentParentUrn, _entityClient)
&& GlossaryUtils.canManageChildrenEntities(context, parentNodeUrn, _entityClient)) {
switch (targetUrn.getEntityType()) {
case Constants.GLOSSARY_TERM_ENTITY_NAME:
return updateGlossaryTermParentNode(targetUrn, parentNodeUrn, input, environment.getContext());
Expand Down
Loading

0 comments on commit 7283080

Please sign in to comment.