Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Centralize one source for configuration #414

Open
wants to merge 25 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
25 commits
Select commit Hold shift + click to select a range
e6c1f3b
test(rate-limiter.test.js): test rate limiter
kdhttps Dec 10, 2021
64e854e
test(app-factory.test.js): test rate limiter middleware
kdhttps Dec 10, 2021
f33c83e
refactor(rate-limiter.js): remove env config
kdhttps Dec 10, 2021
39cfcdf
refactor(app-factory.js): configure rate-limit middleware
kdhttps Dec 10, 2021
9597cac
refactor(app.js): configure rate-limit middleware
kdhttps Dec 10, 2021
89e559f
test(rate-limiter.test.js): test rate limit reconfig
kdhttps Dec 13, 2021
bd2b2ee
test(app.init.spec.js): check rateLimit config
kdhttps Dec 13, 2021
3e7b647
feat(rate-limiter.js): handle the reconfig rate limit
kdhttps Dec 13, 2021
8c6d454
refactor(helper.js): refactor test as per new rate limit config
kdhttps Dec 13, 2021
4ffebd4
test(test.js): add session config
kdhttps Dec 15, 2021
1e34dd7
test(app-factory.test.js): test empty session middleware
kdhttps Dec 15, 2021
b4342fe
test(session.test.js): the module should init express session
kdhttps Dec 15, 2021
cf1cb49
refactor(session.spec.js): use latest config for session test
kdhttps Dec 15, 2021
f67b48b
feat(session.js): configure session with dynamic values
kdhttps Dec 15, 2021
7746e61
feat(app.js): config session in app
kdhttps Dec 15, 2021
f59d816
refactor(rate-limiter.js): refactor code
kdhttps Dec 15, 2021
2a69ced
refactor(app.js): remove commented code
kdhttps Dec 16, 2021
bfff9ba
fix(app-factory.js): fix session middleware problem
kdhttps Dec 16, 2021
b1bef92
test(helper.js): fix bdd test
kdhttps Dec 16, 2021
fe32f68
Merge branch 'master' of github.com:GluuFederation/gluu-passport into…
kdhttps Jan 20, 2022
8095bf9
test(http-proxy.test): update test case
kdhttps Mar 21, 2022
8418ba3
test(http-proxy.spec): update integration test
kdhttps Mar 21, 2022
c797e1c
feat(http-proxy): configurable http proxy
kdhttps Mar 21, 2022
4f631eb
test(http-proxy.test): check configure function exists or not
kdhttps Mar 21, 2022
4f0fc9e
Merge branch 'master' of github.com:GluuFederation/gluu-passport into…
kdhttps Mar 21, 2022
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
22 changes: 14 additions & 8 deletions config/test.js
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,19 @@ const passportConfigAuthorizedResponse = {
password: '',
port: 0
}
},
rateLimit: {
windowMs: 24 * 60 * 60 * 1000,
max: 100
},
session: {
cookieSameSite: 'none',
cookieSecure: true
},
proxy: {
HTTP_PROXY: 'http://localhost:3128',
HTTPS_PROXY: 'http://localhost:3129',
NO_PROXY: 'localhost,127.0.0.1'
}
},
idpInitiated: {
Expand Down Expand Up @@ -238,10 +251,6 @@ const rateLimitMaxRequestAllow = 100
const cookieSameSite = 'none'
const cookieSecure = true

const HTTP_PROXY = 'http://localhost:3128'
const HTTPS_PROXY = 'http://localhost:3129'
const NO_PROXY = 'localhost,127.0.0.1'

module.exports = {
saltFile,
passportConfig,
Expand All @@ -251,8 +260,5 @@ module.exports = {
passportFile,
passportConfigAuthorizedResponse,
cookieSameSite,
cookieSecure,
HTTP_PROXY,
NO_PROXY,
HTTPS_PROXY
cookieSecure
}
20 changes: 13 additions & 7 deletions server/app-factory.js
Original file line number Diff line number Diff line change
Expand Up @@ -9,8 +9,6 @@ const routes = require('./routes')
const metricsMiddleware = require('../server/utils/metrics')
const { globalErrorHandler } = require('./utils/error-handler')
const flash = require('connect-flash')
const { rateLimiter } = require('./utils/rate-limiter')
const { session } = require('./utils/session')
// Setup http proxy config
require('./utils/http-global-proxy')

Expand All @@ -22,10 +20,19 @@ class AppFactory {
app.use(bodyParser.urlencoded({ extended: false }))
app.use(cookieParser())
app.use(flash())
app.use(rateLimiter)
app.set('trust proxy', 1)
app.use(session)

// store rateLimiter for later manipulation/reset
app.rateLimiter = (req, res, next) => next()
app.session = (req, res, next) => next()
app.use((req, res, next) => {
app.rateLimiter(req, res, next)
})

app.use((req, res, next) => {
app.session(req, res, next)
})

app.set('trust proxy', 1)
app.use(passport.initialize())
app.use(passport.session())
app.use('/passport', routes)
Expand All @@ -37,8 +44,7 @@ class AppFactory {
passport.deserializeUser((user, done) => {
done(null, user)
})
// store rateLimiter for later manipulation/reset
app.rateLimiter = rateLimiter

return app
}
}
Expand Down
9 changes: 9 additions & 0 deletions server/app.js
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,9 @@ const confDiscovery = require('./utils/configDiscovery')
const providers = require('./providers')
const passportFile = config.get('passportFile')
const AppFactory = require('./app-factory')
const rateLimiter = require('./utils/rate-limiter')
const session = require('./utils/session')
const httpProxy = require('./utils/http-global-proxy')

let httpServer
let httpPort = -1
Expand Down Expand Up @@ -45,10 +48,16 @@ function recreateHttpServer (serverURI, port) {
function reconfigure (cfg) {
global.config = cfg.conf
global.iiconfig = cfg.idpInitiated
const { windoMs, max } = cfg.conf.rateLimit
const { cookieSameSite, cookieSecure } = cfg.conf.session
const { HTTP_PROXY, HTTPS_PROXY, NO_PROXY } = cfg.conf.proxy

// Apply all runtime configuration changes
logger.configure(cfg.conf.logging)
providers.setup(cfg.providers)
rateLimiter.configure(app, windoMs, max)
session.configure(app, cookieSameSite, cookieSecure)
httpProxy.configure(HTTP_PROXY, HTTPS_PROXY, NO_PROXY)
recreateHttpServer(cfg.conf.serverURI, cfg.conf.serverWebPort)
}

Expand Down
18 changes: 11 additions & 7 deletions server/utils/http-global-proxy.js
Original file line number Diff line number Diff line change
@@ -1,9 +1,13 @@
const config = require('config')
const configure = (HTTP_PROXY, HTTPS_PROXY, NO_PROXY) => {
if (HTTP_PROXY) {
const ga = require('global-agent')
ga.bootstrap()
global.GLOBAL_AGENT.HTTP_PROXY = HTTP_PROXY
global.GLOBAL_AGENT.HTTPS_PROXY = HTTPS_PROXY
global.GLOBAL_AGENT.NO_PROXY = NO_PROXY
}
}

if (config.has('HTTP_PROXY')) {
const ga = require('global-agent')
ga.bootstrap()
global.GLOBAL_AGENT.HTTP_PROXY = config.get('HTTP_PROXY')
global.GLOBAL_AGENT.HTTPS_PROXY = config.get('HTTPS_PROXY')
global.GLOBAL_AGENT.NO_PROXY = config.get('NO_PROXY')
module.exports = {
configure
}
30 changes: 19 additions & 11 deletions server/utils/rate-limiter.js
Original file line number Diff line number Diff line change
@@ -1,16 +1,24 @@
const rateLimit = require('express-rate-limit')
const config = require('config')
const logger = require('./logging')
let checkWindowMs = 0
let checkMax = 0

const windowMs = config.get('rateLimitWindowMs')
const max = config.get('rateLimitMaxRequestAllow')

const rateLimiter = rateLimit({
windowMs,
max,
message: `You have exceeded the ${max} requests in ${windowMs} milliseconds limit!`,
headers: true
})
const configure = (app, windowMs = 86400000, max = 1000) => {
if (windowMs === checkWindowMs && max === checkMax) {
logger.log2('debug', 'Skip ratelimit config, already config with same values')
return
}
checkWindowMs = windowMs
checkMax = max
logger.log2('info', `configure rate-limiter windowMs=${windowMs} maxRequestAllow=${max}`)
app.rateLimiter = rateLimit({
windowMs,
max,
message: `You have exceeded the ${max} requests in ${windowMs} milliseconds limit!`,
headers: true
})
}

module.exports = {
rateLimiter
configure
}
44 changes: 28 additions & 16 deletions server/utils/session.js
Original file line number Diff line number Diff line change
@@ -1,24 +1,36 @@
const expressSession = require('express-session')
const MemoryStore = require('memorystore')(expressSession)
const config = require('config')
const { randomSecret } = require('../utils/misc')
const logger = require('./logging')

const expressSessionConfig = {
cookie: {
maxAge: 86400000,
sameSite: config.get('cookieSameSite'),
secure: config.get('cookieSecure')
},
store: new MemoryStore({
checkPeriod: 86400000 // prune expired entries every 24h
}),
secret: randomSecret(),
resave: false,
saveUninitialized: false
}
let checkCookieSameSite = ''
let checkCookieSecure

const configure = (app, cookieSameSite = 'none', cookieSecure = true) => {
if (cookieSameSite === checkCookieSameSite && cookieSecure === checkCookieSecure) {
logger.log2('debug', 'Skip session config, already config with same values')
return
}

const session = expressSession(expressSessionConfig)
checkCookieSameSite = cookieSameSite
checkCookieSecure = cookieSecure
logger.log2('info', `configure express session sameSite=${cookieSameSite} secure=${cookieSecure}`)
const expressSessionConfig = {
cookie: {
maxAge: 86400000,
sameSite: cookieSameSite,
secure: cookieSecure
},
store: new MemoryStore({
checkPeriod: 86400000 // prune expired entries every 24h
}),
secret: randomSecret(),
resave: false,
saveUninitialized: false
}
app.session = expressSession(expressSessionConfig)
}

module.exports = {
session
configure
}
58 changes: 21 additions & 37 deletions test/app-factory.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@ const assert = chai.assert
const rewire = require('rewire')
const appFactoryRewire = rewire('../server/app-factory.js')
const sinon = require('sinon')
const { rateLimiter } = require('../server/utils/rate-limiter')

/**
* Helper: Returns the argument call number with matching args
Expand Down Expand Up @@ -38,7 +37,7 @@ function spyOnAppUse () {
const appInstance = new AppFactory()

appInstance.createApp()
return appUseSpy
return { appUseSpy, app }
}

describe('connect-flash middleware', () => {
Expand All @@ -58,7 +57,7 @@ describe('connect-flash middleware', () => {

it('should be called once as app.use arg', () => {
const flash = require('connect-flash')
const appUseSpy = spyOnAppUse()
const { appUseSpy } = spyOnAppUse()

assertCalledWithFunctionAsArg(appUseSpy, flash())
sinon.restore()
Expand All @@ -77,56 +76,41 @@ describe('error-handler middleware', () => {
})

it('should be called once as app.use arg', () => {
const appUseSpy = spyOnAppUse()
const { appUseSpy } = spyOnAppUse()
assertCalledWithFunctionAsArg(appUseSpy, rewiredGlobalErrorHandler)
sinon.restore()
})
})

describe('rateLimiter middleware', () => {
const rewiredRateLimiter = appFactoryRewire.__get__('rateLimiter')

it('should exist', () => {
assert.exists(rewiredRateLimiter)
})

it('should be a function', () => {
assert.isFunction(rewiredRateLimiter)
describe('empty rateLimiter middleware', () => {
let app
it('app should have rateLimiter', () => {
const spyApp = spyOnAppUse()
app = spyApp.app
assert.exists(app.rateLimiter)
})

it('should be equal rateLimiter middleware', () => {
assert.strictEqual(rewiredRateLimiter, rateLimiter)
})

it('should be called once as app.use arg', () => {
const appUseSpy = spyOnAppUse()
assertCalledWithFunctionAsArg(appUseSpy, rewiredRateLimiter)
it('app should have rateLimiter as a function', () => {
assert.isFunction(app.rateLimiter)
sinon.restore()
})
})

describe('session middleware', () => {
const rewiredSession = appFactoryRewire.__get__('session')
const { session } = require('../server/utils/session')

it('should exist', () => {
assert.exists(rewiredSession)
describe('empty session middleware', () => {
let app
it('app should have session', () => {
const spyApp = spyOnAppUse()
app = spyApp.app
assert.exists(app.session)
})

it('should be a function', () => {
assert.isFunction(rewiredSession)
})

it('should be equal session middleware', () => {
assert.equal(rewiredSession, session)
})

it('should be called once as app.use arg', () => {
const appUseSpy = spyOnAppUse()
assertCalledWithFunctionAsArg(appUseSpy, rewiredSession)
it('app should have session as a function', () => {
assert.isFunction(app.session)
sinon.restore()
})
})

describe('session middleware', () => {
describe('proxy setup', () => {
it('app.set should be called once w/ params', () => {
const app = appFactoryRewire.__get__('app')
Expand Down
5 changes: 4 additions & 1 deletion test/app.init.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -190,7 +190,10 @@ describe('app.init() - Initialization', () => {
'serverURI',
'serverWebPort',
'spTLSCert',
'spTLSKey'
'spTLSKey',
'rateLimit',
'session',
'proxy'
)
})

Expand Down
23 changes: 18 additions & 5 deletions test/helper.js
Original file line number Diff line number Diff line change
@@ -1,13 +1,18 @@
// Use this file to avoid repeating yourself (DRY!), helper functions.

const InitMock = require('./testdata/init-mock')
const logger = require('../server/utils/logging')
const config = require('config')
const basicConfig = config.get('passportConfig')
const chai = require('chai')
const chaiHttp = require('chai-http')
chai.use(chaiHttp)

const InitMock = require('./testdata/init-mock')
const logger = require('../server/utils/logging')
const rateLimiter = require('../server/utils/rate-limiter')
const session = require('../server/utils/session')

chai.use(chaiHttp)
const basicConfig = config.get('passportConfig')
const passportConfig = config.get('passportConfigAuthorizedResponse').conf
const rateLimitConfig = passportConfig.rateLimit
const sessionConfig = passportConfig.session
/**
* Mocks external endpoints for app initalization
*/
Expand Down Expand Up @@ -37,7 +42,15 @@ const setupServer = async function () {
await app.on('appStarted', () => {
console.log('app started...')
})

const { windowMs, max } = rateLimitConfig
rateLimiter.configure(app, windowMs, max)

const { cookieSameSite, cookieSecure } = sessionConfig
session.configure(app, cookieSameSite, cookieSecure)

await app.rateLimiter.resetKey('::ffff:127.0.0.1')

return chai.request(app).keepOpen()
}

Expand Down
7 changes: 4 additions & 3 deletions test/http-global-proxy.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -4,9 +4,10 @@ const http = require('http')
const https = require('https')

const assert = chai.assert
const HTTP_PROXY = config.get('HTTP_PROXY')
const HTTPS_PROXY = config.get('HTTPS_PROXY')
const NO_PROXY = config.get('NO_PROXY')

const passportConfigAuthorizedResponse = config.get('passportConfigAuthorizedResponse')
const proxy = passportConfigAuthorizedResponse.conf.proxy
const { HTTP_PROXY, HTTPS_PROXY, NO_PROXY } = proxy

describe('global agent proxy setup', () => {
it('global agent object should have global agent', async () => {
Expand Down
Loading