-
Notifications
You must be signed in to change notification settings - Fork 13
expose any additional params passed out of the oauth responses #4
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
base: master
Are you sure you want to change the base?
Conversation
VictorioBerra
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks like there a few formatting issues where there are missing tabs or something.
TinyOAuth1/AccessTokenInfo.cs
Outdated
| { | ||
| public string AccessToken { get; set; } | ||
| public string AccessTokenSecret { get; set; } | ||
| public IDictionary<string, string> AdditionalParams { get; set; } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Formatting.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, I don't see it in my editor. Which is odd, because it isn't a tabs/spaces thing I don't think. Will fix.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah. It is. You use tabs.
TinyOAuth1/RequestTokenInfo.cs
Outdated
| { | ||
| public string RequestToken { get; set; } | ||
| public string RequestTokenSecret { get; set; } | ||
| public IDictionary<string, string> AdditionalParams { get; set; } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Formatting.
TinyOAuth1/TinyOAuth.cs
Outdated
| break; | ||
| } | ||
| } | ||
| var oauthToken = keyValPairs["oauth_token"]; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Does this throw if the key does not exist? The old behavior would simply keep keyValPairs and oauthToken null. Maybe we do not want this new behavior?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fair. It would throw I believe. I did this very quickly last night. Wasn't expecting you to get your eyes on it so quickly 😝
TinyOAuth1/TinyOAuth.cs
Outdated
| } | ||
| var keyValPairs = HttpUtility.ParseQueryString(responseText); | ||
|
|
||
| var oauthToken = keyValPairs["oauth_token"]; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
See above comment.
TinyOAuth1/TinyOAuth.cs
Outdated
| RequestToken = oauthToken, | ||
| RequestTokenSecret = oauthTokenSecret | ||
| RequestTokenSecret = oauthTokenSecret, | ||
| AdditionalParams = ToDictionary(keyValPairs) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Formatting.
TinyOAuth1/TinyOAuth.cs
Outdated
| } | ||
|
|
||
| private static string ConcatList(IEnumerable<string> source, string separator) | ||
| private static IDictionary<string, string> ToDictionary(NameValueCollection col) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Would this be better as an extension method in its own file?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sure.
|
Hi guys! I am no longer using this project myself, are some of you willing to maintain it? I can add you if you want to? @VictorioBerra |
review comments and do not throw if oauth values are not present in response tabs not spaces
49215df to
dfe8377
Compare
|
Apologies for the messy nature of the PR. Have squashed all the changes into one commit that is a bit more reviewable! |
|
@johot You can add me, I am fine with reviewing and accepting PRs and issue replies but I don't think I have time to do any heavy lifting such as bug fixing, or feature additions. |
|
@johot and @VictorioBerra I am happy to contribute some time as well. Maybe between us there will be enough resource to get things done? This and possibly some companion version for OAuth2 are in my own interest currently. |
|
I think there are a large amount of good Oauth2 libs out at the moment |
|
That's true, I'm not suggesting turning |
I love the simplicity of this library, but I have been working with some APIs that implement OAuth1 in a non standard way and require either capturing extra information from the response, or passing extra information in the requests.
Initially it would be really good if I could get these params out of the responses.
Happy to iterate over this suggestion till it suits everyone.