From 66f2d4103304d50aaf792e48a01309139143bfa3 Mon Sep 17 00:00:00 2001 From: Wyatt Johnson Date: Thu, 31 May 2018 14:56:18 -0600 Subject: [PATCH] always call the rest logout command, don't vomit on the error --- client/coral-framework/actions/auth.js | 11 ++++++----- middleware/authorization.js | 10 +++++----- 2 files changed, 11 insertions(+), 10 deletions(-) diff --git a/client/coral-framework/actions/auth.js b/client/coral-framework/actions/auth.js index 7e1785e621..e6e15e8676 100644 --- a/client/coral-framework/actions/auth.js +++ b/client/coral-framework/actions/auth.js @@ -96,13 +96,14 @@ export const logout = () => async ( _, { rest, client, pym, localStorage } ) => { - // Stop if the token doen't exist - if (!localStorage.getItem('token')) { - return; + try { + await rest('/auth', { method: 'DELETE' }); + } catch (err) { + // We ignore any REST related errors from the delete action, which may/may + // not have had a cookie/token attached to it. The logout action was still + // called, so we still want to cleanup. } - await rest('/auth', { method: 'DELETE' }); - // Clear the auth data persisted to localStorage. cleanAuthData(localStorage); diff --git a/middleware/authorization.js b/middleware/authorization.js index 77376f08b5..9462a5d0bd 100644 --- a/middleware/authorization.js +++ b/middleware/authorization.js @@ -40,15 +40,15 @@ authorization.has = (user, ...roles) => { * @return {Callback} connect middleware */ authorization.needed = (...roles) => [ - // Insert the pre-needed middlware. + // Insert the pre-needed middleware. ...authorization.middleware, // Insert the actual middleware to check for the required role. (req, res, next) => { - // All routes that are wrapepd with this middleware actually require a role. + // All routes that are wrapped with this middleware actually require a role. if (!req.user) { - debug(`No user on request, returning with ${ErrNotAuthorized}`); - return next(ErrNotAuthorized); + debug(`No user on request, returning with ErrNotAuthorized`); + return next(new ErrNotAuthorized()); } // Check to see if the current user has all the roles requested for the given @@ -56,7 +56,7 @@ authorization.needed = (...roles) => [ // evaluate to true. if (!authorization.has(req.user, ...roles)) { debug('User does not have all the required roles to access this page'); - return next(ErrNotAuthorized); + return next(new ErrNotAuthorized()); } // Looks like they're allowed!