Skip to content

Commit

Permalink
fix(oidc): fix oidc authentication loop (datahub-project#6848)
Browse files Browse the repository at this point in the history 
* fix(oidc): fix oidc authentication loop
  • Loading branch information
@david-leifker @cccs-Dustin
david-leifker authored and cccs-Dustin committed Feb 1, 2023
1 parent 3fd0dbf commit 5f7b8a8
Show file tree
Hide file tree
Showing 4 changed files with 32 additions and 22 deletions.
13 changes: 11 additions & 2 deletions datahub-frontend/app/auth/AuthUtils.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,8 @@

import java.time.Duration;
import java.time.temporal.ChronoUnit;
import java.util.HashMap;
import java.util.Map;

@Slf4j
public class AuthUtils {
Expand Down Expand Up @@ -77,8 +79,8 @@ public static boolean isEligibleForForwarding(Http.Request req) {
*/
public static boolean hasValidSessionCookie(final Http.Request req) {
return req.session().data().containsKey(ACTOR)
&& req.cookie(ACTOR) != null
&& req.session().data().get(ACTOR).equals(req.cookie(ACTOR).value());
&& req.getCookie(ACTOR).isPresent()
&& req.session().data().get(ACTOR).equals(req.getCookie(ACTOR).get().value());
}

/**
Expand All @@ -101,6 +103,13 @@ public static Http.Cookie createActorCookie(final String actorUrn, final Integer
.build();
}

public static Map<String, String> createSessionMap(final String userUrnStr, final String accessToken) {
final Map<String, String> sessionAttributes = new HashMap<>();
sessionAttributes.put(ACTOR, userUrnStr);
sessionAttributes.put(ACCESS_TOKEN, accessToken);
return sessionAttributes;
}

private AuthUtils() { }

}
12 changes: 6 additions & 6 deletions datahub-frontend/app/auth/sso/oidc/OidcCallbackLogic.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -57,10 +57,10 @@
import play.mvc.Result;
import auth.sso.SsoManager;

import static auth.AuthUtils.ACCESS_TOKEN;
import static auth.AuthUtils.ACTOR;
import static auth.AuthUtils.createActorCookie;
import static com.linkedin.metadata.Constants.*;
import static auth.AuthUtils.createSessionMap;
import static com.linkedin.metadata.Constants.CORP_USER_ENTITY_NAME;
import static com.linkedin.metadata.Constants.GROUP_MEMBERSHIP_ASPECT_NAME;
import static play.mvc.Results.internalServerError;


Expand Down Expand Up @@ -152,9 +152,9 @@ private Result handleOidcCallback(final OidcConfigs oidcConfigs, final Result re

// Successfully logged in - Generate GMS login token
final String accessToken = _authClient.generateSessionTokenForUser(corpUserUrn.getId());
context.getNativeSession().adding(ACCESS_TOKEN, accessToken);
context.getNativeSession().adding(ACTOR, corpUserUrn.toString());
return result.withCookies(createActorCookie(corpUserUrn.toString(), oidcConfigs.getSessionTtlInHours()));
return result
.withSession(createSessionMap(corpUserUrn.toString(), accessToken))
.withCookies(createActorCookie(corpUserUrn.toString(), oidcConfigs.getSessionTtlInHours()));
}
return internalServerError(
"Failed to authenticate current user. Cannot find valid identity provider profile in session.");
Expand Down
12 changes: 1 addition & 11 deletions datahub-frontend/app/controllers/AuthenticationController.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -12,8 +12,6 @@
import com.typesafe.config.Config;
import java.net.URLEncoder;
import java.nio.charset.StandardCharsets;
import java.util.HashMap;
import java.util.Map;
import java.util.Optional;
import javax.annotation.Nonnull;
import javax.inject.Inject;
Expand All @@ -32,8 +30,6 @@
import play.mvc.Results;
import security.AuthenticationManager;

import static auth.AuthUtils.ACCESS_TOKEN;
import static auth.AuthUtils.ACTOR;
import static auth.AuthUtils.DEFAULT_ACTOR_URN;
import static auth.AuthUtils.DEFAULT_SESSION_TTL_HOURS;
import static auth.AuthUtils.EMAIL;
Expand All @@ -46,6 +42,7 @@
import static auth.AuthUtils.TITLE;
import static auth.AuthUtils.USER_NAME;
import static auth.AuthUtils.createActorCookie;
import static auth.AuthUtils.createSessionMap;
import static org.pac4j.core.client.IndirectClient.ATTEMPTED_AUTHENTICATION_SUFFIX;


Expand Down Expand Up @@ -329,11 +326,4 @@ private Result createSession(String userUrnString, String accessToken) {
return Results.ok().withSession(createSessionMap(userUrnString, accessToken))
.withCookies(createActorCookie(userUrnString, ttlInHours));
}

private Map<String, String> createSessionMap(final String userUrnStr, final String accessToken) {
final Map<String, String> sessionAttributes = new HashMap<>();
sessionAttributes.put(ACTOR, userUrnStr);
sessionAttributes.put(ACCESS_TOKEN, accessToken);
return sessionAttributes;
}
}
17 changes: 14 additions & 3 deletions datahub-frontend/test/app/ApplicationTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -24,10 +24,13 @@

import java.io.IOException;
import java.net.InetAddress;
import java.net.URLEncoder;
import java.nio.charset.StandardCharsets;
import java.util.List;
import java.util.Map;

import static org.junit.jupiter.api.Assertions.assertEquals;
import static org.junit.jupiter.api.Assertions.assertTrue;
import static play.mvc.Http.Status.NOT_FOUND;
import static play.mvc.Http.Status.OK;
import static play.test.Helpers.fakeRequest;
Expand Down Expand Up @@ -72,11 +75,14 @@ protected TestBrowser provideBrowser(int port) {

private String _wellKnownUrl;

private static final String TEST_USER = "urn:li:corpuser:[email protected]";
private static final String TEST_TOKEN = "faketoken_YCpYIrjQH4sD3_rAc3VPPFg4";

@BeforeAll
public void init() throws IOException, InterruptedException {
_gmsServer = new MockWebServer();
_gmsServer.enqueue(new MockResponse().setBody("{\"value\":\"urn:li:corpuser:[email protected]\"}"));
_gmsServer.enqueue(new MockResponse().setBody("{\"accessToken\":\"faketoken_YCpYIrjQH4sD3_rAc3VPPFg4\"}"));
_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();
Expand Down Expand Up @@ -140,8 +146,13 @@ public void testOpenIdConfig() {
public void testHappyPathOidc() throws InterruptedException {
browser.goTo("/authenticate");
assertEquals("", browser.url());

Cookie actorCookie = browser.getCookie("actor");
assertEquals("urn:li:corpuser:[email protected]", actorCookie.getValue());
assertEquals(TEST_USER, actorCookie.getValue());

Cookie sessionCookie = browser.getCookie("PLAY_SESSION");
assertTrue(sessionCookie.getValue().contains("token=" + TEST_TOKEN));
assertTrue(sessionCookie.getValue().contains("actor=" + URLEncoder.encode(TEST_USER, StandardCharsets.UTF_8)));
}

}

0 comments on commit 5f7b8a8

Please sign in to comment.