From 64356d42d0f44798d03bbd21951a48d7d1946c49 Mon Sep 17 00:00:00 2001 From: garrettmills Date: Fri, 22 May 2020 09:29:13 -0500 Subject: [PATCH] Add support for session traps; make mfa challenge session trap; remove DMZ middleware --- TODO.text | 3 +- app/controllers/api/v1/Auth.controller.js | 8 ++-- app/controllers/auth/MFA.controller.js | 2 +- app/routing/Middleware.js | 2 +- app/routing/middleware/Traps.middleware.js | 38 +++++++++++++------ .../middleware/auth/DMZOnly.middleware.js | 17 --------- .../middleware/auth/UserOnly.middleware.js | 6 +-- app/routing/routers/api/v1/auth.routes.js | 2 +- app/routing/routers/auth/forms.routes.js | 8 ++-- app/routing/routers/auth/mfa.routes.js | 6 +-- config/traps.config.js | 8 ++++ 11 files changed, 49 insertions(+), 51 deletions(-) delete mode 100644 app/routing/middleware/auth/DMZOnly.middleware.js diff --git a/TODO.text b/TODO.text index d2c4dcd..82a33fa 100644 --- a/TODO.text +++ b/TODO.text @@ -2,6 +2,7 @@ - Forgot password handling - Admin password reset mechanism -> flag users as needing PW resets - OAuth2 -> support refresh tokens -- Traps -> support session traps; convert MFA challenge to use session trap +- Traps - Allow setting user trap from web UI - Don't allow external logins if trap is set +- Trust token page -> force username of current user diff --git a/app/controllers/api/v1/Auth.controller.js b/app/controllers/api/v1/Auth.controller.js index cadd459..5c1562a 100644 --- a/app/controllers/api/v1/Auth.controller.js +++ b/app/controllers/api/v1/Auth.controller.js @@ -481,8 +481,7 @@ class AuthController extends Controller { } if ( user.mfa_enabled && !req.session.mfa_remember ) { - req.session.auth.in_dmz = true - destination = '/auth/mfa/challenge' + await req.trap.begin('mfa_challenge', { session_only: true }) } if ( req.session?.auth?.message ) @@ -496,7 +495,7 @@ class AuthController extends Controller { // Trust re-verification is granted, // but the user might still need to verify MFA const next = req.trust.end() - if ( req.session.auth.in_dmz ) { + if ( req.trap.has_trap('mfa_challenge') ) { req.session.auth.flow = next } else { destination = next @@ -550,7 +549,8 @@ class AuthController extends Controller { let next_destination = undefined if ( is_valid ) { - req.session.auth.in_dmz = false + if ( req.trap.has_trap('mfa_challenge') ) + await req.trap.end() next_destination = req.session.auth.flow || this.configs.get('auth.default_login_route') delete req.session.auth.flow } diff --git a/app/controllers/auth/MFA.controller.js b/app/controllers/auth/MFA.controller.js index 5737829..ecb6462 100644 --- a/app/controllers/auth/MFA.controller.js +++ b/app/controllers/auth/MFA.controller.js @@ -30,7 +30,7 @@ class MFAController extends Controller { }) } - if ( !req.session.auth.in_dmz ) { + if ( !req.trap.has_trap('mfa_challenge') ) { return res.redirect(req.session.auth.flow) } diff --git a/app/routing/Middleware.js b/app/routing/Middleware.js index 49be22c..1b44cf5 100644 --- a/app/routing/Middleware.js +++ b/app/routing/Middleware.js @@ -10,9 +10,9 @@ */ const Middleware = [ "auth:Utility", + "Traps", "auth:TrustTokenUtility", "SAMLUtility", - "Traps", // 'MiddlewareName', diff --git a/app/routing/middleware/Traps.middleware.js b/app/routing/middleware/Traps.middleware.js index 895a180..78ebf0e 100644 --- a/app/routing/middleware/Traps.middleware.js +++ b/app/routing/middleware/Traps.middleware.js @@ -3,34 +3,49 @@ const { Middleware } = require('libflitter') class TrapUtility { constructor(req, res, configs) { this.request = req + this.session = req.session this.response = res this.user = req.user this.configs = configs } - async begin(trap_name) { - this.user.trap = trap_name - this.request.trust.assume() - await this.user.save() + async begin(trap_name, { session_only = false }) { + if ( session_only || !this.user ) { + this.session.trap = trap_name + } else { + this.user.trap = trap_name + await this.user.save() + } + + if ( this.config().assume_trust ) + this.request.trust.assume() } redirect() { - this.request.trust.assume() + if ( this.config().assume_trust ) + this.request.trust.assume() return this.response.redirect(this.config().redirect_to) } async end() { - this.user.trap = '' - this.request.trust.unassume() - await this.user.save() + if ( this.config().assume_trust ) + this.request.trust.unassume() + if ( this.user ) { + this.user.trap = '' + await this.user.save() + } + this.session.trap = '' } - has_trap() { - return !!this.user.trap + has_trap(name = '') { + if ( name ) + return (this.user && this.user.trap === name) || this.session.trap === name + return (this.user && this.user.trap) || this.session.trap } get_trap() { - return this.user.trap + if ( this.session.trap ) return this.session.trap + else if ( this.user ) return this.user.trap } config() { @@ -49,7 +64,6 @@ class TrapsMiddleware extends Middleware { } async test(req, res, next, args = {}) { - if ( !req?.user ) return next() req.trap = new TrapUtility(req, res, this.configs.get('traps.types')) if ( !req.trap.has_trap() ) return next() diff --git a/app/routing/middleware/auth/DMZOnly.middleware.js b/app/routing/middleware/auth/DMZOnly.middleware.js deleted file mode 100644 index 8a0bc82..0000000 --- a/app/routing/middleware/auth/DMZOnly.middleware.js +++ /dev/null @@ -1,17 +0,0 @@ -const Middleware = require('libflitter/middleware/Middleware') -class DMZOnly extends Middleware { - - async test(req, res, next, args = {}){ - - if ( req.is_auth ) return next() - else { - // If not signed in, save the target url so we can redirect back here after auth - req.session.auth.flow = req.originalUrl - return res.redirect('/auth/login') - } - - } - -} - -module.exports = DMZOnly diff --git a/app/routing/middleware/auth/UserOnly.middleware.js b/app/routing/middleware/auth/UserOnly.middleware.js index a0cdd22..44f8cd5 100644 --- a/app/routing/middleware/auth/UserOnly.middleware.js +++ b/app/routing/middleware/auth/UserOnly.middleware.js @@ -12,11 +12,7 @@ class UserOnly extends Middleware { } async test(req, res, next, args = {}){ - if ( req.is_auth && !req.session.auth.in_dmz ) return next() - else if ( req.is_auth ) { // Need an MFA challenge - if ( !req.session.auth.flow ) req.session.auth.flow = req.originalUrl - return res.redirect('/auth/mfa/challenge') - } + if ( req.is_auth ) return next() else { // If not signed in, save the target url so we can redirect back here after auth req.session.auth.flow = req.originalUrl diff --git a/app/routing/routers/api/v1/auth.routes.js b/app/routing/routers/api/v1/auth.routes.js index 77cb294..8711298 100644 --- a/app/routing/routers/api/v1/auth.routes.js +++ b/app/routing/routers/api/v1/auth.routes.js @@ -58,7 +58,7 @@ const auth_routes = { ], '/mfa/attempt': [ - 'middleware::auth:DMZOnly', + 'middleware::auth:UserOnly', 'controller::api:v1:Auth.attempt_mfa' ], diff --git a/app/routing/routers/auth/forms.routes.js b/app/routing/routers/auth/forms.routes.js index 69c68c5..9faf7b3 100644 --- a/app/routing/routers/auth/forms.routes.js +++ b/app/routing/routers/auth/forms.routes.js @@ -51,7 +51,7 @@ const index = { '/:provider/logout': [ 'middleware::auth:ProviderRoute', - 'middleware::auth:DMZOnly', + 'middleware::auth:UserOnly', 'controller::auth:Forms.logout_provider_clean_session', // Note, this separation is between when the auth action has happened properly @@ -62,7 +62,7 @@ const index = { ], '/logout': [ 'middleware::auth:ProviderRoute', - 'middleware::auth:DMZOnly', + 'middleware::auth:UserOnly', 'controller::auth:Forms.logout_provider_clean_session', 'controller::auth:Forms.logout_provider_present_success', ], @@ -100,13 +100,13 @@ const index = { ], '/:provider/logout': [ 'middleware::auth:ProviderRoute', - 'middleware::auth:DMZOnly', + 'middleware::auth:UserOnly', 'controller::auth:Forms.logout_provider_clean_session', 'controller::auth:Forms.logout_provider_present_success', ], '/logout': [ 'middleware::auth:ProviderRoute', - 'middleware::auth:DMZOnly', + 'middleware::auth:UserOnly', 'controller::auth:Forms.logout_provider_clean_session', 'controller::auth:Forms.logout_provider_present_success', ], diff --git a/app/routing/routers/auth/mfa.routes.js b/app/routing/routers/auth/mfa.routes.js index 0eaf1bf..362b702 100644 --- a/app/routing/routers/auth/mfa.routes.js +++ b/app/routing/routers/auth/mfa.routes.js @@ -2,25 +2,21 @@ const mfa_routes = { prefix: '/auth/mfa', middleware: [ - + 'auth:UserOnly', ], get: { '/setup': [ - 'middleware::auth:UserOnly', ['middleware::auth:RequireTrust', { scope: 'mfa.enable' }], 'controller::auth:MFA.setup', ], '/challenge': [ - 'middleware::auth:DMZOnly', 'controller::auth:MFA.challenge', ], '/disable': [ - 'middleware::auth:UserOnly', 'controller::auth:MFA.get_disable', ], '/disable/process': [ - 'middleware::auth:UserOnly', ['middleware::auth:RequireTrust', { scope: 'mfa.disable' }], 'controller::auth:MFA.do_disable', ], diff --git a/config/traps.config.js b/config/traps.config.js index d7b64d5..30bb3e3 100644 --- a/config/traps.config.js +++ b/config/traps.config.js @@ -9,6 +9,14 @@ const traps_config = { '/auth/logout', ], }, + mfa_challenge: { + redirect_to: '/auth/mfa/challenge', + allowed_routes: [ + '/auth/mfa/challenge', + '/api/v1/auth/mfa/attempt', + '/auth/logout', + ], + }, }, }