diff --git a/services/web/app/coffee/Features/Authentication/AuthenticationController.coffee b/services/web/app/coffee/Features/Authentication/AuthenticationController.coffee index 59f55127f1..5a92cc0f75 100644 --- a/services/web/app/coffee/Features/Authentication/AuthenticationController.coffee +++ b/services/web/app/coffee/Features/Authentication/AuthenticationController.coffee @@ -157,7 +157,9 @@ module.exports = AuthenticationController = return isValid _redirectToLoginOrRegisterPage: (req, res, next)-> - if req.query.zipUrl? or req.query.project_name? + if (req.query.zipUrl? or + req.query.project_name? or + req.path == '/user/subscription/new') return AuthenticationController._redirectToRegisterPage(req, res, next) else AuthenticationController._redirectToLoginPage(req, res, next) diff --git a/services/web/app/coffee/Features/Subscription/SubscriptionController.coffee b/services/web/app/coffee/Features/Subscription/SubscriptionController.coffee index 51b3db8f56..d864cac1d1 100644 --- a/services/web/app/coffee/Features/Subscription/SubscriptionController.coffee +++ b/services/web/app/coffee/Features/Subscription/SubscriptionController.coffee @@ -14,10 +14,7 @@ module.exports = SubscriptionController = plansPage: (req, res, next) -> plans = SubscriptionViewModelBuilder.buildViewModel() - if AuthenticationController.isUserLoggedIn(req) - baseUrl = "" - else - baseUrl = "/register?redir=" + baseUrl = "" viewName = "subscriptions/plans" if req.query.v? viewName = "#{viewName}_#{req.query.v}" diff --git a/services/web/app/coffee/Features/User/UserPagesController.coffee b/services/web/app/coffee/Features/User/UserPagesController.coffee index 00494d71e5..54cfd54892 100644 --- a/services/web/app/coffee/Features/User/UserPagesController.coffee +++ b/services/web/app/coffee/Features/User/UserPagesController.coffee @@ -48,6 +48,11 @@ module.exports = token: req.query.token loginPage : (req, res)-> + # if user is being sent to /login with explicit redirect (redir=/foo), + # such as being sent from the editor to /login, then set the redirect explicitly + if req.query.redir? and !AuthenticationController._getRedirectFromSession(req)? + logger.log {redir: req.query.redir}, "setting explicit redirect from login page" + AuthenticationController._setRedirectInSession(req, req.query.redir) res.render 'user/login', title: 'login', email: req.query.email diff --git a/services/web/app/views/user/register.jade b/services/web/app/views/user/register.jade index 5a1196b6a6..2a5ab707a6 100644 --- a/services/web/app/views/user/register.jade +++ b/services/web/app/views/user/register.jade @@ -12,7 +12,7 @@ block content | #{translate("join_sl_to_view_project")}. div | #{translate("if_you_are_registered")}, - a(href="/login?redir=#{getReqQueryParam('redir')}") #{translate("login_here")} + a(href="/login") #{translate("login_here")} else if newTemplateData.templateName !== undefined h1 #{translate("register_to_edit_template", {templateName:newTemplateData.templateName})} diff --git a/services/web/test/UnitTests/coffee/Authentication/AuthenticationControllerTests.coffee b/services/web/test/UnitTests/coffee/Authentication/AuthenticationControllerTests.coffee index 378c3bafd2..72265eac11 100644 --- a/services/web/test/UnitTests/coffee/Authentication/AuthenticationControllerTests.coffee +++ b/services/web/test/UnitTests/coffee/Authentication/AuthenticationControllerTests.coffee @@ -155,10 +155,11 @@ describe "AuthenticationController", -> @AuthenticationController.passportLogin @req, @res, @next @req.login.callCount.should.equal 0 - it 'should send a json response with redirect', () -> + it 'should not send a json response with redirect', () -> @AuthenticationController.passportLogin @req, @res, @next @res.json.callCount.should.equal 1 @res.json.calledWith({message: @info}).should.equal true + expect(@res.json.lastCall.args[0].redir?).to.equal false describe 'afterLoginSessionSetup', -> @@ -269,9 +270,6 @@ describe "AuthenticationController", -> it "should set res.session.justLoggedIn", -> @req.session.justLoggedIn.should.equal true - it "should redirect the user to the specified location", -> - expect(@req._redir).to.deep.equal @redir - it "should record the successful login", -> @AuthenticationController._recordSuccessfulLogin .calledWith(@user._id) diff --git a/services/web/test/UnitTests/coffee/User/UserPagesControllerTests.coffee b/services/web/test/UnitTests/coffee/User/UserPagesControllerTests.coffee index 24ca3eeeda..f0fa84176f 100644 --- a/services/web/test/UnitTests/coffee/User/UserPagesControllerTests.coffee +++ b/services/web/test/UnitTests/coffee/User/UserPagesControllerTests.coffee @@ -30,6 +30,8 @@ describe "UserPagesController", -> @AuthenticationController = getLoggedInUserId: sinon.stub().returns(@user._id) getSessionUser: sinon.stub().returns(@user) + _getRedirectFromSession: sinon.stub() + _setRedirectInSession: sinon.stub() @UserPagesController = SandboxedModule.require modulePath, requires: "settings-sharelatex":@settings "logger-sharelatex": @@ -90,6 +92,20 @@ describe "UserPagesController", -> done() @UserPagesController.loginPage @req, @res + describe 'when an explicit redirect is set via query string', -> + + beforeEach -> + @AuthenticationController._getRedirectFromSession = sinon.stub().returns(null) + @AuthenticationController._setRedirectInSession = sinon.stub() + @req.query.redir = '/somewhere/in/particular' + + it 'should set a redirect', (done) -> + @res.render = (page) => + @AuthenticationController._setRedirectInSession.callCount.should.equal 1 + expect(@AuthenticationController._setRedirectInSession.lastCall.args[1]).to.equal @req.query.redir + done() + @UserPagesController.loginPage @req, @res + describe 'sessionsPage', -> beforeEach -> diff --git a/services/web/test/acceptance/coffee/ProjectInviteTests.coffee b/services/web/test/acceptance/coffee/ProjectInviteTests.coffee index 3068892a68..b599578738 100644 --- a/services/web/test/acceptance/coffee/ProjectInviteTests.coffee +++ b/services/web/test/acceptance/coffee/ProjectInviteTests.coffee @@ -144,10 +144,6 @@ expectInviteRedirectToRegister = (user, link, callback=(err,result)->) -> expect(response.headers.location).to.match new RegExp("^/register.*$") # follow redirect to register page and extract the redirectUrl from form user.request.get response.headers.location, (err, response, body) -> - # redirectUrl = body.match(/input name="redir" type="hidden" value="([^"]*)"/m)?[1] - # loginUrl = body.match(/href="([^"]*)">\s*Login here/m)?[1] - # expect(redirectUrl).to.not.be.oneOf [null, undefined] - # expect(loginUrl).to.not.be.oneOf [null, undefined] callback(null) expectLoginPage = (user, callback=(err, result)->) -> @@ -161,14 +157,12 @@ expectLoginRedirectToInvite = (user, link, callback=(err, result)->) -> tryLoginUser user, (err, response, body) -> expect(err).to.be.oneOf [null, undefined] expect(response.statusCode).to.equal 200 - # expect(link).to.match new RegExp("^.*#{body.redir}\?.*$") callback(null, null) expectRegistrationRedirectToInvite = (user, email, link, callback=(err, result)->) -> tryRegisterUser user, email, (err, response, body) -> expect(err).to.be.oneOf [null, undefined] expect(response.statusCode).to.equal 200 - # expect(link).to.match new RegExp("^.*#{body.redir}\?.*$") callback(null, null) expectInviteRedirectToProject = (user, link, invite, callback=(err,result)->) -> @@ -421,7 +415,6 @@ describe "ProjectInviteTests", -> (cb) => revokeInvite(@sendingUser, @projectId, @invite._id, cb) ], done - # # # # describe 'registration prompt workflow with valid token', -> it 'should redirect to the register page', (done) -> @@ -438,7 +431,6 @@ describe "ProjectInviteTests", -> (cb) => expectProjectAccess @user, @invite.projectId, cb ], done - # # # # describe 'registration prompt workflow with non-valid token', -> before (done)->