remove auth hint on login

tunnel-server/src/app/index.test.ts

@@ -14,7 +14,7 @@ import { SessionStore } from '../session.js'
 import { Claims, cliIdentityProvider, jwtAuthenticator, saasIdentityProvider } from '../auth.js'
 import { ActiveTunnel, ActiveTunnelStore } from '../tunnel-store/index.js'
 import { EntryWatcher } from '../memory-store.js'
-import { proxy } from '../proxy/index.js'
+import { authHintQueryParam, proxy } from '../proxy/index.js'
 import { calcLoginUrl } from './urls.js'
 
 const pinoPretty = pinoPrettyModule.default
@@ -198,13 +198,13 @@ describe('app', () => {
           watcher: undefined as unknown as EntryWatcher,
         }))
         user = { } as Claims
-        response = await request(`${baseAppUrl}/login?env=myenv&returnPath=/bla`, { headers: { host: 'api.base.livecycle.example' } })
+        response = await request(`${baseAppUrl}/login?env=myenv&returnPath=${encodeURIComponent(`/bla?foo=bar&${authHintQueryParam}=basic`)}`, { headers: { host: 'api.base.livecycle.example' } })
       })
 
       it('should return a redirect to the env page', async () => {
         expect(response.statusCode).toBe(302)
         const locationHeader = response.headers.location
-        expect(locationHeader).toBe('http://myenv.base.livecycle.example/bla')
+        expect(locationHeader).toBe(`http://myenv.base.livecycle.example/bla?foo=bar&${authHintQueryParam}=basic`)
       })
     })
   })
