Răsfoiți Sursa

Merge pull request #977 from sharelatex/as-encode-auth-with-v1-query-strings

Encode redirects which will auth with v1 first
Alasdair Smith 8 ani în urmă
părinte
comite
45bd46bc01

+ 12 - 2
services/web/app/coffee/infrastructure/RedirectManager.coffee

@@ -1,5 +1,7 @@
 settings = require("settings-sharelatex")
 settings = require("settings-sharelatex")
 logger = require("logger-sharelatex")
 logger = require("logger-sharelatex")
+URL = require('url')
+querystring = require('querystring')
 
 
 module.exports = RedirectManager =
 module.exports = RedirectManager =
 	apply: (webRouter) ->
 	apply: (webRouter) ->
@@ -15,12 +17,20 @@ module.exports = RedirectManager =
 			else
 			else
 				if req.method == "POST"
 				if req.method == "POST"
 					code = 307
 					code = 307
+
 				if typeof target.url == "function"
 				if typeof target.url == "function"
 					url = target.url(req.params)
 					url = target.url(req.params)
 					if !url
 					if !url
 						return next()
 						return next()
 				else
 				else
 					url = target.url
 					url = target.url
+
+				# Special handling for redirecting to v1, to ensure that query params
+				# are encoded
+				if target.authWithV1
+					url = "/sign_in_to_v1?" + querystring.stringify(return_to: url + getQueryString(req))
+					return res.redirect code, url
+
 				if target.baseUrl?
 				if target.baseUrl?
 					url = "#{target.baseUrl}#{url}"
 					url = "#{target.baseUrl}#{url}"
 			res.redirect code, url + getQueryString(req)
 			res.redirect code, url + getQueryString(req)
@@ -29,5 +39,5 @@ module.exports = RedirectManager =
 # have differences between Express and Rails, so safer to just pass the raw
 # have differences between Express and Rails, so safer to just pass the raw
 # string
 # string
 getQueryString = (req) ->
 getQueryString = (req) ->
-	qs = req.url.match(/\?.*$/)
-	if qs? then qs[0] else ""
+	{search} = URL.parse(req.url)
+	if search then search else ""

+ 10 - 1
services/web/test/acceptance/coffee/RedirectUrlsTests.coffee

@@ -31,4 +31,13 @@ describe "RedirectUrls", ->
 		assertRedirect 'get', '/redirect/get_and_post', 302, '/destination/get_and_post', done
 		assertRedirect 'get', '/redirect/get_and_post', 302, '/destination/get_and_post', done
 
 
 	it 'redirects with query params', (done) ->
 	it 'redirects with query params', (done) ->
-		assertRedirect 'get', '/redirect/qs?foo=bar&baz[]=qux1&baz[]=qux2', 302, '/destination/qs?foo=bar&baz[]=qux1&baz[]=qux2', done
+		assertRedirect 'get', '/redirect/qs?foo=bar&baz[]=qux1&baz[]=qux2', 302, '/destination/qs?foo=bar&baz[]=qux1&baz[]=qux2', done
+
+	it 'redirects to /sign_in_to_v1 with authWithV1 setting', (done) ->
+		assertRedirect(
+			'get',
+			'/docs?zip_uri=http%3A%2F%2Foverleaf.test%2Ffoo%3Fbar%3Dbaz%26qux%3Dthing&bar=baz',
+			302,
+			'/sign_in_to_v1?return_to=%2Fdocs%3Fzip_uri%3Dhttp%253A%252F%252Foverleaf.test%252Ffoo%253Fbar%253Dbaz%2526qux%253Dthing%26bar%3Dbaz',
+			done
+		)

+ 4 - 0
services/web/test/acceptance/config/settings.test.coffee

@@ -128,3 +128,7 @@ module.exports =
 			url: (params) -> "/destination/#{params.id}/params"
 			url: (params) -> "/destination/#{params.id}/params"
 		},
 		},
 		'/redirect/qs': '/destination/qs'
 		'/redirect/qs': '/destination/qs'
+		'/docs': {
+			authWithV1: true
+			url: '/docs'
+		}