301 redirects should not set-cookie (#23360)

* 301 redirects should not set-cookie

Part of #1316

* Update release-notes.js

* No public facing doc changes (#23355)

Co-authored-by: jmarlena <6732600+jmarlena@users.noreply.github.com>

* New translation batch for pt (#23362)

* Add crowdin translations

* Run script/i18n/homogenize-frontmatter.js

* Run script/i18n/fix-translation-errors.js

* Run script/i18n/lint-translation-files.js --check parsing

* Run script/i18n/lint-translation-files.js --check rendering

* run script/i18n/reset-known-broken-translation-files.js

* Check in pt CSV report

* 24h cache all internal redirects without language prefix (#23354)

Part of #1271

* Redirects for root path should not set-cookie in all supported languages (#23377)

* test: add case when unsupported language is specified in redirect

* test: assert root redirects to appropriate language preference if specified

* refactor: remove redundant unit test

* refactor: re-order units starting with affirmative case

* refactor: remove single-use variable from unit

* fix: re-include previous unit with udpated description

Co-authored-by: Octomerger Bot <63058869+Octomerger@users.noreply.github.com>

Co-authored-by: Kevin Heis <heiskr@users.noreply.github.com>
Co-authored-by: Rogan Ferguson <40493721+roferg@users.noreply.github.com>
Co-authored-by: jmarlena <6732600+jmarlena@users.noreply.github.com>
Co-authored-by: docubot <67483024+docubot@users.noreply.github.com>
Co-authored-by: Francis <15894826+francisfuzz@users.noreply.github.com>
Co-authored-by: Octomerger Bot <63058869+Octomerger@users.noreply.github.com>

Author: 7 people

middleware/redirects/external.js

@@ -4,6 +4,8 @@ const externalSites = readJsonFile('./lib/redirects/external-sites.json')
 // blanket redirects to external websites
 export default function externalRedirects(req, res, next) {
   if (req.path in externalSites) {
+    // Undo the cookie setting that CSRF sets.
+    res.removeHeader('set-cookie')
     return res.redirect(301, externalSites[req.path])
   } else {
     return next()

middleware/redirects/handle-redirects.js

@@ -23,7 +23,11 @@ export default function handleRedirects(req, res, next) {
       language = req.context.userLanguage
     }
 
+    // Undo the cookie setting that CSRF sets.
+    res.removeHeader('set-cookie')
+
     noCacheControl(res)
+
     return res.redirect(302, `/${language}`)
   }
 
@@ -67,6 +71,9 @@ export default function handleRedirects(req, res, next) {
     return next()
   }
 
+  // Undo the cookie setting that CSRF sets.
+  res.removeHeader('set-cookie')
+
   // do the redirect if the from-URL already had a language in it
   if (pathLanguagePrefixed(req.path)) {
     cacheControl(res)

tests/rendering/server.js

@@ -7,6 +7,7 @@ import CspParse from 'csp-parse'
 import { productMap } from '../../lib/all-products.js'
 import { SURROGATE_ENUMS } from '../../middleware/set-fastly-surrogate-key.js'
 import { jest } from '@jest/globals'
+import { languageKeys } from '../../lib/languages.js'
 
 const AZURE_STORAGE_URL = 'githubdocs.azureedge.net'
 const activeProducts = Object.values(productMap).filter(
@@ -610,6 +611,7 @@ describe('server', () => {
     test('redirects old articles to their English URL', async () => {
       const res = await get('/articles/deleting-a-team')
       expect(res.statusCode).toBe(302)
+      expect(res.headers['set-cookie']).toBeUndefined()
       // no cache control because a language prefix had to be injected
       expect(res.headers['cache-control']).toBeUndefined()
     })
@@ -621,17 +623,48 @@ describe('server', () => {
       )
     })
 
-    test('redirects / to /en', async () => {
+    test('redirects / to /en when no language preference is specified', async () => {
       const res = await get('/')
       expect(res.statusCode).toBe(302)
       expect(res.headers.location).toBe('/en')
       expect(res.headers['cache-control']).toBe('private, no-store')
+      expect(res.headers['set-cookie']).toBeUndefined()
+    })
+
+    test('redirects / to appropriate language preference if specified', async () => {
+      await Promise.all(
+        languageKeys.map(async (languageKey) => {
+          const res = await get('/', {
+            headers: {
+              'accept-language': `${languageKey}`,
+            },
+          })
+          expect(res.statusCode).toBe(302)
+          expect(res.headers.location).toBe(`/${languageKey}`)
+          expect(res.headers['cache-control']).toBe('private, no-store')
+          expect(res.headers['set-cookie']).toBeUndefined()
+        })
+      )
+    })
+
+    test('redirects / to /en when unsupported language preference is specified', async () => {
+      const res = await get('/', {
+        headers: {
+          // Tagalog: https://www.loc.gov/standards/iso639-2/php/langcodes_name.php?iso_639_1=tl
+          'accept-language': 'tl',
+        },
+      })
+      expect(res.statusCode).toBe(302)
+      expect(res.headers.location).toBe('/en')
+      expect(res.headers['cache-control']).toBe('private, no-store')
+      expect(res.headers['set-cookie']).toBeUndefined()
     })
 
     test('adds English prefix to old article URLs', async () => {
       const res = await get('/articles/deleting-a-team')
       expect(res.statusCode).toBe(302)
       expect(res.headers.location.startsWith('/en/')).toBe(true)
+      expect(res.headers['set-cookie']).toBeUndefined()
       expect(res.headers['cache-control']).toBeUndefined()
     })
 
@@ -646,6 +679,7 @@ describe('server', () => {
       const res = await get('/desktop-classic')
       expect(res.statusCode).toBe(301)
       expect(res.headers.location).toBe('https://desktop.github.com')
+      expect(res.headers['set-cookie']).toBeUndefined()
       expect(res.headers['cache-control']).toBeUndefined()
     })