diff --git a/.github/workflows/build-and-test.yml b/.github/workflows/build-and-test.yml index 7d6df187952197..532ba1102ed579 100644 --- a/.github/workflows/build-and-test.yml +++ b/.github/workflows/build-and-test.yml @@ -70,7 +70,11 @@ jobs: - uses: actions/setup-python@v5 with: python-version: "3.10" - cache: pip + - uses: actions/cache@v4 + with: + path: | + ~/.cache/uv + key: ${{ runner.os }}-uv-${{ hashFiles('**/requirements.txt') }} - name: Set up JDK 17 uses: actions/setup-java@v4 with: diff --git a/.github/workflows/docker-unified.yml b/.github/workflows/docker-unified.yml index a1ae7cee1736fa..49dd26e1cd27e3 100644 --- a/.github/workflows/docker-unified.yml +++ b/.github/workflows/docker-unified.yml @@ -144,6 +144,11 @@ jobs: needs: setup if: ${{ needs.setup.outputs.backend_change == 'true' || needs.setup.outputs.publish == 'true' }} steps: + - name: Free up disk space + run: | + sudo apt-get remove 'dotnet-*' azure-cli || true + sudo rm -rf /usr/local/lib/android/ || true + sudo docker image prune -a -f || true - name: Set up JDK 17 uses: actions/setup-java@v4 with: @@ -210,6 +215,11 @@ jobs: needs: setup if: ${{ needs.setup.outputs.backend_change == 'true' || needs.setup.outputs.publish == 'true' }} steps: + - name: Free up disk space + run: | + sudo apt-get remove 'dotnet-*' azure-cli || true + sudo rm -rf /usr/local/lib/android/ || true + sudo docker image prune -a -f || true - name: Set up JDK 17 uses: actions/setup-java@v4 with: @@ -276,6 +286,11 @@ jobs: needs: setup if: ${{ needs.setup.outputs.backend_change == 'true' || needs.setup.outputs.publish == 'true' }} steps: + - name: Free up disk space + run: | + sudo apt-get remove 'dotnet-*' azure-cli || true + sudo rm -rf /usr/local/lib/android/ || true + sudo docker image prune -a -f || true - name: Set up JDK 17 uses: actions/setup-java@v4 with: @@ -342,6 +357,11 @@ jobs: needs: setup if: ${{ needs.setup.outputs.backend_change == 'true' || needs.setup.outputs.publish == 'true' }} steps: + - name: Free up disk space + run: | + sudo apt-get remove 'dotnet-*' azure-cli || true + sudo rm -rf /usr/local/lib/android/ || true + sudo docker image prune -a -f || true - name: Set up JDK 17 uses: actions/setup-java@v4 with: @@ -408,6 +428,11 @@ jobs: needs: setup if: ${{ needs.setup.outputs.frontend_change == 'true' || needs.setup.outputs.publish == 'true' || needs.setup.outputs.pr-publish == 'true'}} steps: + - name: Free up disk space + run: | + sudo apt-get remove 'dotnet-*' azure-cli || true + sudo rm -rf /usr/local/lib/android/ || true + sudo docker image prune -a -f || true - name: Set up JDK 17 uses: actions/setup-java@v4 with: @@ -476,6 +501,11 @@ jobs: needs: setup if: ${{ needs.setup.outputs.kafka_setup_change == 'true' || (needs.setup.outputs.publish == 'true' || needs.setup.outputs.pr-publish == 'true') }} steps: + - name: Free up disk space + run: | + sudo apt-get remove 'dotnet-*' azure-cli || true + sudo rm -rf /usr/local/lib/android/ || true + sudo docker image prune -a -f || true - name: Check out the repo uses: acryldata/sane-checkout-action@v3 - name: Build and push @@ -532,6 +562,11 @@ jobs: needs: setup if: ${{ needs.setup.outputs.mysql_setup_change == 'true' || (needs.setup.outputs.publish == 'true' || needs.setup.outputs.pr-publish == 'true') }} steps: + - name: Free up disk space + run: | + sudo apt-get remove 'dotnet-*' azure-cli || true + sudo rm -rf /usr/local/lib/android/ || true + sudo docker image prune -a -f || true - name: Check out the repo uses: acryldata/sane-checkout-action@v3 - name: Build and push @@ -588,6 +623,11 @@ jobs: needs: setup if: ${{ needs.setup.outputs.elasticsearch_setup_change == 'true' || (needs.setup.outputs.publish == 'true' || needs.setup.outputs.pr-publish == 'true' ) }} steps: + - name: Free up disk space + run: | + sudo apt-get remove 'dotnet-*' azure-cli || true + sudo rm -rf /usr/local/lib/android/ || true + sudo docker image prune -a -f || true - name: Check out the repo uses: acryldata/sane-checkout-action@v3 - name: Build and push @@ -646,6 +686,11 @@ jobs: needs: setup if: ${{ needs.setup.outputs.ingestion_change == 'true' || needs.setup.outputs.publish == 'true' || needs.setup.outputs.pr-publish == 'true' }} steps: + - name: Free up disk space + run: | + sudo apt-get remove 'dotnet-*' azure-cli || true + sudo rm -rf /usr/local/lib/android/ || true + sudo docker image prune -a -f || true - name: Check out the repo uses: acryldata/sane-checkout-action@v3 - name: Build and push Base Image @@ -674,6 +719,11 @@ jobs: needs: [setup, datahub_ingestion_base_build] if: ${{ needs.setup.outputs.ingestion_change == 'true' || needs.setup.outputs.publish == 'true' || needs.setup.outputs.pr-publish == 'true' }} steps: + - name: Free up disk space + run: | + sudo apt-get remove 'dotnet-*' azure-cli || true + sudo rm -rf /usr/local/lib/android/ || true + sudo docker image prune -a -f || true - name: Check out the repo uses: acryldata/sane-checkout-action@v3 - name: Download Base Image @@ -717,6 +767,11 @@ jobs: needs: [setup, datahub_ingestion_base_build] if: ${{ needs.setup.outputs.ingestion_change == 'true' || needs.setup.outputs.publish == 'true' || needs.setup.outputs.pr-publish == 'true' }} steps: + - name: Free up disk space + run: | + sudo apt-get remove 'dotnet-*' azure-cli || true + sudo rm -rf /usr/local/lib/android/ || true + sudo docker image prune -a -f || true - name: Check out the repo uses: acryldata/sane-checkout-action@v3 - name: Download Base Image @@ -760,6 +815,11 @@ jobs: needs: [setup, datahub_ingestion_base_slim_build] if: ${{ needs.setup.outputs.ingestion_change == 'true' || needs.setup.outputs.publish == 'true' || needs.setup.outputs.pr-publish == 'true' }} steps: + - name: Free up disk space + run: | + sudo apt-get remove 'dotnet-*' azure-cli || true + sudo rm -rf /usr/local/lib/android/ || true + sudo docker image prune -a -f || true - name: Check out the repo uses: acryldata/sane-checkout-action@v3 - uses: actions/setup-python@v5 @@ -856,6 +916,11 @@ jobs: needs: [setup, datahub_ingestion_base_full_build] if: ${{ needs.setup.outputs.ingestion_change == 'true' || needs.setup.outputs.publish == 'true' || needs.setup.outputs.pr-publish == 'true' }} steps: + - name: Free up disk space + run: | + sudo apt-get remove 'dotnet-*' azure-cli || true + sudo rm -rf /usr/local/lib/android/ || true + sudo docker image prune -a -f || true - name: Check out the repo uses: acryldata/sane-checkout-action@v3 - uses: actions/setup-python@v5 diff --git a/.github/workflows/documentation.yml b/.github/workflows/documentation.yml index dbeff7a9c0f9fc..e6044badb1b41c 100644 --- a/.github/workflows/documentation.yml +++ b/.github/workflows/documentation.yml @@ -44,7 +44,11 @@ jobs: - uses: actions/setup-python@v5 with: python-version: "3.10" - cache: pip + - uses: actions/cache@v4 + with: + path: | + ~/.cache/uv + key: ${{ runner.os }}-uv-${{ hashFiles('**/requirements.txt') }} - name: Install Python dependencies run: ./metadata-ingestion/scripts/install_deps.sh - name: Build Docs diff --git a/.github/workflows/gx-plugin.yml b/.github/workflows/gx-plugin.yml index 99121f81099f2d..aa7c3f069c7654 100644 --- a/.github/workflows/gx-plugin.yml +++ b/.github/workflows/gx-plugin.yml @@ -39,6 +39,8 @@ jobs: extraPythonRequirement: "great-expectations~=0.16.0 numpy~=1.26.0" - python-version: "3.11" extraPythonRequirement: "great-expectations~=0.17.0" + - python-version: "3.11" + extraPythonRequirement: "great-expectations~=0.18.0" fail-fast: false steps: - name: Set up JDK 17 diff --git a/.github/workflows/metadata-ingestion.yml b/.github/workflows/metadata-ingestion.yml index 92dcb3d8ac2890..c0eafe891fb0aa 100644 --- a/.github/workflows/metadata-ingestion.yml +++ b/.github/workflows/metadata-ingestion.yml @@ -62,16 +62,11 @@ jobs: - uses: actions/setup-python@v5 with: python-version: ${{ matrix.python-version }} - cache: "pip" - uses: actions/cache@v4 with: path: | ~/.cache/uv key: ${{ runner.os }}-uv-${{ hashFiles('**/requirements.txt') }} - - uses: actions/setup-python@v5 - with: - python-version: ${{ matrix.python-version }} - cache: "pip" - name: Install dependencies run: ./metadata-ingestion/scripts/install_deps.sh - name: Install package diff --git a/build.gradle b/build.gradle index 67968ce3ee2905..77f8395ac898e0 100644 --- a/build.gradle +++ b/build.gradle @@ -45,7 +45,9 @@ buildscript { ext.elasticsearchVersion = '2.11.1' // ES 7.10, Opensearch 1.x, 2.x ext.jacksonVersion = '2.15.3' ext.jettyVersion = '11.0.21' + // see also datahub-frontend/play.gradle ext.playVersion = '2.8.22' + ext.playScalaVersion = '2.13' ext.log4jVersion = '2.23.1' ext.slf4jVersion = '1.7.36' ext.logbackClassic = '1.4.14' @@ -103,7 +105,7 @@ project.ext.spec = [ ] project.ext.externalDependency = [ - 'akkaHttp': 'com.typesafe.akka:akka-http-core_2.12:10.2.10', + 'akkaHttp': "com.typesafe.akka:akka-http-core_$playScalaVersion:10.2.10", 'antlr4Runtime': 'org.antlr:antlr4-runtime:4.9.3', 'antlr4': 'org.antlr:antlr4:4.9.3', 'assertJ': 'org.assertj:assertj-core:3.11.1', @@ -212,18 +214,18 @@ project.ext.externalDependency = [ 'parquet': 'org.apache.parquet:parquet-avro:1.12.3', 'parquetHadoop': 'org.apache.parquet:parquet-hadoop:1.13.1', 'picocli': 'info.picocli:picocli:4.5.0', - 'playCache': "com.typesafe.play:play-cache_2.12:$playVersion", - 'playCaffeineCache': "com.typesafe.play:play-caffeine-cache_2.12:$playVersion", - 'playWs': 'com.typesafe.play:play-ahc-ws-standalone_2.12:2.1.10', - 'playDocs': "com.typesafe.play:play-docs_2.12:$playVersion", - 'playGuice': "com.typesafe.play:play-guice_2.12:$playVersion", - 'playJavaJdbc': "com.typesafe.play:play-java-jdbc_2.12:$playVersion", - 'playAkkaHttpServer': "com.typesafe.play:play-akka-http-server_2.12:$playVersion", - 'playServer': "com.typesafe.play:play-server_2.12:$playVersion", - 'playTest': "com.typesafe.play:play-test_2.12:$playVersion", - 'playFilters': "com.typesafe.play:filters-helpers_2.12:$playVersion", - 'pac4j': 'org.pac4j:pac4j-oidc:4.5.8', - 'playPac4j': 'org.pac4j:play-pac4j_2.12:9.0.2', + 'playCache': "com.typesafe.play:play-cache_$playScalaVersion:$playVersion", + 'playCaffeineCache': "com.typesafe.play:play-caffeine-cache_$playScalaVersion:$playVersion", + 'playWs': "com.typesafe.play:play-ahc-ws-standalone_$playScalaVersion:2.1.10", + 'playDocs': "com.typesafe.play:play-docs_$playScalaVersion:$playVersion", + 'playGuice': "com.typesafe.play:play-guice_$playScalaVersion:$playVersion", + 'playJavaJdbc': "com.typesafe.play:play-java-jdbc_$playScalaVersion:$playVersion", + 'playAkkaHttpServer': "com.typesafe.play:play-akka-http-server_$playScalaVersion:$playVersion", + 'playServer': "com.typesafe.play:play-server_$playScalaVersion:$playVersion", + 'playTest': "com.typesafe.play:play-test_$playScalaVersion:$playVersion", + 'playFilters': "com.typesafe.play:filters-helpers_$playScalaVersion:$playVersion", + 'pac4j': 'org.pac4j:pac4j-oidc:6.0.6', + 'playPac4j': "org.pac4j:play-pac4j_$playScalaVersion:12.0.0-PLAY2.8", 'postgresql': 'org.postgresql:postgresql:42.3.9', 'protobuf': 'com.google.protobuf:protobuf-java:3.25.5', 'grpcProtobuf': 'io.grpc:grpc-protobuf:1.53.0', @@ -407,6 +409,8 @@ subprojects { googleJavaFormat() target project.fileTree(project.projectDir) { include 'src/**/*.java' + include 'app/**/*.java' + include 'test/**/*.java' exclude 'src/**/resources/' exclude 'src/**/generated/' exclude 'src/**/mainGeneratedDataTemplate/' diff --git a/datahub-frontend/app/auth/AuthModule.java b/datahub-frontend/app/auth/AuthModule.java index 7bb25478907011..7fa99ab3cb2621 100644 --- a/datahub-frontend/app/auth/AuthModule.java +++ b/datahub-frontend/app/auth/AuthModule.java @@ -13,6 +13,7 @@ import com.google.inject.Provides; import com.google.inject.Singleton; import com.google.inject.name.Named; +import com.linkedin.entity.client.EntityClientConfig; import com.linkedin.entity.client.SystemEntityClient; import com.linkedin.entity.client.SystemRestliEntityClient; import com.linkedin.metadata.models.registry.EmptyEntityRegistry; @@ -21,27 +22,28 @@ import com.linkedin.util.Configuration; import config.ConfigurationProvider; import controllers.SsoCallbackController; -import io.datahubproject.metadata.context.ValidationContext; -import java.nio.charset.StandardCharsets; -import java.util.Collections; - import io.datahubproject.metadata.context.ActorContext; import io.datahubproject.metadata.context.AuthorizationContext; import io.datahubproject.metadata.context.EntityRegistryContext; import io.datahubproject.metadata.context.OperationContext; import io.datahubproject.metadata.context.OperationContextConfig; import io.datahubproject.metadata.context.SearchContext; +import io.datahubproject.metadata.context.ValidationContext; +import java.nio.charset.StandardCharsets; +import java.util.Collections; +import javax.annotation.Nonnull; import lombok.extern.slf4j.Slf4j; import org.apache.commons.codec.digest.DigestUtils; import org.apache.http.impl.client.CloseableHttpClient; import org.apache.http.impl.client.HttpClients; import org.pac4j.core.config.Config; import org.pac4j.core.context.session.SessionStore; +import org.pac4j.core.profile.ProfileManager; +import org.pac4j.core.util.serializer.JavaSerializer; 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; import org.springframework.context.annotation.AnnotationConfigApplicationContext; import play.Environment; @@ -63,14 +65,16 @@ public class AuthModule extends AbstractModule { private static final String PAC4J_SESSIONSTORE_PROVIDER_CONF = "pac4j.sessionStore.provider"; private static final String ENTITY_CLIENT_RETRY_INTERVAL = "entityClient.retryInterval"; private static final String ENTITY_CLIENT_NUM_RETRIES = "entityClient.numRetries"; - private static final String ENTITY_CLIENT_RESTLI_GET_BATCH_SIZE = "entityClient.restli.get.batchSize"; - private static final String ENTITY_CLIENT_RESTLI_GET_BATCH_CONCURRENCY = "entityClient.restli.get.batchConcurrency"; + private static final String ENTITY_CLIENT_RESTLI_GET_BATCH_SIZE = + "entityClient.restli.get.batchSize"; + private static final String ENTITY_CLIENT_RESTLI_GET_BATCH_CONCURRENCY = + "entityClient.restli.get.batchConcurrency"; private static final String GET_SSO_SETTINGS_ENDPOINT = "auth/getSsoSettings"; - private final com.typesafe.config.Config _configs; + private final com.typesafe.config.Config configs; public AuthModule(final Environment environment, final com.typesafe.config.Config configs) { - _configs = configs; + this.configs = configs; } @Override @@ -84,13 +88,13 @@ protected void configure() { * 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); + 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); + bind(PlayCacheSessionStore.class).toInstance(playCacheSessionStore); } else { PlayCookieSessionStore playCacheCookieStore; try { @@ -98,17 +102,18 @@ protected void configure() { // 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 aesKeyBase = configs.getString(PAC4J_AES_KEY_BASE_CONF); final String aesKeyHash = DigestUtils.sha256Hex(aesKeyBase.getBytes(StandardCharsets.UTF_8)); final String aesEncryptionKey = aesKeyHash.substring(0, 16); playCacheCookieStore = new PlayCookieSessionStore(new ShiroAesDataEncrypter(aesEncryptionKey.getBytes())); + playCacheCookieStore.setSerializer(new JavaSerializer()); } 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(PlayCookieSessionStore.class).toInstance(playCacheCookieStore); } try { @@ -133,9 +138,12 @@ protected void configure() { @Provides @Singleton - protected Config provideConfig() { + protected Config provideConfig(@Nonnull SessionStore sessionStore) { Config config = new Config(); + config.setSessionStoreFactory(parameters -> sessionStore); config.setHttpActionAdapter(new PlayHttpActionAdapter()); + config.setProfileManagerFactory(ProfileManager::new); + return config; } @@ -145,7 +153,7 @@ protected SsoManager provideSsoManager( Authentication systemAuthentication, CloseableHttpClient httpClient) { SsoManager manager = new SsoManager( - _configs, systemAuthentication, getSsoSettingsRequestUrl(_configs), httpClient); + configs, systemAuthentication, getSsoSettingsRequestUrl(configs), httpClient); manager.initializeSsoProvider(); return manager; } @@ -155,8 +163,8 @@ protected SsoManager provideSsoManager( protected Authentication provideSystemAuthentication() { // Returns an instance of Authentication used to authenticate system initiated calls to Metadata // Service. - String systemClientId = _configs.getString(SYSTEM_CLIENT_ID_CONFIG_PATH); - String systemSecret = _configs.getString(SYSTEM_CLIENT_SECRET_CONFIG_PATH); + String systemClientId = configs.getString(SYSTEM_CLIENT_ID_CONFIG_PATH); + String systemSecret = configs.getString(SYSTEM_CLIENT_SECRET_CONFIG_PATH); final Actor systemActor = new Actor(ActorType.USER, systemClientId); // TODO: Change to service actor once supported. return new Authentication( @@ -169,27 +177,25 @@ protected Authentication provideSystemAuthentication() { @Singleton @Named("systemOperationContext") protected OperationContext provideOperationContext( - final Authentication systemAuthentication, - final ConfigurationProvider configurationProvider) { + final Authentication systemAuthentication, + final ConfigurationProvider configurationProvider) { ActorContext systemActorContext = - ActorContext.builder() - .systemAuth(true) - .authentication(systemAuthentication) - .build(); - OperationContextConfig systemConfig = OperationContextConfig.builder() + ActorContext.builder().systemAuth(true).authentication(systemAuthentication).build(); + OperationContextConfig systemConfig = + OperationContextConfig.builder() .viewAuthorizationConfiguration(configurationProvider.getAuthorization().getView()) .allowSystemAuthentication(true) .build(); return OperationContext.builder() - .operationContextConfig(systemConfig) - .systemActorContext(systemActorContext) - // Authorizer.EMPTY is fine since it doesn't actually apply to system auth - .authorizationContext(AuthorizationContext.builder().authorizer(Authorizer.EMPTY).build()) - .searchContext(SearchContext.EMPTY) - .entityRegistryContext(EntityRegistryContext.builder().build(EmptyEntityRegistry.EMPTY)) - .validationContext(ValidationContext.builder().alternateValidation(false).build()) - .build(systemAuthentication); + .operationContextConfig(systemConfig) + .systemActorContext(systemActorContext) + // Authorizer.EMPTY is fine since it doesn't actually apply to system auth + .authorizationContext(AuthorizationContext.builder().authorizer(Authorizer.EMPTY).build()) + .searchContext(SearchContext.EMPTY) + .entityRegistryContext(EntityRegistryContext.builder().build(EmptyEntityRegistry.EMPTY)) + .validationContext(ValidationContext.builder().alternateValidation(false).build()) + .build(systemAuthentication); } @Provides @@ -208,11 +214,13 @@ protected SystemEntityClient provideEntityClient( return new SystemRestliEntityClient( buildRestliClient(), - new ExponentialBackoff(_configs.getInt(ENTITY_CLIENT_RETRY_INTERVAL)), - _configs.getInt(ENTITY_CLIENT_NUM_RETRIES), - configurationProvider.getCache().getClient().getEntityClient(), - Math.max(1, _configs.getInt(ENTITY_CLIENT_RESTLI_GET_BATCH_SIZE)), - Math.max(1, _configs.getInt(ENTITY_CLIENT_RESTLI_GET_BATCH_CONCURRENCY))); + EntityClientConfig.builder() + .backoffPolicy(new ExponentialBackoff(configs.getInt(ENTITY_CLIENT_RETRY_INTERVAL))) + .retryCount(configs.getInt(ENTITY_CLIENT_NUM_RETRIES)) + .batchGetV2Size(configs.getInt(ENTITY_CLIENT_RESTLI_GET_BATCH_SIZE)) + .batchGetV2Concurrency(2) + .build(), + configurationProvider.getCache().getClient().getEntityClient()); } @Provides @@ -220,11 +228,11 @@ protected SystemEntityClient provideEntityClient( protected AuthServiceClient provideAuthClient( Authentication systemAuthentication, CloseableHttpClient httpClient) { // Init a GMS auth client - final String metadataServiceHost = getMetadataServiceHost(_configs); + final String metadataServiceHost = getMetadataServiceHost(configs); - final int metadataServicePort = getMetadataServicePort(_configs); + final int metadataServicePort = getMetadataServicePort(configs); - final boolean metadataServiceUseSsl = doesMetadataServiceUseSsl(_configs); + final boolean metadataServiceUseSsl = doesMetadataServiceUseSsl(configs); return new AuthServiceClient( metadataServiceHost, @@ -243,22 +251,22 @@ protected CloseableHttpClient provideHttpClient() { private com.linkedin.restli.client.Client buildRestliClient() { final String metadataServiceHost = utils.ConfigUtil.getString( - _configs, + configs, METADATA_SERVICE_HOST_CONFIG_PATH, utils.ConfigUtil.DEFAULT_METADATA_SERVICE_HOST); final int metadataServicePort = utils.ConfigUtil.getInt( - _configs, + configs, utils.ConfigUtil.METADATA_SERVICE_PORT_CONFIG_PATH, utils.ConfigUtil.DEFAULT_METADATA_SERVICE_PORT); final boolean metadataServiceUseSsl = utils.ConfigUtil.getBoolean( - _configs, + configs, utils.ConfigUtil.METADATA_SERVICE_USE_SSL_CONFIG_PATH, ConfigUtil.DEFAULT_METADATA_SERVICE_USE_SSL); final String metadataServiceSslProtocol = utils.ConfigUtil.getString( - _configs, + configs, utils.ConfigUtil.METADATA_SERVICE_SSL_PROTOCOL_CONFIG_PATH, ConfigUtil.DEFAULT_METADATA_SERVICE_SSL_PROTOCOL); return DefaultRestliClientFactory.getRestLiClient( diff --git a/datahub-frontend/app/auth/AuthUtils.java b/datahub-frontend/app/auth/AuthUtils.java index 51bb784c61b3b1..490f52bece6517 100644 --- a/datahub-frontend/app/auth/AuthUtils.java +++ b/datahub-frontend/app/auth/AuthUtils.java @@ -75,6 +75,7 @@ public class AuthUtils { public static final String RESPONSE_MODE = "responseMode"; public static final String USE_NONCE = "useNonce"; public static final String READ_TIMEOUT = "readTimeout"; + public static final String CONNECT_TIMEOUT = "connectTimeout"; public static final String EXTRACT_JWT_ACCESS_TOKEN_CLAIMS = "extractJwtAccessTokenClaims"; // Retained for backwards compatibility public static final String PREFERRED_JWS_ALGORITHM = "preferredJwsAlgorithm"; diff --git a/datahub-frontend/app/auth/CookieConfigs.java b/datahub-frontend/app/auth/CookieConfigs.java index 63b2ce61aaf9bb..e77e2001448356 100644 --- a/datahub-frontend/app/auth/CookieConfigs.java +++ b/datahub-frontend/app/auth/CookieConfigs.java @@ -10,34 +10,34 @@ public class CookieConfigs { public static final String AUTH_COOKIE_SECURE = "play.http.session.secure"; public static final boolean DEFAULT_AUTH_COOKIE_SECURE = false; - private final int _ttlInHours; - private final String _authCookieSameSite; - private final boolean _authCookieSecure; + private final int ttlInHours; + private final String authCookieSameSite; + private final boolean authCookieSecure; public CookieConfigs(final Config configs) { - _ttlInHours = + ttlInHours = configs.hasPath(SESSION_TTL_CONFIG_PATH) ? configs.getInt(SESSION_TTL_CONFIG_PATH) : DEFAULT_SESSION_TTL_HOURS; - _authCookieSameSite = + authCookieSameSite = configs.hasPath(AUTH_COOKIE_SAME_SITE) ? configs.getString(AUTH_COOKIE_SAME_SITE) : DEFAULT_AUTH_COOKIE_SAME_SITE; - _authCookieSecure = + authCookieSecure = configs.hasPath(AUTH_COOKIE_SECURE) ? configs.getBoolean(AUTH_COOKIE_SECURE) : DEFAULT_AUTH_COOKIE_SECURE; } public int getTtlInHours() { - return _ttlInHours; + return ttlInHours; } public String getAuthCookieSameSite() { - return _authCookieSameSite; + return authCookieSameSite; } public boolean getAuthCookieSecure() { - return _authCookieSecure; + return authCookieSecure; } } diff --git a/datahub-frontend/app/auth/JAASConfigs.java b/datahub-frontend/app/auth/JAASConfigs.java index 529bf98e1fdcf2..dee4ded68808a3 100644 --- a/datahub-frontend/app/auth/JAASConfigs.java +++ b/datahub-frontend/app/auth/JAASConfigs.java @@ -8,16 +8,16 @@ public class JAASConfigs { public static final String JAAS_ENABLED_CONFIG_PATH = "auth.jaas.enabled"; - private Boolean _isEnabled = true; + private Boolean isEnabled = true; public JAASConfigs(final com.typesafe.config.Config configs) { if (configs.hasPath(JAAS_ENABLED_CONFIG_PATH) && !configs.getBoolean(JAAS_ENABLED_CONFIG_PATH)) { - _isEnabled = false; + isEnabled = false; } } public boolean isJAASEnabled() { - return _isEnabled; + return isEnabled; } } diff --git a/datahub-frontend/app/auth/NativeAuthenticationConfigs.java b/datahub-frontend/app/auth/NativeAuthenticationConfigs.java index 772c2c8f92f28c..a7b8a8bc800672 100644 --- a/datahub-frontend/app/auth/NativeAuthenticationConfigs.java +++ b/datahub-frontend/app/auth/NativeAuthenticationConfigs.java @@ -7,17 +7,17 @@ public class NativeAuthenticationConfigs { public static final String NATIVE_AUTHENTICATION_ENFORCE_VALID_EMAIL_ENABLED_CONFIG_PATH = "auth.native.signUp.enforceValidEmail"; - private Boolean _isEnabled = true; - private Boolean _isEnforceValidEmailEnabled = true; + private Boolean isEnabled = true; + private Boolean isEnforceValidEmailEnabled = true; public NativeAuthenticationConfigs(final com.typesafe.config.Config configs) { if (configs.hasPath(NATIVE_AUTHENTICATION_ENABLED_CONFIG_PATH)) { - _isEnabled = + isEnabled = Boolean.parseBoolean( configs.getValue(NATIVE_AUTHENTICATION_ENABLED_CONFIG_PATH).toString()); } if (configs.hasPath(NATIVE_AUTHENTICATION_ENFORCE_VALID_EMAIL_ENABLED_CONFIG_PATH)) { - _isEnforceValidEmailEnabled = + isEnforceValidEmailEnabled = Boolean.parseBoolean( configs .getValue(NATIVE_AUTHENTICATION_ENFORCE_VALID_EMAIL_ENABLED_CONFIG_PATH) @@ -26,10 +26,10 @@ public NativeAuthenticationConfigs(final com.typesafe.config.Config configs) { } public boolean isNativeAuthenticationEnabled() { - return _isEnabled; + return isEnabled; } public boolean isEnforceValidEmailEnabled() { - return _isEnforceValidEmailEnabled; + return isEnforceValidEmailEnabled; } } diff --git a/datahub-frontend/app/auth/sso/SsoConfigs.java b/datahub-frontend/app/auth/sso/SsoConfigs.java index 976d0826f22770..46a2b7bfd27e8c 100644 --- a/datahub-frontend/app/auth/sso/SsoConfigs.java +++ b/datahub-frontend/app/auth/sso/SsoConfigs.java @@ -1,10 +1,9 @@ package auth.sso; -import com.fasterxml.jackson.databind.JsonNode; -import com.fasterxml.jackson.databind.ObjectMapper; - import static auth.AuthUtils.*; +import com.fasterxml.jackson.databind.JsonNode; +import com.fasterxml.jackson.databind.ObjectMapper; /** * Class responsible for extracting and validating top-level SSO related configurations. TODO: @@ -25,72 +24,72 @@ public class SsoConfigs { private static final String DEFAULT_SUCCESS_REDIRECT_PATH = "/"; - private final String _authBaseUrl; - private final String _authBaseCallbackPath; - private final String _authSuccessRedirectPath; - private final Boolean _oidcEnabled; + private final String authBaseUrl; + private final String authBaseCallbackPath; + private final String authSuccessRedirectPath; + private final Boolean oidcEnabled; public SsoConfigs(Builder builder) { - _authBaseUrl = builder._authBaseUrl; - _authBaseCallbackPath = builder._authBaseCallbackPath; - _authSuccessRedirectPath = builder._authSuccessRedirectPath; - _oidcEnabled = builder._oidcEnabled; + authBaseUrl = builder.authBaseUrl; + authBaseCallbackPath = builder.authBaseCallbackPath; + authSuccessRedirectPath = builder.authSuccessRedirectPath; + oidcEnabled = builder.oidcEnabled; } public String getAuthBaseUrl() { - return _authBaseUrl; + return authBaseUrl; } public String getAuthBaseCallbackPath() { - return _authBaseCallbackPath; + return authBaseCallbackPath; } public String getAuthSuccessRedirectPath() { - return _authSuccessRedirectPath; + return authSuccessRedirectPath; } public Boolean isOidcEnabled() { - return _oidcEnabled; + return oidcEnabled; } public static class Builder> { - protected String _authBaseUrl = null; - private String _authBaseCallbackPath = DEFAULT_BASE_CALLBACK_PATH; - private String _authSuccessRedirectPath = DEFAULT_SUCCESS_REDIRECT_PATH; - protected Boolean _oidcEnabled = false; - private final ObjectMapper _objectMapper = new ObjectMapper(); + protected String authBaseUrl = null; + private String authBaseCallbackPath = DEFAULT_BASE_CALLBACK_PATH; + private String authSuccessRedirectPath = DEFAULT_SUCCESS_REDIRECT_PATH; + protected Boolean oidcEnabled = false; + private final ObjectMapper objectMapper = new ObjectMapper(); protected JsonNode jsonNode = null; // No need to check if changes are made since this method is only called at start-up. public Builder from(final com.typesafe.config.Config configs) { if (configs.hasPath(AUTH_BASE_URL_CONFIG_PATH)) { - _authBaseUrl = configs.getString(AUTH_BASE_URL_CONFIG_PATH); + authBaseUrl = configs.getString(AUTH_BASE_URL_CONFIG_PATH); } if (configs.hasPath(AUTH_BASE_CALLBACK_PATH_CONFIG_PATH)) { - _authBaseCallbackPath = configs.getString(AUTH_BASE_CALLBACK_PATH_CONFIG_PATH); + authBaseCallbackPath = configs.getString(AUTH_BASE_CALLBACK_PATH_CONFIG_PATH); } if (configs.hasPath(OIDC_ENABLED_CONFIG_PATH)) { - _oidcEnabled = + oidcEnabled = Boolean.TRUE.equals(Boolean.parseBoolean(configs.getString(OIDC_ENABLED_CONFIG_PATH))); } if (configs.hasPath(AUTH_SUCCESS_REDIRECT_PATH_CONFIG_PATH)) { - _authSuccessRedirectPath = configs.getString(AUTH_SUCCESS_REDIRECT_PATH_CONFIG_PATH); + authSuccessRedirectPath = configs.getString(AUTH_SUCCESS_REDIRECT_PATH_CONFIG_PATH); } return this; } public Builder from(String ssoSettingsJsonStr) { try { - jsonNode = _objectMapper.readTree(ssoSettingsJsonStr); + jsonNode = objectMapper.readTree(ssoSettingsJsonStr); } catch (Exception e) { throw new RuntimeException( String.format("Failed to parse ssoSettingsJsonStr %s into JSON", ssoSettingsJsonStr)); } if (jsonNode.has(BASE_URL)) { - _authBaseUrl = jsonNode.get(BASE_URL).asText(); + authBaseUrl = jsonNode.get(BASE_URL).asText(); } if (jsonNode.has(OIDC_ENABLED)) { - _oidcEnabled = jsonNode.get(OIDC_ENABLED).asBoolean(); + oidcEnabled = jsonNode.get(OIDC_ENABLED).asBoolean(); } return this; diff --git a/datahub-frontend/app/auth/sso/SsoManager.java b/datahub-frontend/app/auth/sso/SsoManager.java index 8377eb40e237f7..8a8a7f95a1b608 100644 --- a/datahub-frontend/app/auth/sso/SsoManager.java +++ b/datahub-frontend/app/auth/sso/SsoManager.java @@ -26,22 +26,21 @@ public class SsoManager { private SsoProvider _provider; // Only one active provider at a time. - private final Authentication - _authentication; // Authentication used to fetch SSO settings from GMS - private final String _ssoSettingsRequestUrl; // SSO settings request URL. - private final CloseableHttpClient _httpClient; // HTTP client for making requests to GMS. - private com.typesafe.config.Config _configs; + private final Authentication authentication; // Authentication used to fetch SSO settings from GMS + private final String ssoSettingsRequestUrl; // SSO settings request URL. + private final CloseableHttpClient httpClient; // HTTP client for making requests to GMS. + private com.typesafe.config.Config configs; public SsoManager( com.typesafe.config.Config configs, Authentication authentication, String ssoSettingsRequestUrl, CloseableHttpClient httpClient) { - _configs = configs; - _authentication = Objects.requireNonNull(authentication, "authentication cannot be null"); - _ssoSettingsRequestUrl = + this.configs = configs; + this.authentication = Objects.requireNonNull(authentication, "authentication cannot be null"); + this.ssoSettingsRequestUrl = Objects.requireNonNull(ssoSettingsRequestUrl, "ssoSettingsRequestUrl cannot be null"); - _httpClient = Objects.requireNonNull(httpClient, "httpClient cannot be null"); + this.httpClient = Objects.requireNonNull(httpClient, "httpClient cannot be null"); _provider = null; } @@ -52,6 +51,9 @@ public SsoManager( * @return true if SSO logic is enabled, false otherwise. */ public boolean isSsoEnabled() { + if (configs.hasPath("auth.oidc.enabled") && configs.getBoolean("auth.oidc.enabled")) { + return true; + } refreshSsoProvider(); return _provider != null; } @@ -66,7 +68,7 @@ public void setSsoProvider(final SsoProvider provider) { } public void setConfigs(final com.typesafe.config.Config configs) { - _configs = configs; + this.configs = configs; } public void clearSsoProvider() { @@ -87,19 +89,19 @@ public SsoProvider getSsoProvider() { public void initializeSsoProvider() { SsoConfigs ssoConfigs = null; try { - ssoConfigs = new SsoConfigs.Builder().from(_configs).build(); + ssoConfigs = new SsoConfigs.Builder().from(configs).build(); } catch (Exception e) { // Debug-level logging since this is expected to fail if SSO has not been configured. - log.debug(String.format("Missing SSO settings in static configs %s", _configs), e); + log.debug(String.format("Missing SSO settings in static configs %s", configs), e); } if (ssoConfigs != null && ssoConfigs.isOidcEnabled()) { try { - OidcConfigs oidcConfigs = new OidcConfigs.Builder().from(_configs).build(); + OidcConfigs oidcConfigs = new OidcConfigs.Builder().from(configs).build(); maybeUpdateOidcProvider(oidcConfigs); } catch (Exception e) { // Error-level logging since this is unexpected to fail if SSO has been configured. - log.error(String.format("Error building OidcConfigs from static configs %s", _configs), e); + log.error(String.format("Error building OidcConfigs from static configs %s", configs), e); } } else { // Clear the SSO Provider since no SSO is enabled. @@ -132,7 +134,7 @@ private void refreshSsoProvider() { if (ssoConfigs != null && ssoConfigs.isOidcEnabled()) { try { OidcConfigs oidcConfigs = - new OidcConfigs.Builder().from(_configs, ssoSettingsJsonStr).build(); + new OidcConfigs.Builder().from(configs, ssoSettingsJsonStr).build(); maybeUpdateOidcProvider(oidcConfigs); } catch (Exception e) { log.error( @@ -166,15 +168,15 @@ private void maybeUpdateOidcProvider(OidcConfigs oidcConfigs) { private Optional getDynamicSsoSettings() { CloseableHttpResponse response = null; try { - final HttpPost request = new HttpPost(_ssoSettingsRequestUrl); + final HttpPost request = new HttpPost(ssoSettingsRequestUrl); // Build JSON request to verify credentials for a native user. request.setEntity(new StringEntity("")); // Add authorization header with DataHub frontend system id and secret. - request.addHeader(Http.HeaderNames.AUTHORIZATION, _authentication.getCredentials()); + request.addHeader(Http.HeaderNames.AUTHORIZATION, authentication.getCredentials()); - response = _httpClient.execute(request); + response = httpClient.execute(request); final HttpEntity entity = response.getEntity(); if (response.getStatusLine().getStatusCode() == HttpStatus.SC_OK && entity != null) { // Successfully received the SSO settings diff --git a/datahub-frontend/app/auth/sso/SsoProvider.java b/datahub-frontend/app/auth/sso/SsoProvider.java index a0947b52b92ae6..2a6fff728966e5 100644 --- a/datahub-frontend/app/auth/sso/SsoProvider.java +++ b/datahub-frontend/app/auth/sso/SsoProvider.java @@ -1,7 +1,6 @@ package auth.sso; import org.pac4j.core.client.Client; -import org.pac4j.core.credentials.Credentials; /** A thin interface over a Pac4j {@link Client} object and its associated configurations. */ public interface SsoProvider { @@ -12,14 +11,14 @@ enum SsoProtocol { // SAML -- not yet supported. // Common name appears in the Callback URL itself. - private final String _commonName; + private final String commonName; public String getCommonName() { - return _commonName; + return commonName; } SsoProtocol(String commonName) { - _commonName = commonName; + this.commonName = commonName; } } @@ -30,5 +29,5 @@ public String getCommonName() { SsoProtocol protocol(); /** Retrieves an initialized Pac4j {@link Client}. */ - Client client(); + Client client(); } diff --git a/datahub-frontend/app/auth/sso/oidc/OidcAuthorizationGenerator.java b/datahub-frontend/app/auth/sso/oidc/OidcAuthorizationGenerator.java index fa676d2d16c904..3a4433b0ca81e2 100644 --- a/datahub-frontend/app/auth/sso/oidc/OidcAuthorizationGenerator.java +++ b/datahub-frontend/app/auth/sso/oidc/OidcAuthorizationGenerator.java @@ -5,7 +5,7 @@ import java.util.Map.Entry; import java.util.Optional; import org.pac4j.core.authorization.generator.AuthorizationGenerator; -import org.pac4j.core.context.WebContext; +import org.pac4j.core.context.CallContext; import org.pac4j.core.profile.AttributeLocation; import org.pac4j.core.profile.CommonProfile; import org.pac4j.core.profile.UserProfile; @@ -18,24 +18,31 @@ public class OidcAuthorizationGenerator implements AuthorizationGenerator { private static final Logger logger = LoggerFactory.getLogger(OidcAuthorizationGenerator.class); - private final ProfileDefinition profileDef; - + private final ProfileDefinition profileDef; private final OidcConfigs oidcConfigs; public OidcAuthorizationGenerator( - final ProfileDefinition profileDef, final OidcConfigs oidcConfigs) { + final ProfileDefinition profileDef, final OidcConfigs oidcConfigs) { this.profileDef = profileDef; this.oidcConfigs = oidcConfigs; } @Override - public Optional generate(WebContext context, UserProfile profile) { + public Optional generate(final CallContext context, final UserProfile profile) { + if (!(profile instanceof OidcProfile oidcProfile)) { + return Optional.of(profile); + } + if (oidcConfigs.getExtractJwtAccessTokenClaims().orElse(false)) { try { - final JWT jwt = JWTParser.parse(((OidcProfile) profile).getAccessToken().getValue()); + final JWT jwt = JWTParser.parse(oidcProfile.getAccessToken().getValue()); CommonProfile commonProfile = new CommonProfile(); + // Copy existing attributes + profile.getAttributes().forEach(commonProfile::addAttribute); + + // Add JWT claims for (final Entry entry : jwt.getJWTClaimsSet().getClaims().entrySet()) { final String claimName = entry.getKey(); @@ -51,6 +58,6 @@ public Optional generate(WebContext context, UserProfile profile) { } } - return Optional.ofNullable(profile); + return Optional.of(profile); } } diff --git a/datahub-frontend/app/auth/sso/oidc/OidcCallbackLogic.java b/datahub-frontend/app/auth/sso/oidc/OidcCallbackLogic.java index 510804ba17f1a8..ef5833f607efdb 100644 --- a/datahub-frontend/app/auth/sso/oidc/OidcCallbackLogic.java +++ b/datahub-frontend/app/auth/sso/oidc/OidcCallbackLogic.java @@ -39,6 +39,8 @@ import com.linkedin.metadata.utils.GenericRecordUtils; import com.linkedin.mxe.MetadataChangeProposal; import com.linkedin.r2.RemoteInvocationException; +import com.linkedin.util.Pair; +import io.datahubproject.metadata.context.OperationContext; import java.io.UnsupportedEncodingException; import java.net.MalformedURLException; import java.net.URI; @@ -51,27 +53,36 @@ import java.util.Collections; import java.util.List; import java.util.Map; +import java.util.Objects; import java.util.Optional; import java.util.Set; import java.util.regex.Matcher; import java.util.regex.Pattern; import java.util.stream.Collectors; - -import io.datahubproject.metadata.context.OperationContext; +import javax.annotation.Nonnull; import lombok.extern.slf4j.Slf4j; +import org.pac4j.core.client.BaseClient; +import org.pac4j.core.client.Client; +import org.pac4j.core.client.Clients; import org.pac4j.core.config.Config; +import org.pac4j.core.context.CallContext; import org.pac4j.core.context.Cookie; +import org.pac4j.core.context.FrameworkParameters; +import org.pac4j.core.context.WebContext; +import org.pac4j.core.credentials.Credentials; import org.pac4j.core.engine.DefaultCallbackLogic; +import org.pac4j.core.exception.http.HttpAction; 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.CommonHelper; import org.pac4j.core.util.Pac4jConstants; -import org.pac4j.play.PlayWebContext; +import org.pac4j.play.store.PlayCookieSessionStore; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; import play.mvc.Result; -import javax.annotation.Nonnull; - /** * This class contains the logic that is executed when an OpenID Connect Identity Provider redirects * back to D DataHub after an authentication attempt. @@ -83,7 +94,8 @@ * if the user does not already exist. */ @Slf4j -public class OidcCallbackLogic extends DefaultCallbackLogic { +public class OidcCallbackLogic extends DefaultCallbackLogic { + private static final Logger LOGGER = LoggerFactory.getLogger(OidcCallbackLogic.class); private final SsoManager ssoManager; private final SystemEntityClient systemEntityClient; @@ -105,67 +117,129 @@ public OidcCallbackLogic( } @Override - public Result perform( - PlayWebContext context, + public Object perform( Config config, - HttpActionAdapter 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); + String inputDefaultUrl, + Boolean inputRenewSession, + String defaultClient, + FrameworkParameters parameters) { + + final Pair ctxResult = + superPerform(config, inputDefaultUrl, inputRenewSession, defaultClient, parameters); + + CallContext ctx = ctxResult.getFirst(); + Result result = (Result) ctxResult.getSecond(); + + setContextRedirectUrl(ctx); // Handle OIDC authentication errors. - if (OidcResponseErrorHandler.isError(context)) { - return OidcResponseErrorHandler.handleError(context); + if (OidcResponseErrorHandler.isError(ctx)) { + return OidcResponseErrorHandler.handleError(ctx); } // By this point, we know that OIDC is the enabled provider. final OidcConfigs oidcConfigs = (OidcConfigs) ssoManager.getSsoProvider().configs(); - return handleOidcCallback(systemOperationContext, oidcConfigs, result, getProfileManager(context)); + return handleOidcCallback(systemOperationContext, ctx, oidcConfigs, result); + } + + /** Overriding this to be able to intercept the CallContext being created */ + private Pair superPerform( + Config config, + String inputDefaultUrl, + Boolean inputRenewSession, + String defaultClient, + FrameworkParameters parameters) { + LOGGER.debug("=== CALLBACK ==="); + CallContext ctx = this.buildContext(config, parameters); + WebContext webContext = ctx.webContext(); + HttpActionAdapter httpActionAdapter = config.getHttpActionAdapter(); + CommonHelper.assertNotNull("httpActionAdapter", httpActionAdapter); + + HttpAction action; + try { + CommonHelper.assertNotNull("clientFinder", getClientFinder()); + String defaultUrl = (String) Objects.requireNonNullElse(inputDefaultUrl, "/"); + boolean renewSession = inputRenewSession == null || inputRenewSession; + CommonHelper.assertNotBlank("defaultUrl", defaultUrl); + Clients clients = config.getClients(); + CommonHelper.assertNotNull("clients", clients); + List foundClients = getClientFinder().find(clients, webContext, defaultClient); + CommonHelper.assertTrue( + foundClients != null && foundClients.size() == 1, + "unable to find one indirect client for the callback: check the callback URL for a client name parameter or suffix path or ensure that your configuration defaults to one indirect client"); + Client foundClient = (Client) foundClients.get(0); + LOGGER.debug("foundClient: {}", foundClient); + CommonHelper.assertNotNull("foundClient", foundClient); + Credentials credentials = (Credentials) foundClient.getCredentials(ctx).orElse(null); + LOGGER.debug("extracted credentials: {}", credentials); + credentials = (Credentials) foundClient.validateCredentials(ctx, credentials).orElse(null); + LOGGER.debug("validated credentials: {}", credentials); + if (credentials != null && !credentials.isForAuthentication()) { + action = foundClient.processLogout(ctx, credentials); + } else { + if (credentials != null) { + Optional optProfile = foundClient.getUserProfile(ctx, credentials); + LOGGER.debug("optProfile: {}", optProfile); + if (optProfile.isPresent()) { + UserProfile profile = (UserProfile) optProfile.get(); + Boolean saveProfileInSession = + ((BaseClient) foundClient).getSaveProfileInSession(webContext, profile); + boolean multiProfile = ((BaseClient) foundClient).isMultiProfile(webContext, profile); + LOGGER.debug( + "saveProfileInSession: {} / multiProfile: {}", saveProfileInSession, multiProfile); + this.saveUserProfile( + ctx, config, profile, saveProfileInSession, multiProfile, renewSession); + } + } + + action = this.redirectToOriginallyRequestedUrl(ctx, defaultUrl); + } + } catch (RuntimeException var20) { + RuntimeException e = var20; + return Pair.of(ctx, this.handleException(e, httpActionAdapter, webContext)); + } + + return Pair.of(ctx, httpActionAdapter.adapt(action, webContext)); } - @SuppressWarnings("unchecked") - private void setContextRedirectUrl(PlayWebContext context) { + private void setContextRedirectUrl(CallContext ctx) { + WebContext context = ctx.webContext(); + PlayCookieSessionStore sessionStore = (PlayCookieSessionStore) ctx.sessionStore(); + Optional 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( + sessionStore.set( + context, + Pac4jConstants.REQUESTED_URL, + sessionStore + .getSerializer() + .deserializeFromBytes( uncompressBytes(Base64.getDecoder().decode(cookie.getValue()))))); } private Result handleOidcCallback( final OperationContext opContext, + final CallContext ctx, final OidcConfigs oidcConfigs, - final Result result, - final ProfileManager profileManager) { + final Result result) { log.debug("Beginning OIDC Callback Handling..."); + ProfileManager profileManager = + ctx.profileManagerFactory().apply(ctx.webContext(), ctx.sessionStore()); + if (profileManager.isAuthenticated()) { // If authenticated, the user should have a profile. - final CommonProfile profile = (CommonProfile) profileManager.get(true).get(); + final Optional optProfile = profileManager.getProfile(); + if (optProfile.isEmpty()) { + return internalServerError( + "Failed to authenticate current user. Cannot find valid identity provider profile in session."); + } + final CommonProfile profile = (CommonProfile) optProfile.get(); log.debug( String.format( "Found authenticated user with profile %s", profile.getAttributes().toString())); @@ -196,7 +270,8 @@ private Result handleOidcCallback( } // Update user status to active on login. // If we want to prevent certain users from logging in, here's where we'll want to do it. - setUserStatus(opContext, + setUserStatus( + opContext, corpUserUrn, new CorpUserStatus() .setStatus(Constants.CORP_USER_STATUS_ACTIVE) @@ -307,29 +382,32 @@ private CorpUserSnapshot extractUser(CorpuserUrn urn, CommonProfile profile) { return corpUserSnapshot; } - public static Collection getGroupNames(CommonProfile profile, Object groupAttribute, String groupsClaimName) { - Collection groupNames = Collections.emptyList(); - try { - if (groupAttribute instanceof Collection) { - // List of group names - groupNames = (Collection) profile.getAttribute(groupsClaimName, Collection.class); - } else if (groupAttribute instanceof String) { - String groupString = (String) groupAttribute; - ObjectMapper objectMapper = new ObjectMapper(); - try { - // Json list of group names - groupNames = objectMapper.readValue(groupString, new TypeReference>(){}); - } catch (Exception e) { - groupNames = Arrays.asList(groupString.split(",")); - } + public static Collection getGroupNames( + CommonProfile profile, Object groupAttribute, String groupsClaimName) { + Collection groupNames = Collections.emptyList(); + try { + if (groupAttribute instanceof Collection) { + // List of group names + groupNames = (Collection) profile.getAttribute(groupsClaimName, Collection.class); + } else if (groupAttribute instanceof String) { + String groupString = (String) groupAttribute; + ObjectMapper objectMapper = new ObjectMapper(); + try { + // Json list of group names + groupNames = objectMapper.readValue(groupString, new TypeReference>() {}); + } catch (Exception e) { + groupNames = Arrays.asList(groupString.split(",")); } - } catch (Exception e) { - log.error(String.format( - "Failed to parse group names: Expected to find a list of strings for attribute with name %s, found %s", - groupsClaimName, profile.getAttribute(groupsClaimName).getClass())); } - return groupNames; + } catch (Exception e) { + log.error( + String.format( + "Failed to parse group names: Expected to find a list of strings for attribute with name %s, found %s", + groupsClaimName, profile.getAttribute(groupsClaimName).getClass())); + } + return groupNames; } + private List extractGroups(CommonProfile profile) { log.debug( @@ -350,7 +428,8 @@ private List extractGroups(CommonProfile profile) { if (profile.containsAttribute(groupsClaimName)) { try { final List groupSnapshots = new ArrayList<>(); - Collection groupNames = getGroupNames(profile, profile.getAttribute(groupsClaimName), groupsClaimName); + Collection groupNames = + getGroupNames(profile, profile.getAttribute(groupsClaimName), groupsClaimName); for (String groupName : groupNames) { // Create a basic CorpGroupSnapshot from the information. @@ -405,7 +484,8 @@ private GroupMembership createGroupMembership(final List extr return groupMembershipAspect; } - private void tryProvisionUser(@Nonnull OperationContext opContext, CorpUserSnapshot corpUserSnapshot) { + private void tryProvisionUser( + @Nonnull OperationContext opContext, CorpUserSnapshot corpUserSnapshot) { log.debug(String.format("Attempting to provision user with urn %s", corpUserSnapshot.getUrn())); @@ -439,7 +519,8 @@ private void tryProvisionUser(@Nonnull OperationContext opContext, CorpUserSnaps } } - private void tryProvisionGroups(@Nonnull OperationContext opContext, List corpGroups) { + private void tryProvisionGroups( + @Nonnull OperationContext opContext, List corpGroups) { log.debug( String.format( @@ -450,8 +531,7 @@ private void tryProvisionGroups(@Nonnull OperationContext opContext, List urnsToFetch = corpGroups.stream().map(CorpGroupSnapshot::getUrn).collect(Collectors.toSet()); - final Map existingGroups = - systemEntityClient.batchGet(opContext, urnsToFetch); + final Map existingGroups = systemEntityClient.batchGet(opContext, urnsToFetch); log.debug(String.format("Fetched GMS groups with urns %s", existingGroups.keySet())); @@ -489,7 +569,8 @@ private void tryProvisionGroups(@Nonnull OperationContext opContext, List new Entity().setValue(Snapshot.create(groupSnapshot))) .collect(Collectors.toSet())); @@ -505,7 +586,8 @@ private void tryProvisionGroups(@Nonnull OperationContext opContext, List useNonce; private final Optional customParamResource; private final String readTimeout; + private final String connectTimeout; private final Optional extractJwtAccessTokenClaims; private final Optional preferredJwsAlgorithm; private final Optional grantType; @@ -100,6 +103,7 @@ public OidcConfigs(Builder builder) { this.useNonce = builder.useNonce; this.customParamResource = builder.customParamResource; this.readTimeout = builder.readTimeout; + this.connectTimeout = builder.connectTimeout; this.extractJwtAccessTokenClaims = builder.extractJwtAccessTokenClaims; this.preferredJwsAlgorithm = builder.preferredJwsAlgorithm; this.acrValues = builder.acrValues; @@ -127,6 +131,7 @@ public static class Builder extends SsoConfigs.Builder { private Optional useNonce = Optional.empty(); private Optional customParamResource = Optional.empty(); private String readTimeout = DEFAULT_OIDC_READ_TIMEOUT; + private String connectTimeout = DEFAULT_OIDC_CONNECT_TIMEOUT; private Optional extractJwtAccessTokenClaims = Optional.empty(); private Optional preferredJwsAlgorithm = Optional.empty(); private Optional grantType = Optional.empty(); @@ -173,6 +178,7 @@ public Builder from(final com.typesafe.config.Config configs) { useNonce = getOptional(configs, OIDC_USE_NONCE).map(Boolean::parseBoolean); customParamResource = getOptional(configs, OIDC_CUSTOM_PARAM_RESOURCE); readTimeout = getOptional(configs, OIDC_READ_TIMEOUT, DEFAULT_OIDC_READ_TIMEOUT); + connectTimeout = getOptional(configs, OIDC_CONNECT_TIMEOUT, DEFAULT_OIDC_CONNECT_TIMEOUT); extractJwtAccessTokenClaims = getOptional(configs, OIDC_EXTRACT_JWT_ACCESS_TOKEN_CLAIMS).map(Boolean::parseBoolean); preferredJwsAlgorithm = @@ -232,6 +238,9 @@ public Builder from(final com.typesafe.config.Config configs, final String ssoSe if (jsonNode.has(READ_TIMEOUT)) { readTimeout = jsonNode.get(READ_TIMEOUT).asText(); } + if (jsonNode.has(CONNECT_TIMEOUT)) { + connectTimeout = jsonNode.get(CONNECT_TIMEOUT).asText(); + } if (jsonNode.has(EXTRACT_JWT_ACCESS_TOKEN_CLAIMS)) { extractJwtAccessTokenClaims = Optional.of(jsonNode.get(EXTRACT_JWT_ACCESS_TOKEN_CLAIMS).asBoolean()); @@ -250,11 +259,11 @@ public Builder from(final com.typesafe.config.Config configs, final String ssoSe } public OidcConfigs build() { - Objects.requireNonNull(_oidcEnabled, "oidcEnabled is required"); + Objects.requireNonNull(oidcEnabled, "oidcEnabled is required"); Objects.requireNonNull(clientId, "clientId is required"); Objects.requireNonNull(clientSecret, "clientSecret is required"); Objects.requireNonNull(discoveryUri, "discoveryUri is required"); - Objects.requireNonNull(_authBaseUrl, "authBaseUrl is required"); + Objects.requireNonNull(authBaseUrl, "authBaseUrl is required"); return new OidcConfigs(this); } diff --git a/datahub-frontend/app/auth/sso/oidc/OidcProvider.java b/datahub-frontend/app/auth/sso/oidc/OidcProvider.java index a8a3205e8299c8..7fcaa5a9683cb1 100644 --- a/datahub-frontend/app/auth/sso/oidc/OidcProvider.java +++ b/datahub-frontend/app/auth/sso/oidc/OidcProvider.java @@ -2,15 +2,16 @@ import auth.sso.SsoProvider; import auth.sso.oidc.custom.CustomOidcClient; -import com.google.common.collect.ImmutableMap; +import com.nimbusds.jose.JWSAlgorithm; import java.util.HashMap; import java.util.Map; import lombok.extern.slf4j.Slf4j; import org.pac4j.core.client.Client; import org.pac4j.core.http.callback.PathParameterCallbackUrlResolver; import org.pac4j.oidc.config.OidcConfiguration; -import org.pac4j.oidc.credentials.OidcCredentials; import org.pac4j.oidc.profile.OidcProfileDefinition; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; /** * Implementation of {@link SsoProvider} supporting the OIDC protocol. @@ -24,25 +25,25 @@ */ @Slf4j public class OidcProvider implements SsoProvider { - + private static final Logger logger = LoggerFactory.getLogger(OidcProvider.class); private static final String OIDC_CLIENT_NAME = "oidc"; - private final OidcConfigs _oidcConfigs; - private final Client _oidcClient; // Used primarily for redirecting to IdP. + private final OidcConfigs oidcConfigs; + private final Client oidcClient; // Used primarily for redirecting to IdP. public OidcProvider(final OidcConfigs configs) { - _oidcConfigs = configs; - _oidcClient = createPac4jClient(); + oidcConfigs = configs; + oidcClient = createPac4jClient(); } @Override - public Client client() { - return _oidcClient; + public Client client() { + return oidcClient; } @Override public OidcConfigs configs() { - return _oidcConfigs; + return oidcConfigs; } @Override @@ -50,50 +51,52 @@ public SsoProtocol protocol() { return SsoProtocol.OIDC; } - private Client createPac4jClient() { + private Client createPac4jClient() { final OidcConfiguration oidcConfiguration = new OidcConfiguration(); - oidcConfiguration.setClientId(_oidcConfigs.getClientId()); - oidcConfiguration.setSecret(_oidcConfigs.getClientSecret()); - oidcConfiguration.setDiscoveryURI(_oidcConfigs.getDiscoveryUri()); + oidcConfiguration.setClientId(oidcConfigs.getClientId()); + oidcConfiguration.setSecret(oidcConfigs.getClientSecret()); + oidcConfiguration.setDiscoveryURI(oidcConfigs.getDiscoveryUri()); oidcConfiguration.setClientAuthenticationMethodAsString( - _oidcConfigs.getClientAuthenticationMethod()); - oidcConfiguration.setScope(_oidcConfigs.getScope()); + oidcConfigs.getClientAuthenticationMethod()); + oidcConfiguration.setScope(oidcConfigs.getScope()); try { - oidcConfiguration.setReadTimeout(Integer.parseInt(_oidcConfigs.getReadTimeout())); + oidcConfiguration.setConnectTimeout(Integer.parseInt(oidcConfigs.getConnectTimeout())); + } catch (NumberFormatException e) { + log.warn("Invalid connect timeout configuration, defaulting to 1000ms"); + } + try { + oidcConfiguration.setReadTimeout(Integer.parseInt(oidcConfigs.getReadTimeout())); } catch (NumberFormatException e) { log.warn("Invalid read timeout configuration, defaulting to 5000ms"); } - _oidcConfigs.getResponseType().ifPresent(oidcConfiguration::setResponseType); - _oidcConfigs.getResponseMode().ifPresent(oidcConfiguration::setResponseMode); - _oidcConfigs.getUseNonce().ifPresent(oidcConfiguration::setUseNonce); + oidcConfigs.getResponseType().ifPresent(oidcConfiguration::setResponseType); + oidcConfigs.getResponseMode().ifPresent(oidcConfiguration::setResponseMode); + oidcConfigs.getUseNonce().ifPresent(oidcConfiguration::setUseNonce); Map customParamsMap = new HashMap<>(); - _oidcConfigs - .getCustomParamResource() - .ifPresent(value -> customParamsMap.put("resource", value)); - _oidcConfigs - .getGrantType() - .ifPresent(value -> customParamsMap.put("grant_type", value)); - _oidcConfigs - .getAcrValues() - .ifPresent(value -> customParamsMap.put("acr_values", value)); + oidcConfigs.getCustomParamResource().ifPresent(value -> customParamsMap.put("resource", value)); + oidcConfigs.getGrantType().ifPresent(value -> customParamsMap.put("grant_type", value)); + oidcConfigs.getAcrValues().ifPresent(value -> customParamsMap.put("acr_values", value)); if (!customParamsMap.isEmpty()) { oidcConfiguration.setCustomParams(customParamsMap); } - _oidcConfigs + oidcConfigs .getPreferredJwsAlgorithm() .ifPresent( preferred -> { log.info("Setting preferredJwsAlgorithm: " + preferred); - oidcConfiguration.setPreferredJwsAlgorithm(preferred); + oidcConfiguration.setPreferredJwsAlgorithm(JWSAlgorithm.parse(preferred)); }); + // Enable state parameter validation + oidcConfiguration.setWithState(true); + final CustomOidcClient oidcClient = new CustomOidcClient(oidcConfiguration); oidcClient.setName(OIDC_CLIENT_NAME); - oidcClient.setCallbackUrl( - _oidcConfigs.getAuthBaseUrl() + _oidcConfigs.getAuthBaseCallbackPath()); + oidcClient.setCallbackUrl(oidcConfigs.getAuthBaseUrl() + oidcConfigs.getAuthBaseCallbackPath()); oidcClient.setCallbackUrlResolver(new PathParameterCallbackUrlResolver()); oidcClient.addAuthorizationGenerator( - new OidcAuthorizationGenerator(new OidcProfileDefinition(), _oidcConfigs)); + new OidcAuthorizationGenerator(new OidcProfileDefinition(), oidcConfigs)); + return oidcClient; } } diff --git a/datahub-frontend/app/auth/sso/oidc/OidcResponseErrorHandler.java b/datahub-frontend/app/auth/sso/oidc/OidcResponseErrorHandler.java index 9881b5e095b781..2843beee61610f 100644 --- a/datahub-frontend/app/auth/sso/oidc/OidcResponseErrorHandler.java +++ b/datahub-frontend/app/auth/sso/oidc/OidcResponseErrorHandler.java @@ -4,7 +4,8 @@ import static play.mvc.Results.unauthorized; import java.util.Optional; -import org.pac4j.play.PlayWebContext; +import org.pac4j.core.context.CallContext; +import org.pac4j.core.context.WebContext; import org.slf4j.Logger; import org.slf4j.LoggerFactory; import play.mvc.Result; @@ -13,14 +14,14 @@ public class OidcResponseErrorHandler { private OidcResponseErrorHandler() {} - private static final Logger _logger = LoggerFactory.getLogger("OidcResponseErrorHandler"); + private static final Logger logger = LoggerFactory.getLogger("OidcResponseErrorHandler"); private static final String ERROR_FIELD_NAME = "error"; private static final String ERROR_DESCRIPTION_FIELD_NAME = "error_description"; - public static Result handleError(final PlayWebContext context) { - - _logger.warn( + public static Result handleError(final CallContext ctx) { + WebContext context = ctx.webContext(); + logger.warn( "OIDC responded with an error: '{}'. Error description: '{}'", getError(context), getErrorDescription(context)); @@ -44,15 +45,15 @@ public static Result handleError(final PlayWebContext context) { getError(context).orElse(""), getErrorDescription(context).orElse(""))); } - public static boolean isError(final PlayWebContext context) { - return getError(context).isPresent() && !getError(context).get().isEmpty(); + public static boolean isError(final CallContext ctx) { + return getError(ctx.webContext()).isPresent() && !getError(ctx.webContext()).get().isEmpty(); } - public static Optional getError(final PlayWebContext context) { + public static Optional getError(final WebContext context) { return context.getRequestParameter(ERROR_FIELD_NAME); } - public static Optional getErrorDescription(final PlayWebContext context) { + public static Optional getErrorDescription(final WebContext context) { return context.getRequestParameter(ERROR_DESCRIPTION_FIELD_NAME); } } diff --git a/datahub-frontend/app/auth/sso/oidc/custom/CustomOidcAuthenticator.java b/datahub-frontend/app/auth/sso/oidc/custom/CustomOidcAuthenticator.java index 01f8f16171d133..2288547cf6ed1f 100644 --- a/datahub-frontend/app/auth/sso/oidc/custom/CustomOidcAuthenticator.java +++ b/datahub-frontend/app/auth/sso/oidc/custom/CustomOidcAuthenticator.java @@ -2,7 +2,6 @@ import com.nimbusds.oauth2.sdk.AuthorizationCode; import com.nimbusds.oauth2.sdk.AuthorizationCodeGrant; -import com.nimbusds.oauth2.sdk.AuthorizationGrant; import com.nimbusds.oauth2.sdk.ParseException; import com.nimbusds.oauth2.sdk.TokenErrorResponse; import com.nimbusds.oauth2.sdk.TokenRequest; @@ -18,6 +17,7 @@ import com.nimbusds.oauth2.sdk.pkce.CodeVerifier; import com.nimbusds.openid.connect.sdk.OIDCTokenResponse; import com.nimbusds.openid.connect.sdk.OIDCTokenResponseParser; +import com.nimbusds.openid.connect.sdk.op.OIDCProviderMetadata; import com.nimbusds.openid.connect.sdk.token.OIDCTokens; import java.io.IOException; import java.net.URI; @@ -25,9 +25,11 @@ import java.util.Arrays; import java.util.Collection; import java.util.List; +import java.util.Objects; import java.util.Optional; +import org.pac4j.core.context.CallContext; import org.pac4j.core.context.WebContext; -import org.pac4j.core.credentials.authenticator.Authenticator; +import org.pac4j.core.credentials.Credentials; import org.pac4j.core.exception.TechnicalException; import org.pac4j.core.util.CommonHelper; import org.pac4j.oidc.client.OidcClient; @@ -37,9 +39,9 @@ import org.slf4j.Logger; import org.slf4j.LoggerFactory; -public class CustomOidcAuthenticator implements Authenticator { +public class CustomOidcAuthenticator extends OidcAuthenticator { - private static final Logger logger = LoggerFactory.getLogger(OidcAuthenticator.class); + private static final Logger logger = LoggerFactory.getLogger(CustomOidcAuthenticator.class); private static final Collection SUPPORTED_METHODS = Arrays.asList( @@ -47,21 +49,24 @@ public class CustomOidcAuthenticator implements Authenticator { ClientAuthenticationMethod.CLIENT_SECRET_BASIC, ClientAuthenticationMethod.NONE); - protected OidcConfiguration configuration; - - protected OidcClient client; - private final ClientAuthentication clientAuthentication; - public CustomOidcAuthenticator(final OidcClient client) { - CommonHelper.assertNotNull("configuration", client.getConfiguration()); - CommonHelper.assertNotNull("client", client); - this.configuration = client.getConfiguration(); - this.client = client; + public CustomOidcAuthenticator(final OidcClient client) { + super(client.getConfiguration(), client); // check authentication methods - final List metadataMethods = - configuration.findProviderMetadata().getTokenEndpointAuthMethods(); + OIDCProviderMetadata providerMetadata; + try { + providerMetadata = loadWithRetry(); + } catch (TechnicalException e) { + logger.error( + "Could not resolve identity provider's remote configuration from DiscoveryURI: {}", + configuration.getDiscoveryURI()); + throw e; + } + + List metadataMethods = + providerMetadata.getTokenEndpointAuthMethods(); final ClientAuthenticationMethod preferredMethod = getPreferredAuthenticationMethod(configuration); @@ -146,8 +151,11 @@ private static ClientAuthenticationMethod firstSupportedMethod( } @Override - public void validate(final OidcCredentials credentials, final WebContext context) { - final AuthorizationCode code = credentials.getCode(); + public Optional validate(CallContext ctx, Credentials cred) { + OidcCredentials credentials = (OidcCredentials) cred; + WebContext context = ctx.webContext(); + + final AuthorizationCode code = credentials.toAuthorizationCode(); // if we have a code if (code != null) { try { @@ -156,7 +164,7 @@ public void validate(final OidcCredentials credentials, final WebContext context (CodeVerifier) configuration .getValueRetriever() - .retrieve(client.getCodeVerifierSessionAttributeName(), client, context) + .retrieve(ctx, client.getCodeVerifierSessionAttributeName(), client) .orElse(null); // Token request final TokenRequest request = @@ -182,27 +190,49 @@ public void validate(final OidcCredentials credentials, final WebContext context // save tokens in credentials final OIDCTokens oidcTokens = tokenSuccessResponse.getOIDCTokens(); - credentials.setAccessToken(oidcTokens.getAccessToken()); - credentials.setRefreshToken(oidcTokens.getRefreshToken()); - credentials.setIdToken(oidcTokens.getIDToken()); + credentials.setAccessTokenObject(oidcTokens.getAccessToken()); + + // Only set refresh token if it exists + if (oidcTokens.getRefreshToken() != null) { + credentials.setRefreshTokenObject(oidcTokens.getRefreshToken()); + } + + if (oidcTokens.getIDToken() != null) { + credentials.setIdToken(oidcTokens.getIDToken().getParsedString()); + } } catch (final URISyntaxException | IOException | ParseException e) { throw new TechnicalException(e); } } + + return Optional.ofNullable(cred); } - private TokenRequest createTokenRequest(final AuthorizationGrant grant) { - if (clientAuthentication != null) { - return new TokenRequest( - configuration.findProviderMetadata().getTokenEndpointURI(), - this.clientAuthentication, - grant); - } else { - return new TokenRequest( - configuration.findProviderMetadata().getTokenEndpointURI(), - new ClientID(configuration.getClientId()), - grant); + // Simple retry with exponential backoff + public OIDCProviderMetadata loadWithRetry() { + int maxAttempts = 3; + long initialDelay = 1000; // 1 second + + for (int attempt = 1; attempt <= maxAttempts; attempt++) { + try { + OIDCProviderMetadata providerMetadata = configuration.getOpMetadataResolver().load(); + return Objects.requireNonNull(providerMetadata); + } catch (RuntimeException e) { + if (attempt == maxAttempts) { + throw e; // Rethrow on final attempt + } + try { + // Exponential backoff + Thread.sleep(initialDelay * (long) Math.pow(2, attempt - 1)); + } catch (InterruptedException ie) { + Thread.currentThread().interrupt(); + throw new RuntimeException("Retry interrupted", ie); + } + logger.warn("Retry attempt {} of {} failed", attempt, maxAttempts, e); + } } + throw new RuntimeException( + "Failed to load provider metadata after " + maxAttempts + " attempts"); } } diff --git a/datahub-frontend/app/auth/sso/oidc/custom/CustomOidcClient.java b/datahub-frontend/app/auth/sso/oidc/custom/CustomOidcClient.java index 3a0a247cb761e2..28c927c2aa541b 100644 --- a/datahub-frontend/app/auth/sso/oidc/custom/CustomOidcClient.java +++ b/datahub-frontend/app/auth/sso/oidc/custom/CustomOidcClient.java @@ -3,25 +3,38 @@ import org.pac4j.core.util.CommonHelper; import org.pac4j.oidc.client.OidcClient; import org.pac4j.oidc.config.OidcConfiguration; -import org.pac4j.oidc.credentials.extractor.OidcExtractor; +import org.pac4j.oidc.credentials.extractor.OidcCredentialsExtractor; import org.pac4j.oidc.logout.OidcLogoutActionBuilder; +import org.pac4j.oidc.logout.processor.OidcLogoutProcessor; import org.pac4j.oidc.profile.creator.OidcProfileCreator; -import org.pac4j.oidc.redirect.OidcRedirectionActionBuilder; -public class CustomOidcClient extends OidcClient { +public class CustomOidcClient extends OidcClient { - public CustomOidcClient(final OidcConfiguration configuration) { - setConfiguration(configuration); + public CustomOidcClient(OidcConfiguration configuration) { + super(configuration); } @Override - protected void clientInit() { + protected void internalInit(final boolean forceReinit) { + // Validate configuration CommonHelper.assertNotNull("configuration", getConfiguration()); - getConfiguration().init(); - defaultRedirectionActionBuilder(new CustomOidcRedirectionActionBuilder(getConfiguration(), this)); - defaultCredentialsExtractor(new OidcExtractor(getConfiguration(), this)); - defaultAuthenticator(new CustomOidcAuthenticator(this)); - defaultProfileCreator(new OidcProfileCreator<>(getConfiguration(), this)); - defaultLogoutActionBuilder(new OidcLogoutActionBuilder(getConfiguration())); + + // Initialize configuration + getConfiguration().init(forceReinit); + + // Initialize client components + setRedirectionActionBuilderIfUndefined( + new CustomOidcRedirectionActionBuilder(getConfiguration(), this)); + setCredentialsExtractorIfUndefined(new OidcCredentialsExtractor(getConfiguration(), this)); + + // Initialize default authenticator if not set + if (getAuthenticator() == null || forceReinit) { + setAuthenticatorIfUndefined(new CustomOidcAuthenticator(this)); + } + + setProfileCreatorIfUndefined(new OidcProfileCreator(getConfiguration(), this)); + setLogoutProcessorIfUndefined( + new OidcLogoutProcessor(getConfiguration(), findSessionLogoutHandler())); + setLogoutActionBuilderIfUndefined(new OidcLogoutActionBuilder(getConfiguration())); } } diff --git a/datahub-frontend/app/auth/sso/oidc/custom/CustomOidcRedirectionActionBuilder.java b/datahub-frontend/app/auth/sso/oidc/custom/CustomOidcRedirectionActionBuilder.java index bdeeacc895af35..ea5315972344dc 100644 --- a/datahub-frontend/app/auth/sso/oidc/custom/CustomOidcRedirectionActionBuilder.java +++ b/datahub-frontend/app/auth/sso/oidc/custom/CustomOidcRedirectionActionBuilder.java @@ -2,29 +2,35 @@ import java.util.Map; import java.util.Optional; +import org.pac4j.core.context.CallContext; import org.pac4j.core.context.WebContext; import org.pac4j.core.exception.http.RedirectionAction; -import org.pac4j.core.exception.http.RedirectionActionHelper; +import org.pac4j.core.util.HttpActionHelper; import org.pac4j.oidc.client.OidcClient; import org.pac4j.oidc.config.OidcConfiguration; import org.pac4j.oidc.redirect.OidcRedirectionActionBuilder; import org.slf4j.Logger; import org.slf4j.LoggerFactory; - public class CustomOidcRedirectionActionBuilder extends OidcRedirectionActionBuilder { private static final Logger logger = LoggerFactory.getLogger(OidcRedirectionActionBuilder.class); + + private final OidcConfiguration configuration; + public CustomOidcRedirectionActionBuilder(OidcConfiguration configuration, OidcClient client) { - super(configuration, client); + super(client); + this.configuration = configuration; } @Override - public Optional getRedirectionAction(WebContext context) { - Map params = this.buildParams(); + public Optional getRedirectionAction(CallContext ctx) { + WebContext context = ctx.webContext(); + + Map params = this.buildParams(context); String computedCallbackUrl = this.client.computeFinalCallbackUrl(context); params.put("redirect_uri", computedCallbackUrl); - this.addStateAndNonceParameters(context, params); + this.addStateAndNonceParameters(ctx, params); if (this.configuration.getMaxAge() != null) { params.put("max_age", this.configuration.getMaxAge().toString()); } @@ -40,7 +46,6 @@ public Optional getRedirectionAction(WebContext context) { } logger.debug("Authentication request url: {}", location); - return Optional.of(RedirectionActionHelper.buildRedirectUrlAction(context, location)); + return Optional.of(HttpActionHelper.buildRedirectUrlAction(context, location)); } - } diff --git a/datahub-frontend/app/client/KafkaTrackingProducer.java b/datahub-frontend/app/client/KafkaTrackingProducer.java index 058e75100c24ac..a29fe3bb7aef74 100644 --- a/datahub-frontend/app/client/KafkaTrackingProducer.java +++ b/datahub-frontend/app/client/KafkaTrackingProducer.java @@ -27,7 +27,8 @@ @Singleton public class KafkaTrackingProducer { - private final Logger _logger = LoggerFactory.getLogger(KafkaTrackingProducer.class.getName()); + private static final Logger logger = + LoggerFactory.getLogger(KafkaTrackingProducer.class.getName()); private static final List KAFKA_SSL_PROTOCOLS = Collections.unmodifiableList( Arrays.asList( @@ -35,38 +36,38 @@ public class KafkaTrackingProducer { SecurityProtocol.SASL_SSL.name(), SecurityProtocol.SASL_PLAINTEXT.name())); - private final Boolean _isEnabled; - private final KafkaProducer _producer; + private final Boolean isEnabled; + private final KafkaProducer producer; @Inject public KafkaTrackingProducer( @Nonnull Config config, ApplicationLifecycle lifecycle, final ConfigurationProvider configurationProvider) { - _isEnabled = !config.hasPath("analytics.enabled") || config.getBoolean("analytics.enabled"); + isEnabled = !config.hasPath("analytics.enabled") || config.getBoolean("analytics.enabled"); - if (_isEnabled) { - _logger.debug("Analytics tracking is enabled"); - _producer = createKafkaProducer(config, configurationProvider.getKafka()); + if (isEnabled) { + logger.debug("Analytics tracking is enabled"); + producer = createKafkaProducer(config, configurationProvider.getKafka()); lifecycle.addStopHook( () -> { - _producer.flush(); - _producer.close(); + producer.flush(); + producer.close(); return CompletableFuture.completedFuture(null); }); } else { - _logger.debug("Analytics tracking is disabled"); - _producer = null; + logger.debug("Analytics tracking is disabled"); + producer = null; } } public Boolean isEnabled() { - return _isEnabled; + return isEnabled; } public void send(ProducerRecord record) { - _producer.send(record); + producer.send(record); } private static KafkaProducer createKafkaProducer( diff --git a/datahub-frontend/app/config/ConfigurationProvider.java b/datahub-frontend/app/config/ConfigurationProvider.java index d447b28cdcc465..97e916769a6c45 100644 --- a/datahub-frontend/app/config/ConfigurationProvider.java +++ b/datahub-frontend/app/config/ConfigurationProvider.java @@ -6,12 +6,9 @@ import com.linkedin.metadata.config.kafka.KafkaConfiguration; import com.linkedin.metadata.spring.YamlPropertySourceFactory; import lombok.Data; -import org.springframework.boot.autoconfigure.kafka.KafkaProperties; import org.springframework.boot.context.properties.ConfigurationProperties; import org.springframework.boot.context.properties.EnableConfigurationProperties; -import org.springframework.context.annotation.Configuration; import org.springframework.context.annotation.PropertySource; -import org.springframework.stereotype.Component; /** * Minimal sharing between metadata-service and frontend Does not use the factories module to avoid diff --git a/datahub-frontend/app/controllers/Application.java b/datahub-frontend/app/controllers/Application.java index 017847367de053..1c3786f4af526c 100644 --- a/datahub-frontend/app/controllers/Application.java +++ b/datahub-frontend/app/controllers/Application.java @@ -36,8 +36,8 @@ import play.libs.ws.StandaloneWSClient; import play.libs.ws.ahc.StandaloneAhcWSClient; import play.mvc.Controller; -import play.mvc.Http.Cookie; import play.mvc.Http; +import play.mvc.Http.Cookie; import play.mvc.ResponseHeader; import play.mvc.Result; import play.mvc.Security; @@ -48,16 +48,16 @@ import utils.ConfigUtil; public class Application extends Controller { - private final Logger _logger = LoggerFactory.getLogger(Application.class.getName()); - private final Config _config; - private final StandaloneWSClient _ws; - private final Environment _environment; + private static final Logger logger = LoggerFactory.getLogger(Application.class.getName()); + private final Config config; + private final StandaloneWSClient ws; + private final Environment environment; @Inject public Application(Environment environment, @Nonnull Config config) { - _config = config; - _ws = createWsClient(); - _environment = environment; + this.config = config; + ws = createWsClient(); + this.environment = environment; } /** @@ -69,10 +69,10 @@ public Application(Environment environment, @Nonnull Config config) { @Nonnull private Result serveAsset(@Nullable String path) { try { - InputStream indexHtml = _environment.resourceAsStream("public/index.html"); + InputStream indexHtml = environment.resourceAsStream("public/index.html"); return ok(indexHtml).withHeader("Cache-Control", "no-cache").as("text/html"); } catch (Exception e) { - _logger.warn("Cannot load public/index.html resource. Static assets or assets jar missing?"); + logger.warn("Cannot load public/index.html resource. Static assets or assets jar missing?"); return notFound().withHeader("Cache-Control", "no-cache").as("text/html"); } } @@ -106,17 +106,17 @@ public CompletableFuture proxy(String path, Http.Request request) final String metadataServiceHost = ConfigUtil.getString( - _config, + config, ConfigUtil.METADATA_SERVICE_HOST_CONFIG_PATH, ConfigUtil.DEFAULT_METADATA_SERVICE_HOST); final int metadataServicePort = ConfigUtil.getInt( - _config, + config, ConfigUtil.METADATA_SERVICE_PORT_CONFIG_PATH, ConfigUtil.DEFAULT_METADATA_SERVICE_PORT); final boolean metadataServiceUseSsl = ConfigUtil.getBoolean( - _config, + config, ConfigUtil.METADATA_SERVICE_USE_SSL_CONFIG_PATH, ConfigUtil.DEFAULT_METADATA_SERVICE_USE_SSL); @@ -139,7 +139,7 @@ public CompletableFuture proxy(String path, Http.Request request) // Get the current time to measure the duration of the request Instant start = Instant.now(); - return _ws.url( + return ws.url( String.format( "%s://%s:%s%s", protocol, metadataServiceHost, metadataServicePort, resolvedUri)) .setMethod(request.method()) @@ -167,9 +167,10 @@ AuthenticationConstants.LEGACY_X_DATAHUB_ACTOR_HEADER, getDataHubActorHeader(req .execute() .thenApply( apiResponse -> { - // Log the query if it takes longer than the configured threshold and verbose logging is enabled - boolean verboseGraphQLLogging = _config.getBoolean("graphql.verbose.logging"); - int verboseGraphQLLongQueryMillis = _config.getInt("graphql.verbose.slowQueryMillis"); + // Log the query if it takes longer than the configured threshold and verbose logging + // is enabled + boolean verboseGraphQLLogging = config.getBoolean("graphql.verbose.logging"); + int verboseGraphQLLongQueryMillis = config.getInt("graphql.verbose.slowQueryMillis"); Instant finish = Instant.now(); long timeElapsed = Duration.between(start, finish).toMillis(); if (verboseGraphQLLogging && timeElapsed >= verboseGraphQLLongQueryMillis) { @@ -206,32 +207,32 @@ AuthenticationConstants.LEGACY_X_DATAHUB_ACTOR_HEADER, getDataHubActorHeader(req public Result appConfig() { final ObjectNode config = Json.newObject(); config.put("application", "datahub-frontend"); - config.put("appVersion", _config.getString("app.version")); - config.put("isInternal", _config.getBoolean("linkedin.internal")); - config.put("shouldShowDatasetLineage", _config.getBoolean("linkedin.show.dataset.lineage")); + config.put("appVersion", this.config.getString("app.version")); + config.put("isInternal", this.config.getBoolean("linkedin.internal")); + config.put("shouldShowDatasetLineage", this.config.getBoolean("linkedin.show.dataset.lineage")); config.put( "suggestionConfidenceThreshold", - Integer.valueOf(_config.getString("linkedin.suggestion.confidence.threshold"))); + Integer.valueOf(this.config.getString("linkedin.suggestion.confidence.threshold"))); config.set("wikiLinks", wikiLinks()); config.set("tracking", trackingInfo()); // In a staging environment, we can trigger this flag to be true so that the UI can handle based // on // such config and alert users that their changes will not affect production data - config.put("isStagingBanner", _config.getBoolean("ui.show.staging.banner")); - config.put("isLiveDataWarning", _config.getBoolean("ui.show.live.data.banner")); - config.put("showChangeManagement", _config.getBoolean("ui.show.CM.banner")); + config.put("isStagingBanner", this.config.getBoolean("ui.show.staging.banner")); + config.put("isLiveDataWarning", this.config.getBoolean("ui.show.live.data.banner")); + config.put("showChangeManagement", this.config.getBoolean("ui.show.CM.banner")); // Flag to enable people entity elements - config.put("showPeople", _config.getBoolean("ui.show.people")); - config.put("changeManagementLink", _config.getString("ui.show.CM.link")); + config.put("showPeople", this.config.getBoolean("ui.show.people")); + config.put("changeManagementLink", this.config.getString("ui.show.CM.link")); // Flag set in order to warn users that search is experiencing issues - config.put("isStaleSearch", _config.getBoolean("ui.show.stale.search")); - config.put("showAdvancedSearch", _config.getBoolean("ui.show.advanced.search")); + config.put("isStaleSearch", this.config.getBoolean("ui.show.stale.search")); + config.put("showAdvancedSearch", this.config.getBoolean("ui.show.advanced.search")); // Flag to use the new api for browsing datasets - config.put("useNewBrowseDataset", _config.getBoolean("ui.new.browse.dataset")); + config.put("useNewBrowseDataset", this.config.getBoolean("ui.new.browse.dataset")); // show lineage graph in relationships tabs - config.put("showLineageGraph", _config.getBoolean("ui.show.lineage.graph")); + config.put("showLineageGraph", this.config.getBoolean("ui.show.lineage.graph")); // show institutional memory for available entities - config.put("showInstitutionalMemory", _config.getBoolean("ui.show.institutional.memory")); + config.put("showInstitutionalMemory", this.config.getBoolean("ui.show.institutional.memory")); // Insert properties for user profile operations config.set("userEntityProps", userEntityProps()); @@ -250,8 +251,8 @@ public Result appConfig() { @Nonnull private ObjectNode userEntityProps() { final ObjectNode props = Json.newObject(); - props.put("aviUrlPrimary", _config.getString("linkedin.links.avi.urlPrimary")); - props.put("aviUrlFallback", _config.getString("linkedin.links.avi.urlFallback")); + props.put("aviUrlPrimary", config.getString("linkedin.links.avi.urlPrimary")); + props.put("aviUrlFallback", config.getString("linkedin.links.avi.urlFallback")); return props; } @@ -261,19 +262,19 @@ private ObjectNode userEntityProps() { @Nonnull private ObjectNode wikiLinks() { final ObjectNode wikiLinks = Json.newObject(); - wikiLinks.put("appHelp", _config.getString("links.wiki.appHelp")); - wikiLinks.put("gdprPii", _config.getString("links.wiki.gdprPii")); - wikiLinks.put("tmsSchema", _config.getString("links.wiki.tmsSchema")); - wikiLinks.put("gdprTaxonomy", _config.getString("links.wiki.gdprTaxonomy")); - wikiLinks.put("staleSearchIndex", _config.getString("links.wiki.staleSearchIndex")); - wikiLinks.put("dht", _config.getString("links.wiki.dht")); - wikiLinks.put("purgePolicies", _config.getString("links.wiki.purgePolicies")); - wikiLinks.put("jitAcl", _config.getString("links.wiki.jitAcl")); - wikiLinks.put("metadataCustomRegex", _config.getString("links.wiki.metadataCustomRegex")); - wikiLinks.put("exportPolicy", _config.getString("links.wiki.exportPolicy")); - wikiLinks.put("metadataHealth", _config.getString("links.wiki.metadataHealth")); - wikiLinks.put("purgeKey", _config.getString("links.wiki.purgeKey")); - wikiLinks.put("datasetDecommission", _config.getString("links.wiki.datasetDecommission")); + wikiLinks.put("appHelp", config.getString("links.wiki.appHelp")); + wikiLinks.put("gdprPii", config.getString("links.wiki.gdprPii")); + wikiLinks.put("tmsSchema", config.getString("links.wiki.tmsSchema")); + wikiLinks.put("gdprTaxonomy", config.getString("links.wiki.gdprTaxonomy")); + wikiLinks.put("staleSearchIndex", config.getString("links.wiki.staleSearchIndex")); + wikiLinks.put("dht", config.getString("links.wiki.dht")); + wikiLinks.put("purgePolicies", config.getString("links.wiki.purgePolicies")); + wikiLinks.put("jitAcl", config.getString("links.wiki.jitAcl")); + wikiLinks.put("metadataCustomRegex", config.getString("links.wiki.metadataCustomRegex")); + wikiLinks.put("exportPolicy", config.getString("links.wiki.exportPolicy")); + wikiLinks.put("metadataHealth", config.getString("links.wiki.metadataHealth")); + wikiLinks.put("purgeKey", config.getString("links.wiki.purgeKey")); + wikiLinks.put("datasetDecommission", config.getString("links.wiki.datasetDecommission")); return wikiLinks; } @@ -283,8 +284,8 @@ private ObjectNode wikiLinks() { @Nonnull private ObjectNode trackingInfo() { final ObjectNode piwik = Json.newObject(); - piwik.put("piwikSiteId", Integer.valueOf(_config.getString("tracking.piwik.siteid"))); - piwik.put("piwikUrl", _config.getString("tracking.piwik.url")); + piwik.put("piwikSiteId", Integer.valueOf(config.getString("tracking.piwik.siteid"))); + piwik.put("piwikUrl", config.getString("tracking.piwik.url")); final ObjectNode trackers = Json.newObject(); trackers.set("piwik", piwik); @@ -376,9 +377,10 @@ private String mapPath(@Nonnull final String path) { return path; } - /** - * Called if verbose logging is enabled and request takes longer that the slow query milliseconds defined in the config + * Called if verbose logging is enabled and request takes longer that the slow query milliseconds + * defined in the config + * * @param request GraphQL request that was made * @param resolvedUri URI that was requested * @param duration How long the query took to complete @@ -393,16 +395,16 @@ private void logSlowQuery(Http.Request request, String resolvedUri, float durati JsonNode jsonNode = request.body().asJson(); ((ObjectNode) jsonNode).remove("query"); jsonBody.append(mapper.writerWithDefaultPrettyPrinter().writeValueAsString(jsonNode)); - } - catch (Exception e) { - _logger.info("GraphQL Request Received: {}, Unable to parse JSON body", resolvedUri); + } catch (Exception e) { + logger.info("GraphQL Request Received: {}, Unable to parse JSON body", resolvedUri); } String jsonBodyStr = jsonBody.toString(); - _logger.info("Slow GraphQL Request Received: {}, Request query string: {}, Request actor: {}, Request JSON: {}, Request completed in {} ms", - resolvedUri, - request.queryString(), - actorValue, - jsonBodyStr, - duration); + logger.info( + "Slow GraphQL Request Received: {}, Request query string: {}, Request actor: {}, Request JSON: {}, Request completed in {} ms", + resolvedUri, + request.queryString(), + actorValue, + jsonBodyStr, + duration); } } diff --git a/datahub-frontend/app/controllers/AuthenticationController.java b/datahub-frontend/app/controllers/AuthenticationController.java index 87c4b5ba06793b..3c8ace9aee29fc 100644 --- a/datahub-frontend/app/controllers/AuthenticationController.java +++ b/datahub-frontend/app/controllers/AuthenticationController.java @@ -26,12 +26,14 @@ import org.apache.commons.httpclient.InvalidRedirectLocationException; import org.apache.commons.lang3.StringUtils; import org.pac4j.core.client.Client; +import org.pac4j.core.context.CallContext; import org.pac4j.core.context.Cookie; +import org.pac4j.core.context.WebContext; import org.pac4j.core.exception.http.FoundAction; import org.pac4j.core.exception.http.RedirectionAction; import org.pac4j.play.PlayWebContext; import org.pac4j.play.http.PlayHttpActionAdapter; -import org.pac4j.play.store.PlaySessionStore; +import org.pac4j.play.store.PlayCookieSessionStore; import org.slf4j.Logger; import org.slf4j.LoggerFactory; import play.data.validation.Constraints; @@ -51,26 +53,27 @@ public class AuthenticationController extends Controller { private static final String SSO_NO_REDIRECT_MESSAGE = "SSO is configured, however missing redirect from idp"; - private final Logger _logger = LoggerFactory.getLogger(AuthenticationController.class.getName()); - private final CookieConfigs _cookieConfigs; - private final JAASConfigs _jaasConfigs; - private final NativeAuthenticationConfigs _nativeAuthenticationConfigs; - private final boolean _verbose; + private static final Logger logger = + LoggerFactory.getLogger(AuthenticationController.class.getName()); + private final CookieConfigs cookieConfigs; + private final JAASConfigs jaasConfigs; + private final NativeAuthenticationConfigs nativeAuthenticationConfigs; + private final boolean verbose; - @Inject private org.pac4j.core.config.Config _ssoConfig; + @Inject private org.pac4j.core.config.Config ssoConfig; - @Inject private PlaySessionStore _playSessionStore; + @Inject private PlayCookieSessionStore playCookieSessionStore; - @Inject private SsoManager _ssoManager; + @Inject private SsoManager ssoManager; - @Inject AuthServiceClient _authClient; + @Inject AuthServiceClient authClient; @Inject public AuthenticationController(@Nonnull Config configs) { - _cookieConfigs = new CookieConfigs(configs); - _jaasConfigs = new JAASConfigs(configs); - _nativeAuthenticationConfigs = new NativeAuthenticationConfigs(configs); - _verbose = configs.hasPath(AUTH_VERBOSE_LOGGING) && configs.getBoolean(AUTH_VERBOSE_LOGGING); + cookieConfigs = new CookieConfigs(configs); + jaasConfigs = new JAASConfigs(configs); + nativeAuthenticationConfigs = new NativeAuthenticationConfigs(configs); + verbose = configs.hasPath(AUTH_VERBOSE_LOGGING) && configs.getBoolean(AUTH_VERBOSE_LOGGING); } /** @@ -92,11 +95,14 @@ public Result authenticate(Http.Request request) { try { URI redirectUri = new URI(redirectPath); if (redirectUri.getScheme() != null || redirectUri.getAuthority() != null) { - throw new InvalidRedirectLocationException("Redirect location must be relative to the base url, cannot " - + "redirect to other domains: " + redirectPath, redirectPath); + throw new InvalidRedirectLocationException( + "Redirect location must be relative to the base url, cannot " + + "redirect to other domains: " + + redirectPath, + redirectPath); } } catch (URISyntaxException | InvalidRedirectLocationException e) { - _logger.warn(e.getMessage()); + logger.warn(e.getMessage()); redirectPath = "/"; } @@ -105,7 +111,7 @@ public Result authenticate(Http.Request request) { } // 1. If SSO is enabled, redirect to IdP if not authenticated. - if (_ssoManager.isSsoEnabled()) { + if (ssoManager.isSsoEnabled()) { return redirectToIdentityProvider(request, redirectPath) .orElse( Results.redirect( @@ -114,8 +120,8 @@ public Result authenticate(Http.Request request) { } // 2. If either JAAS auth or Native auth is enabled, fallback to it - if (_jaasConfigs.isJAASEnabled() - || _nativeAuthenticationConfigs.isNativeAuthenticationEnabled()) { + if (jaasConfigs.isJAASEnabled() + || nativeAuthenticationConfigs.isNativeAuthenticationEnabled()) { return Results.redirect( LOGIN_ROUTE + String.format("?%s=%s", AUTH_REDIRECT_URI_PARAM, encodeRedirectUri(redirectPath))); @@ -123,21 +129,21 @@ 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()); + final String accessToken = authClient.generateSessionTokenForUser(DEFAULT_ACTOR_URN.getId()); return Results.redirect(redirectPath) .withSession(createSessionMap(DEFAULT_ACTOR_URN.toString(), accessToken)) .withCookies( createActorCookie( DEFAULT_ACTOR_URN.toString(), - _cookieConfigs.getTtlInHours(), - _cookieConfigs.getAuthCookieSameSite(), - _cookieConfigs.getAuthCookieSecure())); + cookieConfigs.getTtlInHours(), + cookieConfigs.getAuthCookieSameSite(), + cookieConfigs.getAuthCookieSecure())); } /** Redirect to the identity provider for authentication. */ @Nonnull public Result sso(Http.Request request) { - if (_ssoManager.isSsoEnabled()) { + if (ssoManager.isSsoEnabled()) { return redirectToIdentityProvider(request, "/") .orElse( Results.redirect( @@ -156,11 +162,11 @@ public Result sso(Http.Request request) { */ @Nonnull public Result logIn(Http.Request request) { - boolean jaasEnabled = _jaasConfigs.isJAASEnabled(); - _logger.debug(String.format("Jaas authentication enabled: %b", jaasEnabled)); + boolean jaasEnabled = jaasConfigs.isJAASEnabled(); + logger.debug(String.format("Jaas authentication enabled: %b", jaasEnabled)); boolean nativeAuthenticationEnabled = - _nativeAuthenticationConfigs.isNativeAuthenticationEnabled(); - _logger.debug(String.format("Native authentication enabled: %b", nativeAuthenticationEnabled)); + nativeAuthenticationConfigs.isNativeAuthenticationEnabled(); + logger.debug(String.format("Native authentication enabled: %b", nativeAuthenticationEnabled)); boolean noAuthEnabled = !jaasEnabled && !nativeAuthenticationEnabled; if (noAuthEnabled) { String message = "Neither JAAS nor native authentication is enabled on the server."; @@ -182,13 +188,13 @@ public Result logIn(Http.Request request) { boolean loginSucceeded = tryLogin(username, password); if (!loginSucceeded) { - _logger.info("Login failed for user: {}", username); + logger.info("Login failed for user: {}", username); return Results.badRequest(invalidCredsJson); } final Urn actorUrn = new CorpuserUrn(username); - _logger.info("Login successful for user: {}, urn: {}", username, actorUrn); - final String accessToken = _authClient.generateSessionTokenForUser(actorUrn.getId()); + logger.info("Login successful for user: {}, urn: {}", username, actorUrn); + final String accessToken = authClient.generateSessionTokenForUser(actorUrn.getId()); return createSession(actorUrn.toString(), accessToken); } @@ -199,8 +205,8 @@ public Result logIn(Http.Request request) { @Nonnull public Result signUp(Http.Request request) { boolean nativeAuthenticationEnabled = - _nativeAuthenticationConfigs.isNativeAuthenticationEnabled(); - _logger.debug(String.format("Native authentication enabled: %b", nativeAuthenticationEnabled)); + nativeAuthenticationConfigs.isNativeAuthenticationEnabled(); + logger.debug(String.format("Native authentication enabled: %b", nativeAuthenticationEnabled)); if (!nativeAuthenticationEnabled) { String message = "Native authentication is not enabled on the server."; final ObjectNode error = Json.newObject(); @@ -224,7 +230,7 @@ public Result signUp(Http.Request request) { JsonNode invalidCredsJson = Json.newObject().put("message", "Email must not be empty."); return Results.badRequest(invalidCredsJson); } - if (_nativeAuthenticationConfigs.isEnforceValidEmailEnabled()) { + if (nativeAuthenticationConfigs.isEnforceValidEmailEnabled()) { Constraints.EmailValidator emailValidator = new Constraints.EmailValidator(); if (!emailValidator.isValid(email)) { JsonNode invalidCredsJson = Json.newObject().put("message", "Email must not be empty."); @@ -250,9 +256,9 @@ public Result signUp(Http.Request request) { final Urn userUrn = new CorpuserUrn(email); final String userUrnString = userUrn.toString(); - _authClient.signUp(userUrnString, fullName, email, title, password, inviteToken); - _logger.info("Signed up user {} using invite tokens", userUrnString); - final String accessToken = _authClient.generateSessionTokenForUser(userUrn.getId()); + authClient.signUp(userUrnString, fullName, email, title, password, inviteToken); + logger.info("Signed up user {} using invite tokens", userUrnString); + final String accessToken = authClient.generateSessionTokenForUser(userUrn.getId()); return createSession(userUrnString, accessToken); } @@ -260,8 +266,8 @@ public Result signUp(Http.Request request) { @Nonnull public Result resetNativeUserCredentials(Http.Request request) { boolean nativeAuthenticationEnabled = - _nativeAuthenticationConfigs.isNativeAuthenticationEnabled(); - _logger.debug(String.format("Native authentication enabled: %b", nativeAuthenticationEnabled)); + nativeAuthenticationConfigs.isNativeAuthenticationEnabled(); + logger.debug(String.format("Native authentication enabled: %b", nativeAuthenticationEnabled)); if (!nativeAuthenticationEnabled) { String message = "Native authentication is not enabled on the server."; final ObjectNode error = Json.newObject(); @@ -291,26 +297,27 @@ public Result resetNativeUserCredentials(Http.Request request) { final Urn userUrn = new CorpuserUrn(email); final String userUrnString = userUrn.toString(); - _authClient.resetNativeUserCredentials(userUrnString, password, resetToken); - final String accessToken = _authClient.generateSessionTokenForUser(userUrn.getId()); + authClient.resetNativeUserCredentials(userUrnString, password, resetToken); + final String accessToken = authClient.generateSessionTokenForUser(userUrn.getId()); return createSession(userUrnString, accessToken); } private Optional redirectToIdentityProvider( Http.RequestHeader request, String redirectPath) { - final PlayWebContext playWebContext = new PlayWebContext(request, _playSessionStore); - final Client client = _ssoManager.getSsoProvider().client(); - configurePac4jSessionStore(playWebContext, client, redirectPath); + CallContext ctx = buildCallContext(request); + + final Client client = ssoManager.getSsoProvider().client(); + configurePac4jSessionStore(ctx, client, redirectPath); try { - final Optional action = client.getRedirectionAction(playWebContext); - return action.map(act -> new PlayHttpActionAdapter().adapt(act, playWebContext)); + final Optional action = client.getRedirectionAction(ctx); + return action.map(act -> new PlayHttpActionAdapter().adapt(act, ctx.webContext())); } catch (Exception e) { - if (_verbose) { - _logger.error( + if (verbose) { + logger.error( "Caught exception while attempting to redirect to SSO identity provider! It's likely that SSO integration is mis-configured", e); } else { - _logger.error( + logger.error( "Caught exception while attempting to redirect to SSO identity provider! It's likely that SSO integration is mis-configured"); } return Optional.of( @@ -324,22 +331,33 @@ private Optional redirectToIdentityProvider( } } - private void configurePac4jSessionStore( - PlayWebContext context, Client client, String redirectPath) { + private CallContext buildCallContext(Http.RequestHeader request) { + // First create PlayWebContext from the request + PlayWebContext webContext = new PlayWebContext(request); + + // Then create CallContext using the web context and session store + return new CallContext(webContext, playCookieSessionStore); + } + + private void configurePac4jSessionStore(CallContext ctx, Client client, String redirectPath) { + WebContext context = ctx.webContext(); + // 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); + byte[] javaSerBytes = + ((PlayCookieSessionStore) ctx.sessionStore()).getSerializer().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) { - _logger.debug( + Optional attempt = + playCookieSessionStore.get(context, client.getName() + ATTEMPTED_AUTHENTICATION_SUFFIX); + if (attempt.isPresent() && !"".equals(attempt.get())) { + logger.debug( "Found previous login attempt. Removing it manually to prevent unexpected errors."); - _playSessionStore.set(context, client.getName() + ATTEMPTED_AUTHENTICATION_SUFFIX, ""); + playCookieSessionStore.set(context, client.getName() + ATTEMPTED_AUTHENTICATION_SUFFIX, ""); } } @@ -351,27 +369,27 @@ private boolean tryLogin(String username, String password) { boolean loginSucceeded = false; // First try jaas login, if enabled - if (_jaasConfigs.isJAASEnabled()) { + if (jaasConfigs.isJAASEnabled()) { try { - _logger.debug("Attempting JAAS authentication for user: {}", username); + logger.debug("Attempting JAAS authentication for user: {}", username); AuthenticationManager.authenticateJaasUser(username, password); - _logger.debug("JAAS authentication successful. Login succeeded"); + logger.debug("JAAS authentication successful. Login succeeded"); loginSucceeded = true; } catch (Exception e) { - if (_verbose) { - _logger.debug("JAAS authentication error. Login failed", e); + if (verbose) { + logger.debug("JAAS authentication error. Login failed", e); } else { - _logger.debug("JAAS authentication error. Login failed"); + logger.debug("JAAS authentication error. Login failed"); } } } // If jaas login fails or is disabled, try native auth login - if (_nativeAuthenticationConfigs.isNativeAuthenticationEnabled() && !loginSucceeded) { + if (nativeAuthenticationConfigs.isNativeAuthenticationEnabled() && !loginSucceeded) { final Urn userUrn = new CorpuserUrn(username); final String userUrnString = userUrn.toString(); loginSucceeded = - loginSucceeded || _authClient.verifyNativeUserCredentials(userUrnString, password); + loginSucceeded || authClient.verifyNativeUserCredentials(userUrnString, password); } return loginSucceeded; @@ -383,8 +401,8 @@ private Result createSession(String userUrnString, String accessToken) { .withCookies( createActorCookie( userUrnString, - _cookieConfigs.getTtlInHours(), - _cookieConfigs.getAuthCookieSameSite(), - _cookieConfigs.getAuthCookieSecure())); + cookieConfigs.getTtlInHours(), + cookieConfigs.getAuthCookieSameSite(), + cookieConfigs.getAuthCookieSecure())); } } diff --git a/datahub-frontend/app/controllers/CentralLogoutController.java b/datahub-frontend/app/controllers/CentralLogoutController.java index eea1c662ebf894..d2e3027236adcd 100644 --- a/datahub-frontend/app/controllers/CentralLogoutController.java +++ b/datahub-frontend/app/controllers/CentralLogoutController.java @@ -1,6 +1,6 @@ package controllers; -import com.typesafe.config.Config; +import auth.sso.SsoManager; import java.net.URLEncoder; import java.nio.charset.StandardCharsets; import javax.inject.Inject; @@ -15,13 +15,10 @@ public class CentralLogoutController extends LogoutController { private static final String AUTH_URL_CONFIG_PATH = "/login"; private static final String DEFAULT_BASE_URL_PATH = "/"; - private static Boolean _isOidcEnabled = false; + @Inject private SsoManager ssoManager; - @Inject - public CentralLogoutController(Config config) { - _isOidcEnabled = config.hasPath("auth.oidc.enabled") && config.getBoolean("auth.oidc.enabled"); - - setDefaultUrl(DEFAULT_BASE_URL_PATH); + public CentralLogoutController() { + setDefaultUrl(AUTH_URL_CONFIG_PATH); setLogoutUrlPattern(DEFAULT_BASE_URL_PATH + ".*"); setLocalLogout(true); setCentralLogout(true); @@ -29,7 +26,7 @@ public CentralLogoutController(Config config) { /** logout() method should not be called if oidc is not enabled */ public Result executeLogout(Http.Request request) { - if (_isOidcEnabled) { + if (ssoManager.isSsoEnabled()) { try { return logout(request).toCompletableFuture().get().withNewSession(); } catch (Exception e) { diff --git a/datahub-frontend/app/controllers/RedirectController.java b/datahub-frontend/app/controllers/RedirectController.java index 17f86b7fbffae3..a86584e24ca29f 100644 --- a/datahub-frontend/app/controllers/RedirectController.java +++ b/datahub-frontend/app/controllers/RedirectController.java @@ -16,7 +16,10 @@ public Result favicon(Http.Request request) { if (config.getVisualConfig().getAssets().getFaviconUrl().startsWith("http")) { return permanentRedirect(config.getVisualConfig().getAssets().getFaviconUrl()); } else { - final String prefix = config.getVisualConfig().getAssets().getFaviconUrl().startsWith("/") ? "/public" : "/public/"; + final String prefix = + config.getVisualConfig().getAssets().getFaviconUrl().startsWith("/") + ? "/public" + : "/public/"; return ok(Application.class.getResourceAsStream( prefix + config.getVisualConfig().getAssets().getFaviconUrl())) .as("image/x-icon"); diff --git a/datahub-frontend/app/controllers/SsoCallbackController.java b/datahub-frontend/app/controllers/SsoCallbackController.java index 750886570bf406..385b02a56ba233 100644 --- a/datahub-frontend/app/controllers/SsoCallbackController.java +++ b/datahub-frontend/app/controllers/SsoCallbackController.java @@ -5,8 +5,8 @@ import auth.sso.SsoProvider; import auth.sso.oidc.OidcCallbackLogic; import client.AuthServiceClient; -import com.datahub.authentication.Authentication; import com.linkedin.entity.client.SystemEntityClient; +import io.datahubproject.metadata.context.OperationContext; import java.net.URLEncoder; import java.nio.charset.StandardCharsets; import java.util.ArrayList; @@ -16,16 +16,15 @@ import javax.annotation.Nonnull; import javax.inject.Inject; import javax.inject.Named; - -import io.datahubproject.metadata.context.OperationContext; import lombok.extern.slf4j.Slf4j; +import org.pac4j.core.adapter.FrameworkAdapter; import org.pac4j.core.client.Client; import org.pac4j.core.client.Clients; import org.pac4j.core.config.Config; +import org.pac4j.core.context.FrameworkParameters; import org.pac4j.core.engine.CallbackLogic; -import org.pac4j.core.http.adapter.HttpActionAdapter; import org.pac4j.play.CallbackController; -import org.pac4j.play.PlayWebContext; +import org.pac4j.play.context.PlayFrameworkParameters; import play.mvc.Http; import play.mvc.Result; import play.mvc.Results; @@ -40,8 +39,9 @@ @Slf4j public class SsoCallbackController extends CallbackController { - private final SsoManager _ssoManager; - private final Config _config; + private final SsoManager ssoManager; + private final Config config; + private final CallbackLogic callbackLogic; @Inject public SsoCallbackController( @@ -51,23 +51,41 @@ public SsoCallbackController( @Nonnull AuthServiceClient authClient, @Nonnull Config config, @Nonnull com.typesafe.config.Config configs) { - _ssoManager = ssoManager; - _config = config; + this.ssoManager = ssoManager; + this.config = config; setDefaultUrl("/"); // By default, redirects to Home Page on log in. - setSaveInSession(false); - setCallbackLogic( + + callbackLogic = new SsoCallbackLogic( ssoManager, - systemOperationContext, + systemOperationContext, entityClient, authClient, - new CookieConfigs(configs))); + new CookieConfigs(configs)); + } + + @Override + public CompletionStage callback(Http.Request request) { + FrameworkAdapter.INSTANCE.applyDefaultSettingsIfUndefined(this.config); + + return CompletableFuture.supplyAsync( + () -> { + return (Result) + callbackLogic.perform( + this.config, + getDefaultUrl(), + getRenewSession(), + getDefaultClient(), + new PlayFrameworkParameters(request)); + }, + this.ec.current()); } public CompletionStage handleCallback(String protocol, Http.Request request) { if (shouldHandleCallback(protocol)) { - log.debug("Handling SSO callback. Protocol: {}", - _ssoManager.getSsoProvider().protocol().getCommonName()); + log.debug( + "Handling SSO callback. Protocol: {}", + ssoManager.getSsoProvider().protocol().getCommonName()); return callback(request) .handle( (res, e) -> { @@ -94,9 +112,9 @@ public CompletionStage handleCallback(String protocol, Http.Request requ } /** Logic responsible for delegating to protocol-specific callback logic. */ - public class SsoCallbackLogic implements CallbackLogic { + public class SsoCallbackLogic implements CallbackLogic { - private final OidcCallbackLogic _oidcCallbackLogic; + private final OidcCallbackLogic oidcCallbackLogic; SsoCallbackLogic( final SsoManager ssoManager, @@ -104,31 +122,21 @@ public class SsoCallbackLogic implements CallbackLogic { final SystemEntityClient entityClient, final AuthServiceClient authClient, final CookieConfigs cookieConfigs) { - _oidcCallbackLogic = + oidcCallbackLogic = new OidcCallbackLogic( ssoManager, systemOperationContext, entityClient, authClient, cookieConfigs); } @Override - public Result perform( - PlayWebContext context, + public Object perform( Config config, - HttpActionAdapter httpActionAdapter, - String defaultUrl, - Boolean saveInSession, - Boolean multiProfile, - Boolean renewSession, - String defaultClient) { - if (SsoProvider.SsoProtocol.OIDC.equals(_ssoManager.getSsoProvider().protocol())) { - return _oidcCallbackLogic.perform( - context, - config, - httpActionAdapter, - defaultUrl, - saveInSession, - multiProfile, - renewSession, - defaultClient); + String inputDefaultUrl, + Boolean inputRenewSession, + String defaultClient, + FrameworkParameters parameters) { + if (SsoProvider.SsoProtocol.OIDC.equals(ssoManager.getSsoProvider().protocol())) { + return oidcCallbackLogic.perform( + config, inputDefaultUrl, inputRenewSession, defaultClient, parameters); } // Should never occur. throw new UnsupportedOperationException( @@ -137,18 +145,18 @@ public Result perform( } private boolean shouldHandleCallback(final String protocol) { - if (!_ssoManager.isSsoEnabled()) { + if (!ssoManager.isSsoEnabled()) { return false; } updateConfig(); - return _ssoManager.getSsoProvider().protocol().getCommonName().equals(protocol); + return ssoManager.getSsoProvider().protocol().getCommonName().equals(protocol); } private void updateConfig() { final Clients clients = new Clients(); final List clientList = new ArrayList<>(); - clientList.add(_ssoManager.getSsoProvider().client()); + clientList.add(ssoManager.getSsoProvider().client()); clients.setClients(clientList); - _config.setClients(clients); + config.setClients(clients); } } diff --git a/datahub-frontend/app/controllers/TrackingController.java b/datahub-frontend/app/controllers/TrackingController.java index 254a8cc640d0c5..5d12c96ed77cbc 100644 --- a/datahub-frontend/app/controllers/TrackingController.java +++ b/datahub-frontend/app/controllers/TrackingController.java @@ -22,23 +22,23 @@ @Singleton public class TrackingController extends Controller { - private final Logger _logger = LoggerFactory.getLogger(TrackingController.class.getName()); + private static final Logger logger = LoggerFactory.getLogger(TrackingController.class.getName()); - private final String _topic; + private final String topic; - @Inject KafkaTrackingProducer _producer; + @Inject KafkaTrackingProducer producer; - @Inject AuthServiceClient _authClient; + @Inject AuthServiceClient authClient; @Inject public TrackingController(@Nonnull Config config) { - _topic = config.getString("analytics.tracking.topic"); + topic = config.getString("analytics.tracking.topic"); } @Security.Authenticated(Authenticator.class) @Nonnull public Result track(Http.Request request) throws Exception { - if (!_producer.isEnabled()) { + if (!producer.isEnabled()) { // If tracking is disabled, simply return a 200. return status(200); } @@ -51,15 +51,15 @@ public Result track(Http.Request request) throws Exception { } final String actor = request.session().data().get(ACTOR); try { - _logger.debug( + logger.debug( String.format("Emitting product analytics event. actor: %s, event: %s", actor, event)); final ProducerRecord record = - new ProducerRecord<>(_topic, actor, event.toString()); - _producer.send(record); - _authClient.track(event.toString()); + new ProducerRecord<>(topic, actor, event.toString()); + producer.send(record); + authClient.track(event.toString()); return ok(); } catch (Exception e) { - _logger.error( + logger.error( String.format( "Failed to emit product analytics event. actor: %s, event: %s", actor, event)); return internalServerError(e.getMessage()); diff --git a/datahub-frontend/build.gradle b/datahub-frontend/build.gradle index ab4ce405a55411..7750e169b11fbe 100644 --- a/datahub-frontend/build.gradle +++ b/datahub-frontend/build.gradle @@ -12,6 +12,12 @@ ext { docker_dir = 'datahub-frontend' } +java { + toolchain { + languageVersion = JavaLanguageVersion.of(jdkVersion(project)) + } +} + model { // Must specify the dependency here as "stage" is added by rule based model. tasks.myTar { diff --git a/datahub-frontend/conf/application.conf b/datahub-frontend/conf/application.conf index be57a33b13564d..db982b595e2482 100644 --- a/datahub-frontend/conf/application.conf +++ b/datahub-frontend/conf/application.conf @@ -184,6 +184,7 @@ auth.oidc.responseMode = ${?AUTH_OIDC_RESPONSE_MODE} auth.oidc.useNonce = ${?AUTH_OIDC_USE_NONCE} auth.oidc.customParam.resource = ${?AUTH_OIDC_CUSTOM_PARAM_RESOURCE} auth.oidc.readTimeout = ${?AUTH_OIDC_READ_TIMEOUT} +auth.oidc.connectTimeout = ${?AUTH_OIDC_CONNECT_TIMEOUT} auth.oidc.extractJwtAccessTokenClaims = ${?AUTH_OIDC_EXTRACT_JWT_ACCESS_TOKEN_CLAIMS} # Whether to extract claims from JWT access token. Defaults to false. auth.oidc.preferredJwsAlgorithm = ${?AUTH_OIDC_PREFERRED_JWS_ALGORITHM} # Which jws algorithm to use auth.oidc.acrValues = ${?AUTH_OIDC_ACR_VALUES} diff --git a/datahub-frontend/play.gradle b/datahub-frontend/play.gradle index ff43e4a93a80f8..266962721a80a8 100644 --- a/datahub-frontend/play.gradle +++ b/datahub-frontend/play.gradle @@ -11,16 +11,29 @@ configurations { play } +ext { + nimbusJoseJwtVersion = "9.41.2" + oauth2OidcSdkVersion = "11.20.1" +} + dependencies { implementation project(':datahub-web-react') constraints { + play(externalDependency.pac4j) + play(externalDependency.playPac4j) + play("com.nimbusds:oauth2-oidc-sdk:$oauth2OidcSdkVersion") + play("com.nimbusds:nimbus-jose-jwt:$nimbusJoseJwtVersion") + implementation(externalDependency.pac4j) + implementation(externalDependency.playPac4j) + implementation("com.nimbusds:nimbus-jose-jwt:$nimbusJoseJwtVersion") + testImplementation("com.nimbusds:oauth2-oidc-sdk:$oauth2OidcSdkVersion") + play(externalDependency.jacksonDataBind) - play('com.nimbusds:oauth2-oidc-sdk:8.36.2') - play('com.nimbusds:nimbus-jose-jwt:8.18') - play('com.typesafe.akka:akka-actor_2.12:2.6.20') + play("com.typesafe.akka:akka-actor_$playScalaVersion:2.6.20") play(externalDependency.jsonSmart) play('io.netty:netty-all:4.1.114.Final') + implementation(externalDependency.commonsText) { because("previous versions are vulnerable to CVE-2022-42889") } @@ -46,14 +59,11 @@ dependencies { implementation externalDependency.jerseyCore implementation externalDependency.jerseyGuava - implementation(externalDependency.pac4j) { - exclude group: "net.minidev", module: "json-smart" - exclude group: "com.nimbusds", module: "nimbus-jose-jwt" - } - - implementation 'com.nimbusds:nimbus-jose-jwt:8.18' - implementation externalDependency.jsonSmart + implementation externalDependency.pac4j implementation externalDependency.playPac4j + implementation "com.nimbusds:nimbus-jose-jwt:$nimbusJoseJwtVersion" + implementation externalDependency.jsonSmart + implementation externalDependency.shiroCore implementation externalDependency.playCache @@ -69,7 +79,7 @@ dependencies { testImplementation externalDependency.mockito testImplementation externalDependency.playTest testImplementation 'org.awaitility:awaitility:4.2.0' - testImplementation 'no.nav.security:mock-oauth2-server:0.3.1' + testImplementation 'no.nav.security:mock-oauth2-server:2.1.9' testImplementation 'org.junit-pioneer:junit-pioneer:1.9.1' testImplementation externalDependency.junitJupiterApi testRuntimeOnly externalDependency.junitJupiterEngine @@ -78,7 +88,7 @@ dependencies { compileOnly externalDependency.lombok runtimeOnly externalDependency.guicePlay runtimeOnly (externalDependency.playDocs) { - exclude group: 'com.typesafe.akka', module: 'akka-http-core_2.12' + exclude group: 'com.typesafe.akka', module: "akka-http-core_$playScalaVersion" } runtimeOnly externalDependency.playGuice implementation externalDependency.log4j2Api @@ -90,9 +100,9 @@ dependencies { play { platform { - playVersion = '2.8.21' - scalaVersion = '2.12' - javaVersion = JavaVersion.VERSION_11 + playVersion = "2.8.22" // see also top level build.gradle + scalaVersion = "2.13" + javaVersion = JavaVersion.VERSION_17 } injectedRoutesGenerator = true diff --git a/datahub-frontend/test/app/ApplicationTest.java b/datahub-frontend/test/app/ApplicationTest.java index 534cffb5cc7fe4..3ad9e228571681 100644 --- a/datahub-frontend/test/app/ApplicationTest.java +++ b/datahub-frontend/test/app/ApplicationTest.java @@ -12,17 +12,27 @@ import com.nimbusds.jwt.JWTParser; import controllers.routes; import java.io.IOException; +import java.net.HttpURLConnection; import java.net.InetAddress; +import java.net.URL; import java.text.ParseException; import java.util.Date; import java.util.List; import java.util.Map; +import java.util.concurrent.CompletableFuture; +import java.util.concurrent.TimeUnit; +import java.util.concurrent.TimeoutException; import no.nav.security.mock.oauth2.MockOAuth2Server; +import no.nav.security.mock.oauth2.http.OAuth2HttpRequest; +import no.nav.security.mock.oauth2.http.OAuth2HttpResponse; +import no.nav.security.mock.oauth2.http.Route; import no.nav.security.mock.oauth2.token.DefaultOAuth2TokenCallback; +import okhttp3.Headers; import okhttp3.mockwebserver.MockResponse; import okhttp3.mockwebserver.MockWebServer; import org.awaitility.Awaitility; import org.awaitility.Durations; +import org.jetbrains.annotations.NotNull; import org.junit.jupiter.api.AfterAll; import org.junit.jupiter.api.BeforeAll; import org.junit.jupiter.api.Test; @@ -30,6 +40,8 @@ import org.junitpioneer.jupiter.SetEnvironmentVariable; import org.openqa.selenium.Cookie; import org.openqa.selenium.htmlunit.HtmlUnitDriver; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; import play.Application; import play.Environment; import play.Mode; @@ -48,7 +60,9 @@ @SetEnvironmentVariable(key = "AUTH_OIDC_JIT_PROVISIONING_ENABLED", value = "false") @SetEnvironmentVariable(key = "AUTH_OIDC_CLIENT_ID", value = "testclient") @SetEnvironmentVariable(key = "AUTH_OIDC_CLIENT_SECRET", value = "testsecret") +@SetEnvironmentVariable(key = "AUTH_VERBOSE_LOGGING", value = "true") public class ApplicationTest extends WithBrowser { + private static final Logger logger = LoggerFactory.getLogger(ApplicationTest.class); private static final String ISSUER_ID = "testIssuer"; @Override @@ -80,41 +94,34 @@ public int gmsServerPort() { return providePort() + 2; } - private MockOAuth2Server _oauthServer; - private MockWebServer _gmsServer; + private MockOAuth2Server oauthServer; + private Thread oauthServerThread; + private CompletableFuture oauthServerStarted; - private String _wellKnownUrl; + private MockWebServer gmsServer; + + private String wellKnownUrl; private static final String TEST_USER = "urn:li:corpuser:testUser@myCompany.com"; private static final String TEST_TOKEN = "faketoken_YCpYIrjQH4sD3_rAc3VPPFg4"; @BeforeAll public void init() throws IOException { - _gmsServer = new MockWebServer(); - _gmsServer.enqueue(new MockResponse().setResponseCode(404)); // dynamic settings - not tested - _gmsServer.enqueue(new MockResponse().setResponseCode(404)); // dynamic settings - not tested - _gmsServer.enqueue(new MockResponse().setResponseCode(404)); // dynamic settings - not tested - _gmsServer.enqueue(new MockResponse().setBody(String.format("{\"value\":\"%s\"}", TEST_USER))); - _gmsServer.enqueue( + // Start Mock GMS + gmsServer = new MockWebServer(); + gmsServer.enqueue(new MockResponse().setResponseCode(404)); // dynamic settings - not tested + gmsServer.enqueue(new MockResponse().setResponseCode(404)); // dynamic settings - not tested + gmsServer.enqueue(new MockResponse().setResponseCode(404)); // dynamic settings - not tested + gmsServer.enqueue(new MockResponse().setBody(String.format("{\"value\":\"%s\"}", TEST_USER))); + gmsServer.enqueue( new MockResponse().setBody(String.format("{\"accessToken\":\"%s\"}", TEST_TOKEN))); - _gmsServer.start(gmsServerPort()); - - _oauthServer = new MockOAuth2Server(); - _oauthServer.enqueueCallback( - new DefaultOAuth2TokenCallback( - ISSUER_ID, - "testUser", - List.of(), - Map.of( - "email", "testUser@myCompany.com", - "groups", "myGroup"), - 600)); - _oauthServer.start(InetAddress.getByName("localhost"), oauthServerPort()); - - // Discovery url to authorization server metadata - _wellKnownUrl = _oauthServer.wellKnownUrl(ISSUER_ID).toString(); + gmsServer.start(gmsServerPort()); + // Start Mock Identity Provider + startMockOauthServer(); + // Start Play Frontend startServer(); + // Start Browser createBrowser(); Awaitility.await().timeout(Durations.TEN_SECONDS).until(() -> app != null); @@ -122,13 +129,131 @@ public void init() throws IOException { @AfterAll public void shutdown() throws IOException { - if (_gmsServer != null) { - _gmsServer.shutdown(); - } - if (_oauthServer != null) { - _oauthServer.shutdown(); + if (gmsServer != null) { + logger.info("Shutdown Mock GMS"); + gmsServer.shutdown(); } + logger.info("Shutdown Play Frontend"); stopServer(); + if (oauthServer != null) { + logger.info("Shutdown MockOAuth2Server"); + oauthServer.shutdown(); + } + if (oauthServerThread != null && oauthServerThread.isAlive()) { + logger.info("Shutdown MockOAuth2Server thread"); + oauthServerThread.interrupt(); + try { + oauthServerThread.join(2000); // Wait up to 2 seconds for thread to finish + } catch (InterruptedException e) { + logger.warn("Shutdown MockOAuth2Server thread failed to join."); + } + } + } + + private void startMockOauthServer() { + // Configure HEAD responses + Route[] routes = + new Route[] { + new Route() { + @Override + public boolean match(@NotNull OAuth2HttpRequest oAuth2HttpRequest) { + return "HEAD".equals(oAuth2HttpRequest.getMethod()) + && (String.format("/%s/.well-known/openid-configuration", ISSUER_ID) + .equals(oAuth2HttpRequest.getUrl().url().getPath()) + || String.format("/%s/token", ISSUER_ID) + .equals(oAuth2HttpRequest.getUrl().url().getPath())); + } + + @Override + public OAuth2HttpResponse invoke(OAuth2HttpRequest oAuth2HttpRequest) { + return new OAuth2HttpResponse( + Headers.of( + Map.of( + "Content-Type", "application/json", + "Cache-Control", "no-store", + "Pragma", "no-cache", + "Content-Length", "-1")), + 200, + null, + null); + } + } + }; + oauthServer = new MockOAuth2Server(routes); + oauthServerStarted = new CompletableFuture<>(); + + // Create and start server in separate thread + oauthServerThread = + new Thread( + () -> { + try { + // Configure mock responses + oauthServer.enqueueCallback( + new DefaultOAuth2TokenCallback( + ISSUER_ID, + "testUser", + "JWT", + List.of(), + Map.of( + "email", "testUser@myCompany.com", + "groups", "myGroup"), + 600)); + + oauthServer.start(InetAddress.getByName("localhost"), oauthServerPort()); + + oauthServerStarted.complete(null); + + // Keep thread alive until server is stopped + while (!Thread.currentThread().isInterrupted() && testServer.isRunning()) { + try { + Thread.sleep(1000); + } catch (InterruptedException e) { + Thread.currentThread().interrupt(); + break; + } + } + } catch (Exception e) { + oauthServerStarted.completeExceptionally(e); + } + }); + + oauthServerThread.setDaemon(true); // Ensure thread doesn't prevent JVM shutdown + oauthServerThread.start(); + + // Wait for server to start with timeout + oauthServerStarted + .orTimeout(10, TimeUnit.SECONDS) + .whenComplete( + (result, throwable) -> { + if (throwable != null) { + if (throwable instanceof TimeoutException) { + throw new RuntimeException( + "MockOAuth2Server failed to start within timeout", throwable); + } + throw new RuntimeException("MockOAuth2Server failed to start", throwable); + } + }); + + // Discovery url to authorization server metadata + wellKnownUrl = oauthServer.wellKnownUrl(ISSUER_ID).toString(); + + // Wait for server to return configuration + // Validate mock server returns data + try { + URL url = new URL(wellKnownUrl); + HttpURLConnection conn = (HttpURLConnection) url.openConnection(); + conn.setRequestMethod("GET"); + int responseCode = conn.getResponseCode(); + logger.info("Well-known endpoint response code: {}", responseCode); + + if (responseCode != 200) { + throw new RuntimeException( + "MockOAuth2Server not accessible. Response code: " + responseCode); + } + logger.info("Successfully started MockOAuth2Server."); + } catch (Exception e) { + throw new RuntimeException("Failed to connect to MockOAuth2Server", e); + } } @Test @@ -158,7 +283,7 @@ public void testIndexNotFound() { public void testOpenIdConfig() { assertEquals( "http://localhost:" + oauthServerPort() + "/testIssuer/.well-known/openid-configuration", - _wellKnownUrl); + wellKnownUrl); } @Test @@ -188,10 +313,10 @@ public void testHappyPathOidc() throws ParseException { @Test public void testAPI() throws ParseException { testHappyPathOidc(); - int requestCount = _gmsServer.getRequestCount(); + int requestCount = gmsServer.getRequestCount(); browser.goTo("/api/v2/graphql/"); - assertEquals(++requestCount, _gmsServer.getRequestCount()); + assertEquals(++requestCount, gmsServer.getRequestCount()); } @Test @@ -201,8 +326,9 @@ public void testOidcRedirectToRequestedUrl() { } /** - * The Redirect Uri parameter is used to store a previous relative location within the app to be able to - * take a user back to their expected page. Redirecting to other domains should be blocked. + * The Redirect Uri parameter is used to store a previous relative location within the app to be + * able to take a user back to their expected page. Redirecting to other domains should be + * blocked. */ @Test public void testInvalidRedirectUrl() { diff --git a/datahub-frontend/test/oidc/OidcCallbackLogicTest.java b/datahub-frontend/test/oidc/OidcCallbackLogicTest.java index f4784c29e91f2e..9eb3833cbc897a 100644 --- a/datahub-frontend/test/oidc/OidcCallbackLogicTest.java +++ b/datahub-frontend/test/oidc/OidcCallbackLogicTest.java @@ -1,64 +1,65 @@ package oidc; -import auth.sso.oidc.OidcConfigs; - -import java.util.Arrays; -import java.util.Collection; -import java.util.Collections; -import java.util.List; - import static auth.sso.oidc.OidcCallbackLogic.getGroupNames; import static org.junit.jupiter.api.Assertions.assertEquals; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.when; +import java.util.Arrays; +import java.util.Collection; import org.junit.jupiter.api.Test; -import org.mockito.Mockito; import org.pac4j.core.profile.CommonProfile; public class OidcCallbackLogicTest { - @Test - public void testGetGroupsClaimNamesJsonArray() { - CommonProfile profile = createMockProfileWithAttribute("[\"group1\", \"group2\"]", "groupsClaimName"); - Collection result = getGroupNames(profile, "[\"group1\", \"group2\"]", "groupsClaimName"); - assertEquals(Arrays.asList("group1", "group2"), result); - } - @Test - public void testGetGroupNamesWithSingleGroup() { - CommonProfile profile = createMockProfileWithAttribute("group1", "groupsClaimName"); - Collection result = getGroupNames(profile, "group1", "groupsClaimName"); - assertEquals(Arrays.asList("group1"), result); - } + @Test + public void testGetGroupsClaimNamesJsonArray() { + CommonProfile profile = + createMockProfileWithAttribute("[\"group1\", \"group2\"]", "groupsClaimName"); + Collection result = + getGroupNames(profile, "[\"group1\", \"group2\"]", "groupsClaimName"); + assertEquals(Arrays.asList("group1", "group2"), result); + } - @Test - public void testGetGroupNamesWithCommaSeparated() { - CommonProfile profile = createMockProfileWithAttribute("group1,group2", "groupsClaimName"); - Collection result = getGroupNames(profile, "group1,group2", "groupsClaimName"); - assertEquals(Arrays.asList("group1", "group2"), result); - } + @Test + public void testGetGroupNamesWithSingleGroup() { + CommonProfile profile = createMockProfileWithAttribute("group1", "groupsClaimName"); + Collection result = getGroupNames(profile, "group1", "groupsClaimName"); + assertEquals(Arrays.asList("group1"), result); + } - @Test - public void testGetGroupNamesWithCollection() { - CommonProfile profile = createMockProfileWithAttribute(Arrays.asList("group1", "group2"), "groupsClaimName"); - Collection result = getGroupNames(profile, Arrays.asList("group1", "group2"), "groupsClaimName"); - assertEquals(Arrays.asList("group1", "group2"), result); - } - // Helper method to create a mock CommonProfile with given attribute - private CommonProfile createMockProfileWithAttribute(Object attribute, String attributeName) { - CommonProfile profile = mock(CommonProfile.class); + @Test + public void testGetGroupNamesWithCommaSeparated() { + CommonProfile profile = createMockProfileWithAttribute("group1,group2", "groupsClaimName"); + Collection result = getGroupNames(profile, "group1,group2", "groupsClaimName"); + assertEquals(Arrays.asList("group1", "group2"), result); + } + + @Test + public void testGetGroupNamesWithCollection() { + CommonProfile profile = + createMockProfileWithAttribute(Arrays.asList("group1", "group2"), "groupsClaimName"); + Collection result = + getGroupNames(profile, Arrays.asList("group1", "group2"), "groupsClaimName"); + assertEquals(Arrays.asList("group1", "group2"), result); + } - // Mock for getAttribute(String) - when(profile.getAttribute(attributeName)).thenReturn(attribute); + // Helper method to create a mock CommonProfile with given attribute + private CommonProfile createMockProfileWithAttribute(Object attribute, String attributeName) { + CommonProfile profile = mock(CommonProfile.class); - // Mock for getAttribute(String, Class) - if (attribute instanceof Collection) { - when(profile.getAttribute(attributeName, Collection.class)).thenReturn((Collection) attribute); - } else if (attribute instanceof String) { - when(profile.getAttribute(attributeName, String.class)).thenReturn((String) attribute); - } - // Add more conditions here if needed for other types + // Mock for getAttribute(String) + when(profile.getAttribute(attributeName)).thenReturn(attribute); - return profile; + // Mock for getAttribute(String, Class) + if (attribute instanceof Collection) { + when(profile.getAttribute(attributeName, Collection.class)) + .thenReturn((Collection) attribute); + } else if (attribute instanceof String) { + when(profile.getAttribute(attributeName, String.class)).thenReturn((String) attribute); } + // Add more conditions here if needed for other types + + return profile; + } } diff --git a/datahub-frontend/test/security/OidcConfigurationTest.java b/datahub-frontend/test/security/OidcConfigurationTest.java index 1c52d45af5f9e0..ec19979c56120e 100644 --- a/datahub-frontend/test/security/OidcConfigurationTest.java +++ b/datahub-frontend/test/security/OidcConfigurationTest.java @@ -23,9 +23,9 @@ import java.util.Map; import java.util.Set; import java.util.concurrent.TimeUnit; +import org.json.JSONObject; import org.junit.jupiter.api.Test; import org.pac4j.oidc.client.OidcClient; -import org.json.JSONObject; public class OidcConfigurationTest { @@ -328,17 +328,28 @@ public void readPreferredJwsAlgorithmPropagationFromConfig() { oidcConfigsBuilder.from(CONFIG, SSO_SETTINGS_JSON_STR); OidcConfigs oidcConfigs = new OidcConfigs(oidcConfigsBuilder); OidcProvider oidcProvider = new OidcProvider(oidcConfigs); - assertEquals("RS256", ((OidcClient) oidcProvider.client()).getConfiguration().getPreferredJwsAlgorithm().toString()); + assertEquals( + "RS256", + ((OidcClient) oidcProvider.client()) + .getConfiguration() + .getPreferredJwsAlgorithm() + .toString()); } @Test public void readPreferredJwsAlgorithmPropagationFromJSON() { - final String SSO_SETTINGS_JSON_STR = new JSONObject().put(PREFERRED_JWS_ALGORITHM, "HS256").toString(); + final String SSO_SETTINGS_JSON_STR = + new JSONObject().put(PREFERRED_JWS_ALGORITHM, "HS256").toString(); CONFIG.withValue(OIDC_PREFERRED_JWS_ALGORITHM, ConfigValueFactory.fromAnyRef("RS256")); OidcConfigs.Builder oidcConfigsBuilder = new OidcConfigs.Builder(); oidcConfigsBuilder.from(CONFIG, SSO_SETTINGS_JSON_STR); OidcConfigs oidcConfigs = new OidcConfigs(oidcConfigsBuilder); OidcProvider oidcProvider = new OidcProvider(oidcConfigs); - assertEquals("HS256", ((OidcClient) oidcProvider.client()).getConfiguration().getPreferredJwsAlgorithm().toString()); + assertEquals( + "HS256", + ((OidcClient) oidcProvider.client()) + .getConfiguration() + .getPreferredJwsAlgorithm() + .toString()); } } diff --git a/datahub-graphql-core/src/main/java/com/linkedin/datahub/graphql/types/container/ContainerType.java b/datahub-graphql-core/src/main/java/com/linkedin/datahub/graphql/types/container/ContainerType.java index db44a5be8bdd37..37b021fcb1091a 100644 --- a/datahub-graphql-core/src/main/java/com/linkedin/datahub/graphql/types/container/ContainerType.java +++ b/datahub-graphql-core/src/main/java/com/linkedin/datahub/graphql/types/container/ContainerType.java @@ -52,7 +52,8 @@ public class ContainerType Constants.DEPRECATION_ASPECT_NAME, Constants.DATA_PRODUCTS_ASPECT_NAME, Constants.STRUCTURED_PROPERTIES_ASPECT_NAME, - Constants.FORMS_ASPECT_NAME); + Constants.FORMS_ASPECT_NAME, + Constants.ACCESS_ASPECT_NAME); private static final Set FACET_FIELDS = ImmutableSet.of("origin", "platform"); private static final String ENTITY_NAME = "container"; diff --git a/datahub-graphql-core/src/main/java/com/linkedin/datahub/graphql/types/container/mappers/ContainerMapper.java b/datahub-graphql-core/src/main/java/com/linkedin/datahub/graphql/types/container/mappers/ContainerMapper.java index 2c0dc142bee3d9..02357b3ddc349e 100644 --- a/datahub-graphql-core/src/main/java/com/linkedin/datahub/graphql/types/container/mappers/ContainerMapper.java +++ b/datahub-graphql-core/src/main/java/com/linkedin/datahub/graphql/types/container/mappers/ContainerMapper.java @@ -2,6 +2,7 @@ import static com.linkedin.metadata.Constants.*; +import com.linkedin.common.Access; import com.linkedin.common.DataPlatformInstance; import com.linkedin.common.Deprecation; import com.linkedin.common.Forms; @@ -30,6 +31,7 @@ import com.linkedin.datahub.graphql.types.domain.DomainAssociationMapper; import com.linkedin.datahub.graphql.types.form.FormsMapper; import com.linkedin.datahub.graphql.types.glossary.mappers.GlossaryTermsMapper; +import com.linkedin.datahub.graphql.types.rolemetadata.mappers.AccessMapper; import com.linkedin.datahub.graphql.types.structuredproperty.StructuredPropertiesMapper; import com.linkedin.datahub.graphql.types.tag.mappers.GlobalTagsMapper; import com.linkedin.domain.Domains; @@ -105,6 +107,11 @@ public static Container map( context, new GlossaryTerms(envelopedTerms.getValue().data()), entityUrn)); } + final EnvelopedAspect accessAspect = aspects.get(ACCESS_ASPECT_NAME); + if (accessAspect != null) { + result.setAccess(AccessMapper.map(new Access(accessAspect.getValue().data()), entityUrn)); + } + final EnvelopedAspect envelopedInstitutionalMemory = aspects.get(Constants.INSTITUTIONAL_MEMORY_ASPECT_NAME); if (envelopedInstitutionalMemory != null) { diff --git a/datahub-graphql-core/src/main/java/com/linkedin/datahub/graphql/types/dataset/DatasetType.java b/datahub-graphql-core/src/main/java/com/linkedin/datahub/graphql/types/dataset/DatasetType.java index 65b5d39e315692..6a3f9cb9b21f38 100644 --- a/datahub-graphql-core/src/main/java/com/linkedin/datahub/graphql/types/dataset/DatasetType.java +++ b/datahub-graphql-core/src/main/java/com/linkedin/datahub/graphql/types/dataset/DatasetType.java @@ -86,7 +86,7 @@ public class DatasetType EMBED_ASPECT_NAME, DATA_PRODUCTS_ASPECT_NAME, BROWSE_PATHS_V2_ASPECT_NAME, - ACCESS_DATASET_ASPECT_NAME, + ACCESS_ASPECT_NAME, STRUCTURED_PROPERTIES_ASPECT_NAME, FORMS_ASPECT_NAME, SUB_TYPES_ASPECT_NAME); diff --git a/datahub-graphql-core/src/main/java/com/linkedin/datahub/graphql/types/dataset/mappers/DatasetMapper.java b/datahub-graphql-core/src/main/java/com/linkedin/datahub/graphql/types/dataset/mappers/DatasetMapper.java index a7b5f6de0c183d..0869463ba73ac2 100644 --- a/datahub-graphql-core/src/main/java/com/linkedin/datahub/graphql/types/dataset/mappers/DatasetMapper.java +++ b/datahub-graphql-core/src/main/java/com/linkedin/datahub/graphql/types/dataset/mappers/DatasetMapper.java @@ -166,7 +166,7 @@ public Dataset apply( (dataset, dataMap) -> dataset.setBrowsePathV2(BrowsePathsV2Mapper.map(context, new BrowsePathsV2(dataMap)))); mappingHelper.mapToResult( - ACCESS_DATASET_ASPECT_NAME, + ACCESS_ASPECT_NAME, ((dataset, dataMap) -> dataset.setAccess(AccessMapper.map(new Access(dataMap), entityUrn)))); mappingHelper.mapToResult( diff --git a/datahub-graphql-core/src/main/resources/entity.graphql b/datahub-graphql-core/src/main/resources/entity.graphql index 16ef59114f86c8..732a782139b616 100644 --- a/datahub-graphql-core/src/main/resources/entity.graphql +++ b/datahub-graphql-core/src/main/resources/entity.graphql @@ -645,7 +645,7 @@ type Mutation { updateERModelRelationship( "The urn of the ERModelRelationship to update" urn: String!, - "Input required to updat an existing DataHub View" + "Input required to update an existing DataHub View" input: ERModelRelationshipUpdateInput!): Boolean """ @@ -795,7 +795,7 @@ type Mutation { updateView( "The urn of the View to update" urn: String!, - "Input required to updat an existing DataHub View" + "Input required to update an existing DataHub View" input: UpdateViewInput!): DataHubView """ @@ -2837,6 +2837,11 @@ type Container implements Entity { """ exists: Boolean + """ + The Roles and the properties to access the container + """ + access: Access + """ Experimental API. For fetching extra entities that do not have custom UI code yet diff --git a/datahub-upgrade/src/main/java/com/linkedin/datahub/upgrade/config/restoreindices/PropertyDefinitionsConfig.java b/datahub-upgrade/src/main/java/com/linkedin/datahub/upgrade/config/restoreindices/PropertyDefinitionsConfig.java new file mode 100644 index 00000000000000..49cd0beb98ea23 --- /dev/null +++ b/datahub-upgrade/src/main/java/com/linkedin/datahub/upgrade/config/restoreindices/PropertyDefinitionsConfig.java @@ -0,0 +1,30 @@ +package com.linkedin.datahub.upgrade.config.restoreindices; + +import com.linkedin.datahub.upgrade.config.SystemUpdateCondition; +import com.linkedin.datahub.upgrade.system.NonBlockingSystemUpgrade; +import com.linkedin.datahub.upgrade.system.restoreindices.structuredproperties.PropertyDefinitions; +import com.linkedin.metadata.entity.AspectDao; +import com.linkedin.metadata.entity.EntityService; +import io.datahubproject.metadata.context.OperationContext; +import org.springframework.beans.factory.annotation.Value; +import org.springframework.context.annotation.Bean; +import org.springframework.context.annotation.Conditional; +import org.springframework.context.annotation.Configuration; + +@Configuration +@Conditional(SystemUpdateCondition.NonBlockingSystemUpdateCondition.class) +public class PropertyDefinitionsConfig { + + @Bean + public NonBlockingSystemUpgrade propertyDefinitions( + final OperationContext opContext, + final EntityService entityService, + final AspectDao aspectDao, + @Value("${systemUpdate.propertyDefinitions.enabled}") final boolean enabled, + @Value("${systemUpdate.propertyDefinitions.batchSize}") final Integer batchSize, + @Value("${systemUpdate.propertyDefinitions.delayMs}") final Integer delayMs, + @Value("${systemUpdate.propertyDefinitions.limit}") final Integer limit) { + return new PropertyDefinitions( + opContext, entityService, aspectDao, enabled, batchSize, delayMs, limit); + } +} diff --git a/datahub-upgrade/src/main/java/com/linkedin/datahub/upgrade/config/ReindexDomainDescriptionConfig.java b/datahub-upgrade/src/main/java/com/linkedin/datahub/upgrade/config/restoreindices/ReindexDomainDescriptionConfig.java similarity index 84% rename from datahub-upgrade/src/main/java/com/linkedin/datahub/upgrade/config/ReindexDomainDescriptionConfig.java rename to datahub-upgrade/src/main/java/com/linkedin/datahub/upgrade/config/restoreindices/ReindexDomainDescriptionConfig.java index 3cdab0dc4d4bc6..0ec9eb38e2b1cc 100644 --- a/datahub-upgrade/src/main/java/com/linkedin/datahub/upgrade/config/ReindexDomainDescriptionConfig.java +++ b/datahub-upgrade/src/main/java/com/linkedin/datahub/upgrade/config/restoreindices/ReindexDomainDescriptionConfig.java @@ -1,7 +1,8 @@ -package com.linkedin.datahub.upgrade.config; +package com.linkedin.datahub.upgrade.config.restoreindices; +import com.linkedin.datahub.upgrade.config.SystemUpdateCondition; import com.linkedin.datahub.upgrade.system.NonBlockingSystemUpgrade; -import com.linkedin.datahub.upgrade.system.domaindescription.ReindexDomainDescription; +import com.linkedin.datahub.upgrade.system.restoreindices.domaindescription.ReindexDomainDescription; import com.linkedin.metadata.entity.AspectDao; import com.linkedin.metadata.entity.EntityService; import io.datahubproject.metadata.context.OperationContext; diff --git a/datahub-upgrade/src/main/java/com/linkedin/datahub/upgrade/config/graph/ReindexDataJobViaNodesCLLConfig.java b/datahub-upgrade/src/main/java/com/linkedin/datahub/upgrade/config/restoreindices/graph/ReindexDataJobViaNodesCLLConfig.java similarity index 88% rename from datahub-upgrade/src/main/java/com/linkedin/datahub/upgrade/config/graph/ReindexDataJobViaNodesCLLConfig.java rename to datahub-upgrade/src/main/java/com/linkedin/datahub/upgrade/config/restoreindices/graph/ReindexDataJobViaNodesCLLConfig.java index a973876c6715f0..730991cd0a697e 100644 --- a/datahub-upgrade/src/main/java/com/linkedin/datahub/upgrade/config/graph/ReindexDataJobViaNodesCLLConfig.java +++ b/datahub-upgrade/src/main/java/com/linkedin/datahub/upgrade/config/restoreindices/graph/ReindexDataJobViaNodesCLLConfig.java @@ -1,8 +1,8 @@ -package com.linkedin.datahub.upgrade.config.graph; +package com.linkedin.datahub.upgrade.config.restoreindices.graph; import com.linkedin.datahub.upgrade.config.SystemUpdateCondition; import com.linkedin.datahub.upgrade.system.NonBlockingSystemUpgrade; -import com.linkedin.datahub.upgrade.system.graph.vianodes.ReindexDataJobViaNodesCLL; +import com.linkedin.datahub.upgrade.system.restoreindices.graph.vianodes.ReindexDataJobViaNodesCLL; import com.linkedin.metadata.entity.AspectDao; import com.linkedin.metadata.entity.EntityService; import io.datahubproject.metadata.context.OperationContext; diff --git a/datahub-upgrade/src/main/java/com/linkedin/datahub/upgrade/config/graph/ReindexEdgeStatusConfig.java b/datahub-upgrade/src/main/java/com/linkedin/datahub/upgrade/config/restoreindices/graph/ReindexEdgeStatusConfig.java similarity index 89% rename from datahub-upgrade/src/main/java/com/linkedin/datahub/upgrade/config/graph/ReindexEdgeStatusConfig.java rename to datahub-upgrade/src/main/java/com/linkedin/datahub/upgrade/config/restoreindices/graph/ReindexEdgeStatusConfig.java index 97715573eb51ff..14b60f44c09a0b 100644 --- a/datahub-upgrade/src/main/java/com/linkedin/datahub/upgrade/config/graph/ReindexEdgeStatusConfig.java +++ b/datahub-upgrade/src/main/java/com/linkedin/datahub/upgrade/config/restoreindices/graph/ReindexEdgeStatusConfig.java @@ -1,8 +1,8 @@ -package com.linkedin.datahub.upgrade.config.graph; +package com.linkedin.datahub.upgrade.config.restoreindices.graph; import com.linkedin.datahub.upgrade.config.SystemUpdateCondition; import com.linkedin.datahub.upgrade.system.NonBlockingSystemUpgrade; -import com.linkedin.datahub.upgrade.system.graph.edgestatus.ReindexEdgeStatus; +import com.linkedin.datahub.upgrade.system.restoreindices.graph.edgestatus.ReindexEdgeStatus; import com.linkedin.metadata.entity.AspectDao; import com.linkedin.metadata.entity.EntityService; import io.datahubproject.metadata.context.OperationContext; diff --git a/datahub-upgrade/src/main/java/com/linkedin/datahub/upgrade/system/bootstrapmcps/BootstrapMCPUtil.java b/datahub-upgrade/src/main/java/com/linkedin/datahub/upgrade/system/bootstrapmcps/BootstrapMCPUtil.java index b8b7e828c16c6b..4cc3edff3eb52d 100644 --- a/datahub-upgrade/src/main/java/com/linkedin/datahub/upgrade/system/bootstrapmcps/BootstrapMCPUtil.java +++ b/datahub-upgrade/src/main/java/com/linkedin/datahub/upgrade/system/bootstrapmcps/BootstrapMCPUtil.java @@ -31,6 +31,7 @@ import java.util.Map; import java.util.stream.Collectors; import javax.annotation.Nonnull; +import javax.annotation.Nullable; import lombok.extern.slf4j.Slf4j; import org.apache.commons.io.IOUtils; import org.springframework.core.io.ClassPathResource; @@ -53,6 +54,7 @@ static List generateSteps( .getBootstrap() .getTemplates() .stream() + .map(cfg -> cfg.withOverride(opContext.getObjectMapper())) .filter(cfg -> cfg.isBlocking() == isBlocking) .map(cfg -> new BootstrapMCPStep(opContext, entityService, cfg)) .collect(Collectors.toList()); @@ -109,13 +111,29 @@ static List resolveMCPTemplate( AuditStamp auditStamp) throws IOException { - String template = loadTemplate(mcpTemplate.getMcps_location()); - Mustache mustache = MUSTACHE_FACTORY.compile(new StringReader(template), mcpTemplate.getName()); + final String template = loadTemplate(mcpTemplate.getMcps_location()); Map scopeValues = resolveValues(opContext, mcpTemplate, auditStamp); + StringWriter writer = new StringWriter(); - mustache.execute(writer, scopeValues); + try { + Mustache mustache = + MUSTACHE_FACTORY.compile(new StringReader(template), mcpTemplate.getName()); + mustache.execute(writer, scopeValues); + } catch (Exception e) { + log.error( + "Failed to apply mustache template. Template: {} Values: {}", + template, + resolveEnv(mcpTemplate)); + throw e; + } - return opContext.getYamlMapper().readValue(writer.toString(), new TypeReference<>() {}); + final String yaml = writer.toString(); + try { + return opContext.getYamlMapper().readValue(yaml, new TypeReference<>() {}); + } catch (Exception e) { + log.error("Failed to parse rendered MCP bootstrap yaml: {}", yaml); + throw e; + } } static Map resolveValues( @@ -128,13 +146,21 @@ static Map resolveValues( // built-in scopeValues.put("auditStamp", RecordUtils.toJsonString(auditStamp)); + String envValue = resolveEnv(mcpTemplate); + if (envValue != null) { + scopeValues.putAll(opContext.getObjectMapper().readValue(envValue, new TypeReference<>() {})); + } + return scopeValues; + } + + @Nullable + private static String resolveEnv(BootstrapMCPConfigFile.MCPTemplate mcpTemplate) { if (mcpTemplate.getValues_env() != null && !mcpTemplate.getValues_env().isEmpty() && System.getenv().containsKey(mcpTemplate.getValues_env())) { - String envValue = System.getenv(mcpTemplate.getValues_env()); - scopeValues.putAll(opContext.getObjectMapper().readValue(envValue, new TypeReference<>() {})); + return System.getenv(mcpTemplate.getValues_env()); } - return scopeValues; + return null; } private static String loadTemplate(String source) throws IOException { diff --git a/datahub-upgrade/src/main/java/com/linkedin/datahub/upgrade/system/bootstrapmcps/model/BootstrapMCPConfigFile.java b/datahub-upgrade/src/main/java/com/linkedin/datahub/upgrade/system/bootstrapmcps/model/BootstrapMCPConfigFile.java index 8fd3dd7c7d8975..009d19e453b6a9 100644 --- a/datahub-upgrade/src/main/java/com/linkedin/datahub/upgrade/system/bootstrapmcps/model/BootstrapMCPConfigFile.java +++ b/datahub-upgrade/src/main/java/com/linkedin/datahub/upgrade/system/bootstrapmcps/model/BootstrapMCPConfigFile.java @@ -1,5 +1,7 @@ package com.linkedin.datahub.upgrade.system.bootstrapmcps.model; +import com.fasterxml.jackson.databind.ObjectMapper; +import java.io.IOException; import java.util.List; import javax.annotation.Nonnull; import javax.annotation.Nullable; @@ -7,6 +9,7 @@ import lombok.Builder; import lombok.Data; import lombok.NoArgsConstructor; +import lombok.extern.slf4j.Slf4j; @AllArgsConstructor @NoArgsConstructor @@ -23,6 +26,7 @@ public static class Bootstrap { private List templates; } + @Slf4j @AllArgsConstructor @NoArgsConstructor @Data @@ -36,5 +40,19 @@ public static class MCPTemplate { @Builder.Default private boolean optional = false; @Nonnull private String mcps_location; @Nullable private String values_env; + @Nullable private String revision_env; + + public MCPTemplate withOverride(ObjectMapper objectMapper) { + if (revision_env != null) { + String overrideJson = System.getenv().getOrDefault(revision_env, "{}"); + try { + return objectMapper.readerForUpdating(this).readValue(overrideJson); + } catch (IOException e) { + log.error("Error applying override {} to {}", overrideJson, this); + throw new RuntimeException(e); + } + } + return this; + } } } diff --git a/datahub-upgrade/src/main/java/com/linkedin/datahub/upgrade/system/domaindescription/ReindexDomainDescription.java b/datahub-upgrade/src/main/java/com/linkedin/datahub/upgrade/system/restoreindices/domaindescription/ReindexDomainDescription.java similarity index 94% rename from datahub-upgrade/src/main/java/com/linkedin/datahub/upgrade/system/domaindescription/ReindexDomainDescription.java rename to datahub-upgrade/src/main/java/com/linkedin/datahub/upgrade/system/restoreindices/domaindescription/ReindexDomainDescription.java index 85af912e24f68a..6eb7e831356275 100644 --- a/datahub-upgrade/src/main/java/com/linkedin/datahub/upgrade/system/domaindescription/ReindexDomainDescription.java +++ b/datahub-upgrade/src/main/java/com/linkedin/datahub/upgrade/system/restoreindices/domaindescription/ReindexDomainDescription.java @@ -1,4 +1,4 @@ -package com.linkedin.datahub.upgrade.system.domaindescription; +package com.linkedin.datahub.upgrade.system.restoreindices.domaindescription; import com.google.common.collect.ImmutableList; import com.linkedin.datahub.upgrade.UpgradeStep; diff --git a/datahub-upgrade/src/main/java/com/linkedin/datahub/upgrade/system/domaindescription/ReindexDomainDescriptionStep.java b/datahub-upgrade/src/main/java/com/linkedin/datahub/upgrade/system/restoreindices/domaindescription/ReindexDomainDescriptionStep.java similarity index 93% rename from datahub-upgrade/src/main/java/com/linkedin/datahub/upgrade/system/domaindescription/ReindexDomainDescriptionStep.java rename to datahub-upgrade/src/main/java/com/linkedin/datahub/upgrade/system/restoreindices/domaindescription/ReindexDomainDescriptionStep.java index 1fa8bc92af078f..c452300e7f61ce 100644 --- a/datahub-upgrade/src/main/java/com/linkedin/datahub/upgrade/system/domaindescription/ReindexDomainDescriptionStep.java +++ b/datahub-upgrade/src/main/java/com/linkedin/datahub/upgrade/system/restoreindices/domaindescription/ReindexDomainDescriptionStep.java @@ -1,4 +1,4 @@ -package com.linkedin.datahub.upgrade.system.domaindescription; +package com.linkedin.datahub.upgrade.system.restoreindices.domaindescription; import static com.linkedin.metadata.Constants.*; diff --git a/datahub-upgrade/src/main/java/com/linkedin/datahub/upgrade/system/graph/edgestatus/ReindexEdgeStatus.java b/datahub-upgrade/src/main/java/com/linkedin/datahub/upgrade/system/restoreindices/graph/edgestatus/ReindexEdgeStatus.java similarity index 94% rename from datahub-upgrade/src/main/java/com/linkedin/datahub/upgrade/system/graph/edgestatus/ReindexEdgeStatus.java rename to datahub-upgrade/src/main/java/com/linkedin/datahub/upgrade/system/restoreindices/graph/edgestatus/ReindexEdgeStatus.java index 6b7286a6a06393..7296d0880a8634 100644 --- a/datahub-upgrade/src/main/java/com/linkedin/datahub/upgrade/system/graph/edgestatus/ReindexEdgeStatus.java +++ b/datahub-upgrade/src/main/java/com/linkedin/datahub/upgrade/system/restoreindices/graph/edgestatus/ReindexEdgeStatus.java @@ -1,4 +1,4 @@ -package com.linkedin.datahub.upgrade.system.graph.edgestatus; +package com.linkedin.datahub.upgrade.system.restoreindices.graph.edgestatus; import com.google.common.collect.ImmutableList; import com.linkedin.datahub.upgrade.UpgradeStep; diff --git a/datahub-upgrade/src/main/java/com/linkedin/datahub/upgrade/system/graph/edgestatus/ReindexReindexEdgeStatusStep.java b/datahub-upgrade/src/main/java/com/linkedin/datahub/upgrade/system/restoreindices/graph/edgestatus/ReindexReindexEdgeStatusStep.java similarity index 95% rename from datahub-upgrade/src/main/java/com/linkedin/datahub/upgrade/system/graph/edgestatus/ReindexReindexEdgeStatusStep.java rename to datahub-upgrade/src/main/java/com/linkedin/datahub/upgrade/system/restoreindices/graph/edgestatus/ReindexReindexEdgeStatusStep.java index 6543f82e745635..955442ab85379e 100644 --- a/datahub-upgrade/src/main/java/com/linkedin/datahub/upgrade/system/graph/edgestatus/ReindexReindexEdgeStatusStep.java +++ b/datahub-upgrade/src/main/java/com/linkedin/datahub/upgrade/system/restoreindices/graph/edgestatus/ReindexReindexEdgeStatusStep.java @@ -1,4 +1,4 @@ -package com.linkedin.datahub.upgrade.system.graph.edgestatus; +package com.linkedin.datahub.upgrade.system.restoreindices.graph.edgestatus; import static com.linkedin.metadata.Constants.STATUS_ASPECT_NAME; diff --git a/datahub-upgrade/src/main/java/com/linkedin/datahub/upgrade/system/graph/vianodes/ReindexDataJobViaNodesCLL.java b/datahub-upgrade/src/main/java/com/linkedin/datahub/upgrade/system/restoreindices/graph/vianodes/ReindexDataJobViaNodesCLL.java similarity index 94% rename from datahub-upgrade/src/main/java/com/linkedin/datahub/upgrade/system/graph/vianodes/ReindexDataJobViaNodesCLL.java rename to datahub-upgrade/src/main/java/com/linkedin/datahub/upgrade/system/restoreindices/graph/vianodes/ReindexDataJobViaNodesCLL.java index 7a4ca9586f155d..75bfcabdb82d3b 100644 --- a/datahub-upgrade/src/main/java/com/linkedin/datahub/upgrade/system/graph/vianodes/ReindexDataJobViaNodesCLL.java +++ b/datahub-upgrade/src/main/java/com/linkedin/datahub/upgrade/system/restoreindices/graph/vianodes/ReindexDataJobViaNodesCLL.java @@ -1,4 +1,4 @@ -package com.linkedin.datahub.upgrade.system.graph.vianodes; +package com.linkedin.datahub.upgrade.system.restoreindices.graph.vianodes; import com.google.common.collect.ImmutableList; import com.linkedin.datahub.upgrade.UpgradeStep; diff --git a/datahub-upgrade/src/main/java/com/linkedin/datahub/upgrade/system/graph/vianodes/ReindexDataJobViaNodesCLLStep.java b/datahub-upgrade/src/main/java/com/linkedin/datahub/upgrade/system/restoreindices/graph/vianodes/ReindexDataJobViaNodesCLLStep.java similarity index 95% rename from datahub-upgrade/src/main/java/com/linkedin/datahub/upgrade/system/graph/vianodes/ReindexDataJobViaNodesCLLStep.java rename to datahub-upgrade/src/main/java/com/linkedin/datahub/upgrade/system/restoreindices/graph/vianodes/ReindexDataJobViaNodesCLLStep.java index e3e07f99bb1ee7..4087e35c0e96bf 100644 --- a/datahub-upgrade/src/main/java/com/linkedin/datahub/upgrade/system/graph/vianodes/ReindexDataJobViaNodesCLLStep.java +++ b/datahub-upgrade/src/main/java/com/linkedin/datahub/upgrade/system/restoreindices/graph/vianodes/ReindexDataJobViaNodesCLLStep.java @@ -1,4 +1,4 @@ -package com.linkedin.datahub.upgrade.system.graph.vianodes; +package com.linkedin.datahub.upgrade.system.restoreindices.graph.vianodes; import static com.linkedin.metadata.Constants.*; diff --git a/datahub-upgrade/src/main/java/com/linkedin/datahub/upgrade/system/restoreindices/structuredproperties/PropertyDefinitions.java b/datahub-upgrade/src/main/java/com/linkedin/datahub/upgrade/system/restoreindices/structuredproperties/PropertyDefinitions.java new file mode 100644 index 00000000000000..2c3a26cdc43dc3 --- /dev/null +++ b/datahub-upgrade/src/main/java/com/linkedin/datahub/upgrade/system/restoreindices/structuredproperties/PropertyDefinitions.java @@ -0,0 +1,49 @@ +package com.linkedin.datahub.upgrade.system.restoreindices.structuredproperties; + +import com.google.common.collect.ImmutableList; +import com.linkedin.datahub.upgrade.UpgradeStep; +import com.linkedin.datahub.upgrade.system.NonBlockingSystemUpgrade; +import com.linkedin.metadata.entity.AspectDao; +import com.linkedin.metadata.entity.EntityService; +import io.datahubproject.metadata.context.OperationContext; +import java.util.List; +import javax.annotation.Nonnull; +import lombok.extern.slf4j.Slf4j; + +/** + * A job that reindexes all domain aspects as part of reindexing descriptions This is required to + * fix the analytics for domains + */ +@Slf4j +public class PropertyDefinitions implements NonBlockingSystemUpgrade { + + private final List _steps; + + public PropertyDefinitions( + @Nonnull OperationContext opContext, + EntityService entityService, + AspectDao aspectDao, + boolean enabled, + Integer batchSize, + Integer batchDelayMs, + Integer limit) { + if (enabled) { + _steps = + ImmutableList.of( + new PropertyDefinitionsStep( + opContext, entityService, aspectDao, batchSize, batchDelayMs, limit)); + } else { + _steps = ImmutableList.of(); + } + } + + @Override + public String id() { + return this.getClass().getName(); + } + + @Override + public List steps() { + return _steps; + } +} diff --git a/datahub-upgrade/src/main/java/com/linkedin/datahub/upgrade/system/restoreindices/structuredproperties/PropertyDefinitionsStep.java b/datahub-upgrade/src/main/java/com/linkedin/datahub/upgrade/system/restoreindices/structuredproperties/PropertyDefinitionsStep.java new file mode 100644 index 00000000000000..1f5e979a163af3 --- /dev/null +++ b/datahub-upgrade/src/main/java/com/linkedin/datahub/upgrade/system/restoreindices/structuredproperties/PropertyDefinitionsStep.java @@ -0,0 +1,42 @@ +package com.linkedin.datahub.upgrade.system.restoreindices.structuredproperties; + +import static com.linkedin.metadata.Constants.*; + +import com.linkedin.datahub.upgrade.system.AbstractMCLStep; +import com.linkedin.metadata.entity.AspectDao; +import com.linkedin.metadata.entity.EntityService; +import io.datahubproject.metadata.context.OperationContext; +import javax.annotation.Nonnull; +import lombok.extern.slf4j.Slf4j; +import org.jetbrains.annotations.Nullable; + +@Slf4j +public class PropertyDefinitionsStep extends AbstractMCLStep { + + public PropertyDefinitionsStep( + OperationContext opContext, + EntityService entityService, + AspectDao aspectDao, + Integer batchSize, + Integer batchDelayMs, + Integer limit) { + super(opContext, entityService, aspectDao, batchSize, batchDelayMs, limit); + } + + @Override + public String id() { + return "structured-property-definitions-v1"; + } + + @Nonnull + @Override + protected String getAspectName() { + return STRUCTURED_PROPERTY_DEFINITION_ASPECT_NAME; + } + + @Nullable + @Override + protected String getUrnLike() { + return "urn:li:" + STRUCTURED_PROPERTY_ENTITY_NAME + ":%"; + } +} diff --git a/datahub-upgrade/src/test/java/com/linkedin/datahub/upgrade/DatahubUpgradeNonBlockingTest.java b/datahub-upgrade/src/test/java/com/linkedin/datahub/upgrade/DatahubUpgradeNonBlockingTest.java index 845d8185273432..f340e688ad7f77 100644 --- a/datahub-upgrade/src/test/java/com/linkedin/datahub/upgrade/DatahubUpgradeNonBlockingTest.java +++ b/datahub-upgrade/src/test/java/com/linkedin/datahub/upgrade/DatahubUpgradeNonBlockingTest.java @@ -12,7 +12,7 @@ import com.linkedin.datahub.upgrade.impl.DefaultUpgradeManager; import com.linkedin.datahub.upgrade.system.SystemUpdateNonBlocking; import com.linkedin.datahub.upgrade.system.bootstrapmcps.BootstrapMCPStep; -import com.linkedin.datahub.upgrade.system.graph.vianodes.ReindexDataJobViaNodesCLL; +import com.linkedin.datahub.upgrade.system.restoreindices.graph.vianodes.ReindexDataJobViaNodesCLL; import com.linkedin.metadata.boot.kafka.MockSystemUpdateDeserializer; import com.linkedin.metadata.boot.kafka.MockSystemUpdateSerializer; import com.linkedin.metadata.config.kafka.KafkaConfiguration; diff --git a/datahub-upgrade/src/test/java/com/linkedin/datahub/upgrade/system/bootstrapmcps/BootstrapMCPUtilTest.java b/datahub-upgrade/src/test/java/com/linkedin/datahub/upgrade/system/bootstrapmcps/BootstrapMCPUtilTest.java index 68023a084bbd20..f914b355fe7802 100644 --- a/datahub-upgrade/src/test/java/com/linkedin/datahub/upgrade/system/bootstrapmcps/BootstrapMCPUtilTest.java +++ b/datahub-upgrade/src/test/java/com/linkedin/datahub/upgrade/system/bootstrapmcps/BootstrapMCPUtilTest.java @@ -4,6 +4,7 @@ import static org.testng.Assert.assertFalse; import static org.testng.Assert.assertTrue; +import com.fasterxml.jackson.databind.ObjectMapper; import com.fasterxml.jackson.databind.node.ObjectNode; import com.linkedin.common.AuditStamp; import com.linkedin.common.urn.UrnUtils; @@ -17,6 +18,7 @@ import io.datahubproject.test.metadata.context.TestOperationContexts; import java.io.IOException; import java.util.List; +import org.testng.annotations.BeforeMethod; import org.testng.annotations.Listeners; import org.testng.annotations.Test; import uk.org.webcompere.systemstubs.environment.EnvironmentVariables; @@ -28,10 +30,17 @@ public class BootstrapMCPUtilTest { static final OperationContext OP_CONTEXT = TestOperationContexts.systemContextNoSearchAuthorization(); private static final String DATAHUB_TEST_VALUES_ENV = "DATAHUB_TEST_VALUES_ENV"; + private static final String DATAHUB_TEST_REVISION_ENV = "DATAHUB_TEST_REVISION_ENV"; private static final AuditStamp TEST_AUDIT_STAMP = AuditStampUtils.createDefaultAuditStamp(); @SystemStub private EnvironmentVariables environmentVariables; + @BeforeMethod + private void resetEnvironment() { + environmentVariables.remove(DATAHUB_TEST_VALUES_ENV); + environmentVariables.remove(DATAHUB_TEST_REVISION_ENV); + } + @Test public void testResolveYamlConf() throws IOException { BootstrapMCPConfigFile initConfig = @@ -51,9 +60,28 @@ public void testResolveYamlConf() throws IOException { } @Test - public void testResolveMCPTemplateDefaults() throws IOException { - environmentVariables.remove(DATAHUB_TEST_VALUES_ENV); + public void testResolveYamlConfOverride() throws IOException { + environmentVariables.set(DATAHUB_TEST_REVISION_ENV, "{\"version\":\"2024110600\"}"); + + BootstrapMCPConfigFile initConfig = + BootstrapMCPUtil.resolveYamlConf( + OP_CONTEXT, "bootstrapmcp/test.yaml", BootstrapMCPConfigFile.class); + assertEquals(initConfig.getBootstrap().getTemplates().size(), 1); + + BootstrapMCPConfigFile.MCPTemplate template = + initConfig.getBootstrap().getTemplates().get(0).withOverride(new ObjectMapper()); + assertEquals(template.getName(), "datahub-test"); + assertEquals(template.getVersion(), "2024110600"); + assertFalse(template.isForce()); + assertFalse(template.isBlocking()); + assertTrue(template.isAsync()); + assertFalse(template.isOptional()); + assertEquals(template.getMcps_location(), "bootstrapmcp/datahub-test-mcp.yaml"); + assertEquals(template.getValues_env(), "DATAHUB_TEST_VALUES_ENV"); + } + @Test + public void testResolveMCPTemplateDefaults() throws IOException { BootstrapMCPConfigFile.MCPTemplate template = BootstrapMCPUtil.resolveYamlConf( OP_CONTEXT, "bootstrapmcp/test.yaml", BootstrapMCPConfigFile.class) @@ -186,8 +214,6 @@ public void testResolveMCPTemplateOverride() throws IOException { @Test public void testMCPBatch() throws IOException { - environmentVariables.remove(DATAHUB_TEST_VALUES_ENV); - BootstrapMCPConfigFile.MCPTemplate template = BootstrapMCPUtil.resolveYamlConf( OP_CONTEXT, "bootstrapmcp/test.yaml", BootstrapMCPConfigFile.class) @@ -219,6 +245,6 @@ public void testMCPBatch() throws IOException { OP_CONTEXT .getObjectMapper() .readTree( - "{\"source\":{\"type\":\"datahub-gc\",\"config\":{\"cleanup_expired_tokens\":false,\"truncate_indices\":true,\"dataprocess_cleanup\":{\"retention_days\":10,\"delete_empty_data_jobs\":true,\"delete_empty_data_flows\":true,\"hard_delete_entities\":false,\"keep_last_n\":5},\"soft_deleted_entities_cleanup\":{\"retention_days\":10}}}}")); + "{\"source\":{\"type\":\"datahub-gc\",\"config\":{\"cleanup_expired_tokens\":false,\"truncate_indices\":true,\"dataprocess_cleanup\":{\"retention_days\":10,\"delete_empty_data_jobs\":true,\"delete_empty_data_flows\":true,\"hard_delete_entities\":false,\"keep_last_n\":5},\"soft_deleted_entities_cleanup\":{\"retention_days\":10},\"execution_request_cleanup\":{\"keep_history_min_count\":10,\"keep_history_max_count\":1000,\"keep_history_max_days\":30,\"batch_read_size\":100,\"enabled\":false}}}}")); } } diff --git a/datahub-upgrade/src/test/resources/bootstrapmcp/datahub-test-mcp.yaml b/datahub-upgrade/src/test/resources/bootstrapmcp/datahub-test-mcp.yaml index d049a807ac1d88..233db06d61c3fc 100644 --- a/datahub-upgrade/src/test/resources/bootstrapmcp/datahub-test-mcp.yaml +++ b/datahub-upgrade/src/test/resources/bootstrapmcp/datahub-test-mcp.yaml @@ -23,7 +23,13 @@ keep_last_n: {{dataprocess_cleanup.keep_last_n}}{{^dataprocess_cleanup.keep_last_n}}5{{/dataprocess_cleanup.keep_last_n}} soft_deleted_entities_cleanup: retention_days: {{soft_deleted_entities_cleanup.retention_days}}{{^soft_deleted_entities_cleanup.retention_days}}10{{/soft_deleted_entities_cleanup.retention_days}} + execution_request_cleanup: + keep_history_min_count: {{execution_request_cleanup.keep_history_min_count}}{{^execution_request_cleanup.keep_history_min_count}}10{{/execution_request_cleanup.keep_history_min_count}} + keep_history_max_count: {{execution_request_cleanup.keep_history_max_count}}{{^execution_request_cleanup.keep_history_max_count}}1000{{/execution_request_cleanup.keep_history_max_count}} + keep_history_max_days: {{execution_request_cleanup.keep_history_max_days}}{{^execution_request_cleanup.keep_history_max_days}}30{{/execution_request_cleanup.keep_history_max_days}} + batch_read_size: {{execution_request_cleanup.batch_read_size}}{{^execution_request_cleanup.batch_read_size}}100{{/execution_request_cleanup.batch_read_size}} + enabled: {{execution_request_cleanup.enabled}}{{^execution_request_cleanup.enabled}}false{{/execution_request_cleanup.enabled}} extraArgs: {} debugMode: false executorId: default - headers: {} \ No newline at end of file + headers: {} diff --git a/datahub-upgrade/src/test/resources/bootstrapmcp/test.yaml b/datahub-upgrade/src/test/resources/bootstrapmcp/test.yaml index 649cc09632fc2a..5718ea3ac0e048 100644 --- a/datahub-upgrade/src/test/resources/bootstrapmcp/test.yaml +++ b/datahub-upgrade/src/test/resources/bootstrapmcp/test.yaml @@ -6,4 +6,5 @@ bootstrap: # blocking: false # async: true mcps_location: "bootstrapmcp/datahub-test-mcp.yaml" - values_env: "DATAHUB_TEST_VALUES_ENV" \ No newline at end of file + values_env: "DATAHUB_TEST_VALUES_ENV" + revision_env: "DATAHUB_TEST_REVISION_ENV" \ No newline at end of file diff --git a/datahub-web-react/src/app/entity/container/ContainerEntity.tsx b/datahub-web-react/src/app/entity/container/ContainerEntity.tsx index 9cd32cf33a013f..89f9122c6287f8 100644 --- a/datahub-web-react/src/app/entity/container/ContainerEntity.tsx +++ b/datahub-web-react/src/app/entity/container/ContainerEntity.tsx @@ -8,7 +8,7 @@ import { DocumentationTab } from '../shared/tabs/Documentation/DocumentationTab' import { SidebarAboutSection } from '../shared/containers/profile/sidebar/AboutSection/SidebarAboutSection'; import { SidebarOwnerSection } from '../shared/containers/profile/sidebar/Ownership/sidebar/SidebarOwnerSection'; import { getDataForEntityType } from '../shared/containers/profile/utils'; -import { useGetContainerQuery } from '../../../graphql/container.generated'; +import { useGetContainerQuery, GetContainerQuery } from '../../../graphql/container.generated'; import { ContainerEntitiesTab } from './ContainerEntitiesTab'; import { SidebarTagsSection } from '../shared/containers/profile/sidebar/SidebarTagsSection'; import { PropertiesTab } from '../shared/tabs/Properties/PropertiesTab'; @@ -17,6 +17,8 @@ import { capitalizeFirstLetterOnly } from '../../shared/textUtil'; import DataProductSection from '../shared/containers/profile/sidebar/DataProduct/DataProductSection'; import { getDataProduct } from '../shared/utils'; import EmbeddedProfile from '../shared/embed/EmbeddedProfile'; +import AccessManagement from '../shared/tabs/Dataset/AccessManagement/AccessManagement'; +import { useAppConfig } from '../../useAppConfig'; /** * Definition of the DataHub Container entity. @@ -65,6 +67,8 @@ export class ContainerEntity implements Entity { useEntityQuery = useGetContainerQuery; + appconfig = useAppConfig; + renderProfile = (urn: string) => ( { name: 'Properties', component: PropertiesTab, }, + { + name: 'Access Management', + component: AccessManagement, + display: { + visible: (_, container: GetContainerQuery) => { + return ( + this.appconfig().config.featureFlags.showAccessManagement && + !!container?.container?.access + ); + }, + enabled: (_, container: GetContainerQuery) => { + const accessAspect = container?.container?.access; + const rolesList = accessAspect?.roles; + return !!accessAspect && !!rolesList && rolesList.length > 0; + }, + }, + }, ]} sidebarSections={this.getSidebarSections()} /> diff --git a/datahub-web-react/src/app/entity/schemaField/SchemaFieldPropertiesEntity.tsx b/datahub-web-react/src/app/entity/schemaField/SchemaFieldPropertiesEntity.tsx index 7e74b43e68afbb..88743012ddbc8a 100644 --- a/datahub-web-react/src/app/entity/schemaField/SchemaFieldPropertiesEntity.tsx +++ b/datahub-web-react/src/app/entity/schemaField/SchemaFieldPropertiesEntity.tsx @@ -1,9 +1,10 @@ import * as React from 'react'; import { PicCenterOutlined } from '@ant-design/icons'; -import { EntityType, SchemaFieldEntity, SearchResult } from '../../../types.generated'; +import { Dataset, EntityType, SchemaFieldEntity, SearchResult } from '../../../types.generated'; import { Entity, IconStyleType, PreviewType } from '../Entity'; import { getDataForEntityType } from '../shared/containers/profile/utils'; import { Preview } from './preview/Preview'; +import { capitalizeFirstLetterOnly } from '../../shared/textUtil'; export class SchemaFieldPropertiesEntity implements Entity { type: EntityType = EntityType.SchemaField; @@ -18,6 +19,16 @@ export class SchemaFieldPropertiesEntity implements Entity { isLineageEnabled = () => false; + getParentDataset = (parent) => { + return { + urn: parent?.urn, + name: parent?.name, + type: parent?.type, + platform: parent?.platfrom, + properties: parent?.properties, + } as Dataset; + }; + // Currently unused. getAutoCompleteFieldName = () => 'schemaField'; @@ -33,9 +44,23 @@ export class SchemaFieldPropertiesEntity implements Entity { // Currently unused. renderProfile = (_: string) => <>; - renderPreview = (previewType: PreviewType, data: SchemaFieldEntity) => ( - - ); + renderPreview = (previewType: PreviewType, data: SchemaFieldEntity) => { + const parent = data.parent as Dataset; + return ( + + ); + }; renderSearch = (result: SearchResult) => this.renderPreview(PreviewType.SEARCH, result.entity as SchemaFieldEntity); diff --git a/datahub-web-react/src/app/entity/schemaField/preview/Preview.tsx b/datahub-web-react/src/app/entity/schemaField/preview/Preview.tsx index b22e988c76672c..086b1a38021889 100644 --- a/datahub-web-react/src/app/entity/schemaField/preview/Preview.tsx +++ b/datahub-web-react/src/app/entity/schemaField/preview/Preview.tsx @@ -1,6 +1,6 @@ import React from 'react'; import { PicCenterOutlined } from '@ant-design/icons'; -import { EntityType, Owner } from '../../../../types.generated'; +import { Dataset, EntityType, Owner, ParentContainersResult } from '../../../../types.generated'; import DefaultPreviewCard from '../../../preview/DefaultPreviewCard'; import { useEntityRegistry } from '../../../useEntityRegistry'; import { IconStyleType, PreviewType } from '../../Entity'; @@ -11,12 +11,22 @@ export const Preview = ({ description, owners, previewType, + parentContainers, + platformName, + platformLogo, + platformInstanceId, + parentDataset, }: { datasetUrn: string; name: string; description?: string | null; owners?: Array | null; previewType: PreviewType; + parentContainers?: ParentContainersResult | null; + platformName?: string; + platformLogo?: string | null; + platformInstanceId?: string; + parentDataset?: Dataset; }): JSX.Element => { const entityRegistry = useEntityRegistry(); @@ -35,6 +45,11 @@ export const Preview = ({ logoComponent={} type="Column" typeIcon={entityRegistry.getIcon(EntityType.SchemaField, 14, IconStyleType.ACCENT)} + logoUrl={platformLogo || ''} + platform={platformName} + platformInstanceId={platformInstanceId} + parentContainers={parentContainers} + parentDataset={parentDataset} /> ); }; diff --git a/datahub-web-react/src/app/entity/shared/containers/profile/header/PlatformContent/DatasetLink.tsx b/datahub-web-react/src/app/entity/shared/containers/profile/header/PlatformContent/DatasetLink.tsx new file mode 100644 index 00000000000000..cf4891acd384de --- /dev/null +++ b/datahub-web-react/src/app/entity/shared/containers/profile/header/PlatformContent/DatasetLink.tsx @@ -0,0 +1,46 @@ +import React from 'react'; +import styled from 'styled-components'; +import { Link } from 'react-router-dom'; +import { Typography } from 'antd'; +import { Dataset, EntityType } from '../../../../../../../types.generated'; +import { ANTD_GRAY } from '../../../../constants'; +import { useEntityRegistry } from '../../../../../../useEntityRegistry'; +import { IconStyleType } from '../../../../../Entity'; + +const DatasetText = styled(Typography.Text)` + font-size: 12px; + line-height: 20px; + color: ${ANTD_GRAY[7]}; +`; + +export const DatasetIcon = styled.span` + color: ${ANTD_GRAY[7]}; + margin-right: 4px; + font-size: 12px; +`; + +const StyledLink = styled(Link)` + white-space: nowrap; +`; + +interface Props { + parentDataset: Dataset; +} + +function DatasetLink(props: Props) { + const { parentDataset } = props; + const entityRegistry = useEntityRegistry(); + if (!parentDataset) return null; + + const datasetUrl = entityRegistry.getEntityUrl(EntityType.Dataset, parentDataset.urn); + const datasetName = entityRegistry.getDisplayName(EntityType.Dataset, parentDataset); + + return ( + + {entityRegistry.getIcon(EntityType.Dataset, 14, IconStyleType.ACCENT)} + {datasetName} + + ); +} + +export default DatasetLink; diff --git a/datahub-web-react/src/app/entity/shared/containers/profile/header/PlatformContent/PlatformContentView.tsx b/datahub-web-react/src/app/entity/shared/containers/profile/header/PlatformContent/PlatformContentView.tsx index 3104dfd911aabb..70cc5fa3b36d13 100644 --- a/datahub-web-react/src/app/entity/shared/containers/profile/header/PlatformContent/PlatformContentView.tsx +++ b/datahub-web-react/src/app/entity/shared/containers/profile/header/PlatformContent/PlatformContentView.tsx @@ -2,9 +2,10 @@ import React from 'react'; import styled from 'styled-components'; import { Typography, Image } from 'antd'; import { Maybe } from 'graphql/jsutils/Maybe'; -import { Container, Entity } from '../../../../../../../types.generated'; +import { Container, Dataset, Entity } from '../../../../../../../types.generated'; import { ANTD_GRAY } from '../../../../constants'; import ContainerLink from './ContainerLink'; +import DatasetLink from './DatasetLink'; import { StyledRightOutlined, ParentNodesWrapper as ParentContainersWrapper, @@ -80,6 +81,7 @@ interface Props { parentEntities?: Entity[] | null; parentContainersRef: React.RefObject; areContainersTruncated: boolean; + parentDataset?: Dataset; } function PlatformContentView(props: Props) { @@ -96,6 +98,7 @@ function PlatformContentView(props: Props) { parentContainers, parentContainersRef, areContainersTruncated, + parentDataset, } = props; const directParentContainer = parentContainers && parentContainers[0]; @@ -152,6 +155,12 @@ function PlatformContentView(props: Props) { {directParentContainer && } + {parentDataset && ( + + + + + )} ); diff --git a/datahub-web-react/src/app/entity/shared/tabs/Dataset/AccessManagement/AccessManagement.tsx b/datahub-web-react/src/app/entity/shared/tabs/Dataset/AccessManagement/AccessManagement.tsx index 5de532d17a6769..f5789447d953db 100644 --- a/datahub-web-react/src/app/entity/shared/tabs/Dataset/AccessManagement/AccessManagement.tsx +++ b/datahub-web-react/src/app/entity/shared/tabs/Dataset/AccessManagement/AccessManagement.tsx @@ -3,8 +3,8 @@ import styled from 'styled-components'; import { Button, Table } from 'antd'; import { SpinProps } from 'antd/es/spin'; import { LoadingOutlined } from '@ant-design/icons'; -import { useBaseEntity } from '../../../EntityContext'; -import { GetDatasetQuery, useGetExternalRolesQuery } from '../../../../../../graphql/dataset.generated'; +import { useEntityData } from '../../../EntityContext'; +import { useGetExternalRolesQuery } from '../../../../../../graphql/dataset.generated'; import { handleAccessRoles } from './utils'; import AccessManagerDescription from './AccessManagerDescription'; @@ -60,11 +60,12 @@ const AccessButton = styled(Button)` `; export default function AccessManagement() { - const baseEntity = useBaseEntity(); + const { entityData } = useEntityData(); + const entityUrn = (entityData as any)?.urn; const { data: externalRoles, loading: isLoading } = useGetExternalRolesQuery({ - variables: { urn: baseEntity?.dataset?.urn as string }, - skip: !baseEntity?.dataset?.urn, + variables: { urn: entityUrn as string }, + skip: !entityUrn, }); const columns = [ diff --git a/datahub-web-react/src/app/entity/shared/tabs/Incident/components/AddIncidentModal.tsx b/datahub-web-react/src/app/entity/shared/tabs/Incident/components/AddIncidentModal.tsx index 9f5047b3c73fe2..9ae03fce144235 100644 --- a/datahub-web-react/src/app/entity/shared/tabs/Incident/components/AddIncidentModal.tsx +++ b/datahub-web-react/src/app/entity/shared/tabs/Incident/components/AddIncidentModal.tsx @@ -1,7 +1,7 @@ import React, { useState } from 'react'; import { message, Modal, Button, Form, Input, Typography, Select } from 'antd'; import { useApolloClient } from '@apollo/client'; -import TextArea from 'antd/lib/input/TextArea'; +import styled from 'styled-components'; import analytics, { EventType, EntityActionType } from '../../../../../analytics'; import { useEntityData } from '../../../EntityContext'; import { EntityType, IncidentSourceType, IncidentState, IncidentType } from '../../../../../../types.generated'; @@ -9,6 +9,12 @@ import { INCIDENT_DISPLAY_TYPES, PAGE_SIZE, addActiveIncidentToCache } from '../ import { useRaiseIncidentMutation } from '../../../../../../graphql/mutations.generated'; import handleGraphQLError from '../../../../../shared/handleGraphQLError'; import { useUserContext } from '../../../../../context/useUserContext'; +import { Editor } from '../../Documentation/components/editor/Editor'; +import { ANTD_GRAY } from '../../../constants'; + +const StyledEditor = styled(Editor)` + border: 1px solid ${ANTD_GRAY[4.5]}; +`; type AddIncidentProps = { open: boolean; @@ -113,6 +119,7 @@ export const AddIncidentModal = ({ open, onClose, refetch }: AddIncidentProps) = open={open} destroyOnClose onCancel={handleClose} + width={600} footer={[