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

Changes to make ETrade work #26

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

Conversation

linuxrocks123
Copy link

Hey there,

I made some changes to your library to make ETrade work. The primary change is adding a parameter to add additional key/value pairs when getting an HTTP header, but I also changed it so that URL encoding is done for Authorization headers as well as query URLs.

If you know of a case where Authorization headers should definitely NOT be URL encoded, you may wish to make this a parameter instead of accepting my change as-is. However, the only way to make ETrade work was by flipping that boolean, so it should definitely be an option.

This is a very useful library. Thanks for writing it, and have a nice day.

@ewencp
Copy link
Member

ewencp commented Oct 20, 2018

@linuxrocks123 You didn't mention what headers needed to be added. I'm curious if etrade is requiring some non-standard header or if there might just be a gap in the set of headers handled by this library currently. In particular, I notice that you're using the extra headers when generating the Authorization string, but from a quick look at the etrade docs I can't see anything that looks like it would be required and not one of the standard oauth headers.

In particular, I'm noticing from https://us.etrade.com/ctnt/dev-portal/getDetail?contentUri=V0_Documentation-AuthorizationAPI-GetRequestToken that perhaps the oauth_callback is omitted from the list used for the Authorization header currently. I'm wondering if this could be fixed more simply by just fixing the list of headers to include there (i.e. here in this patch).

@linuxrocks123
Copy link
Author

linuxrocks123 commented Oct 21, 2018

The parameter I added in my client code was exactly oauth_callback=oob. For ETrade, oauth_callback is always supposed to be oob, but I think it's not supposed to always be oob for other sites. My initial reaction is that supporting arbitrary key-value pairs is desirable since it's more flexible than hard-coding support for oauth_callback some kind of way, but perhaps I'm missing something.

You'd still need to support URL-encoding the Authorization string after hard-coding in oauth_callback. That's line 478/480 of liboauthcpp.cpp in the patch.

@CharMyFloat
Copy link

@linuxrocks123

Hey I have been stuck trying to get this to work with etrade. I'm able to get the request token and authorize with the pin but when trying to make the call to get the access token something is not working. I'm able to get it working in Postman from there if I percent decode the token key and secret. Was there anything you had to change on the last call to make it work?

@CharMyFloat
Copy link

@linuxrocks123
Nevermind I got it. Had to decode, then with the bool flipped you pointed out, encode the token string in the header.

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.

3 participants