From d13145e32debff221f39208ca30cd4c47328e8be Mon Sep 17 00:00:00 2001 From: Aditya Radhakrishnan Date: Thu, 22 Sep 2022 18:26:42 -0700 Subject: [PATCH] fix(frontend): refactoring AuthServiceClient (#6029) --- .../app/client/AuthServiceClient.java | 49 ++++++---- .../controllers/AuthenticationController.java | 92 +++++++++---------- datahub-frontend/conf/routes | 8 +- 3 files changed, 80 insertions(+), 69 deletions(-) diff --git a/datahub-frontend/app/client/AuthServiceClient.java b/datahub-frontend/app/client/AuthServiceClient.java index e32064f608..c7b443b1ec 100644 --- a/datahub-frontend/app/client/AuthServiceClient.java +++ b/datahub-frontend/app/client/AuthServiceClient.java @@ -1,6 +1,8 @@ package client; +import com.datahub.authentication.Authentication; import com.fasterxml.jackson.databind.ObjectMapper; +import com.fasterxml.jackson.databind.node.ObjectNode; import java.util.Objects; import javax.annotation.Nonnull; import lombok.extern.slf4j.Slf4j; @@ -13,7 +15,6 @@ import org.apache.http.impl.client.CloseableHttpClient; import org.apache.http.impl.client.HttpClients; import org.apache.http.util.EntityUtils; import play.mvc.Http; -import com.datahub.authentication.Authentication; /** @@ -66,11 +67,15 @@ public class AuthServiceClient { try { final String protocol = this.metadataServiceUseSsl ? "https" : "http"; - final HttpPost request = new HttpPost(String.format("%s://%s:%s/%s", protocol, this.metadataServiceHost, - this.metadataServicePort, GENERATE_SESSION_TOKEN_ENDPOINT)); + final HttpPost request = new HttpPost( + String.format("%s://%s:%s/%s", protocol, this.metadataServiceHost, this.metadataServicePort, + GENERATE_SESSION_TOKEN_ENDPOINT)); // Build JSON request to generate a token on behalf of a user. - String json = String.format("{ \"%s\":\"%s\" }", USER_ID_FIELD, userId); + final ObjectMapper objectMapper = new ObjectMapper(); + final ObjectNode objectNode = objectMapper.createObjectNode(); + objectNode.put(USER_ID_FIELD, userId); + final String json = objectMapper.writerWithDefaultPrettyPrinter().writeValueAsString(objectNode); request.setEntity(new StringEntity(json)); // Add authorization header with DataHub frontend system id and secret. @@ -101,7 +106,6 @@ public class AuthServiceClient { /** * Call the Auth Service to create a native Datahub user. */ - @Nonnull public boolean signUp(@Nonnull final String userUrn, @Nonnull final String fullName, @Nonnull final String email, @Nonnull final String title, @Nonnull final String password, @Nonnull final String inviteToken) { Objects.requireNonNull(userUrn, "userUrn must not be null"); @@ -115,15 +119,20 @@ public class AuthServiceClient { try { final String protocol = this.metadataServiceUseSsl ? "https" : "http"; - final HttpPost request = - new HttpPost(String.format("%s://%s:%s/%s", protocol, this.metadataServiceHost, this.metadataServicePort, + final HttpPost request = new HttpPost( + String.format("%s://%s:%s/%s", protocol, this.metadataServiceHost, this.metadataServicePort, SIGN_UP_ENDPOINT)); // Build JSON request to verify credentials for a native user. - String json = - String.format("{ \"%s\":\"%s\", \"%s\":\"%s\", \"%s\":\"%s\", \"%s\":\"%s\", \"%s\":\"%s\", \"%s\":\"%s\" }", - USER_URN_FIELD, userUrn, FULL_NAME_FIELD, fullName, EMAIL_FIELD, email, TITLE_FIELD, title, - PASSWORD_FIELD, password, INVITE_TOKEN_FIELD, inviteToken); + final ObjectMapper objectMapper = new ObjectMapper(); + final ObjectNode objectNode = objectMapper.createObjectNode(); + objectNode.put(USER_URN_FIELD, userUrn); + objectNode.put(FULL_NAME_FIELD, fullName); + objectNode.put(EMAIL_FIELD, email); + objectNode.put(TITLE_FIELD, title); + objectNode.put(PASSWORD_FIELD, password); + objectNode.put(INVITE_TOKEN_FIELD, inviteToken); + final String json = objectMapper.writerWithDefaultPrettyPrinter().writeValueAsString(objectNode); request.setEntity(new StringEntity(json)); // Add authorization header with DataHub frontend system id and secret. @@ -154,7 +163,6 @@ public class AuthServiceClient { /** * Call the Auth Service to reset credentials for a native DataHub user. */ - @Nonnull public boolean resetNativeUserCredentials(@Nonnull final String userUrn, @Nonnull final String password, @Nonnull final String resetToken) { Objects.requireNonNull(userUrn, "userUrn must not be null"); @@ -170,9 +178,12 @@ public class AuthServiceClient { RESET_NATIVE_USER_CREDENTIALS_ENDPOINT)); // Build JSON request to verify credentials for a native user. - String json = - String.format("{ \"%s\":\"%s\", \"%s\":\"%s\", \"%s\":\"%s\" }", USER_URN_FIELD, userUrn, - PASSWORD_FIELD, password, RESET_TOKEN_FIELD, resetToken); + final ObjectMapper objectMapper = new ObjectMapper(); + final ObjectNode objectNode = objectMapper.createObjectNode(); + objectNode.put(USER_URN_FIELD, userUrn); + objectNode.put(PASSWORD_FIELD, password); + objectNode.put(RESET_TOKEN_FIELD, resetToken); + final String json = objectMapper.writerWithDefaultPrettyPrinter().writeValueAsString(objectNode); request.setEntity(new StringEntity(json)); // Add authorization header with DataHub frontend system id and secret. @@ -203,7 +214,6 @@ public class AuthServiceClient { /** * Call the Auth Service to verify the credentials for a native Datahub user. */ - @Nonnull public boolean verifyNativeUserCredentials(@Nonnull final String userUrn, @Nonnull final String password) { Objects.requireNonNull(userUrn, "userUrn must not be null"); Objects.requireNonNull(password, "password must not be null"); @@ -217,8 +227,11 @@ public class AuthServiceClient { VERIFY_NATIVE_USER_CREDENTIALS_ENDPOINT)); // Build JSON request to verify credentials for a native user. - String json = - String.format("{ \"%s\":\"%s\", \"%s\":\"%s\" }", USER_URN_FIELD, userUrn, PASSWORD_FIELD, password); + final ObjectMapper objectMapper = new ObjectMapper(); + final ObjectNode objectNode = objectMapper.createObjectNode(); + objectNode.put(USER_URN_FIELD, userUrn); + objectNode.put(PASSWORD_FIELD, password); + final String json = objectMapper.writerWithDefaultPrettyPrinter().writeValueAsString(objectNode); request.setEntity(new StringEntity(json)); // Add authorization header with DataHub frontend system id and secret. diff --git a/datahub-frontend/app/controllers/AuthenticationController.java b/datahub-frontend/app/controllers/AuthenticationController.java index 6317c4f950..023eda4533 100644 --- a/datahub-frontend/app/controllers/AuthenticationController.java +++ b/datahub-frontend/app/controllers/AuthenticationController.java @@ -1,5 +1,9 @@ package controllers; +import auth.AuthUtils; +import auth.JAASConfigs; +import auth.NativeAuthenticationConfigs; +import auth.sso.SsoManager; import client.AuthServiceClient; import com.fasterxml.jackson.databind.JsonNode; import com.fasterxml.jackson.databind.node.ObjectNode; @@ -8,7 +12,13 @@ import com.linkedin.common.urn.Urn; import com.typesafe.config.Config; import java.io.UnsupportedEncodingException; import java.net.URLEncoder; +import java.time.Duration; +import java.time.temporal.ChronoUnit; +import java.util.HashMap; +import java.util.Map; import java.util.Optional; +import javax.annotation.Nonnull; +import javax.inject.Inject; import org.apache.commons.lang3.StringUtils; import org.pac4j.core.client.Client; import org.pac4j.core.context.session.SessionStore; @@ -21,18 +31,8 @@ import play.libs.Json; import play.mvc.Controller; import play.mvc.Http; import play.mvc.Result; -import auth.AuthUtils; -import auth.JAASConfigs; -import auth.NativeAuthenticationConfigs; -import auth.sso.SsoManager; import security.AuthenticationManager; -import javax.annotation.Nonnull; -import javax.inject.Inject; - -import java.time.Duration; -import java.time.temporal.ChronoUnit; - import static auth.AuthUtils.*; import static org.pac4j.core.client.IndirectClient.*; @@ -70,14 +70,14 @@ public class AuthenticationController extends Controller { * Route used to perform authentication, or redirect to log in if authentication fails. * * If indirect SSO (eg. oidc) is configured, this route will redirect to the identity provider (Indirect auth). - * If not, we will fallback to the default username / password login experience (Direct auth). + * If not, we will fall back to the default username / password login experience (Direct auth). */ @Nonnull - public Result authenticate() { + public Result authenticate(Http.Request request) { // TODO: Call getAuthenticatedUser and then generate a session cookie for the UI if the user is authenticated. - final Optional maybeRedirectPath = Optional.ofNullable(ctx().request().getQueryString(AUTH_REDIRECT_URI_PARAM)); + final Optional maybeRedirectPath = Optional.ofNullable(request.getQueryString(AUTH_REDIRECT_URI_PARAM)); final String redirectPath = maybeRedirectPath.orElse("/"); if (AuthUtils.hasValidSessionCookie(ctx())) { @@ -98,10 +98,9 @@ public class AuthenticationController extends Controller { // 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()); - session().put(ACCESS_TOKEN, accessToken); - session().put(ACTOR, DEFAULT_ACTOR_URN.toString()); - return redirect(redirectPath).withCookies(createActorCookie(DEFAULT_ACTOR_URN.toString(), _configs.hasPath(SESSION_TTL_CONFIG_PATH) - ? _configs.getInt(SESSION_TTL_CONFIG_PATH) + request.session().adding(createSessionMap(DEFAULT_ACTOR_URN.toString(), accessToken)); + return redirect(redirectPath).withCookies(createActorCookie(DEFAULT_ACTOR_URN.toString(), + _configs.hasPath(SESSION_TTL_CONFIG_PATH) ? _configs.getInt(SESSION_TTL_CONFIG_PATH) : DEFAULT_SESSION_TTL_HOURS)); } @@ -111,7 +110,7 @@ public class AuthenticationController extends Controller { * TODO: Implement built-in support for LDAP auth. Currently dummy jaas authentication is the default. */ @Nonnull - public Result logIn() { + public Result logIn(Http.Request request) { boolean jaasEnabled = _jaasConfigs.isJAASEnabled(); _logger.debug(String.format("Jaas authentication enabled: %b", jaasEnabled)); boolean nativeAuthenticationEnabled = _nativeAuthenticationConfigs.isNativeAuthenticationEnabled(); @@ -124,7 +123,7 @@ public class AuthenticationController extends Controller { return badRequest(error); } - final JsonNode json = request().body().asJson(); + final JsonNode json = request.body().asJson(); final String username = json.findPath(USER_NAME).textValue(); final String password = json.findPath(PASSWORD).textValue(); @@ -133,8 +132,6 @@ public class AuthenticationController extends Controller { return badRequest(invalidCredsJson); } - ctx().session().clear(); - JsonNode invalidCredsJson = Json.newObject().put("message", "Invalid Credentials"); boolean loginSucceeded = tryLogin(username, password); @@ -144,12 +141,12 @@ public class AuthenticationController extends Controller { final Urn actorUrn = new CorpuserUrn(username); final String accessToken = _authClient.generateSessionTokenForUser(actorUrn.getId()); - ctx().session().put(ACTOR, actorUrn.toString()); - ctx().session().put(ACCESS_TOKEN, accessToken); - return ok().withCookies(Http.Cookie.builder(ACTOR, actorUrn.toString()) - .withHttpOnly(false) - .withMaxAge(Duration.of(30, ChronoUnit.DAYS)) - .build()); + Result result = ok().withSession(createSessionMap(actorUrn.toString(), accessToken)) + .withCookies(Http.Cookie.builder(ACTOR, actorUrn.toString()) + .withHttpOnly(false) + .withMaxAge(Duration.of(30, ChronoUnit.DAYS)) + .build()); + return result; } /** @@ -158,7 +155,7 @@ public class AuthenticationController extends Controller { * */ @Nonnull - public Result signUp() { + public Result signUp(Http.Request request) { boolean nativeAuthenticationEnabled = _nativeAuthenticationConfigs.isNativeAuthenticationEnabled(); _logger.debug(String.format("Native authentication enabled: %b", nativeAuthenticationEnabled)); if (!nativeAuthenticationEnabled) { @@ -168,7 +165,7 @@ public class AuthenticationController extends Controller { return badRequest(error); } - final JsonNode json = request().body().asJson(); + final JsonNode json = request.body().asJson(); final String fullName = json.findPath(FULL_NAME).textValue(); final String email = json.findPath(EMAIL).textValue(); final String title = json.findPath(TITLE).textValue(); @@ -200,18 +197,15 @@ public class AuthenticationController extends Controller { return badRequest(invalidCredsJson); } - ctx().session().clear(); - final Urn userUrn = new CorpuserUrn(email); final String userUrnString = userUrn.toString(); boolean isNativeUserCreated = _authClient.signUp(userUrnString, fullName, email, title, password, inviteToken); final String accessToken = _authClient.generateSessionTokenForUser(userUrn.getId()); - ctx().session().put(ACTOR, userUrnString); - ctx().session().put(ACCESS_TOKEN, accessToken); - return ok().withCookies(Http.Cookie.builder(ACTOR, userUrnString) - .withHttpOnly(false) - .withMaxAge(Duration.of(30, ChronoUnit.DAYS)) - .build()); + return ok().withSession(createSessionMap(userUrnString, accessToken)) + .withCookies(Http.Cookie.builder(ACTOR, userUrnString) + .withHttpOnly(false) + .withMaxAge(Duration.of(30, ChronoUnit.DAYS)) + .build()); } /** @@ -219,7 +213,7 @@ public class AuthenticationController extends Controller { * */ @Nonnull - public Result resetNativeUserCredentials() { + public Result resetNativeUserCredentials(Http.Request request) { boolean nativeAuthenticationEnabled = _nativeAuthenticationConfigs.isNativeAuthenticationEnabled(); _logger.debug(String.format("Native authentication enabled: %b", nativeAuthenticationEnabled)); if (!nativeAuthenticationEnabled) { @@ -229,7 +223,7 @@ public class AuthenticationController extends Controller { return badRequest(error); } - final JsonNode json = request().body().asJson(); + final JsonNode json = request.body().asJson(); final String email = json.findPath(EMAIL).textValue(); final String password = json.findPath(PASSWORD).textValue(); final String resetToken = json.findPath(RESET_TOKEN).textValue(); @@ -249,20 +243,17 @@ public class AuthenticationController extends Controller { return badRequest(invalidCredsJson); } - ctx().session().clear(); - final Urn userUrn = new CorpuserUrn(email); final String userUrnString = userUrn.toString(); boolean areNativeUserCredentialsReset = _authClient.resetNativeUserCredentials(userUrnString, password, resetToken); _logger.debug(String.format("Are native user credentials reset: %b", areNativeUserCredentialsReset)); final String accessToken = _authClient.generateSessionTokenForUser(userUrn.getId()); - ctx().session().put(ACTOR, userUrnString); - ctx().session().put(ACCESS_TOKEN, accessToken); - return ok().withCookies(Http.Cookie.builder(ACTOR, userUrnString) - .withHttpOnly(false) - .withMaxAge(Duration.of(30, ChronoUnit.DAYS)) - .build()); + return ok().withSession(createSessionMap(userUrnString, accessToken)) + .withCookies(Http.Cookie.builder(ACTOR, userUrnString) + .withHttpOnly(false) + .withMaxAge(Duration.of(30, ChronoUnit.DAYS)) + .build()); } private Result redirectToIdentityProvider() { @@ -321,4 +312,11 @@ public class AuthenticationController extends Controller { return loginSucceeded; } + + private Map createSessionMap(final String userUrnStr, final String accessToken) { + final Map sessionAttributes = new HashMap<>(); + sessionAttributes.put(ACTOR, userUrnStr); + sessionAttributes.put(ACCESS_TOKEN, accessToken); + return sessionAttributes; + } } \ No newline at end of file diff --git a/datahub-frontend/conf/routes b/datahub-frontend/conf/routes index 77d8aef9c4..ef6bd32559 100644 --- a/datahub-frontend/conf/routes +++ b/datahub-frontend/conf/routes @@ -12,10 +12,10 @@ GET /config co # Routes used exclusively by the React application. # Authentication in React -GET /authenticate controllers.AuthenticationController.authenticate() -POST /logIn controllers.AuthenticationController.logIn() -POST /signUp controllers.AuthenticationController.signUp() -POST /resetNativeUserCredentials controllers.AuthenticationController.resetNativeUserCredentials() +GET /authenticate controllers.AuthenticationController.authenticate(request: Request) +POST /logIn controllers.AuthenticationController.logIn(request: Request) +POST /signUp controllers.AuthenticationController.signUp(request: Request) +POST /resetNativeUserCredentials controllers.AuthenticationController.resetNativeUserCredentials(request: Request) GET /callback/:protocol controllers.SsoCallbackController.handleCallback(protocol: String) POST /callback/:protocol controllers.SsoCallbackController.handleCallback(protocol: String) GET /logOut controllers.CentralLogoutController.executeLogout()