@@ -274,7 +274,7 @@ describe('app', () => {
 
         describe('with basic auth hint', () => {
           beforeEach(async () => {
-            response = await request(`${baseAppUrl}/bla?_preevy_auth_hint=basic`, { headers: { host: 'my-tunnel.base.livecycle.example' } })
+            response = await request(`${baseAppUrl}/bla?${authHintQueryParam}=basic`, { headers: { host: 'my-tunnel.base.livecycle.example' } })
           })
 
           it('should return an unauthorized status with basic auth header', async () => {
@@ -285,21 +285,45 @@ describe('app', () => {
 
         describe('with basic auth', () => {
           const originServer = setupOriginServer()
+          let jwt: string
           beforeEach(async () => {
             activeTunnel.target = originServer.listenPath
             sessionStoreStore.set.mockImplementation(u => { user = u })
-            const jwt = await jwtGenerator(envKey)
-            response = await request(`${baseAppUrl}/bla`, {
-              headers: {
-                host: 'my-tunnel.base.livecycle.example',
-                authorization: `Basic ${Buffer.from(`x-preevy-profile-key:${jwt}`).toString('base64')}`,
-              },
+            jwt = await jwtGenerator(envKey)
+          })
+
+          describe('from a non-browser', () => {
+            beforeEach(async () => {
+              response = await request(`${baseAppUrl}/bla`, {
+                headers: {
+                  host: 'my-tunnel.base.livecycle.example',
+                  authorization: `Basic ${Buffer.from(`x-preevy-profile-key:${jwt}`).toString('base64')}`,
+                },
+              })
+            })
+
+            it('should return the origin response', async () => {
+              expect(response.statusCode).toBe(200)
+              expect(await response.body.text()).toBe('hello')
             })
           })
 
-          it('should return the origin response', async () => {
-            expect(response.statusCode).toBe(200)
-            expect(await response.body.text()).toBe('hello')
+          describe('from a browser', () => {
+            beforeEach(async () => {
+              response = await request(`${baseAppUrl}/bla?${authHintQueryParam}=basic`, {
+                headers: {
+                  host: 'my-tunnel.base.livecycle.example',
+                  'user-agent': 'chrome',
+                  authorization: `Basic ${Buffer.from(`x-preevy-profile-key:${jwt}`).toString('base64')}`,
+                },
+              })
+            })
+
+            it('should return a redirect to the login page', async () => {
+              expect(response.statusCode).toBe(307)
+              const locationHeader = response.headers.location
+              expect(locationHeader).toBe('http://auth.base.livecycle.example/login?env=my-tunnel&returnPath=%2Fbla')
+            })
           })
         })
 

tunnel-server/src/app/login.ts

@@ -52,10 +52,8 @@ export const login: FastifyPluginAsync<{
       session.set(result.claims)
       session.save()
     }
-    const redirectTo = editUrl(baseUrl, {
-      hostname: `${envId}.${baseUrl.hostname}`,
-      path: returnPath ?? '/',
-    })
+    const envBaseUrl = editUrl(baseUrl, { hostname: `${envId}.${baseUrl.hostname}` })
+    const redirectTo = new URL(returnPath ?? '/', envBaseUrl)
     return await res.redirect(redirectTo.toString())
   })
 }

tunnel-server/src/proxy/index.ts

@@ -12,9 +12,21 @@ import { SessionStore } from '../session.js'
 import { BadGatewayError, BadRequestError, BasicAuthUnauthorizedError, RedirectError, UnauthorizedError, errorHandler, errorUpgradeHandler, tryHandler, tryUpgradeHandler } from '../http-server-helpers.js'
 import { TunnelFinder, proxyRouter } from './router.js'
 import { proxyInjectionHandlers } from './injection/index.js'
+import { editUrl } from '../url.js'
+
+export const authHintQueryParam = '_preevy_auth_hint'
 
 const hasBasicAuthQueryParamHint = (url: string) =>
-  new URL(url, 'http://a').searchParams.get('_preevy_auth_hint') === 'basic'
+  new URL(url, 'http://a').searchParams.get(authHintQueryParam) === 'basic'
+
+const removeBasicAuthQueryParamHint = (pathAndSearch: string) => {
+  const u = editUrl(
+    new URL(pathAndSearch, 'http://a'),
+    { removeQueryParams: [authHintQueryParam] },
+  )
+
+  return u.pathname + u.search
+}
 
 export const proxy = ({
   activeTunnelStore,
@@ -44,7 +56,7 @@ export const proxy = ({
     if (!session.user) {
       const redirectToLoginError = () => new RedirectError(
         307,
-        loginUrl({ env: tunnel.hostname, returnPath: req.url }),
+        loginUrl({ env: tunnel.hostname, returnPath: req.url && removeBasicAuthQueryParamHint(req.url) }),
       )
 
       const authenticate = authFactory(tunnel)

tunnel-server/src/url.test.ts

@@ -31,5 +31,12 @@ describe('url', () => {
         expect(editUrl(baseUrl, { path: 'otherpath' }).toJSON()).toEqual(new URL('http://example.com/otherpath?x=12&y=13').toJSON())
       })
     })
+
+    describe('when given removeQueryParams', () => {
+      it('should remove them', () => {
+        expect(editUrl(baseUrl, { removeQueryParams: ['y'] }).toJSON()).toEqual(new URL('http://example.com/mypath?x=12').toJSON())
+        expect(editUrl(baseUrl, { queryParams: { y: '14' }, removeQueryParams: ['y'] }).toJSON()).toEqual(new URL('http://example.com/mypath?x=12').toJSON())
+      })
+    })
   })
 })

tunnel-server/src/url.ts

@@ -1,21 +1,25 @@
-import { defaults } from 'lodash-es'
+import { defaults, omit } from 'lodash-es'
 
 export const editUrl = (
   url: URL | string,
-  { hostname, queryParams, username, password, path }: Partial<{
+  { hostname, queryParams, username, password, path, removeQueryParams }: Partial<{
     hostname: string
     queryParams: Record<string, string>
     username: string
     password: string
     path: string
+    removeQueryParams: string[]
   }>,
 ): URL => {
   const u = new URL(url.toString())
   return Object.assign(u, {
     ...hostname ? { hostname } : {},
-    ...queryParams ? {
-      search: new URLSearchParams(defaults(queryParams, Object.fromEntries(u.searchParams.entries()))),
-    } : {},
+    ...{
+      search: new URLSearchParams(omit(
+        defaults(queryParams, Object.fromEntries(u.searchParams.entries())),
+        ...(removeQueryParams ?? [])
+      )),
+    },
     ...username ? { username } : {},
     ...password ? { password } : {},
     ...path ? { pathname: path } : {},