If user is sent to login page with explicit redirect, obey

This commit is contained in:
Shane Kilkelly 2016-11-24 11:38:13 +00:00
parent cee3326ce3
commit 22101d0305
7 changed files with 28 additions and 18 deletions

View file

@ -157,7 +157,9 @@ module.exports = AuthenticationController =
return isValid return isValid
_redirectToLoginOrRegisterPage: (req, res, next)-> _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) return AuthenticationController._redirectToRegisterPage(req, res, next)
else else
AuthenticationController._redirectToLoginPage(req, res, next) AuthenticationController._redirectToLoginPage(req, res, next)

View file

@ -14,10 +14,7 @@ module.exports = SubscriptionController =
plansPage: (req, res, next) -> plansPage: (req, res, next) ->
plans = SubscriptionViewModelBuilder.buildViewModel() plans = SubscriptionViewModelBuilder.buildViewModel()
if AuthenticationController.isUserLoggedIn(req) baseUrl = ""
baseUrl = ""
else
baseUrl = "/register?redir="
viewName = "subscriptions/plans" viewName = "subscriptions/plans"
if req.query.v? if req.query.v?
viewName = "#{viewName}_#{req.query.v}" viewName = "#{viewName}_#{req.query.v}"

View file

@ -48,6 +48,11 @@ module.exports =
token: req.query.token token: req.query.token
loginPage : (req, res)-> 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', res.render 'user/login',
title: 'login', title: 'login',
email: req.query.email email: req.query.email

View file

@ -12,7 +12,7 @@ block content
| #{translate("join_sl_to_view_project")}. | #{translate("join_sl_to_view_project")}.
div div
| #{translate("if_you_are_registered")}, | #{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 else if newTemplateData.templateName !== undefined
h1 #{translate("register_to_edit_template", {templateName:newTemplateData.templateName})} h1 #{translate("register_to_edit_template", {templateName:newTemplateData.templateName})}

View file

@ -155,10 +155,11 @@ describe "AuthenticationController", ->
@AuthenticationController.passportLogin @req, @res, @next @AuthenticationController.passportLogin @req, @res, @next
@req.login.callCount.should.equal 0 @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 @AuthenticationController.passportLogin @req, @res, @next
@res.json.callCount.should.equal 1 @res.json.callCount.should.equal 1
@res.json.calledWith({message: @info}).should.equal true @res.json.calledWith({message: @info}).should.equal true
expect(@res.json.lastCall.args[0].redir?).to.equal false
describe 'afterLoginSessionSetup', -> describe 'afterLoginSessionSetup', ->
@ -269,9 +270,6 @@ describe "AuthenticationController", ->
it "should set res.session.justLoggedIn", -> it "should set res.session.justLoggedIn", ->
@req.session.justLoggedIn.should.equal true @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", -> it "should record the successful login", ->
@AuthenticationController._recordSuccessfulLogin @AuthenticationController._recordSuccessfulLogin
.calledWith(@user._id) .calledWith(@user._id)

View file

@ -30,6 +30,8 @@ describe "UserPagesController", ->
@AuthenticationController = @AuthenticationController =
getLoggedInUserId: sinon.stub().returns(@user._id) getLoggedInUserId: sinon.stub().returns(@user._id)
getSessionUser: sinon.stub().returns(@user) getSessionUser: sinon.stub().returns(@user)
_getRedirectFromSession: sinon.stub()
_setRedirectInSession: sinon.stub()
@UserPagesController = SandboxedModule.require modulePath, requires: @UserPagesController = SandboxedModule.require modulePath, requires:
"settings-sharelatex":@settings "settings-sharelatex":@settings
"logger-sharelatex": "logger-sharelatex":
@ -90,6 +92,20 @@ describe "UserPagesController", ->
done() done()
@UserPagesController.loginPage @req, @res @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', -> describe 'sessionsPage', ->
beforeEach -> beforeEach ->

View file

@ -144,10 +144,6 @@ expectInviteRedirectToRegister = (user, link, callback=(err,result)->) ->
expect(response.headers.location).to.match new RegExp("^/register.*$") expect(response.headers.location).to.match new RegExp("^/register.*$")
# follow redirect to register page and extract the redirectUrl from form # follow redirect to register page and extract the redirectUrl from form
user.request.get response.headers.location, (err, response, body) -> 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) callback(null)
expectLoginPage = (user, callback=(err, result)->) -> expectLoginPage = (user, callback=(err, result)->) ->
@ -161,14 +157,12 @@ expectLoginRedirectToInvite = (user, link, callback=(err, result)->) ->
tryLoginUser user, (err, response, body) -> tryLoginUser user, (err, response, body) ->
expect(err).to.be.oneOf [null, undefined] expect(err).to.be.oneOf [null, undefined]
expect(response.statusCode).to.equal 200 expect(response.statusCode).to.equal 200
# expect(link).to.match new RegExp("^.*#{body.redir}\?.*$")
callback(null, null) callback(null, null)
expectRegistrationRedirectToInvite = (user, email, link, callback=(err, result)->) -> expectRegistrationRedirectToInvite = (user, email, link, callback=(err, result)->) ->
tryRegisterUser user, email, (err, response, body) -> tryRegisterUser user, email, (err, response, body) ->
expect(err).to.be.oneOf [null, undefined] expect(err).to.be.oneOf [null, undefined]
expect(response.statusCode).to.equal 200 expect(response.statusCode).to.equal 200
# expect(link).to.match new RegExp("^.*#{body.redir}\?.*$")
callback(null, null) callback(null, null)
expectInviteRedirectToProject = (user, link, invite, callback=(err,result)->) -> expectInviteRedirectToProject = (user, link, invite, callback=(err,result)->) ->
@ -421,7 +415,6 @@ describe "ProjectInviteTests", ->
(cb) => revokeInvite(@sendingUser, @projectId, @invite._id, cb) (cb) => revokeInvite(@sendingUser, @projectId, @invite._id, cb)
], done ], done
# # # #
describe 'registration prompt workflow with valid token', -> describe 'registration prompt workflow with valid token', ->
it 'should redirect to the register page', (done) -> it 'should redirect to the register page', (done) ->
@ -438,7 +431,6 @@ describe "ProjectInviteTests", ->
(cb) => expectProjectAccess @user, @invite.projectId, cb (cb) => expectProjectAccess @user, @invite.projectId, cb
], done ], done
# # # #
describe 'registration prompt workflow with non-valid token', -> describe 'registration prompt workflow with non-valid token', ->
before (done)-> before (done)->