From d0042b550a088384e09bf31de2f714c6319f5db3 Mon Sep 17 00:00:00 2001 From: Simon Backx Date: Tue, 4 Apr 2023 18:07:37 +0200 Subject: [PATCH] Fixed redirecting back to account home after sign in in Portal (#16487) refs https://github.com/TryGhost/Team/issues/2674 When going to /#/portal/account when not signed in, you are redirected to the login page. But once signed in, you aren't redirected back to the account page. This fixes this issue by adding an extra and optional redirect parameter when requesting a magic token via email. This new parameter allows to override the default behaviour of using the Referer HTTP header, which doesn't include the hash/fragment part of the URL. The referrer is already restricted to only allow redirects to the site, not external URLs. --- ghost/core/core/server/services/members/middleware.js | 2 +- .../unit/server/services/members/middleware.test.js | 4 ++-- ghost/members-api/lib/controllers/router.js | 11 ++++++++++- .../pages/AccountHomePage/AccountHomePage.js | 5 ++++- ghost/portal/src/components/pages/SigninPage.js | 3 ++- ghost/portal/src/utils/api.js | 5 +++-- 6 files changed, 22 insertions(+), 8 deletions(-) diff --git a/ghost/core/core/server/services/members/middleware.js b/ghost/core/core/server/services/members/middleware.js index 6ebaad9307..42a58c51a4 100644 --- a/ghost/core/core/server/services/members/middleware.js +++ b/ghost/core/core/server/services/members/middleware.js @@ -267,7 +267,7 @@ const createSessionFromMagicLink = async function (req, res, next) { const redirectUrl = new URL(referrer); redirectUrl.searchParams.set('success', true); redirectUrl.searchParams.set('action', 'signin'); - return res.redirect(redirectUrl.pathname + redirectUrl.search); + return res.redirect(redirectUrl.href); } } diff --git a/ghost/core/test/unit/server/services/members/middleware.test.js b/ghost/core/test/unit/server/services/members/middleware.test.js index be5a4a945b..b1c3a210e0 100644 --- a/ghost/core/test/unit/server/services/members/middleware.test.js +++ b/ghost/core/test/unit/server/services/members/middleware.test.js @@ -146,7 +146,7 @@ describe('Members Service Middleware', function () { it('redirects member to referrer param path on signup if it is on the site', async function () { req.url = '/members?token=test&action=signin&r=https%3A%2F%2Fsite.com%2Fblah%2Fmy-post%2F'; - req.query = {token: 'test', action: 'signin', r: 'https://site.com/blah/my-post/'}; + req.query = {token: 'test', action: 'signin', r: 'https://site.com/blah/my-post/#comment-123'}; // Fake token handling failure membersService.ssr.exchangeTokenForSession.resolves({}); @@ -157,7 +157,7 @@ describe('Members Service Middleware', function () { // Check behavior next.calledOnce.should.be.false(); res.redirect.calledOnce.should.be.true(); - res.redirect.firstCall.args[0].should.eql('/blah/my-post/?success=true&action=signin'); + res.redirect.firstCall.args[0].should.eql('https://site.com/blah/my-post/?success=true&action=signin#comment-123'); }); it('does not redirect to referrer param if it is external', async function () { diff --git a/ghost/members-api/lib/controllers/router.js b/ghost/members-api/lib/controllers/router.js index 3b133f9636..fc3fd29cbe 100644 --- a/ghost/members-api/lib/controllers/router.js +++ b/ghost/members-api/lib/controllers/router.js @@ -319,12 +319,21 @@ module.exports = class RouterController { async sendMagicLink(req, res) { const {email, autoRedirect} = req.body; - let {emailType} = req.body; + let {emailType, redirect} = req.body; let referer = req.get('referer'); if (autoRedirect === false){ referer = null; } + if (redirect) { + try { + // Validate URL + referer = new URL(redirect).href; + } catch (e) { + logging.warn(e); + } + } + if (!email) { throw new errors.BadRequestError({ message: tpl(messages.emailRequired) diff --git a/ghost/portal/src/components/pages/AccountHomePage/AccountHomePage.js b/ghost/portal/src/components/pages/AccountHomePage/AccountHomePage.js index 05dc00a0a8..372c795e3c 100644 --- a/ghost/portal/src/components/pages/AccountHomePage/AccountHomePage.js +++ b/ghost/portal/src/components/pages/AccountHomePage/AccountHomePage.js @@ -12,7 +12,10 @@ export default class AccountHomePage extends React.Component { const {member} = this.context; if (!member) { this.context.onAction('switchPage', { - page: 'signin' + page: 'signin', + pageData: { + redirect: window.location.href // This includes the search/fragment of the URL (#/portal/account) which is missing from the default referer header + } }); } } diff --git a/ghost/portal/src/components/pages/SigninPage.js b/ghost/portal/src/components/pages/SigninPage.js index 15b2bc4886..45922a0357 100644 --- a/ghost/portal/src/components/pages/SigninPage.js +++ b/ghost/portal/src/components/pages/SigninPage.js @@ -33,9 +33,10 @@ export default class SigninPage extends React.Component { }; }, async () => { const {email, errors} = this.state; + const {redirect} = this.context.pageData ?? {}; const hasFormErrors = (errors && Object.values(errors).filter(d => !!d).length > 0); if (!hasFormErrors) { - this.context.onAction('signin', {email}); + this.context.onAction('signin', {email, redirect}); } }); } diff --git a/ghost/portal/src/utils/api.js b/ghost/portal/src/utils/api.js index 9895208752..06b05cf828 100644 --- a/ghost/portal/src/utils/api.js +++ b/ghost/portal/src/utils/api.js @@ -213,7 +213,7 @@ function setupGhostApi({siteUrl = window.location.origin, apiUrl, apiKey}) { }); }, - async sendMagicLink({email, emailType, labels, name, oldEmail, newsletters}) { + async sendMagicLink({email, emailType, labels, name, oldEmail, newsletters, redirect}) { const url = endpointFor({type: 'members', resource: 'send-magic-link'}); const body = { name, @@ -222,7 +222,8 @@ function setupGhostApi({siteUrl = window.location.origin, apiUrl, apiKey}) { oldEmail, emailType, labels, - requestSrc: 'portal' + requestSrc: 'portal', + redirect }; const urlHistory = getUrlHistory(); if (urlHistory) {