Skip to content

Commit

Permalink
Added maxAge to cookie to be more secure by default, auth service now… (
Browse files Browse the repository at this point in the history
#23)

* Added maxAge to cookie to be more secure by default, auth service now only listens on localhost for security reasons

* Removed old config var no longer used

* Added default maxAge to cookie for security reasons

* Fixed a bug with setting of cookie expiry
  • Loading branch information
rorylshanks authored Mar 17, 2024
1 parent 65011f5 commit 143f954
Show file tree
Hide file tree
Showing 9 changed files with 69 additions and 42 deletions.
6 changes: 2 additions & 4 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -76,16 +76,15 @@ This will generate a base64 encoded 4096-bit RSA private key that can be used in

An example configuration file can be found in `example-config.yaml`. A breakdown of each option is below

- `auth_listen_port`: Port on which the authentication server listens.
- `data_listen_port`: Port on which the data server listens.
- `metrics_listen_port`: Port where the metrics server should listen.
- `service_url`: URL of the Veriflow service.
- `cookie_secret`: Secret key used for cookie encryption and verification.
- `cookie_settings`: Settings related to the session cookie set by Veriflow for each site.
- `sameSite`: Sets the sameSite attribute of the cookie. Default "Lax"
- `secure`: Sets whether the cookie should be secure. Default "false"
- `maxAge`: Sets the maximim time of the authenticated sessions in milliseconds (default 3 hours [3600000])
- `redis_connection_string`: Connection string for the redis server to connect to (e.g. redis://127.0.0.1:6379)
- `redis_host`: Hostname of the Redis database server. (deprecated, for backwards compatibility only)
- `redis_port`: Port of the Redis database server. (deprecated, for backwards compatibility only)
- `idp_client_id`: Client ID for communication with the Identity Provider (IdP).
- `idp_client_secret`: Secret key for authenticating with the Identity Provider (IdP).
- `idp_tenant_id`: Identifier for the specific tenant in the Identity Provider's system.
Expand All @@ -95,7 +94,6 @@ An example configuration file can be found in `example-config.yaml`. A breakdown
- `idp_provider_url`: URL of the Identity Provider service.
- `idp_refresh_directory_interval`: How often the directory information should be refreshed from the Identity Provider.
- `idp_refresh_directory_timeout`: How long the system should wait for a directory refresh before timing out.
- `metrics_address`: Address and port where the metrics server should listen.
- `signing_key`: RSA private key for signing JWT tokens, encoded in base64.
- `redirect_base_path`: Base path for redirection URLs. By default `/.veriflow`
- `jwks_path`: Location of the JSON Web Key Set (JWKS) that can be called to get the public keys of the signing key.
Expand Down
3 changes: 1 addition & 2 deletions example-config.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -5,8 +5,8 @@ ui:
logo_image_src: https://upload.wikimedia.org/wikipedia/commons/thumb/2/2f/Google_2015_logo.svg/1200px-Google_2015_logo.svg.png
error_page_show_error_code: false

auth_listen_port: 3000
data_listen_port: 2080
metrics_listen_port: 9090
service_url: https://veriflow.codemo.re
cookie_secret: "ThisIsAFakeCookieSecret"
cookie_settings:
Expand All @@ -23,7 +23,6 @@ idp_provider_user_id_claim: oid
idp_provider_url: https://login.microsoftonline.com/00000000-1111-2222-3333-444444444444/v2.0
idp_refresh_directory_interval: 10m
idp_refresh_directory_timeout: 5m
metrics_address: 0.0.0.0:9090
signing_key: "BASE64_ENCODED_RSA_PRIVATE_KEY"
redirect_base_path: /.veriflow
jwks_path: /.well-known/veriflow/jwks.json
Expand Down
18 changes: 9 additions & 9 deletions lib/http.js
Original file line number Diff line number Diff line change
Expand Up @@ -6,21 +6,18 @@ import ssoController from './sso.js';

import redisHelper from "../util/redis.js"
import log from '../util/logging.js'
import { getConfig, getRedirectBasepath } from '../util/config.js';
import { getConfig, getRedirectBasepath, getAuthListenPort } from '../util/config.js';
import { pem2jwk } from 'pem-jwk';
import crypto from 'crypto';
import errorpages from '../util/errorpage.js'



var trusted_ranges = ["loopback"].concat(getConfig().trusted_ranges || [])
log.info({ message: `Setting trusted proxies to ${trusted_ranges}` })
app.set('trust proxy', trusted_ranges)

if (trusted_ranges) {
log.info({ message: `Setting trusted proxies to ${trusted_ranges}` })
app.set('trust proxy', trusted_ranges)
}


var defaultCookieOptions = { maxAge: 3600000 }

// Initialize sesssion storage.
app.use(
Expand All @@ -30,7 +27,10 @@ app.use(
resave: false,
saveUninitialized: false,
secret: getConfig().cookie_secret,
cookie: getConfig().cookie_settings || {}
cookie: {
...defaultCookieOptions,
...getConfig().cookie_settings
}
})
)

Expand Down Expand Up @@ -84,4 +84,4 @@ app.get(getConfig().jwks_path, (req, res) => {
});
});

