Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Per-request scoped scope #118

Open
coolaj86 opened this issue Mar 15, 2013 · 5 comments
Open

Per-request scoped scope #118

coolaj86 opened this issue Mar 15, 2013 · 5 comments

Comments

@coolaj86
Copy link

Maybe I'm just not understanding something, but it seems that you can only define the scope-to-be-used inside the strategy as an all-or-nothing deal when the strategy is created.

I'd like to be able to have my app request scopes by degrees - just getting the scope that's needed exactly when it's needed - like facebook encourages.

I'm trying to think of a way to add this feature without breaking anything. Maybe adding an options hash after the callback?

that.authenticate(req, res, cb, reqOpts);

req.authenticate(['facebook'], cb, { "scope": ["email", "birthday"] });

What would you suggest?

@coolaj86
Copy link
Author

It seems like the best thing to do would probably be to change strategies into a hash or an array of hashes

req.authenticate([{ "name": "facebook", "scope": ["email", "birthday"] }], cb);

This way all of the internals could be changed to turn the string into a hash unobtrusively. It would be a new opt-in feature from the existing api user's perspective.

Would you accept such a change?

@ciaranj
Copy link
Owner

ciaranj commented Mar 15, 2013

Hmm, yes I see what you're saying, currently to pass per-request authentication settings you would need to set something on the request (or response) so that it could be used in facebook.authenticate etc.

I think that we could achieve this with minimal changes, using the existing APIs;

  1. requestMethods#authenticate already takes an optional hash or 'options' as the second argument the signature is mixed-in to become req.authenticate(strategy, options, cb) or req.authenticate(strategy, cb) (in auth_middleware.js). Currently the only thing on that opts that does 'anything' is 'scope' the idea being a user can be authenticated as a 'guest' user or 'admin' user or 'someone else'
  2. Currently these options are discarded within requestMethods#authenticate, we could pass them through to the authContext
  3. We could then in authExecutionScope.ctr() add these options to itself and provide a nice tidy method to expose them (the methods of these instances are available on 'this' within facebook#authenticate
  4. Then in facebook#authenticate could just check these options as well as my.scope to allow overriding of the requested scope?

Thoughts?

@coolaj86
Copy link
Author

To help me understand a little better. The new call would look like this?

req.authenticate(['facebook'], { scopes: ["email", "birthday"] }, onAuthenticated);

@coolaj86
Copy link
Author

Bump. Did I understand you correctly or not?

From what I saw in the code I think that it would be less work and cleaner to change the signature to this, but maybe I missed something.

req.authenticate([{ "name": "facebook", "scope": ["email", "birthday"] }], cb);

@ciaranj
Copy link
Owner

ciaranj commented Mar 19, 2013

Sorry, I meant, something like:

req.authenticate( 'facebook', { "facebook" : { scope: ["email", "birthday"] } } , cb)

That middle argument hash already exists on the API, just needs plumbing through, however it isn't as clean as your suggestion, in this scenario. (there's an unfortunate collision here between the already supported 'scope' property on that hash and facebook's scope :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants