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

Support functions for provider.auth and provider.token #463

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

Conversation

charlesbjohnson
Copy link

Resolves #461

@charlesbjohnson charlesbjohnson changed the title Support functions for provider.auth and `provider.token Support functions for provider.auth and provider.token Jun 8, 2020
@charlesbjohnson charlesbjohnson force-pushed the charlesbjohnson/dynamic-endpoints branch from 57816a0 to eadaa03 Compare June 8, 2020 00:12
headers: Joi.object(),
profile: Joi.func(),
profileMethod: Joi.valid('get', 'post').default('get')
})
.when('.protocol', {
is: 'oauth',
then: Joi.object({
temporary: Joi.string().required(),
temporary: Joi.alternatives(Joi.string(), Joi.func()).required(),
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My interest is only in auth and token, but I thought it might be odd to leave this out.

Comment on lines +2239 to +2243
it('passes through without port', () => {

expect(OAuth.Client.baseUri('http://example.com/x')).to.equal('http://example.com/x');
expect(OAuth.Client.baseUri('https://example.com/x')).to.equal('https://example.com/x');
});
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is unrelated, but after my changes we lost coverage on this case. This adds it back.

@hueniverse hueniverse self-assigned this Jul 5, 2020
@hueniverse hueniverse added the feature New functionality or improvement label Jul 5, 2020
lib/oauth.js Outdated
clientId: options.clientId,
clientSecret: options.provider.signatureMethod === 'RSA-SHA1' ? options.clientSecret : internals.encode(options.clientSecret || '') + '&',
signatureMethod: options.provider.signatureMethod
};
};


internals.Client.prototype.temporary = function (oauth_callback) {
internals.Client.prototype.temporary = function (uri, oauth_callback) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a breaking change. The Client is a public interface. Can't change it.

lib/oauth.js Outdated
};


internals.Client.prototype.token = function (oauthToken, oauthVerifier, tokenSecret) {
internals.Client.prototype.token = function (uri, oauthToken, oauthVerifier, tokenSecret) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same on every Client endpoint... I thought that was implicit... :-)

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I reverted the changes to these method signatures but kept their ability to resolve endpoints from a function, if a function is provided. It's a bit ugly since it potentially requires setting client.settings.token and client.settings.temporary in two passes, but I think it only applies to those who choose to upgrade to using a function instead of a string.

I didn't catch that Client was exported before (d'oh 😅). I think this should resolve the breaking change, right? Is there something else that I'm missing?

@hueniverse
Copy link
Contributor

Thanks for the changes. I hope to get to it in the next few days. If not, please ping me.

@hueniverse hueniverse removed their assignment Aug 18, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature New functionality or improvement
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Support functions for provider.auth and provider.token
3 participants