From fd89047ee23db81f4b96f1e29e1871aa996338f2 Mon Sep 17 00:00:00 2001 From: Chris Collins Date: Tue, 14 Feb 2023 13:36:47 -0500 Subject: [PATCH] fix(sso) Retrieve cookie configs separately from SSO configs (#7330) --- datahub-frontend/app/auth/AuthModule.java | 5 ++- datahub-frontend/app/auth/AuthUtils.java | 8 ---- datahub-frontend/app/auth/CookieConfigs.java | 38 ++++++++++++++++ datahub-frontend/app/auth/sso/SsoConfigs.java | 28 ------------ .../app/auth/sso/oidc/OidcCallbackLogic.java | 11 +++-- .../controllers/AuthenticationController.java | 44 +++++++++---------- .../controllers/SsoCallbackController.java | 10 +++-- 7 files changed, 74 insertions(+), 70 deletions(-) create mode 100644 datahub-frontend/app/auth/CookieConfigs.java diff --git a/datahub-frontend/app/auth/AuthModule.java b/datahub-frontend/app/auth/AuthModule.java index e62118859376f..eb95078b1a640 100644 --- a/datahub-frontend/app/auth/AuthModule.java +++ b/datahub-frontend/app/auth/AuthModule.java @@ -105,9 +105,10 @@ protected void configure() { SsoManager.class, Authentication.class, EntityClient.class, - AuthServiceClient.class)); + AuthServiceClient.class, + com.typesafe.config.Config.class)); } catch (NoSuchMethodException | SecurityException e) { - throw new RuntimeException("Failed to bind to SsoCallbackController. Cannot find constructor, e"); + throw new RuntimeException("Failed to bind to SsoCallbackController. Cannot find constructor", e); } // logout final LogoutController logoutController = new LogoutController(); diff --git a/datahub-frontend/app/auth/AuthUtils.java b/datahub-frontend/app/auth/AuthUtils.java index f71f1a97f1a3a..80bd631d0db70 100644 --- a/datahub-frontend/app/auth/AuthUtils.java +++ b/datahub-frontend/app/auth/AuthUtils.java @@ -41,16 +41,8 @@ public class AuthUtils { */ public static final String SYSTEM_CLIENT_SECRET_CONFIG_PATH = "systemClientSecret"; - public static final String SESSION_TTL_CONFIG_PATH = "auth.session.ttlInHours"; - - public static final Integer DEFAULT_SESSION_TTL_HOURS = 720; public static final CorpuserUrn DEFAULT_ACTOR_URN = new CorpuserUrn("datahub"); - public static final String AUTH_COOKIE_SAME_SITE = "play.http.session.sameSite"; - public static final String DEFAULT_AUTH_COOKIE_SAME_SITE = "LAX"; - public static final String AUTH_COOKIE_SECURE = "play.http.session.secure"; - public static final boolean DEFAULT_AUTH_COOKIE_SECURE = false; - public static final String LOGIN_ROUTE = "/login"; public static final String USER_NAME = "username"; public static final String PASSWORD = "password"; diff --git a/datahub-frontend/app/auth/CookieConfigs.java b/datahub-frontend/app/auth/CookieConfigs.java new file mode 100644 index 0000000000000..b6da9b7a1833c --- /dev/null +++ b/datahub-frontend/app/auth/CookieConfigs.java @@ -0,0 +1,38 @@ +package auth; + + +import com.typesafe.config.Config; + +public class CookieConfigs { + public static final String SESSION_TTL_CONFIG_PATH = "auth.session.ttlInHours"; + public static final Integer DEFAULT_SESSION_TTL_HOURS = 720; + public static final String AUTH_COOKIE_SAME_SITE = "play.http.session.sameSite"; + public static final String DEFAULT_AUTH_COOKIE_SAME_SITE = "LAX"; + public static final String AUTH_COOKIE_SECURE = "play.http.session.secure"; + public static final boolean DEFAULT_AUTH_COOKIE_SECURE = false; + + private final int _ttlInHours; + private final String _authCookieSameSite; + private final boolean _authCookieSecure; + + public CookieConfigs(final Config configs) { + _ttlInHours = configs.hasPath(SESSION_TTL_CONFIG_PATH) ? configs.getInt(SESSION_TTL_CONFIG_PATH) + : DEFAULT_SESSION_TTL_HOURS; + _authCookieSameSite = configs.hasPath(AUTH_COOKIE_SAME_SITE) ? configs.getString(AUTH_COOKIE_SAME_SITE) + : DEFAULT_AUTH_COOKIE_SAME_SITE; + _authCookieSecure = configs.hasPath(AUTH_COOKIE_SECURE) ? configs.getBoolean(AUTH_COOKIE_SECURE) + : DEFAULT_AUTH_COOKIE_SECURE; + } + + public int getTtlInHours() { + return _ttlInHours; + } + + public String getAuthCookieSameSite() { + return _authCookieSameSite; + } + + public boolean getAuthCookieSecure() { + return _authCookieSecure; + } +} diff --git a/datahub-frontend/app/auth/sso/SsoConfigs.java b/datahub-frontend/app/auth/sso/SsoConfigs.java index e6e0c6a86af60..062054173bddb 100644 --- a/datahub-frontend/app/auth/sso/SsoConfigs.java +++ b/datahub-frontend/app/auth/sso/SsoConfigs.java @@ -1,6 +1,5 @@ package auth.sso; -import static auth.AuthUtils.*; import static auth.ConfigUtil.*; @@ -26,10 +25,7 @@ public class SsoConfigs { private final String _authBaseUrl; private final String _authBaseCallbackPath; private final String _authSuccessRedirectPath; - private final Integer _sessionTtlInHours; private final Boolean _oidcEnabled; - private final String _authCookieSameSite; - private final Boolean _authCookieSecure; public SsoConfigs(final com.typesafe.config.Config configs) { _authBaseUrl = getRequired(configs, AUTH_BASE_URL_CONFIG_PATH); @@ -41,21 +37,9 @@ public SsoConfigs(final com.typesafe.config.Config configs) { configs, AUTH_SUCCESS_REDIRECT_PATH_CONFIG_PATH, DEFAULT_SUCCESS_REDIRECT_PATH); - _sessionTtlInHours = Integer.parseInt(getOptional( - configs, - SESSION_TTL_CONFIG_PATH, - DEFAULT_SESSION_TTL_HOURS.toString())); _oidcEnabled = configs.hasPath(OIDC_ENABLED_CONFIG_PATH) && Boolean.TRUE.equals( Boolean.parseBoolean(configs.getString(OIDC_ENABLED_CONFIG_PATH))); - _authCookieSameSite = getOptional( - configs, - AUTH_COOKIE_SAME_SITE, - DEFAULT_AUTH_COOKIE_SAME_SITE); - _authCookieSecure = Boolean.parseBoolean(getOptional( - configs, - AUTH_COOKIE_SECURE, - String.valueOf(DEFAULT_AUTH_COOKIE_SECURE))); } public String getAuthBaseUrl() { @@ -70,18 +54,6 @@ public String getAuthSuccessRedirectPath() { return _authSuccessRedirectPath; } - public Integer getSessionTtlInHours() { - return _sessionTtlInHours; - } - - public String getAuthCookieSameSite() { - return _authCookieSameSite; - } - - public boolean getAuthCookieSecure() { - return _authCookieSecure; - } - public Boolean isOidcEnabled() { return _oidcEnabled; } diff --git a/datahub-frontend/app/auth/sso/oidc/OidcCallbackLogic.java b/datahub-frontend/app/auth/sso/oidc/OidcCallbackLogic.java index 67a788e1e4df3..85139d1db0868 100644 --- a/datahub-frontend/app/auth/sso/oidc/OidcCallbackLogic.java +++ b/datahub-frontend/app/auth/sso/oidc/OidcCallbackLogic.java @@ -1,5 +1,6 @@ package auth.sso.oidc; +import auth.CookieConfigs; import client.AuthServiceClient; import com.datahub.authentication.Authentication; import com.linkedin.common.AuditStamp; @@ -80,13 +81,15 @@ public class OidcCallbackLogic extends DefaultCallbackLogic handleCallback(String protocol, Http.Request request) { @@ -77,8 +79,8 @@ public class SsoCallbackLogic implements CallbackLogic { private final OidcCallbackLogic _oidcCallbackLogic; SsoCallbackLogic(final SsoManager ssoManager, final Authentication systemAuthentication, - final EntityClient entityClient, final AuthServiceClient authClient) { - _oidcCallbackLogic = new OidcCallbackLogic(ssoManager, systemAuthentication, entityClient, authClient); + final EntityClient entityClient, final AuthServiceClient authClient, final CookieConfigs cookieConfigs) { + _oidcCallbackLogic = new OidcCallbackLogic(ssoManager, systemAuthentication, entityClient, authClient, cookieConfigs); } @Override