diff --git a/pom.xml b/pom.xml index 130a9f43707..97cc58fbecf 100644 --- a/pom.xml +++ b/pom.xml @@ -28,9 +28,8 @@ 1.2 4.5.5 4.12 - 5.3.1 - 5.3.1 - 1.3.1 + 5.5.2 + ${junit.jupiter.version} 2.28.2 5.2.4 1.20.1 @@ -114,7 +113,7 @@ org.junit.jupiter - junit-jupiter-api + junit-jupiter ${junit.jupiter.version} test @@ -124,24 +123,18 @@ ${junit.version} test - - org.junit.jupiter - junit-jupiter-engine - ${junit.jupiter.version} - test - - - org.junit.jupiter - junit-jupiter-params - ${junit.jupiter.version} - test - org.junit.vintage junit-vintage-engine ${junit.vintage.version} test + + org.hamcrest + hamcrest-library + 2.2 + test + org.glassfish javax.json @@ -753,7 +746,7 @@ maven-surefire-plugin - 2.22.0 + 2.22.2 ${testsToExclude} diff --git a/src/main/java/edu/harvard/iq/dataverse/authorization/providers/oauth2/AbstractOAuth2AuthenticationProvider.java b/src/main/java/edu/harvard/iq/dataverse/authorization/providers/oauth2/AbstractOAuth2AuthenticationProvider.java index 700bd17a176..9671621ac73 100644 --- a/src/main/java/edu/harvard/iq/dataverse/authorization/providers/oauth2/AbstractOAuth2AuthenticationProvider.java +++ b/src/main/java/edu/harvard/iq/dataverse/authorization/providers/oauth2/AbstractOAuth2AuthenticationProvider.java @@ -6,6 +6,7 @@ import com.github.scribejava.core.model.OAuthRequest; import com.github.scribejava.core.model.Response; import com.github.scribejava.core.model.Verb; +import com.github.scribejava.core.oauth.AuthorizationUrlBuilder; import com.github.scribejava.core.oauth.OAuth20Service; import edu.harvard.iq.dataverse.authorization.AuthenticatedUserDisplayInfo; import edu.harvard.iq.dataverse.authorization.AuthenticationProvider; @@ -100,7 +101,22 @@ public String toString() { protected abstract ParsedUserResponse parseUserResponse( String responseBody ); /** - * Build an OAuth20Service based on client ID & secret. Add default scope and insert + * Build an Authorization URL for this identity provider + * @param state A randomized state, necessary to secure the authorization flow. @see OAuth2LoginBackingBean.createState() + * @param callbackUrl URL where the provider should send the browser after authn in code flow + */ + public String buildAuthzUrl(String state, String callbackUrl) { + OAuth20Service svc = this.getService(callbackUrl); + + AuthorizationUrlBuilder aub = svc.createAuthorizationUrlBuilder().state(state); + // Do not include scope if empty string (necessary for GitHub) + if (!this.getSpacedScope().isEmpty()) { aub.scope(this.getSpacedScope()); } + + return aub.build(); + } + + /** + * Build an OAuth20Service based on client ID & secret, also inserting the * callback URL. Build uses the real API object for the target service like GitHub etc. * @param callbackUrl URL where the OAuth2 Provider should send browsers to after authz. * @return A usable OAuth20Service object @@ -116,18 +132,19 @@ public OAuth20Service getService(String callbackUrl) { * Receive user data from OAuth2 provider after authn/z has been successfull. (Callback view uses this) * Request a token and access the resource, parse output and return user details. * @param code The authz code sent from the provider - * @param service The service object in use to communicate with the provider + * @param redirectUrl The redirect URL (some providers require this when fetching the access token, e. g. Google) * @return A user record containing all user details accessible for us * @throws IOException Thrown when communication with the provider fails * @throws OAuth2Exception Thrown when we cannot access the user details for some reason * @throws InterruptedException Thrown when the requests thread is failing * @throws ExecutionException Thrown when the requests thread is failing */ - public OAuth2UserRecord getUserRecord(String code, @NotNull OAuth20Service service) + public OAuth2UserRecord getUserRecord(String code, String redirectUrl) throws IOException, OAuth2Exception, InterruptedException, ExecutionException { + OAuth20Service service = getService(redirectUrl); OAuth2AccessToken accessToken = service.getAccessToken(code); - //final String userEndpoint = getUserEndpoint(accessToken); + // We need to check if scope is null first: GitHub is used without scope, so the responses scope is null. // Checking scopes via Stream to be independent from order. if ( ( accessToken.getScope() != null && ! getScope().stream().allMatch(accessToken.getScope()::contains) ) || diff --git a/src/main/java/edu/harvard/iq/dataverse/authorization/providers/oauth2/OAuth2LoginBackingBean.java b/src/main/java/edu/harvard/iq/dataverse/authorization/providers/oauth2/OAuth2LoginBackingBean.java index 5a43c255fcf..618b8e5ca1d 100644 --- a/src/main/java/edu/harvard/iq/dataverse/authorization/providers/oauth2/OAuth2LoginBackingBean.java +++ b/src/main/java/edu/harvard/iq/dataverse/authorization/providers/oauth2/OAuth2LoginBackingBean.java @@ -1,16 +1,16 @@ package edu.harvard.iq.dataverse.authorization.providers.oauth2; -import com.github.scribejava.core.oauth.AuthorizationUrlBuilder; -import com.github.scribejava.core.oauth.OAuth20Service; import edu.harvard.iq.dataverse.DataverseSession; import edu.harvard.iq.dataverse.authorization.AuthenticationServiceBean; import edu.harvard.iq.dataverse.authorization.UserRecordIdentifier; import edu.harvard.iq.dataverse.authorization.users.AuthenticatedUser; +import edu.harvard.iq.dataverse.util.ClockUtil; import edu.harvard.iq.dataverse.util.StringUtil; import java.io.BufferedReader; import java.io.IOException; import java.io.Serializable; import java.security.SecureRandom; +import java.time.Clock; import java.util.Comparator; import java.util.List; import java.util.Optional; @@ -19,16 +19,15 @@ import java.util.logging.Logger; import static java.util.stream.Collectors.toList; import javax.ejb.EJB; -import javax.faces.context.FacesContext; import javax.inject.Named; import javax.faces.view.ViewScoped; import javax.inject.Inject; import javax.servlet.http.HttpServletRequest; -import javax.servlet.http.HttpServletResponse; import javax.validation.constraints.NotNull; import static edu.harvard.iq.dataverse.util.StringUtil.toOption; import edu.harvard.iq.dataverse.util.SystemConfig; +import org.omnifaces.util.Faces; /** * Backing bean of the oauth2 login process. Used from the login and the @@ -41,11 +40,14 @@ public class OAuth2LoginBackingBean implements Serializable { private static final Logger logger = Logger.getLogger(OAuth2LoginBackingBean.class.getName()); - private static final long STATE_TIMEOUT = 1000 * 60 * 15; // 15 minutes in msec + static final long STATE_TIMEOUT = 1000 * 60 * 15; // 15 minutes in msec private int responseCode; private String responseBody; - private Optional redirectPage; + Optional redirectPage = Optional.empty(); private OAuth2Exception error; + /** + * TODO: Only used in exchangeCodeForToken(). Make local var in method. + */ private OAuth2UserRecord oauthUser; @EJB @@ -62,7 +64,11 @@ public class OAuth2LoginBackingBean implements Serializable { @Inject OAuth2FirstLoginPage newAccountPage; - + + @Inject + @ClockUtil.LocalTime + Clock clock; + /** * Generate the OAuth2 Provider URL to be used in the login page link for the provider. * @param idpId Unique ID for the provider (used to lookup in authn service bean) @@ -71,16 +77,8 @@ public class OAuth2LoginBackingBean implements Serializable { */ public String linkFor(String idpId, String redirectPage) { AbstractOAuth2AuthenticationProvider idp = authenticationSvc.getOAuth2Provider(idpId); - OAuth20Service svc = idp.getService(systemConfig.getOAuth2CallbackUrl()); String state = createState(idp, toOption(redirectPage)); - - AuthorizationUrlBuilder aub = svc.createAuthorizationUrlBuilder() - .state(state); - - // Do not include scope if empty string (necessary for GitHub) - if (!idp.getSpacedScope().isEmpty()) { aub.scope(idp.getSpacedScope()); } - - return aub.build(); + return idp.buildAuthzUrl(state, systemConfig.getOAuth2CallbackUrl()); } /** @@ -88,17 +86,15 @@ public String linkFor(String idpId, String redirectPage) { * @throws IOException */ public void exchangeCodeForToken() throws IOException { - HttpServletRequest req = (HttpServletRequest) FacesContext.getCurrentInstance().getExternalContext().getRequest(); + HttpServletRequest req = Faces.getRequest(); try { - Optional oIdp = parseStateFromRequest(req); + Optional oIdp = parseStateFromRequest(req.getParameter("state")); Optional code = parseCodeFromRequest(req); if (oIdp.isPresent() && code.isPresent()) { AbstractOAuth2AuthenticationProvider idp = oIdp.get(); - - OAuth20Service svc = idp.getService(systemConfig.getOAuth2CallbackUrl()); - oauthUser = idp.getUserRecord(code.get(), svc); + oauthUser = idp.getUserRecord(code.get(), systemConfig.getOAuth2CallbackUrl()); UserRecordIdentifier idtf = oauthUser.getUserRecordIdentifier(); AuthenticatedUser dvUser = authenticationSvc.lookupUser(idtf); @@ -106,7 +102,7 @@ public void exchangeCodeForToken() throws IOException { if (dvUser == null) { // need to create the user newAccountPage.setNewUser(oauthUser); - FacesContext.getCurrentInstance().getExternalContext().redirect("/oauth2/firstLogin.xhtml"); + Faces.redirect("/oauth2/firstLogin.xhtml"); } else { // login the user and redirect to HOME of intended page (if any). @@ -116,10 +112,8 @@ public void exchangeCodeForToken() throws IOException { tokenData.setUser(dvUser); tokenData.setOauthProviderId(idp.getId()); oauth2Tokens.store(tokenData); - String destination = redirectPage.orElse("/"); - HttpServletResponse response = (HttpServletResponse) FacesContext.getCurrentInstance().getExternalContext().getResponse(); - String prettyUrl = response.encodeRedirectURL(destination); - FacesContext.getCurrentInstance().getExternalContext().redirect(prettyUrl); + + Faces.redirect(redirectPage.orElse("/")); } } } catch (OAuth2Exception ex) { @@ -132,6 +126,10 @@ public void exchangeCodeForToken() throws IOException { } } + /** + * TODO: Refactor this to be included in calling method. + * TODO: Use org.apache.commons.io.IOUtils.toString(req.getReader()) instead of overcomplicated code below. + */ private Optional parseCodeFromRequest(@NotNull HttpServletRequest req) { String code = req.getParameter("code"); if (code == null || code.trim().isEmpty()) { @@ -152,11 +150,21 @@ private Optional parseCodeFromRequest(@NotNull HttpServletRequest req) { } return Optional.of(code); } - - private Optional parseStateFromRequest(@NotNull HttpServletRequest req) { - String state = req.getParameter("state"); - - if (state == null) { + + /** + * Parse and verify the state returned from the provider. + * + * As it contains the providers implementation "id" field when send by us, + * we can return the corresponding provider object. + * + * This function is not side effect free: it will (if present) set {@link #redirectPage} + * to the value received from the state. + * + * @param state The state string, created in {@link #createState(AbstractOAuth2AuthenticationProvider, Optional)}, send and returned by provider + * @return A corresponding provider object when state verification succeeded. + */ + Optional parseStateFromRequest(@NotNull String state) { + if (state == null || state.trim().equals("")) { logger.log(Level.INFO, "No state present in request"); return Optional.empty(); } @@ -177,10 +185,10 @@ private Optional parseStateFromRequest(@No String[] stateFields = raw.split("~", -1); if (idp.getId().equals(stateFields[0])) { long timeOrigin = Long.parseLong(stateFields[1]); - long timeDifference = System.currentTimeMillis() - timeOrigin; + long timeDifference = this.clock.millis() - timeOrigin; if (timeDifference > 0 && timeDifference < STATE_TIMEOUT) { if ( stateFields.length > 3) { - redirectPage = Optional.ofNullable(stateFields[3]); + this.redirectPage = Optional.ofNullable(stateFields[3]); } return Optional.of(idp); } else { @@ -192,14 +200,20 @@ private Optional parseStateFromRequest(@No return Optional.empty(); } } - - private String createState(AbstractOAuth2AuthenticationProvider idp, Optional redirectPage ) { + + /** + * Create a randomized unique state string to be used while crafting the autorization request + * @param idp + * @param redirectPage + * @return Random state string, composed from system time, random numbers and redirectPage parameter + */ + String createState(AbstractOAuth2AuthenticationProvider idp, Optional redirectPage) { if (idp == null) { throw new IllegalArgumentException("idp cannot be null"); } SecureRandom rand = new SecureRandom(); - String base = idp.getId() + "~" + System.currentTimeMillis() + String base = idp.getId() + "~" + this.clock.millis() + "~" + rand.nextInt(1000) + redirectPage.map( page -> "~"+page).orElse(""); @@ -207,15 +221,24 @@ private String createState(AbstractOAuth2AuthenticationProvider idp, Optional getProviders() { return authenticationSvc.getOAuth2Providers().stream() .sorted(Comparator.comparing(AbstractOAuth2AuthenticationProvider::getTitle)) .collect(toList()); } - + + /** + * TODO: Unused. Remove. + */ public boolean isOAuth2ProvidersDefined() { return !authenticationSvc.getOAuth2Providers().isEmpty(); } diff --git a/src/main/java/edu/harvard/iq/dataverse/util/ClockUtil.java b/src/main/java/edu/harvard/iq/dataverse/util/ClockUtil.java new file mode 100644 index 00000000000..9c1c89430d5 --- /dev/null +++ b/src/main/java/edu/harvard/iq/dataverse/util/ClockUtil.java @@ -0,0 +1,26 @@ +package edu.harvard.iq.dataverse.util; + +import javax.enterprise.inject.Produces; +import javax.inject.Qualifier; +import javax.inject.Singleton; +import java.lang.annotation.ElementType; +import java.lang.annotation.Retention; +import java.lang.annotation.RetentionPolicy; +import java.lang.annotation.Target; +import java.time.Clock; + +@Singleton +public class ClockUtil { + + + @Qualifier + @Retention(RetentionPolicy.RUNTIME) + @Target({ElementType.FIELD, ElementType.METHOD, ElementType.TYPE, ElementType.PARAMETER}) + public @interface LocalTime { + } + + @Produces + @LocalTime + public static final Clock LOCAL_CLOCK = Clock.systemDefaultZone(); + +} diff --git a/src/test/java/edu/harvard/iq/dataverse/LocaleTest.java b/src/test/java/edu/harvard/iq/dataverse/LocaleTest.java new file mode 100644 index 00000000000..1ca43cde138 --- /dev/null +++ b/src/test/java/edu/harvard/iq/dataverse/LocaleTest.java @@ -0,0 +1,30 @@ +package edu.harvard.iq.dataverse; + +import org.junit.jupiter.api.Test; + +import java.util.Locale; +import java.util.TimeZone; + +import static org.hamcrest.MatcherAssert.assertThat; +import static org.hamcrest.Matchers.equalTo; + +public class LocaleTest { + + /** + * Many of our tests require setting the locale and timezone appropriately + * as formating dates, numbers, etc are based on it. Ensure consistency. + */ + @Test + void ensureLocale() { + Locale l = Locale.getDefault(); + TimeZone tz = TimeZone.getDefault(); + + System.out.println("user.language="+l.getLanguage()); + System.out.println("user.region="+l.getCountry()); + System.out.println("user.timezone="+tz.getDefault().getID()); + + assertThat(l.getLanguage(), equalTo("en")); + assertThat(l.getCountry(), equalTo("US")); + assertThat(tz.getID(), equalTo("UTC")); + } +} diff --git a/src/test/java/edu/harvard/iq/dataverse/authorization/providers/oauth2/OAuth2LoginBackingBeanTest.java b/src/test/java/edu/harvard/iq/dataverse/authorization/providers/oauth2/OAuth2LoginBackingBeanTest.java new file mode 100644 index 00000000000..80249cc89e8 --- /dev/null +++ b/src/test/java/edu/harvard/iq/dataverse/authorization/providers/oauth2/OAuth2LoginBackingBeanTest.java @@ -0,0 +1,258 @@ +package edu.harvard.iq.dataverse.authorization.providers.oauth2; + +import edu.harvard.iq.dataverse.DataverseSession; +import edu.harvard.iq.dataverse.authorization.AuthenticationServiceBean; +import edu.harvard.iq.dataverse.authorization.UserRecordIdentifier; +import edu.harvard.iq.dataverse.authorization.providers.oauth2.impl.GitHubOAuth2APTest; +import edu.harvard.iq.dataverse.authorization.users.AuthenticatedUser; +import edu.harvard.iq.dataverse.util.StringUtil; +import edu.harvard.iq.dataverse.util.SystemConfig; +import org.hamcrest.Matchers; +import org.hamcrest.core.StringContains; +import org.junit.jupiter.api.*; +import org.junit.jupiter.api.extension.ExtendWith; +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.*; +import org.mockito.ArgumentCaptor; +import org.mockito.Mock; +import org.mockito.Spy; +import org.mockito.junit.jupiter.MockitoExtension; +import org.omnifaces.util.Faces; + +import javax.faces.context.ExternalContext; +import javax.faces.context.FacesContext; +import javax.faces.context.Flash; +import javax.servlet.http.HttpServletRequest; + +import static org.junit.jupiter.api.Assertions.assertDoesNotThrow; + +import java.time.Clock; +import java.time.Duration; +import java.time.Instant; +import java.time.ZoneId; +import java.util.Optional; +import java.util.stream.Stream; +import java.io.BufferedReader; +import java.io.IOException; + +import static org.junit.jupiter.api.Assertions.assertThrows; +import static org.hamcrest.MatcherAssert.assertThat; +import static org.hamcrest.Matchers.*; +import static org.mockito.Mockito.*; + +@ExtendWith(MockitoExtension.class) +class OAuth2LoginBackingBeanTest { + + OAuth2LoginBackingBean loginBackingBean = new OAuth2LoginBackingBean(); + @Spy static AbstractOAuth2AuthenticationProvider testIdp = new GitHubOAuth2APTest(); + + @Mock AuthenticationServiceBean authenticationServiceBean; + @Mock SystemConfig systemConfig; + + Clock constantClock = Clock.fixed(Instant.now(), ZoneId.systemDefault()); + + /** + * Save the current JSF context to reset after all test cases done. + * Without doing this, many tests will fail with NPEs while fetching the bundle, + * when the locale is received from the context. + */ + static FacesContext save = Faces.getContext(); + @AfterAll + static void cleanupFaces() { + Faces.setContext(save); + } + + @BeforeEach + void setUp() { + + // create a fixed (not running) clock for testing. + // --> reset the clock for every test to the fixed time, to avoid sideeffects of a DeLorean time travel + this.loginBackingBean.clock = constantClock; + this.loginBackingBean.authenticationSvc = this.authenticationServiceBean; + this.loginBackingBean.systemConfig = this.systemConfig; + lenient().when(this.authenticationServiceBean.getOAuth2Provider(testIdp.getId())).thenReturn(testIdp); + } + + /** + * TODO: this should be a parameterized test case testing all providers available + */ + @Test + void linkFor() { + // given + String redirectPage = "dataverse.xhtml"; // @see LoginPage.redirectPage + String callbackURL = "oauth2/callback.xhtml"; + + // when + when(this.systemConfig.getOAuth2CallbackUrl()).thenReturn(callbackURL); + + String link = loginBackingBean.linkFor(testIdp.getId(), redirectPage); + + // then + assertThat(link, notNullValue()); + assertThat(link, not(isEmptyString())); + assertThat(link, StringContains.containsString(testIdp.getService(callbackURL).getAuthorizationUrl())); + } + + @Nested + @DisplayName("Tests for exchangeCodeForToken()") + class ecft { + @Mock FacesContext facesContextMock; + @Mock ExternalContext externalContextMock; + @Mock Flash flashMock; + @Mock HttpServletRequest requestMock; + @Mock BufferedReader reader; + @Mock OAuth2FirstLoginPage newAccountPage; + @Mock DataverseSession session; + @Mock OAuth2TokenDataServiceBean oauth2Tokens; + Optional redirect = Optional.of("/hellotest"); + + @BeforeEach + void setUp() throws IOException { + loginBackingBean.newAccountPage = this.newAccountPage; + loginBackingBean.session = this.session; + loginBackingBean.oauth2Tokens = this.oauth2Tokens; + + // mock FacesContext to make the method testable + Faces.setContext(facesContextMock); + when(facesContextMock.getExternalContext()).thenReturn(externalContextMock); + when(externalContextMock.getRequest()).thenReturn(requestMock); + lenient().when(externalContextMock.getFlash()).thenReturn(flashMock); + lenient().when(requestMock.getReader()).thenReturn(reader); + doReturn(loginBackingBean.createState(testIdp, this.redirect)).when(requestMock).getParameter("state"); + // travel in time at least 10 milliseconds (remote calls & redirects are much likely longer) + // (if not doing this tests become flaky on fast machinas) + loginBackingBean.clock = Clock.offset(constantClock, Duration.ofMillis(10)); + } + + @Test + void noCode() { + assertDoesNotThrow(() -> loginBackingBean.exchangeCodeForToken()); + assertThat(loginBackingBean.getError(), Matchers.isA(OAuth2Exception.class)); + } + + @Test + void newUser() throws Exception { + // GIVEN + String code = "randomstring"; + OAuth2UserRecord userRecord = mock(OAuth2UserRecord.class); + String newUserRedirect = "/oauth2/firstLogin.xhtml"; + + // fake the code received from the provider + when(requestMock.getParameter("code")).thenReturn(code); + // let's deep-fake the result of getUserRecord() + doReturn(userRecord).when(testIdp).getUserRecord(code, null); + + // WHEN (& then) + // capture the redirect target from the faces context + ArgumentCaptor redirectUrlCaptor = ArgumentCaptor.forClass(String.class); + doNothing().when(externalContextMock).redirect(redirectUrlCaptor.capture()); + + assertDoesNotThrow(() -> loginBackingBean.exchangeCodeForToken()); + + // THEN + // verify that the user object is passed on to the first login page + verify(newAccountPage, times(1)).setNewUser(userRecord); + // verify that the user is redirected to the first login page + assertThat(redirectUrlCaptor.getValue(), equalTo(newUserRedirect)); + } + + @Test + void existingUser() throws Exception { + // GIVEN + String code = "randomstring"; + OAuth2UserRecord userRecord = mock(OAuth2UserRecord.class); + UserRecordIdentifier userIdentifier = mock(UserRecordIdentifier.class); + AuthenticatedUser user = mock(AuthenticatedUser.class); + OAuth2TokenData tokenData = mock(OAuth2TokenData.class); + + // fake the code received from the provider + when(requestMock.getParameter("code")).thenReturn(code); + // let's deep-fake the result of getUserRecord() + doReturn(userRecord).when(testIdp).getUserRecord(code, null); + doReturn(tokenData).when(userRecord).getTokenData(); + // also fake the result of the lookup in the auth service + doReturn(userIdentifier).when(userRecord).getUserRecordIdentifier(); + doReturn(user).when(authenticationServiceBean).lookupUser(userIdentifier); + + // WHEN (& then) + // capture the redirect target from the faces context + ArgumentCaptor redirectUrlCaptor = ArgumentCaptor.forClass(String.class); + doNothing().when(externalContextMock).redirect(redirectUrlCaptor.capture()); + + assertDoesNotThrow(() -> loginBackingBean.exchangeCodeForToken()); + + // THEN + // verify session handling: set the looked up user + verify(session, times(1)).setUser(user); + // verify that the user is redirected to the first login page + assertThat(redirectUrlCaptor.getValue(), equalTo(redirect.get())); + } + } + + @Test + void createStateFailNullIdp() { + // given + AbstractOAuth2AuthenticationProvider idp = null; + Optional page = Optional.empty(); + // when & then + assertThrows(IllegalArgumentException.class, () -> { + loginBackingBean.createState(idp, page); + }); + } + + @ParameterizedTest(name = "\"{0}\" should be Optional.empty") + @NullSource + @EmptySource + @ValueSource(strings = {" ", "\t"}) + void parseStateFromRequestStateNullOrEmpty(String state) { + assertThat(loginBackingBean.parseStateFromRequest(state), is(Optional.empty())); + } + + @ParameterizedTest(name = "\"{0}\" should be Optional.empty") + @ValueSource(strings = {"test", "test~", "test~test"}) + void parseStateFromRequestStateInvalidString(String state) { + assertThat(loginBackingBean.parseStateFromRequest(state), is(Optional.empty())); + } + + static Stream tamperedStates() { + return Stream.of( + // encrypted nonsense + Arguments.of(testIdp.getId() + "~" + StringUtil.encrypt("test", testIdp.getClientSecret())), + // expired timestamp < 0 + Arguments.of(testIdp.getId() + "~" + StringUtil.encrypt(testIdp.getId()+"~-1", testIdp.getClientSecret())), + // expired timestamp (10 milliseconds to old...) + Arguments.of(testIdp.getId() + "~" + StringUtil.encrypt(testIdp.getId()+"~"+(System.currentTimeMillis()-OAuth2LoginBackingBean.STATE_TIMEOUT-10), testIdp.getClientSecret())) + ); + } + @ParameterizedTest + @MethodSource("tamperedStates") + void parseStateFromRequestStateTampered(String state) { + assertThat(loginBackingBean.parseStateFromRequest(state), is(Optional.empty())); + } + + /** + * Testing for side effect with proper parameters. + */ + static Stream provideStates() { + return Stream.of( + Arguments.of(Optional.of("dataverse.xhtml"), true), + Arguments.of(Optional.empty(), false) + ); + } + @ParameterizedTest + @MethodSource("provideStates") + void parseStateFromRequestStateValid(Optional redirectPage, boolean present) { + // given + String stateWithRedirect = loginBackingBean.createState(testIdp, redirectPage); + // travel in time at least 10 milliseconds (remote calls & redirects are much likely longer) + // (if not doing this tests become flaky on fast machinas) + loginBackingBean.clock = Clock.offset(constantClock, Duration.ofMillis(10)); + + // when & then + assertThat(loginBackingBean.parseStateFromRequest(stateWithRedirect), is(Optional.of(testIdp))); + assertThat(loginBackingBean.redirectPage.isPresent(), is(present)); + if (present) { + assertThat(loginBackingBean.redirectPage.get(), equalTo(redirectPage.get())); + } + } +} \ No newline at end of file diff --git a/src/test/java/edu/harvard/iq/dataverse/authorization/providers/oauth2/impl/GitHubOAuth2APTest.java b/src/test/java/edu/harvard/iq/dataverse/authorization/providers/oauth2/impl/GitHubOAuth2APTest.java index 8b43b65f324..786c30fb2d7 100644 --- a/src/test/java/edu/harvard/iq/dataverse/authorization/providers/oauth2/impl/GitHubOAuth2APTest.java +++ b/src/test/java/edu/harvard/iq/dataverse/authorization/providers/oauth2/impl/GitHubOAuth2APTest.java @@ -3,6 +3,8 @@ import edu.harvard.iq.dataverse.authorization.AuthenticatedUserDisplayInfo; import edu.harvard.iq.dataverse.authorization.providers.oauth2.AbstractOAuth2AuthenticationProvider; import static org.junit.Assert.assertEquals; + +import edu.harvard.iq.dataverse.authorization.providers.oauth2.OAuth2UserRecord; import org.junit.Test; public class GitHubOAuth2APTest extends GitHubOAuth2AP { @@ -57,5 +59,10 @@ public void testParseUserResponse() { assertEquals("21006", result.userIdInProvider); } + + public OAuth2UserRecord getExampleUserRecord() { + ParsedUserResponse res = parseUserResponse(GITHUB_RESPONSE); + return new OAuth2UserRecord(this.getId(), res.userIdInProvider, res.username, null, res.displayInfo, res.emails); + } }