Skip to content
This repository has been archived by the owner on Dec 13, 2018. It is now read-only.

Scope factory and client_credentials #550

Open
wants to merge 3 commits into
base: 4.0.0
Choose a base branch
from
Open

Conversation

the-overengineer
Copy link

Integrates new Node SDK client_credentials authenticator and adds support for scope factories.

Depends on stormpath/stormpath-sdk-node#575 PR in the Node SDK and will not work without it. This PR allows the OAuthAuthenticator class to proxy the scope factory and signing key between the Express app and the concrete authenticator instances.

The scope factory is set as a property in the config (config.web.scopeFactory), and will be used if defined there. The client secret will be used automatically, and no configuration is required.

It adds support for sending the client id and secret as parameters, but remains backwards-compatible with sending them via the Authorization header.

Note: There still seems to be some sort of an issue with the code, which I am trying to resolve. When using client_credentials in the test, it will sometimes fail unless it is awaited for a short time between creating the api keys and using them. Still not sure why this is happening, but am looking into it. Could always use a second pair of eyes. At the moment, I'm trying to solve it in a hacky way, using a setTimeout.

Fixes #535

@the-overengineer
Copy link
Author

The guys who worked on the Ruby and Rails SDKs tell me that this has been known to happen - elastic search syncing or something like that. Don't know if that's a problem. I do assume that it's extremely unlikely that the exact conditions from the test will be replicated, though - even delaying with something as simple as a console.log will make the error not appear, and I doubt someone is creating and using the client credentials in the same breath in any real-world application. Maybe we don't have a problem there after all.

@robertjd robertjd changed the base branch from master to 4.0.0 February 3, 2017 23:51
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant