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

Add a whitelist in options for header parameters that do not begin with oauth_ #88

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

williamkmlau
Copy link

Right now the toHeader() function will strip away all parameters that don't begin with "oauth_", I have added a white list option to deal with cases where there are non-standard headers such as xoauth that is required in the Authorization string.

@ddo
Copy link
Owner

ddo commented Jun 10, 2019

is this really necessary? you can modify the header after got it from toHeader()

@williamkmlau
Copy link
Author

is this really necessary? you can modify the header after got it from toHeader()

I think its necessary for 2 main reasons, abstraction and consistency.

Firstly for abstraction.
The toHeader() method takes the oauth parameters and creates an authorization string that is comma separated, URI encoded and sorted by name.
You are correct in saying the header can be modified, but that would mean the user has to URI encode and then insert each extra parameter in the correct sorted position. (It would be simpler to just generate the entire string from scratch)
However my point here is, in order to do the above, the user is required to have the knowledge of URI Encoding and Sorting standard of OAuth protocols.
The use of the library is to abstract away these sophisticated protocol steps so the user can simply throw in everything and the oauth works.

Secondly, for consistency.
The getSignature() or getBaseString() function already accounts for extra parameters included in the oauth_data object, which makes it awkward when passing the same oauth_data object to toHeader() function that doesn't support extra parameters.

An alternative solution instead of using a whitelist is simply to add a new boolean option (such as allowExtraOAuthParams) which disables the
if (sorted[i].key.indexOf('oauth_') !== 0 checking.
I didn't opt for this solution because it seems to dissuade precaution when making the oauth_data.

@ddo
Copy link
Owner

ddo commented Jun 14, 2019

good point, im still consider about it. ty

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

Successfully merging this pull request may close these issues.

None yet

2 participants