Add support for providing clientSecret as a function #476
Conversation
Partially resolves #462
| @@ -226,6 +226,8 @@ The `server.auth.strategy()` method requires the following strategy options: | |||
| object will be merged with the Wreck request object used to call the token endpoint. Such an | |||
| object can contain custom HTTP headers or TLS options (e.g. | |||
| `{ agent: new Https.Agent({ cert: myClientCert, key: myClientKey}) }`). | |||
| To allow dynamically updating secret, this option can be passed as a *function* returning string | |||
There was a problem hiding this comment.
We should mention that the fonction is called synchronously.
There was a problem hiding this comment.
I think function returning string is equivalent to () => string in TypeScript, not () => string | Promise<string>. Though, we can just add await in lib/oauth.js:267
There was a problem hiding this comment.
I agree but we didn't express any type definition here. This was just to prevent people from using async function and then wonder why it does not work the way they intended to. I don't expect every user to look at the code. I'm in favor of leaving sync for now especially you don't seem to need it. We can always reassess later if someone needs it.
| @@ -263,6 +263,9 @@ exports.v2 = function (settings) { | |||
| if (typeof settings.clientSecret === 'string') { | |||
| query.client_secret = settings.clientSecret; | |||
| } | |||
| else if (typeof settings.clientSecret === 'function') { | |||
| query.client_secret = settings.clientSecret(); | |||
There was a problem hiding this comment.
Is there any parameters that could be interesting to pass on here?
There was a problem hiding this comment.
I'm not much of a bell user myself that's just a question that crossed my mind. I just wondered if that's something you considered on your end. If you don't need it let's keep it that way then.
|
This blocks our application development. @Nargonath |
|
Any update on this? @Nargonath |
|
Sorry for the lack of udpate on this @Ginden. I was waiting for others to chime in. I'll try to get some feedbacks and we'll see. 😃 |
|
It has been published as |
Partially resolves #462