[KEYCLOAK-13202] Reset password redirects to account client

services/src/main/java/org/keycloak/protocol/oidc/utils/RedirectUtils.java

@@ -28,7 +28,6 @@ import org.keycloak.services.Urls;
 import org.keycloak.services.util.ResolveRelative;
 
 import java.net.URI;
-import java.net.URISyntaxException;
 import java.util.Collection;
 import java.util.HashSet;
 import java.util.Set;
@@ -189,10 +188,26 @@ public class RedirectUtils {
     private static String getSingleValidRedirectUri(Collection<String> validRedirects) {
         if (validRedirects.size() != 1) return null;
         String validRedirect = validRedirects.iterator().next();
-        int idx = validRedirect.indexOf("/*");
+        return validateRedirectUriWildcard(validRedirect);
+    }
+
+    public static String validateRedirectUriWildcard(String redirectUri) {
+        if (redirectUri == null)
+            return null;
+
+        int idx = redirectUri.indexOf("/*");
         if (idx > -1) {
-            validRedirect = validRedirect.substring(0, idx);
+            redirectUri = redirectUri.substring(0, idx);
         }
-        return validRedirect;
+        return redirectUri;
+    }
+
+    private static String getFirstValidRedirectUri(Collection<String> validRedirects) {
+        final String redirectUri = validRedirects.stream().findFirst().orElse(null);
+        return (redirectUri != null) ? validateRedirectUriWildcard(redirectUri) : null;
+    }
+
+    public static String getFirstValidRedirectUri(KeycloakSession session, String rootUrl, Set<String> validRedirects) {
+        return getFirstValidRedirectUri(resolveValidRedirects(session, rootUrl, validRedirects));
     }
 }

services/src/main/java/org/keycloak/services/resources/LoginActionsService.java

@@ -48,7 +48,6 @@ import org.keycloak.events.EventBuilder;
 import org.keycloak.events.EventType;
 import org.keycloak.exceptions.TokenNotActiveException;
 import org.keycloak.locale.LocaleSelectorProvider;
-import org.keycloak.locale.LocaleSelectorSPI;
 import org.keycloak.locale.LocaleUpdaterProvider;
 import org.keycloak.models.ActionTokenKeyModel;
 import org.keycloak.models.AuthenticationFlowModel;
@@ -70,6 +69,7 @@ import org.keycloak.protocol.LoginProtocol.Error;
 import org.keycloak.protocol.oidc.OIDCLoginProtocol;
 import org.keycloak.protocol.oidc.utils.OIDCResponseMode;
 import org.keycloak.protocol.oidc.utils.OIDCResponseType;
+import org.keycloak.protocol.oidc.utils.RedirectUtils;
 import org.keycloak.representations.JsonWebToken;
 import org.keycloak.services.ErrorPage;
 import org.keycloak.services.ServicesLogger;
@@ -380,10 +380,10 @@ public class LoginActionsService {
             if (!realm.isResetPasswordAllowed()) {
                 event.event(EventType.RESET_PASSWORD);
                 event.error(Errors.NOT_ALLOWED);
-                return ErrorPage.error(session, authSession, Response.Status.BAD_REQUEST, Messages.RESET_CREDENTIAL_NOT_ALLOWED);
+                return ErrorPage.error(session, null, Response.Status.BAD_REQUEST, Messages.RESET_CREDENTIAL_NOT_ALLOWED);
 
             }
-            authSession = createAuthenticationSessionForClient();
+            authSession = createAuthenticationSessionForClient(clientId);
             return processResetCredentials(false, null, authSession, null);
         }
 
@@ -391,12 +391,19 @@ public class LoginActionsService {
         return resetCredentials(authSessionId, code, execution, clientId, tabId);
     }
 
