ScopedUser Logout Error #103

Open
wants to merge 4 commits into
from

2 participants

@djwglpuppy

I modified the logout request for scopes to try to delete a user account. If the user account for that scope does not exist... It will throw a more contextual error instead of simply user is undefined

throw new Error('ScopeError: there are no user credentials associated with the SCOPE requested (' +  authContext.scope + ')');
@ciaranj
Owner

I wonder if it would be better to log a warning and callback to the middlewareCallback (if it is defined) with an error object, rather than throw an error ? We could then change the logout callback in auth_middleware to pass this error back to the middlewarecallback that was passed to it (optionally passed) if something wanted to handle that dodgy logout ? What do you think?

@ciaranj - That is what I was originally thinking, but decided to go with a more conservative approach 😄 I like the way you are proposing much better since it is non obtrusive and doesn't leave the user dead in their tracks. Hold off on the pull request and I will implement.

@djwglpuppy

I added an error object to the middlewareCallback

Line 93 of auth_middleware.js

          if( middlewareCallback) middlewareCallback(err);

Bottom of requestMethods.js

    } catch(error){
      err = new Error('There are no user credentials associated with the scope: "' +  authContext.scope + '"');
    }
  }
  logoutHandler( authContext, user, middlewareCallback(err));
@ciaranj
Owner

Sorry for the delay, quick question, I see you've chosen to attempt to access the object literal and catch any usage of 'undefined' in an exception handler, would you be adverse to testing the presence in a condition before trying to de-reference it, or is this a performance thing ?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment