[Enhancement] Improve _checkAuth function to return promise from authHandler #enhancement

[colceagus]

The authorization handler will be async (in most cases) and the private check authorization function should handle the authorization result in an async manner.
The implementation of the authorization handler can be with a callback or using promises.
The private check authorization function should be agnostic of the authorization handler's end-user implementation(callback/promise).

[colceagus]

In order to make a wrapper over the handler (agnostic of callback/promise style) there should be support from bluebird to do such a thing (or think of an ingenious solution ourselves).
It might be possible that we will have to reconsider, if we don't find a solution until Thursday.

http://stackoverflow.com/questions/39851023/make-javascript-function-with-callback-promise-implementation-to-always-return-a/39854555#39854555

[colceagus]

Update:
Using Promise.method on the authHandler var promisifiedAuthHandler = Promise.method(self.authHandler); works for the synchronous implementation of the authHandler.
Fails for the callback implementation.

Using Promise.promisify on the authHandler wrapped in the Promise.method works for the callback implementation var promisifiedAuthHandler = Promise.promisify(Promise.method(self.authHandler));.
Fails for the synchronous implementation.

Providing a callback for the authHandler (even if it does not use it in the implementation) works for all methods. It goes like this (writing for a general case):

function _checkAuth(req) {
  var credentials = _getCredentials(req);
  var promisifiedAuthHandler = Promise.method(self.authHandler); // for the sync implementation

  switch (authType) {

  case Authorization.WHATEVERTYPE: 
    return promisifiedAuthHandler(credentials, function callback(err, result) {
      if (err) return Promise.reject(err);
      return Promise.resolve(result);
    }
  }
}

and the server.enableCookie/JWT/BasicAuth handler can be implemented in the three ways mentioned: sync/callback/promise; As follows:

server.enableCookieAuth(function (cookie) {
  return (cookie === validCookieValue);
});

server.enableCookieAuth(function (cookie, callback) { 
  apiCall('path/to/auth', function(err, result) { 
    // apiCall could have a promise implementation, as well, and could 
    // be used with .then/.catch, but this is not that important here, since we care 
    // about the handler implementation)
    if (err) return callback(err, null);
    callback(null, result); // can be returned
  }
});

server.enableCookieAuth(function (cookie) {
  // let's write the apiCall with promise handling, since we mentioned it above
  return apiCall('path/to/auth').then(function (result) {
    return Promise.resolve(result);
  }).catch(function (err) {
    return Promise.reject(err);
  });
});

Now, we can use our _checkAuth function internally using only promises, agnostic of the authHandler function implementation. As in:

handleHttp: function(req, res) {
  var self = this;
  // ...other processing
  self._checkAuth(req).then(function (result) {
    // check if the user is authed or not 
    if (result) {
      // further process the request
    } else { 
      // handle unauthorized request
    }
  }).catch(function (err) {
     // handle internal server or api call (or whatever) error 
  });
}

The trick was to write our callback with a promise implementation. We always provide the authHandler a callback implementation, even if the authHandler does not use it. This way, we always make sure that the auth handler implementation returns a promise if it uses a callback style implementation.

Don't listen to the naysayers!

I will submit a pull request for this issue tomorrow, after writing the remaining tests.

[colceagus]

can we close this issue, since the pull request was merged into develop ?
and apply 'hacktoberfest' label on it, please ?

[colceagus]

Thanks, @pocesar !

[colceagus]

I forgot to ask you another thing, could you please assign this issue to me ? Thank you!