-    AuthenticationSessionModel createAuthenticationSessionForClient()
+    AuthenticationSessionModel createAuthenticationSessionForClient(String clientID)
-      throws UriBuilderException, IllegalArgumentException {
+            throws UriBuilderException, IllegalArgumentException {
         AuthenticationSessionModel authSession;
 
-        // set up the account service as the endpoint to call.
+        ClientModel client = session.realms().getClientByClientId(clientID, realm);
-        ClientModel client = SystemClientUtil.getSystemClient(realm);
+        String redirectUri;
+
+        if (client == null) {
+            client = SystemClientUtil.getSystemClient(realm);
+            redirectUri = Urls.accountBase(session.getContext().getUri().getBaseUri()).path("/").build(realm.getName()).toString();
+        } else {
+            redirectUri = RedirectUtils.getFirstValidRedirectUri(session, client.getRootUrl(), client.getRedirectUris());
+        }
 
         RootAuthenticationSessionModel rootAuthSession = new AuthenticationSessionManager(session).createAuthenticationSession(realm, true);
         authSession = rootAuthSession.createAuthenticationSession(client);
@@ -404,7 +411,6 @@ public class LoginActionsService {
         authSession.setAction(AuthenticationSessionModel.Action.AUTHENTICATE.name());
         //authSession.setNote(AuthenticationManager.END_AFTER_REQUIRED_ACTIONS, "true");
         authSession.setProtocol(OIDCLoginProtocol.LOGIN_PROTOCOL);
-        String redirectUri = Urls.accountBase(session.getContext().getUri().getBaseUri()).path("/").build(realm.getName()).toString();
         authSession.setRedirectUri(redirectUri);
         authSession.setClientNote(OIDCLoginProtocol.RESPONSE_TYPE_PARAM, OAuth2Constants.CODE);
         authSession.setClientNote(OIDCLoginProtocol.REDIRECT_URI_PARAM, redirectUri);

testsuite/integration-arquillian/tests/base/src/main/java/org/keycloak/testsuite/util/BrowserTabUtil.java

@@ -0,0 +1,158 @@
+/*
+ * Copyright 2020 Red Hat, Inc. and/or its affiliates
+ * and other contributors as indicated by the @author tags.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.keycloak.testsuite.util;
+
+import com.gargoylesoftware.htmlunit.WebClient;
+import org.jboss.arquillian.drone.webdriver.htmlunit.DroneHtmlUnitDriver;
+import org.openqa.selenium.JavascriptExecutor;
+import org.openqa.selenium.WebDriver;
+import org.openqa.selenium.htmlunit.HtmlUnitDriver;
+
+import java.util.ArrayList;
+import java.util.List;
+import java.util.Set;
+
+/**
+ * Helper class for managing tabs in browser.
+ * Tabs are indexed from 0. (f.e. first tab has index 0)
+ *
+ * <p>Note: For one particular WebDriver has to exist only one BrowserTabUtil instance. (Right order of tabs)</p>
+ *
+ * @author <a href="mailto:mabartos@redhat.com">Martin Bartos</a>
+ */
+public class BrowserTabUtil implements AutoCloseable {
+
+    private WebDriver driver;
+    private JavascriptExecutor jsExecutor;
+    private List<String> tabs;
+    private static List<BrowserTabUtil> instances;
+
+    private BrowserTabUtil(WebDriver driver) {
+        this.driver = driver;
+
+        if (driver instanceof JavascriptExecutor) {
+            this.jsExecutor = (JavascriptExecutor) driver;
+        } else {
+            throw new RuntimeException("WebDriver must be instance of JavascriptExecutor");
+        }
+
+        // HtmlUnit doesn't work very well with JS and it's recommended to use this settings.
+        // HtmlUnit validates all scripts and then fails. It turned off the validation.
+        if (driver instanceof HtmlUnitDriver) {
+            WebClient client = ((DroneHtmlUnitDriver) driver).getWebClient();
+            client.getOptions().setThrowExceptionOnScriptError(false);
+            client.getOptions().setThrowExceptionOnFailingStatusCode(false);
+        }
+
+        tabs = new ArrayList<>(driver.getWindowHandles());
+    }
+
+    public static BrowserTabUtil getInstanceAndSetEnv(WebDriver driver) {
+        if (instances == null) {
+            instances = new ArrayList<>();
+        }
+
+        BrowserTabUtil instance = instances.stream()
+                .filter(inst -> inst.getDriver().toString().equals(driver.toString()))
+                .findFirst()
+                .orElse(null);
+
+        if (instance == null) {
+            instance = new BrowserTabUtil(driver);
+            instances.add(instance);
+        }
+        return instance;
+    }
+
+    public WebDriver getDriver() {
+        return driver;
+    }
+
+    public String getActualWindowHandle() {
+        return driver.getWindowHandle();
+    }
+
+    public void switchToTab(String windowHandle) {
+        driver.switchTo().window(windowHandle);
+        WaitUtils.waitForPageToLoad();
+    }
+
+    public void switchToTab(int index) {
+        assertValidIndex(index);
+        switchToTab(tabs.get(index));
+    }
+
+    public void newTab(String url) {
+        jsExecutor.executeScript("window.open(arguments[0]);", url);
+
+        final Set<String> handles = driver.getWindowHandles();
+        final String tabHandle = handles.stream()
+                .filter(tab -> !tabs.contains(tab))
+                .findFirst()
+                .orElse(null);
+
+        if (handles.size() > tabs.size() + 1) {
+            throw new RuntimeException("Too many window handles. You can only create a new one by this method.");
+        }
+
+        if (tabHandle == null) {
+            throw new RuntimeException("Creating the new tab failed.");
+        }
+
+        tabs.add(tabHandle);
+        switchToTab(tabHandle);
+    }
+
+    public void closeTab(int index) {
+        assertValidIndex(index);
+
+        if (index == 0 || getCountOfTabs() == 1)
+            throw new RuntimeException("You must not close the original tab.");
+
+        switchToTab(index);
+        driver.close();
+
+        tabs.remove(index);
+        switchToTab(index - 1);
+    }
+
+    public int getCountOfTabs() {
+        return tabs.size();
+    }
+
+    public void destroy() {
+        for (int i = 1; i < getCountOfTabs(); i++) {
+            closeTab(i);
+        }
+        instances.removeIf(inst -> inst.getDriver().toString().equals(driver.toString()));
+    }
+
+    private boolean validIndex(int index) {
+        return (index >= 0 && tabs != null && index < tabs.size());
+    }
+
+    private void assertValidIndex(int index) {
+        if (!validIndex(index))
+            throw new IndexOutOfBoundsException("Invalid index of tab.");
+    }
+
+    @Override
+    public void close() {
+        destroy();
+    }
+}

testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/forms/ResetPasswordTest.java

@@ -27,6 +27,8 @@ import org.keycloak.events.Errors;
 import org.keycloak.events.EventType;
 import org.keycloak.models.Constants;
 import org.keycloak.models.utils.SystemClientUtil;
+import org.keycloak.protocol.oidc.utils.RedirectUtils;
+import org.keycloak.representations.idm.ClientRepresentation;
 import org.keycloak.representations.idm.EventRepresentation;
 import org.keycloak.representations.idm.RealmRepresentation;
 import org.keycloak.representations.idm.UserRepresentation;
@@ -43,6 +45,7 @@ import org.keycloak.testsuite.pages.LoginPasswordResetPage;
 import org.keycloak.testsuite.pages.LoginPasswordUpdatePage;
 import org.keycloak.testsuite.pages.VerifyEmailPage;
 import org.keycloak.testsuite.updaters.ClientAttributeUpdater;
+import org.keycloak.testsuite.util.BrowserTabUtil;
 import org.keycloak.testsuite.util.GreenMailRule;
 import org.keycloak.testsuite.util.MailUtils;
 import org.keycloak.testsuite.util.OAuthClient;
@@ -56,18 +59,23 @@ import javax.mail.internet.MimeMessage;
 
 import java.io.Closeable;
 import java.io.IOException;
+import java.net.MalformedURLException;
+import java.net.URL;
+import java.util.Collection;
 import java.util.Collections;
 import java.util.HashMap;
 import java.util.Map;
 import java.util.concurrent.atomic.AtomicInteger;
 
 import org.junit.*;
+import org.keycloak.testsuite.util.WaitUtils;
 import org.openqa.selenium.By;
 import org.openqa.selenium.WebDriver;
 import org.openqa.selenium.WebElement;
 
 import static org.hamcrest.MatcherAssert.assertThat;
 import static org.junit.Assert.*;
+
 import org.keycloak.testsuite.arquillian.annotation.AuthServerContainerExclude.AuthServer;
 
 /**
@@ -78,6 +86,7 @@ import org.keycloak.testsuite.arquillian.annotation.AuthServerContainerExclude.A
 public class ResetPasswordTest extends AbstractTestRealmKeycloakTest {
 
     private String userId;
+    private UserRepresentation defaultUser;
 
     @Drone
     @SecondBrowser
@@ -92,13 +101,14 @@ public class ResetPasswordTest extends AbstractTestRealmKeycloakTest {
     @Before
     public void setup() {
         log.info("Adding login-test user");
-        UserRepresentation user = UserBuilder.create()
+        defaultUser = UserBuilder.create()
                 .username("login-test")
                 .email("login@test.com")
                 .enabled(true)
                 .build();
 
-        userId = ApiUtil.createUserAndResetPasswordWithAdminClient(testRealm(), user, "password");
+        userId = ApiUtil.createUserAndResetPasswordWithAdminClient(testRealm(), defaultUser, "password");
+        defaultUser.setId(userId);
         expectedMessagesCount = 0;
         getCleanup().addUserId(userId);
     }
@@ -1048,12 +1058,7 @@ public class ResetPasswordTest extends AbstractTestRealmKeycloakTest {
 
         driver2.navigate().to(changePasswordUrl.trim());
 
-        final WebElement newPassword = driver2.findElement(By.id("password-new"));
+        changePasswordOnUpdatePage(driver2);
-        newPassword.sendKeys("resetPassword");
-        final WebElement confirmPassword = driver2.findElement(By.id("password-confirm"));
-        confirmPassword.sendKeys("resetPassword");
-        final WebElement submit = driver2.findElement(By.cssSelector("input[type=\"submit\"]"));
-        submit.click();
 
         assertThat(driver2.getCurrentUrl(), Matchers.containsString("client_id=test-app"));
 
@@ -1073,7 +1078,7 @@ public class ResetPasswordTest extends AbstractTestRealmKeycloakTest {
         serviceAccount = serviceAccount1.toRepresentation();
         serviceAccount.setEmail("client-user@test.com");
         serviceAccount1.update(serviceAccount);
-        
+
         String resetUri = oauth.AUTH_SERVER_ROOT + "/realms/test/login-actions/reset-credentials";
         driver.navigate().to(resetUri);
 
@@ -1084,4 +1089,162 @@ public class ResetPasswordTest extends AbstractTestRealmKeycloakTest {
         loginPage.assertCurrent();
         assertEquals("Invalid username or password.", errorPage.getError());
     }
+
+    @Test
+    public void resetPasswordLinkNewTabAndProperRedirectAccount() throws IOException {
+        final String REQUIRED_URI = OAuthClient.AUTH_SERVER_ROOT + "/realms/test/account/applications";
+        final String REDIRECT_URI = getAccountRedirectUrl() + "?path=applications";
+        final String CLIENT_ID = "account";
+
+        try (BrowserTabUtil tabUtil = BrowserTabUtil.getInstanceAndSetEnv(driver)) {
+            assertThat(tabUtil.getCountOfTabs(), Matchers.is(1));
+
+            driver.navigate().to(REQUIRED_URI);
+            resetPasswordTwiceInNewTab(defaultUser, CLIENT_ID, false, REDIRECT_URI, REQUIRED_URI);
+            assertThat(driver.getTitle(), Matchers.equalTo("Keycloak Account Management"));
+
+            oauth.openLogout();
+
+            driver.navigate().to(REQUIRED_URI);
+            resetPasswordTwiceInNewTab(defaultUser, CLIENT_ID, true, REDIRECT_URI, REQUIRED_URI);
+            assertThat(driver.getTitle(), Matchers.equalTo("Keycloak Account Management"));
+        }
+    }
+
+    @Test
+    public void resetPasswordLinkNewTabAndProperRedirectClient() throws IOException {
+        final String REDIRECT_URI = OAuthClient.AUTH_SERVER_ROOT + "/realms/master/app/auth";
+        final String CLIENT_ID = "test-app";
+
+        try (BrowserTabUtil tabUtil = BrowserTabUtil.getInstanceAndSetEnv(driver)) {
+            assertThat(tabUtil.getCountOfTabs(), Matchers.is(1));
+
+            loginPage.open();
+            resetPasswordTwiceInNewTab(defaultUser, CLIENT_ID, false, REDIRECT_URI);
+            assertThat(driver.getCurrentUrl(), Matchers.containsString(REDIRECT_URI));
+
+            oauth.openLogout();
+
+            loginPage.open();
+            resetPasswordTwiceInNewTab(defaultUser, CLIENT_ID, true, REDIRECT_URI);
+            assertThat(driver.getCurrentUrl(), Matchers.containsString(REDIRECT_URI));
+
+        }
+    }
+
+    private void changePasswordOnUpdatePage(WebDriver driver) {
+        assertThat(driver.getPageSource(), Matchers.containsString("You need to change your password."));
+
+        final WebElement newPassword = driver.findElement(By.id("password-new"));
+        newPassword.sendKeys("resetPassword");
+        final WebElement confirmPassword = driver.findElement(By.id("password-confirm"));
+        confirmPassword.sendKeys("resetPassword");
+        final WebElement submit = driver.findElement(By.cssSelector("input[type=\"submit\"]"));
+
+        submit.click();
+    }
+
+    private void resetPasswordTwiceInNewTab(UserRepresentation user, String clientId, boolean shouldLogOut, String redirectUri) throws IOException {
+        resetPasswordTwiceInNewTab(user, clientId, shouldLogOut, redirectUri, redirectUri);
+    }
+
+    private void resetPasswordTwiceInNewTab(UserRepresentation user, String clientId, boolean shouldLogOut, String redirectUri, String requiredUri) throws IOException {
+        events.clear();
+        updateForgottenPassword(user, clientId, redirectUri, requiredUri);
+
+        if (shouldLogOut) {
+            String sessionId = events.expectLogin().user(user.getId()).detail(Details.USERNAME, user.getUsername())
+                    .detail(Details.REDIRECT_URI, redirectUri)
+                    .client(clientId)
+                    .assertEvent().getSessionId();
+            oauth.openLogout();
+            events.expectLogout(sessionId).user(user.getId()).session(sessionId).assertEvent();
+        }
+
+        BrowserTabUtil util = BrowserTabUtil.getInstanceAndSetEnv(driver);
+        assertThat(util.getCountOfTabs(), Matchers.equalTo(2));
+        util.closeTab(1);
+        assertThat(util.getCountOfTabs(), Matchers.equalTo(1));
+
+        if (shouldLogOut) {
+            final ClientRepresentation client = testRealm().clients()
+                    .findByClientId(clientId)
+                    .stream()
+                    .findFirst()
+                    .orElse(null);
+
+            assertThat(client, Matchers.notNullValue());
+            System.out.println("HEE");
+            System.out.println(client.getRootUrl());
+            updateForgottenPassword(user, clientId, getValidRedirectUriWithRootUrl(client.getRootUrl(), client.getRedirectUris()));
+        } else {
+            doForgotPassword(user.getUsername());
+        }
+    }
+
+    private void updateForgottenPassword(UserRepresentation user, String clientId, String redirectUri) throws IOException {
+        updateForgottenPassword(user, clientId, redirectUri, redirectUri);
+    }
+
+    private void updateForgottenPassword(UserRepresentation user, String clientId, String redirectUri, String requiredUri) throws IOException {
+        final int emailCount = greenMail.getReceivedMessages().length;
+
+        doForgotPassword(user.getUsername());
+        assertEquals("You should receive an email shortly with further instructions.", loginPage.getSuccessMessage());
+
+        events.expectRequiredAction(EventType.SEND_RESET_PASSWORD)
+                .user(user.getId())
+                .client(clientId)
+                .detail(Details.REDIRECT_URI, redirectUri)
+                .detail(Details.USERNAME, user.getUsername())
+                .detail(Details.EMAIL, user.getEmail())
+                .session((String) null)
+                .assertEvent();
+
+        assertEquals(emailCount + 1, greenMail.getReceivedMessages().length);
+
+        final MimeMessage message = greenMail.getReceivedMessages()[emailCount];
+        final String changePasswordUrl = MailUtils.getPasswordResetEmailLink(message);
+
+        BrowserTabUtil util = BrowserTabUtil.getInstanceAndSetEnv(driver);
+        util.newTab(changePasswordUrl.trim());
+
+        changePasswordOnUpdatePage(driver);
+
+        events.expectRequiredAction(EventType.UPDATE_PASSWORD)
+                .detail(Details.REDIRECT_URI, redirectUri)
+                .client(clientId)
+                .user(user.getId()).detail(Details.USERNAME, user.getUsername()).assertEvent();
+
+        assertThat(driver.getCurrentUrl(), Matchers.containsString(requiredUri));
+    }
+
+    private void doForgotPassword(String username) {
+        loginPage.assertCurrent();
+        loginPage.resetPassword();
+
+        resetPasswordPage.assertCurrent();
+        resetPasswordPage.changePassword(username);
+        WaitUtils.waitForPageToLoad();
+    }
+
+    private String getValidRedirectUriWithRootUrl(String rootUrl, Collection<String> redirectUris) {
+        final boolean isRootUrlValid = isValidUrl(rootUrl);
+
+        return redirectUris.stream()
+                .map(uri -> isRootUrlValid && uri.startsWith("/") ? rootUrl + uri : uri)
+                .map(uri -> uri.startsWith("/") ? OAuthClient.AUTH_SERVER_ROOT + uri : uri)
+                .map(RedirectUtils::validateRedirectUriWildcard)
+                .findFirst()
+                .orElse(null);
+    }
+
+    private boolean isValidUrl(String url) {
+        try {
+            new URL(url);
+            return true;
+        } catch (MalformedURLException e) {
+            return false;
+        }
+    }
 }