fix(sso) Retrieve cookie configs separately from SSO configs (#7330)

datahub-project · Feb 14, 2023 · fd89047 · fd89047
1 parent 3a095f9
commit fd89047
Show file tree

Hide file tree

Showing 7 changed files with 74 additions and 70 deletions.
diff --git 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
@@ -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
@@ -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
@@ -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
@@ -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<Result, PlayWebConte
   private final EntityClient _entityClient;
   private final Authentication _systemAuthentication;
   private final AuthServiceClient _authClient;
+  private final CookieConfigs _cookieConfigs;
 
   public OidcCallbackLogic(final SsoManager ssoManager, final Authentication systemAuthentication,
-      final EntityClient entityClient, final AuthServiceClient authClient) {
+      final EntityClient entityClient, final AuthServiceClient authClient, final CookieConfigs cookieConfigs) {
     _ssoManager = ssoManager;
     _systemAuthentication = systemAuthentication;
     _entityClient = entityClient;
     _authClient = authClient;
+    _cookieConfigs = cookieConfigs;
   }
 
   @Override
@@ -157,9 +160,9 @@ private Result handleOidcCallback(final OidcConfigs oidcConfigs, final Result re
               .withCookies(
                   createActorCookie(
                       corpUserUrn.toString(),
-                      oidcConfigs.getSessionTtlInHours(),
-                      oidcConfigs.getAuthCookieSameSite(),
-                      oidcConfigs.getAuthCookieSecure()
+                      _cookieConfigs.getTtlInHours(),
+                      _cookieConfigs.getAuthCookieSameSite(),
+                      _cookieConfigs.getAuthCookieSecure()
                   )
               );
     }

diff --git a/datahub-frontend/app/controllers/AuthenticationController.java b/datahub-frontend/app/controllers/AuthenticationController.java
@@ -1,6 +1,7 @@
 package controllers;
 
 import auth.AuthUtils;
+import auth.CookieConfigs;
 import auth.JAASConfigs;
 import auth.NativeAuthenticationConfigs;
 import auth.sso.SsoManager;
@@ -32,19 +33,13 @@
 import play.mvc.Results;
 import security.AuthenticationManager;
 
-import static auth.AuthUtils.AUTH_COOKIE_SAME_SITE;
-import static auth.AuthUtils.AUTH_COOKIE_SECURE;
 import static auth.AuthUtils.DEFAULT_ACTOR_URN;
-import static auth.AuthUtils.DEFAULT_AUTH_COOKIE_SAME_SITE;
-import static auth.AuthUtils.DEFAULT_AUTH_COOKIE_SECURE;
-import static auth.AuthUtils.DEFAULT_SESSION_TTL_HOURS;
 import static auth.AuthUtils.EMAIL;
 import static auth.AuthUtils.FULL_NAME;
 import static auth.AuthUtils.INVITE_TOKEN;
 import static auth.AuthUtils.LOGIN_ROUTE;
 import static auth.AuthUtils.PASSWORD;
 import static auth.AuthUtils.RESET_TOKEN;
-import static auth.AuthUtils.SESSION_TTL_CONFIG_PATH;
 import static auth.AuthUtils.TITLE;
 import static auth.AuthUtils.USER_NAME;
 import static auth.AuthUtils.createActorCookie;
@@ -62,7 +57,7 @@ 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 Config _configs;
+    private final CookieConfigs _cookieConfigs;
     private final JAASConfigs _jaasConfigs;
     private final NativeAuthenticationConfigs _nativeAuthenticationConfigs;
 
@@ -80,7 +75,7 @@ public class AuthenticationController extends Controller {
 
     @Inject
     public AuthenticationController(@Nonnull Config configs) {
-        _configs = configs;
+        _cookieConfigs = new CookieConfigs(configs);
         _jaasConfigs = new JAASConfigs(configs);
         _nativeAuthenticationConfigs = new NativeAuthenticationConfigs(configs);
     }
@@ -119,15 +114,15 @@ public Result authenticate(Http.Request request) {
         // 3. If no auth enabled, fallback to using default user account & redirect.
         // Generate GMS session token, TODO:
         final String accessToken = _authClient.generateSessionTokenForUser(DEFAULT_ACTOR_URN.getId());
-        int ttlInHours = _configs.hasPath(SESSION_TTL_CONFIG_PATH) ? _configs.getInt(SESSION_TTL_CONFIG_PATH)
-            : DEFAULT_SESSION_TTL_HOURS;
-        String authCookieSameSite = _configs.hasPath(AUTH_COOKIE_SAME_SITE) ? _configs.getString(AUTH_COOKIE_SAME_SITE)
-            : DEFAULT_AUTH_COOKIE_SAME_SITE;
-        boolean authCookieSecure = _configs.hasPath(AUTH_COOKIE_SECURE) ? _configs.getBoolean(AUTH_COOKIE_SECURE)
-            : DEFAULT_AUTH_COOKIE_SECURE;
-
         return Results.redirect(redirectPath).withSession(createSessionMap(DEFAULT_ACTOR_URN.toString(), accessToken))
-            .withCookies(createActorCookie(DEFAULT_ACTOR_URN.toString(), ttlInHours, authCookieSameSite, authCookieSecure));
+            .withCookies(
+                createActorCookie(
+                    DEFAULT_ACTOR_URN.toString(),
+                    _cookieConfigs.getTtlInHours(),
+                    _cookieConfigs.getAuthCookieSameSite(),
+                    _cookieConfigs.getAuthCookieSecure()
+                )
+            );
     }
 
     /**
@@ -336,14 +331,15 @@ private boolean tryLogin(String username, String password) {
     }
 
     private Result createSession(String userUrnString, String accessToken) {
-        int ttlInHours = _configs.hasPath(SESSION_TTL_CONFIG_PATH) ? _configs.getInt(SESSION_TTL_CONFIG_PATH)
-            : DEFAULT_SESSION_TTL_HOURS;
-        String authCookieSameSite = _configs.hasPath(AUTH_COOKIE_SAME_SITE) ? _configs.getString(AUTH_COOKIE_SAME_SITE)
-            : DEFAULT_AUTH_COOKIE_SAME_SITE;
-        boolean authCookieSecure = _configs.hasPath(AUTH_COOKIE_SECURE) ? _configs.getBoolean(AUTH_COOKIE_SECURE)
-            : DEFAULT_AUTH_COOKIE_SECURE;
-
         return Results.ok().withSession(createSessionMap(userUrnString, accessToken))
-            .withCookies(createActorCookie(userUrnString, ttlInHours, authCookieSameSite,  authCookieSecure));
+            .withCookies(
+                createActorCookie(
+                    userUrnString,
+                    _cookieConfigs.getTtlInHours(),
+                    _cookieConfigs.getAuthCookieSameSite(),
+                    _cookieConfigs.getAuthCookieSecure()
+                )
+            );
+
     }
 }
diff --git a/datahub-frontend/app/controllers/SsoCallbackController.java b/datahub-frontend/app/controllers/SsoCallbackController.java
@@ -1,5 +1,6 @@
 package controllers;
 
+import auth.CookieConfigs;
 import client.AuthServiceClient;
 import com.datahub.authentication.Authentication;
 import com.linkedin.entity.client.EntityClient;
@@ -40,11 +41,12 @@ public SsoCallbackController(
       @Nonnull SsoManager ssoManager,
       @Nonnull Authentication systemAuthentication,
       @Nonnull EntityClient entityClient,
-      @Nonnull AuthServiceClient authClient) {
+      @Nonnull AuthServiceClient authClient,
+      @Nonnull com.typesafe.config.Config configs) {
     _ssoManager = ssoManager;
     setDefaultUrl("/"); // By default, redirects to Home Page on log in.
     setSaveInSession(false);
-    setCallbackLogic(new SsoCallbackLogic(ssoManager, systemAuthentication, entityClient, authClient));
+    setCallbackLogic(new SsoCallbackLogic(ssoManager, systemAuthentication, entityClient, authClient, new CookieConfigs(configs)));
   }
 
   public CompletionStage<Result> handleCallback(String protocol, Http.Request request) {
@@ -77,8 +79,8 @@ public class SsoCallbackLogic implements CallbackLogic<Result, PlayWebContext> {
     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