Scope support and dependencies fixes

[sybeck2k]

Hello,
I've added support for an optional scope parameter for the token requests and an optional hook called authorizeToken to authorize a token with a scope to access the resource.
I've added some Unit tests also about that.
Moreover, I've updated the test system to work with the current coffee-script version (it seems that vows is not maintained anymore....)

[domenic]

This is good stuff, thanks. Will dig into it and merge this weekend.

[beniwtv]

A +1 for this, I just have finished implementing restify-oauth2 and was looking for scopes support. Awesome :)

[domenic]

Hi! I spent a decent amount of time on this today!!

It turns out this isn't quite the approach I think is best for Restify–OAuth2. Although the general idea was good, I think the API usability suffered. Plus, these commits got all confusing, mixing up test-fixing stuff and changing of clientId/username and so on with the actual scope work.

Let me describe what I think would be best, and then one of us can implement it:

• Add a new (optional) hook to both flows, grantScopes(credentials, scopesRequested, cb).
  • credentials is either { clientId, clientSecret } (CC) or { clientId, clientSecret, username, password } (ROPC)
  • scopesRequested is an array of scope strings, derived via splitting on whitespace. Note that the scope is part of the body, like username and password and grant_type; it is not part of the query string, as in this pull request.
  • You can do either cb(err), cb(null, true) to grant all requested scopes, or cb(null, scopesGranted) to grant a different set of scopes. Again, scopesGranted is an array.
• Allow authenticateToken to call back with either credential (as it does now), or { credential, scopes }. If it calls back with the latter, then set req.scopesGranted = scopes.
• Add a new method to requests, res.sendUnauthorized(), that sends the appropriate error and headers, kind of like the current res.sendUnauthenticated() does.

The workflow now is more like it is for existing credentials. You set everything up, and Restify–OAuth2 will call the appropriate hooks, resulting in giving you useful information attached to your request. You can then check that information on appropriate routes that need such checks yourself, and use res.sendUnauthorized() to respond if res.scopesGranted doesn't allow access to the given route.

Finally, we'd want to illustrate this via a new example server + integration test. It'd probably be fine to just do a CC version instead of both ROPC and CC versions.

---

In the meantime, I've released 3.0.0 which fixes the tests and renames res.sendUnauthorized to res.sendUnauthenticated, since as you note in this PR the difference is important and I was not correctly distinguishing. It also deduplicates a lot of the code in the grantToken.js files, which was an area of heavy effort in this PR. The upside is, it should set the stage for proper scope support according to the above plan.

Sadly I don't have enough time left this weekend to carry out the above plan myself, so feel free to give it a shot. Otherwise we'll have to wait until my next free weekend...

[sybeck2k]

it is true that this commit got messed up a bit!

I agree completly on the new hook grantScopes as it allows for more flexibility. Also, I didn't know about the existing whitespace split definition - my commit was based on the Heroku API that uses comma-separated scopes in the query string - but it's better to follow the standard when available.

About the proposed authenticateToken, my idea of the hook authorizeToken was to define in this method a centralized business logic of the authorization, to avoid each route to authorize the request. Moreover I wanted to enforce that the authorization happens as soon as the user is authenticated, preventing any BL (i.e. the pre methods of restify) to get executed for nothing.
Do you have any thought about this?

[domenic]

Do you have any thought about this?

I think that it is not as flexible as an approach which gives the application the correct information, and lets it handle the processing (similar to how authentication is already handled). You can build a generic centralized handler as a simple Restify plugin on top of the information and methods given by Restify–OAuth2, but you cannot be flexible if you are forced to use a generic centralized handler.

Besides, I prefer not to keep business logic in my routes, but instead to handle exactly this kind of stuff inside them. I put the business logic in services, and put stuff like authentication and authorization in the routes.

[sybeck2k]

Ok, it's good to know different options!
I've started some work on the new optional hook grantScopes(credentials, scopesRequested, cb), I've opened a new branch on my fork starting from your tag 3.0.
At this point I will close this PR - but we should keep it for further reference!

[sybeck2k]

I'm re-opening this pull because I still have something to clarify. You can see that I implemented the 3 functionalities in this branch - now I'm trying to implement the server example - and I'm not clear how the grantScopes function is supposed to be used. The workflow implemented is grantToken -> grantScopes - but the grantScopes hook does not have access to the generated token and thus cannot attach the scopes to the token.

In my opinion, when this workflow is implemented, the grantScopes function accepts the token generated by the grantToken function. Finally, grantScopes will be responsible also for the persistance of the token.

In other word, we should have a grantScopes(clientId, token, scopesRequested)

[domenic]

Well, I would think the application code could track the credentials <-> token correspondence itself, but sure, we could have credentials be { clientId, clientSecret, token } or { clientId, clientSecret, username, password, token } using the above grantScopes(credentials, scopesRequested) idea.

[sybeck2k]

now tracked in #17