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

Dynamic clientID and clientSecret #48

Open
rbjarnason opened this issue Feb 1, 2016 · 9 comments
Open

Dynamic clientID and clientSecret #48

rbjarnason opened this issue Feb 1, 2016 · 9 comments

Comments

@rbjarnason
Copy link

My app allows many different domains to be used so I need to store clientID and clientSecret in the database with each domain in the database. Can you give me pointers on what I'd need to do to make this work in passport?

After looking at the code for passport-facebook, passport-oauth2 and node-oauth2 then it looks like I would have to change those to somehow reset the oauth2 object with the dynamically on each authorize call in a similar place as the dynamic callbackURL is supported.

@mattmccarty
Copy link

I'm working on a similar issue. You simply store a unique name for your provider in with the clientID and clientSecret then pass that name to passport.use().

For example, pull all of your providers from the database, iterate through all of them one by one and fire off a call to passport.use with the first parameter as the unique name.

In sails.js, I do something like this:

`AuthProviders.find({host: hostName}).exec(function findCB(err, providers) {
if (err) {
// Do something with error
}

for (provider in providers) {
passport.use(providers[provider].uniqueName, {}, function authCallback(param1, param2) {
// Do something when this callback is triggered by the passport authentication
}
}
});`

@rbjarnason
Copy link
Author

Thanks for the pointer @mattmccarty :) I will get back to working on those features in a couple of weeks time. Will report here how it goes.

@mattmccarty
Copy link

No problem. I'm working on an npm module called passport-sso that handles everything you have mentioned plus some. Maybe it's better to collaborate than both us us writing something new? Let me know if you are interested

@rbjarnason
Copy link
Author

@mattmccarty I finally have the time to start looking at this again. I will try out your https://github.com/mattmccarty/passport-sso module - looks like you've already done most (if not all) of the work to get this working! :)

@rbjarnason
Copy link
Author

rbjarnason commented Apr 28, 2016

@mattmccarty I've integrated passport-sso into my project and it works well, thanks for your great code :) I will need to add Twitter and SAML support in the next few weeks, I will do PR's in the case I need to add something to your base to support other models than local and oauth2, I'm not sure I will do. https://github.com/rbjarnason/your-priorities-app

@kara-ryli
Copy link

kara-ryli commented May 4, 2016

For what it's worth, I had to work around this issue as well. I wrote my own OAuth strategy for a couple reasons, but solved the client issue with a dedicated Client class. the API was basically:

const client = new Client(clientId, clientSecret);
passport.use(new CustomOAuth2Strategy({
  client: client,
  ...
});

The client class includes an update method that I call in response to configuration changes, so I can update without restarting the server.

@crung
Copy link

crung commented May 31, 2018

@ry7n How did you get around the Oauth2 constructor requiring things like clientId and secret up front?

@twelve17
Copy link

twelve17 commented Jun 8, 2018

I'm running into this limitation as well, by way of the facebook-strategy, which inherits this one. Trying to get the existing passport-oauth2 strategy to use dynamic clients seems like a bit of a shoehorn.

It looks like by design, there's an expectation for there to only be one instance of a type of strategy to be present in the passport configuration, as the strategy names are set in the strategy's constructor. While I do find passport to have been suitable for my needs, I ran across authom, which has allowed me to register multiple instances of the same kind of strategy, and is low level enough that it has allowed me to have it even work along side passport strategies.

@basememara
Copy link

I implemented a workaround downstream by dynamically reinstantiating them as they are accessed per request, which isn't desirable but works. It would be great to have this done on this level of the OAuth strategy so derived strategies will have this feature.

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

6 participants