app.listen(getConfig().auth_listen_port, () => log.debug("Veriflow HTTP server running"));
app.listen(getAuthListenPort(), 'localhost', () => log.debug("Veriflow HTTP server running"));
10 changes: 9 additions & 1 deletion lib/sso.js
Original file line number Diff line number Diff line change
Expand Up @@ -144,7 +144,15 @@ async function setSessionCookie(req, res) {

// This sets the cookie for the accessed domain to expire at the same time as the "main" veriflow cookie, to
// prevent a user from being deauthenticated from Veriflow, but still authenticated on the subdomains.
req.session.cookie.expires = decoded.cookieExpires
var expireDate = false
try {
var date = new Date(decoded.cookieExpires)
expireDate = date
} catch (error) {
log.error({ message: "Unable to get cookie expiry from decoded date", context: decoded})
}

req.session.cookie.expires = expireDate

var redirectProtocol = decoded.protocol
var redirectHost = decoded.host
Expand Down
3 changes: 2 additions & 1 deletion test/e2e/configs/dex.yaml
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
issuer: http://127.0.0.1:5556/dex
issuer: http://dex.localtest.me:5556/dex
storage:
type: sqlite3
config:
Expand All @@ -18,6 +18,7 @@ staticClients:
- id: 0c4860a4-ae2b-4f49-97d7-b581252a7166
redirectURIs:
- 'http://localhost:2080/.veriflow/callback'
- 'http://veriflow.localtest.me/.veriflow/callback'
name: 'Veriflow'
secret: supersecret

Expand Down
10 changes: 4 additions & 6 deletions test/e2e/configs/veriflow.yaml
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
---
auth_listen_port: 3000
data_listen_port: 2080
service_url: http://localhost:2080
data_listen_port: 80
metrics_listen_port: 9090
service_url: http://veriflow.localtest.me
cookie_secret: "fakeCookieSecret"
redis_connection_string: redis://127.0.0.1:6379
idp_client_id: 0c4860a4-ae2b-4f49-97d7-b581252a7166
Expand All @@ -10,14 +10,12 @@ idp_provider: localfile
idp_provider_localfile_location: output.json
idp_provider_scope: openid email profile
idp_provider_user_id_claim: email
idp_provider_url: http://localhost:5556/dex
idp_provider_url: http://dex.localtest.me:5556/dex
idp_refresh_directory_interval: 10m
idp_refresh_directory_timeout: 5m
refresh_idp_at_start: true
metrics_address: 0.0.0.0:9090
# BELOW KEY IS ONLY FOR TESTING AND NOT A REAL KEY
signing_key: "LS0tLS1CRUdJTiBSU0EgUFJJVkFURSBLRVktLS0tLQpNSUlFcFFJQkFBS0NBUUVBazdPT1kyZzk4QnVRWFFGN0d6VXQ3V3pQc1hsQXBXYWg5blozMDZwZzJUV0NXYm95CmIwekJEVWpmeWZHWDNEUnl1NTdZSFgwL2pwWUdhcGt6YW41SkxWSkl2MmpsQnZkWGZOREFvRkFWVURnTFJYa3QKN3FlMnRNazFGMEFoQnBpbWdQSUlMbU9HRVJrSzZ4cmFmRHorTXRyV2NOVnNqdmJUZ1ZuSC85K2lBSGZXSTQ3Zwp4NWd1MmJuR1U2N3cyN3BvZGo1ZWsyUGdBR0cwb2M0dUJiY3V4cGFOSy9xdERnZU1samtyRWc0S0NpYmlFMUFuCm5aUUc2aGkrNkJuQnNWQkRWTDV5L0xkOUhrS0FxNDB2MHQ4VThKZ0xqNElmcUJ4c2Z0WllyL2Y4UDhoR2Z1N0UKek13ZS91MnU2NXZCRVBlRnNYQk14UmZqQ041enVlVWI2L1RRaXdJREFRQUJBb0lCQVFDUlZ1bXhKZjEwelJyVQpla1dTYzFUN1FjeHFQZitBQXFzelpFWHJVY2UxVlhNc0tnM0ErYzBwN21EUVRkeDZRbDM0QTRsMEV6QThkYUpnCnVOb2dXNTVVYTVqTVNVSzlCUnpnNUdYNEduV3VsMGQ0R0pNN09XdVBJRU1PMnZya2k4ZWtNUVlkNTY4Z0dmMWwKZGVveXdLMytpdHJpOHhDODZXTWM4S1RlUTBnZG5rKzBOM0h0cWx5N005TG9laElpVHMyMGRBSGcwT1NGaXdyYwpiL2Y1T0MybFRhbmdJb1FpbzFvTEwwRUM2dk4wdnpUUkxnNW9hMXhUMTlDN21FcFprL1ZnUjBuQlpRcU4vNDlICklvTGpIWWNFbXVsZDh1UU14dUZ1RzVLN0lpQlBVKys1QWhvbmUwTDlvQnB2NFlYeVM2OXE0amU0ZjNFSllPa2QKS2VndmVJa3BBb0dCQU1RVUhheDBhOHVaTWNaZnhvaG9CMjVuaE53Nng1VEZtVGRQeC9DamxYWllBYWcrbUFGYwpTWnF4SEJHcVJTTFRQMVkreFp6VExlS0VJNnlLSXJwbHozMXZPMEdsU05Xb3UybGkvUFRBQlBUVnl3eEcvWVpHCmZUL3BhZXJXYzJuYkxnUngxMkl5TzhsQXIwUVFJU3ZHdlF0cUdHUlpQY2YxSHJMQU81dkgzK2U5QW9HQkFNRFcKdXgxMmo5U1BZT3NBN2E1b2JmSzZmazNnTVFjeEZVRGVUaU1GSHl3bFdoWThQOUhqelRHVUNEQ3gzT2lwSHdNZApBUWZOTXRMRDR4WThZS05Ha2t1YklFemFRL0hWckRIQklZdGNsMHFobUZzRDAyYXJqWnlNV2xQN0FySEZlenNhCnZGMHR5eEIzQlFjVWE0REpqcG1jN1V2S2l6Zm4xVEl0a2NXL1lsbm5Bb0dCQUpndEFJYXFhRXJBWDNnVk52RUEKdzl1MHJkRjZNUkZPZGtZT1BoK041ZDdPR0tNcHlURXRIZGJYNCsvMTFPaGRTUWUzZWdqbmdQSVBHZHk3N0kzNwpuQmcrcnArWkZyanoxbGZKUW9iMVRDTjBsYnkyaithWmFIV2t3dFpHajVZMVREYVkzODlQSzBWYlZXc2VsWS96CkV4Nzd2V2lNTmoydENLRTBQazc5eGRHRkFvR0JBSUVNc3JmcTZpSWp1WVpMWHNSQzJxRi9zSnJKRjhacVVJRFMKeEpPbkQ4OXBSN3B0bzRBQTVRYnl1L0JxZHgyMFlDNmpNRmRhT1ZMWENKZU8zRlVvR3l0QnF3SURaMGpsNTVCOApZTWgwdEVLYmxld0N5V3lDRGdqZjNHc3JKZ2gxMGh3aHJrRGxMbW5jWEo3NlNWOHNnNlBGWXdBL2taOWVKRXlxCk5rMlI0RzJ0QW9HQWZVL092bHdCSWlBN0p3Wjc4c2x5MVFPMkkrbWdybHo5OWRERlNRcTAwQTVUZjd2dGFEYncKNzQwRXBGc0lNRm5BNlZRQUg5YVV1RnVxSVV4K1RQazMyQXdJTy9SNVpLR2dNelNqcndDRVNBeXMyR0JJdWx1TwpFcktHNUp1NmFteWUyWk90eEl5emdWV3Zmc0hERG5MNDRuTmNhYkNDbnRQR1dJb3QyeXhnZllvPQotLS0tLUVORCBSU0EgUFJJVkFURSBLRVktLS0tLQo=" # ONLY FOR TESTING
xff_num_trusted_hops: 2
redirect_base_path: /.veriflow
jwks_path: /.well-known/pomerium/jwks.json
kid_override: "0"
Expand Down
28 changes: 14 additions & 14 deletions test/e2e/tests/basic_test.js
Original file line number Diff line number Diff line change
@@ -1,47 +1,47 @@
Feature('Login').retry(3);

Scenario('Basic login test', async ({ I }) => {
I.amOnPage('http://test-basic-login.localtest.me:2080/get');
I.amOnPage('http://test-basic-login.localtest.me/get');
I.login();
I.see("x-veriflow-user-id")
});

Scenario('HTTPS Upstream Test', async ({ I }) => {
I.amOnPage('http://test-https-upstream.localtest.me:2080/get');
I.amOnPage('http://test-https-upstream.localtest.me/get');
I.login();
I.see("x-veriflow-user-id")
});

Scenario('Removing headers', async ({ I }) => {
I.amOnPage('http://test-removing-headers.localtest.me:2080/get');
I.amOnPage('http://test-removing-headers.localtest.me/get');
I.login();
I.dontSee("x-veriflow-user-id")
});

Scenario('Adding headers', async ({ I }) => {
I.amOnPage('http://test-adding-headers.localtest.me:2080/get');
I.amOnPage('http://test-adding-headers.localtest.me/get');
I.login();
I.see("x-pomerium-claim-email")
I.see("x-test-header")
});

Scenario('Testing mTLS', async ({ I }) => {
I.amOnPage('http://test-mtls-auth.localtest.me:2080/');
I.amOnPage('http://test-mtls-auth.localtest.me/');
I.login();
I.see("Veriflow-Test-Cert")
I.see("Veriflow-Test-CA")
});

Scenario('Testing Header Mapping', async ({ I }) => {
I.amOnPage('http://test-header-mapping.localtest.me:2080/');
I.amOnPage('http://test-header-mapping.localtest.me/');
I.login();
I.see("ThisIsATestHeaderFromTheHeaderMapping")
I.see("TestHeaderFromGroup")
I.dontSee("TestAbsentHeaderFromGroup")
});

Scenario('Testing Header Mapping Inline', async ({ I }) => {
I.amOnPage('http://test-header-mapping-inline.localtest.me:2080/');
I.amOnPage('http://test-header-mapping-inline.localtest.me/');
I.login();
I.see("ThisIsATestHeaderFromTheHeaderMapping")
I.see("TestHeaderFromGroup")
Expand All @@ -52,37 +52,37 @@ Scenario('Testing Token Auth', async ({ I }) => {
I.setPuppeteerRequestHeaders({
'Authorization': 'Bearer ThisIsATestToken',
});
I.amOnPage('http://test-token-auth.localtest.me:2080/');
I.amOnPage('http://test-token-auth.localtest.me/');
I.see("x-veriflow-user-id")
});

Scenario('Testing Unauthorized Flow', async ({ I }) => {
I.amOnPage('http://test-unauthorized-login.localtest.me:2080/');
I.amOnPage('http://test-unauthorized-login.localtest.me/');
I.login()
I.see("ERR_NOT_AUTHORIZED")
});

Scenario('Advanced matchers test', async ({ I }) => {
I.amOnPage('http://test-advanced-matchers.localtest.me:2080/get');
I.amOnPage('http://test-advanced-matchers.localtest.me/get');
I.login();
I.see("x-veriflow-user-id")
I.amOnPage('http://test-advanced-matchers.localtest.me:2080/should404');
I.amOnPage('http://test-advanced-matchers.localtest.me/should404');
I.see("ERR_ROUTE_NOT_FOUND")
});

Scenario('Dynamic Upstreams Test', async ({ I }) => {
I.amOnPage('http://test-dynamic-upstreams.localtest.me:2080/get');
I.amOnPage('http://test-dynamic-upstreams.localtest.me/get');
I.login();
I.see("x-veriflow-user-id")
});

Scenario('Dynamic Upstreams Test SRV', async ({ I }) => {
I.amOnPage('http://test-dynamic-upstreams-srv.localtest.me:2080/get');
I.amOnPage('http://test-dynamic-upstreams-srv.localtest.me/get');
I.login();
I.see("x-veriflow-user-id")
});

Scenario('Unauthenticated Access test', async ({ I }) => {
I.amOnPage('http://test-unauthenticated-access.localtest.me:2080/get');
I.amOnPage('http://test-unauthenticated-access.localtest.me/get');
I.see("x-public-access")
});
8 changes: 4 additions & 4 deletions util/caddyModels.js
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { getConfig } from "./config.js"
import { getConfig, getAuthListenPort } from "./config.js"
import log from './logging.js';
import { writeFile, stat } from "fs/promises";
import axios from 'axios';
Expand Down Expand Up @@ -173,7 +173,7 @@ async function saturateRoute(route, routeId) {
handler: "reverse_proxy",
upstreams: [
{
dial: "localhost:" + config.auth_listen_port
dial: "localhost:" + getAuthListenPort()
}
]
}
Expand Down Expand Up @@ -243,7 +243,7 @@ async function saturateRoute(route, routeId) {
},
upstreams: [
{
dial: "localhost:" + config.auth_listen_port
dial: "localhost:" + getAuthListenPort()
}
]
}
Expand Down Expand Up @@ -360,7 +360,7 @@ async function generateCaddyConfig() {
"handler": "reverse_proxy",
"upstreams": [
{
"dial": "localhost:" + config.auth_listen_port
"dial": "localhost:" + getAuthListenPort()
}
]
}
Expand Down
25 changes: 24 additions & 1 deletion util/config.js
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,8 @@ import log from './logging.js';
import reloadCaddy from './caddyModels.js';
import chokidar from 'chokidar';

let foundPort = false

let configFileLocation = process.env.CONFIG_FILE || "config.yaml"

let currentConfig = yaml.load(fsSync.readFileSync(configFileLocation, 'utf8'))
Expand Down Expand Up @@ -55,9 +57,30 @@ function getRedirectBasepath() {
return redirectBasePath
}

function getAuthListenPort() {
if (foundPort) {
return foundPort
}
var config = getConfig()
var metricsListenPort = config.metrics_listen_port
var dataListenPort = config.data_listen_port
var authListenPort = 9847
while (foundPort == false) {
if ((metricsListenPort == authListenPort) || (dataListenPort == authListenPort)) {
log.info({ message: `Port ${authListenPort} is taken, trying another port for the auth service`})
authListenPort++
} else {
foundPort = authListenPort
log.info({ message: `Found port ${foundPort} for the auth service to listen on`})
}
}
return foundPort
}

export {
reloadConfig,
getConfig,
getRouteFromRequest,
getRedirectBasepath
getRedirectBasepath,
getAuthListenPort
};

0 comments on commit 143f954

Please sign in to comment.