Merge pull request #3476 from metabase/fix-login-redirect-2

Fix login redirect

frontend/src/metabase/auth/auth.js

@@ -16,7 +16,7 @@ const SessionApi = new AngularResourceProxy("Session", ["create", "createWithGoo
 
 
 // login
-export const login = createThunkAction("AUTH_LOGIN", function(credentials) {
+export const login = createThunkAction("AUTH_LOGIN", function(credentials, redirectUrl) {
     return async function(dispatch, getState) {
 
         if (!MetabaseUtils.validEmail(credentials.email)) {
@@ -32,7 +32,7 @@ export const login = createThunkAction("AUTH_LOGIN", function(credentials) {
             MetabaseAnalytics.trackEvent('Auth', 'Login');
             // TODO: redirect after login (carry user to intended destination)
             await dispatch(refreshCurrentUser());
-            dispatch(push("/"));
+            dispatch(push(redirectUrl || "/"));
 
         } catch (error) {
             return error;
@@ -42,7 +42,7 @@ export const login = createThunkAction("AUTH_LOGIN", function(credentials) {
 
 
 // login Google
-export const loginGoogle = createThunkAction("AUTH_LOGIN_GOOGLE", function(googleUser) {
+export const loginGoogle = createThunkAction("AUTH_LOGIN_GOOGLE", function(googleUser, redirectUrl) {
     return async function(dispatch, getState) {
         try {
             let newSession = await SessionApi.createWithGoogleAuth({
@@ -56,7 +56,7 @@ export const loginGoogle = createThunkAction("AUTH_LOGIN_GOOGLE", function(googl
 
             // TODO: redirect after login (carry user to intended destination)
             await dispatch(refreshCurrentUser());
-            dispatch(push("/"));
+            dispatch(push(redirectUrl || "/"));
 
         } catch (error) {
             clearGoogleAuthCredentials();

frontend/src/metabase/auth/containers/LoginApp.jsx

@@ -57,7 +57,7 @@ export default class LoginApp extends Component {
 
         this.validateForm();
 
-        const { loginGoogle } = this.props;
+        const { loginGoogle, location } = this.props;
 
         let ssoLoginButton = findDOMNode(this.refs.ssoLoginButton);
 
@@ -74,7 +74,7 @@ export default class LoginApp extends Component {
                       cookiepolicy: 'single_host_origin',
                   });
                   auth2.attachClickHandler(ssoLoginButton, {},
-                      (googleUser) => loginGoogle(googleUser),
+                      (googleUser) => loginGoogle(googleUser, location.query.redirect),
                       (error) => console.error('There was an error logging in', error)
                   );
                 })
@@ -96,10 +96,10 @@ export default class LoginApp extends Component {
     formSubmitted(e) {
         e.preventDefault();
 
-        let { login } = this.props;
+        let { login, location } = this.props;
         let { credentials } = this.state;
 
-        login(credentials);
+        login(credentials, location.query.redirect);
     }
 
     render() {

frontend/test/e2e/auth/login.spec.js

@@ -25,14 +25,14 @@ describeE2E("auth/login", () => {
             await driver.manage().deleteAllCookies();
         });
 
-        it ("should take you to the login page", async () => {
+        it("should take you to the login page", async () => {
             await driver.get(`${server.host}/`);
             await waitForUrl(driver, `${server.host}/auth/login?redirect=%2F`);
             expect(await driver.isElementPresent(By.css("[name=email]"))).toEqual(true);
             await screenshot(driver, "screenshots/auth-login.png");
         });
 
-        it ("should log you in", async () => {
+        it("should log you in", async () => {
             await driver.get(`${server.host}/`);
             await loginMetabase(driver, "bob@metabase.com", "12341234");
             await waitForUrl(driver, `${server.host}/`);
@@ -40,7 +40,7 @@ describeE2E("auth/login", () => {
             sessionId = sessionCookie.value;
         });
 
-        xit ("should redirect you after logging in", async () => {
+        it("should redirect you after logging in", async () => {
             await driver.get(`${server.host}/questions`);
             await waitForUrl(driver, `${server.host}/auth/login?redirect=%2Fquestions`);
             await loginMetabase(driver, "bob@metabase.com", "12341234");
@@ -55,13 +55,13 @@ describeE2E("auth/login", () => {
             await driver.manage().addCookie("metabase.SESSION_ID", sessionId);
         });
 
-        it ("is logged in", async () => {
+        it("is logged in", async () => {
             await driver.get(`${server.host}/`);
             await waitForUrl(driver, `${server.host}/`);
             await screenshot(driver, "screenshots/loggedin.png");
         });
 
-        it ("loads the qb", async () => {
+        it("loads the qb", async () => {
             await driver.get(`${server.host}/q#eyJuYW1lIjpudWxsLCJkYXRhc2V0X3F1ZXJ5Ijp7ImRhdGFiYXNlIjoxLCJ0eXBlIjoibmF0aXZlIiwibmF0aXZlIjp7InF1ZXJ5Ijoic2VsZWN0ICdvaCBoYWkgZ3Vpc2Ug8J-QsScifSwicGFyYW1ldGVycyI6W119LCJkaXNwbGF5Ijoic2NhbGFyIiwidmlzdWFsaXphdGlvbl9zZXR0aW5ncyI6e319`);
             await waitForUrl(driver, `${server.host}/q#eyJuYW1lIjpudWxsLCJkYXRhc2V0X3F1ZXJ5Ijp7ImRhdGFiYXNlIjoxLCJ0eXBlIjoibmF0aXZlIiwibmF0aXZlIjp7InF1ZXJ5Ijoic2VsZWN0ICdvaCBoYWkgZ3Vpc2Ug8J-QsScifSwicGFyYW1ldGVycyI6W119LCJkaXNwbGF5Ijoic2NhbGFyIiwidmlzdWFsaXphdGlvbl9zZXR0aW5ncyI6e319`);
             await screenshot(driver, "screenshots/qb.png");

frontend/test/e2e/support/utils.js

@@ -72,7 +72,7 @@ export const waitForElementAndClick = async (driver, selector, timeout = DEFAULT
 };
 
 export const waitForElementAndSendKeys = async (driver, selector, keys, timeout = DEFAULT_TIMEOUT) => {
-    log(`waiting for element to send ${keys}: ${selector}`);
+    log(`waiting for element to send "${keys}": ${selector}`);
     const element = await waitForElement(driver, selector, timeout);
     await element.clear();
     return await element.sendKeys(keys);
@@ -149,6 +149,7 @@ export const ensureLoggedIn = async (server, driver, email, password) => {
         return;
     }
     console.log("logging in");
+    await driver.get(`${server.host}/`);
     await driver.manage().deleteAllCookies();
     await driver.get(`${server.host}/`);
     await loginMetabase(driver, email, password);
@@ -186,11 +187,13 @@ export const describeE2E = (name, options, describeCallback) => {
                 SauceConnectResource.start(sauce).then(()=>
                     WebdriverResource.start(webdriver)),
             ]);
-            await webdriver.driver.manage().deleteAllCookies();
-            await webdriver.driver.manage().timeouts().implicitlyWait(100);
 
             global.driver = webdriver.driver;
             global.server = server;
+
+            await driver.get(`${server.host}/`);
+            await driver.manage().deleteAllCookies();
+            await driver.manage().timeouts().implicitlyWait(100);
         });
 
         it ("should start", async () => {
@@ -224,13 +227,34 @@ function jasmineMultipleSetupTeardown(fn) {
         const handlers = { beforeAll: [], beforeEach: [], afterEach: [], afterAll: [] }
         const originals = {};
 
+        // hook the global "describe" so we know if we're in an inner describe call,
+        // since we only want to grab the top-level setup/teardown handlers
+        let originalDescribe = global.describe;
+        let innerDescribe = false;
+        global.describe = (...args) => {
+            innerDescribe = true;
+            try {
+                return originalDescribe.apply(this, args);
+            } finally {
+                innerDescribe = false;
+            }
+        }
+
         Object.keys(handlers).map((name) => {
             originals[name] = global[name];
-            global[name] = (fn) => handlers[name].push(fn);
+            global[name] = (fn) => {
+                if (innerDescribe) {
+                    return originals[name](fn);
+                } else {
+                    return handlers[name].push(fn);
+                }
+            };
         });
 
         fn.apply(this, args);
 
+        global.describe = originalDescribe;
+
         // restore and register actual handler
         Object.keys(handlers).map((name) => {
             global[name] = originals[name];