stormpath / stormpath-sdk-node

Official Node.js SDK for the Stormpath User Management REST API
https://docs.stormpath.com/nodejs/jsdoc/
Apache License 2.0
92 stars 40 forks source link

OauthAccessTokenAuthenticator does not attach application to AuthenticationResult #342

Open areichman opened 8 years ago

areichman commented 8 years ago

When creating JWTs using the AuthenticationResult.getJwt method, it is expected that the AuthenticationResult object has a reference to an application property so it can properly set the iss value in the token.

This application object is set by the OAuthBasicExchangeAuthenticator when creating an AuthenticationResult:

var result = new AuthenticationResult(apiKey,self.application.dataStore);
result.forApiKey = apiKey;
result.application = self.application;
// ...
callback(null, result);

But it is not set by the OauthAccessTokenAuthenticator:

callback(null,new AuthenticationResult(data,self.application.dataStore));

The result is that if you try to authenticate a request using an existing token (in the header, body, or url), you cannot call the getJwt method to create a new token since that method will fail trying to lookup the application.href value.

timothyej commented 8 years ago

Thanks for reporting and fixing this! I've issued a new PR based on your changes: #343

robertjd commented 8 years ago

Hi Aaron, thanks for the report!

The getJwt method should only available on an AuthenticationResult instance if the authentication result is created during the grant_type=client_credentials request, where the client is presenting api key credentials in exchange for an access token. It’s presence otherwise is a mistake on our part, my apologies.

Can you tell us more about your use case? Typically, with the Oauth2 scheme, the client has to present a refresh token in order to get a new access token. Access tokens are not used to get new access tokens.

We do support access tokens and refresh tokens for our password grant flow. Are you working with our client_crendential flow, or passwords?

Thanks in advance.

areichman commented 8 years ago

Hi Robert- In our API, all of our token access is off a /oauth/token endpoint. From there, we primarily support grant_type=client_credentials for our end users. But for our own first class Backbone app, we also have undocumented password and refresh_token support as well. This is the basic setup for our API method:

// user sends api keys
if (grant_type == 'client_credentials') {
  application.authenticateApiRequest({request: req}, function(err, authResult) {
    handleAuthResult(authResult);
  });
} 

// user sends username and password
else if (grant_type == 'password') {
  application.authenticateAccount({username: req.body.username, password: req.body.password}, function(err, authResult) {
    handleAuthResult(authResult);
  });
}

// user sends token
else if (grant_type == 'refresh_token') {
  application.authenticateApiRequest({request: req}, function(err, authResult) {
    handleAuthResult(authResult);
  });
}

function handleAuthResult(authResult) {
  var jwt = authResult.getJwt();
  ... // get account, set scopes, send the token etc.
}

We are not currently using the Stormpath API's /oauth/token endpoint to get access and refresh tokens since we need to customize the scope, and that is not currently an option there.

For our refresh flow, I'm admittedly cheating a bit by allowing users to send a valid access token instead of a refresh token, partially since I wasn't sure if there was refresh token capability elsewhere in the SDK. But it also simplifies things for us by not having to store separate refresh tokens as well. Whether it be an access or refresh token, we still perform a full account validation with Stormpath on the refresh requests so we've been comfortable with this modified flow. If you have suggestions on a better approach though, I'd love the feedback.

Thanks...