Session management enhancements

.gitignore

@@ -34,6 +34,7 @@ scripts/bin
 node_modules/
 bin/
 dist/
+ssl/
 tmp/
 coverage/
 my*

backend/__fixtures__/config.js

@@ -41,7 +41,7 @@ const defaultConfig = {
     ca,
     client_id: 'dashboard',
     redirect_uris: [
-      'http://localhost:8080/auth/callback'
+      'https://localhost:8443/auth/callback'
     ],
     scope: 'openid email profile groups audience:server:client_id:dashboard audience:server:client_id:kube-kubectl',
     clockTolerance: 42,

backend/lib/app.js

@@ -53,16 +53,18 @@ app.set('trust proxy', 1)
 app.set('etag', false)
 app.set('x-powered-by', false)
 
-app.use(helmet.dnsPrefetchControl())
-app.use(helmet.permittedCrossDomainPolicies())
-app.use(helmet.noSniff())
-app.use(helmet.hsts())
+app.use(helmet.xDnsPrefetchControl())
+app.use(helmet.xPermittedCrossDomainPolicies())
+app.use(helmet.xContentTypeOptions())
+if (process.env.NODE_ENV !== 'development') {
+  app.use(helmet.strictTransportSecurity())
+}
 app.use(noCache(STATIC_PATHS))
 app.use('/auth', auth.router)
 app.use('/webhook', githubWebhook.router)
 app.use('/api', api.router)
 
-app.use(helmet.xssFilter())
+app.use(helmet.xXssProtection())
 app.use(helmet.contentSecurityPolicy({
   directives: {
     defaultSrc: ['\'self\''],
@@ -88,7 +90,7 @@ app.use(expressStaticGzip(PUBLIC_DIRNAME, {
 }))
 app.use(STATIC_PATHS, notFound)
 
-app.use(helmet.frameguard({
+app.use(helmet.xFrameOptions({
   action: 'deny'
 }))
 app.use(historyFallback(INDEX_FILENAME))

backend/lib/security/constants.js

@@ -6,10 +6,13 @@
 
 'use strict'
 
+const HOST_PREFIX = '__Host-'
+
 module.exports = {
-  COOKIE_HEADER_PAYLOAD: 'gHdrPyl',
-  COOKIE_SIGNATURE: 'gSgn',
-  COOKIE_TOKEN: 'gTkn',
-  COOKIE_CODE_VERIFIER: 'gCdVrfr',
-  GARDENER_AUDIENCE: 'gardener'
+  GARDENER_AUDIENCE: 'gardener',
+  COOKIE_HEADER_PAYLOAD: HOST_PREFIX + 'gHdrPyl',
+  COOKIE_SIGNATURE: HOST_PREFIX + 'gSgn',
+  COOKIE_TOKEN: HOST_PREFIX + 'gTkn',
+  COOKIE_CODE_VERIFIER: HOST_PREFIX + 'gCdVrfr',
+  COOKIE_STATE: HOST_PREFIX + 'gStt'
 }

backend/lib/security/index.js

@@ -8,7 +8,7 @@
 
 const assert = require('assert').strict
 const crypto = require('crypto')
-const { split, join, some, every, includes, head, chain, pick } = require('lodash')
+const { split, join, includes, head, chain, pick } = require('lodash')
 const { Issuer, custom, generators, TokenSet } = require('openid-client')
 const pRetry = require('p-retry')
 const pTimeout = require('p-timeout')
@@ -42,6 +42,7 @@ const {
   COOKIE_TOKEN,
   COOKIE_SIGNATURE,
   COOKIE_CODE_VERIFIER,
+  COOKIE_STATE,
   GARDENER_AUDIENCE
 } = require('./constants')
 
@@ -66,14 +67,6 @@ if (ca) {
   httpOptions.ca = ca
 }
 
-const hasHttpsProtocol = uri => /^https:/.test(uri)
-const secure = some(redirectUris, hasHttpsProtocol)
-if (secure) {
-  assert.ok(every(redirectUris, hasHttpsProtocol), 'All \'redirect_uris\' must have the same protocol')
-} else if (process.env.NODE_ENV === 'production') {
-  logger.warn('The Gardener Dashboard is running in production but you don\'t use Transport Layer Security (TLS) to secure the connection and the data')
-}
-
 let clientPromise
 
 /**
@@ -162,9 +155,16 @@ async function authorizationUrl (req, res) {
   const redirectPath = frontendRedirectUrl.pathname + frontendRedirectUrl.search
   const redirectOrigin = frontendRedirectUrl.origin
   const backendRedirectUri = getBackendRedirectUri(redirectOrigin)
-  const state = encodeState({
+  const state = generators.state()
+  res.cookie(COOKIE_STATE, {
     redirectPath,
-    redirectOrigin
+    redirectOrigin,
+    state
+  }, {
+    secure: true,
+    httpOnly: true,
+    maxAge: 180_000, // cookie will be removed after 3 minutes
+    sameSite: 'Lax'
   })
   const client = await exports.getIssuerClient()
   if (!includes(redirectUris, backendRedirectUri)) {
@@ -179,11 +179,10 @@ async function authorizationUrl (req, res) {
     const codeChallengeMethod = getCodeChallengeMethod(client)
     const codeVerifier = generators.codeVerifier()
     res.cookie(COOKIE_CODE_VERIFIER, codeVerifier, {
-      secure,
+      secure: true,
       httpOnly: true,
-      maxAge: 300000, // cookie will be removed after 5 minutes
-      sameSite: 'Lax',
-      path: '/auth/callback'
+      maxAge: 180_000, // cookie will be removed after 3 minutes
+      sameSite: 'Lax'
     })
     switch (codeChallengeMethod) {
       case 'S256':
@@ -254,12 +253,12 @@ async function setCookies (res, tokenSet) {
   const accessToken = tokenSet.access_token
   const [header, payload, signature] = split(accessToken, '.')
   res.cookie(COOKIE_HEADER_PAYLOAD, join([header, payload], '.'), {
-    secure,
+    secure: true,
     expires: undefined,
     sameSite: 'Lax'
   })
   res.cookie(COOKIE_SIGNATURE, signature, {
-    secure,
+    secure: true,
     httpOnly: true,
     expires: undefined,
     sameSite: 'Lax'
@@ -270,7 +269,7 @@ async function setCookies (res, tokenSet) {
   }
   const encryptedValues = await encrypt(values.join(','))
   res.cookie(COOKIE_TOKEN, encryptedValues, {
-    secure,
+    secure: true,
     httpOnly: true,
     expires: undefined,
     sameSite: 'Lax'
@@ -279,19 +278,29 @@ async function setCookies (res, tokenSet) {
 }
 
 async function authorizationCallback (req, res) {
-  const { code, state } = req.query
-  const parameters = { code }
+  const options = {
+    secure: true,
+    path: '/'
+  }
+  const stateObject = {}
+  if (COOKIE_STATE in req.cookies) {
+    Object.assign(stateObject, req.cookies[COOKIE_STATE])
+    res.clearCookie(COOKIE_STATE, options)
+  }
   const {
     redirectPath,
-    redirectOrigin
-  } = decodeState(state)
+    redirectOrigin,
+    state
+  } = stateObject
+  const parameters = pick(req.query, ['code', 'state'])
   const backendRedirectUri = getBackendRedirectUri(redirectOrigin)
   const checks = {
-    response_type: 'code'
+    response_type: 'code',
+    state
   }
   if (COOKIE_CODE_VERIFIER in req.cookies) {
     checks.code_verifier = req.cookies[COOKIE_CODE_VERIFIER]
-    res.clearCookie(COOKIE_CODE_VERIFIER)
+    res.clearCookie(COOKIE_CODE_VERIFIER, options)
   }
   const tokenSet = await authorizationCodeExchange(backendRedirectUri, parameters, checks)
   await setCookies(res, tokenSet)
@@ -473,9 +482,13 @@ function authenticate (options = {}) {
 }
 
 function clearCookies (res) {
-  res.clearCookie(COOKIE_HEADER_PAYLOAD)
-  res.clearCookie(COOKIE_SIGNATURE)
-  res.clearCookie(COOKIE_TOKEN)
+  const options = {
+    secure: true,
+    path: '/'
+  }
+  res.clearCookie(COOKIE_HEADER_PAYLOAD, options)
+  res.clearCookie(COOKIE_SIGNATURE, options)
+  res.clearCookie(COOKIE_TOKEN, options)
 }
 
 exports = module.exports = {

backend/package.json

@@ -26,9 +26,9 @@
     "server.js"
   ],
   "scripts": {
-    "serve": "node server.js",
+    "serve": "cross-env NODE_ENV=development node server.js",
     "lint": "eslint --ext .js server.js .",
-    "test": "jest",
+    "test": "cross-env NODE_ENV=test jest",
     "test-coverage": "yarn test --coverage"
   },
   "dependencies": {
@@ -74,6 +74,7 @@
   "devDependencies": {
     "@gardener-dashboard/test-utils": "workspace:^",
     "abort-controller": "^3.0.0",
+    "cross-env": "^7.0.3",
     "dockerfile-ast": "^0.6.1",
     "eslint": "^8.57.0",
     "eslint-config-standard": "^17.1.0",

backend/test/acceptance/auth.spec.js

@@ -8,7 +8,7 @@
 
 const { pick, head } = require('lodash')
 const assert = require('assert').strict
-const { TokenSet } = require('openid-client')
+const { TokenSet, generators } = require('openid-client')
 const setCookieParser = require('set-cookie-parser')
 const { mockRequest } = require('@gardener-dashboard/request')
 
@@ -17,7 +17,6 @@ const {
   COOKIE_HEADER_PAYLOAD,
   COOKIE_SIGNATURE,
   COOKIE_TOKEN,
-  decodeState,
   setCookies,
   sign,
   decrypt,
@@ -118,6 +117,7 @@ describe('auth', function () {
   let agent
   let getIssuerClientStub
   let mockRefresh
+  let mockState
 
   beforeAll(() => {
     agent = createAgent()
@@ -134,34 +134,30 @@ describe('auth', function () {
     })
     getIssuerClientStub = jest.spyOn(security, 'getIssuerClient').mockResolvedValue(client)
     mockRefresh = jest.spyOn(client, 'refresh')
+    mockState = jest.spyOn(generators, 'state').mockReturnValue('state')
   })
 
   it('should redirect to authorization url without frontend redirectUrl', async function () {
-    const redirectPath = '/'
     const redirectUri = head(oidc.redirect_uris)
-    const redirectOrigin = new URL(redirectUri).origin
 
     const res = await agent
       .get('/auth')
       .redirects(0)
       .expect(302)
 
     expect(getIssuerClientStub).toBeCalledTimes(1)
+    expect(mockState).toBeCalledTimes(1)
     const url = new URL(res.headers.location)
     expect(url.searchParams.get('client_id')).toBe(oidc.client_id)
     expect(url.searchParams.get('redirect_uri')).toBe(redirectUri)
     expect(url.searchParams.get('scope')).toBe(oidc.scope)
     const state = url.searchParams.get('state')
-    expect(decodeState(state)).toEqual({
-      redirectPath,
-      redirectOrigin
-    })
+    expect(state).toEqual('state')
   })
 
   it('should redirect to authorization url with frontend redirectUrl', async function () {
     const redirectPath = '/namespace/garden-foo/administration'
     const redirectUri = head(oidc.redirect_uris)
-    const redirectOrigin = new URL(redirectUri).origin
     const redirectUrl = new URL(redirectPath, redirectUri).toString()
 
     const res = await agent
@@ -174,10 +170,7 @@ describe('auth', function () {
     const url = new URL(res.headers.location)
     expect(url.searchParams.get('redirect_uri')).toBe(redirectUri)
     const state = url.searchParams.get('state')
-    expect(decodeState(state)).toEqual({
-      redirectPath,
-      redirectOrigin
-    })
+    expect(state).toEqual('state')
   })
 
   it('should fail to redirect to authorization url', async function () {