Supply access token through Authorization header#308
Supply access token through Authorization header#308ConradIrwin merged 1 commit intodefunkt:masterfrom
Conversation
Instead of through query parameter, since that method is being actively deprecated. * https://developer.github.com/changes/2019-11-05-deprecated-passwords-and-authorizations-api/#authenticating-using-query-parameters * https://developer.github.com/v3/#oauth2-token-sent-in-a-header
| url << "?access_token=" << CGI.escape(access_token) if access_token.to_s != '' | ||
|
|
||
| request = Net::HTTP::Post.new(url) | ||
| request['Authorization'] = "token #{access_token}" if access_token.to_s != '' |
There was a problem hiding this comment.
@keynslug thanks for this change!
In general it's unsafe to echo user-provided input into HTTP headers (as if the input contains \r\n then they can control more than just one header). The risk is very low in this context (as the access token is already assumed to be trusted), but as good practice I think we should do something like:
"token #{header_safe access_token}" where
def header_safe(x)
raise "invalid access_token" if x =~ /[\r\n]/
x
end
There was a problem hiding this comment.
As Conrad notes, this is probably needed, but the input reader presently stops this anyway, so it's only an extra security step (a very good one).
There was a problem hiding this comment.
Oh. I had an impression that it's already taken care of by these lines of code from Net::HTTPHeader:
https://github.com/ruby/ruby/blob/v2_7_0/lib/net/http/header.rb#L84-L86
There was a problem hiding this comment.
Perfect, I was not aware of that!
|
Thanks for this! Will this be released soon, or should we rather install from the repo? |
|
Published as v5.1.0
…On Thu, Feb 06, 2020 at 3:51 AM, Michael Marino < ***@***.*** > wrote:
Thanks for this! Will this be released soon, or should we rather install
from the repo?
—
You are receiving this because you modified the open/close state.
Reply to this email directly, view it on GitHub (
#308?email_source=notifications&email_token=AAAXAQCSUIN7VGNCQO4FN43RBP2VRA5CNFSM4KPVKZE2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEK66EBI#issuecomment-582869509
) , or unsubscribe (
https://github.com/notifications/unsubscribe-auth/AAAXAQEIG3OZBGXDWPWNTILRBP2VRANCNFSM4KPVKZEQ
).
|
Wonderful! Would be great to have it tagged and provided as a release also. |
GitHub has changed the authentication method slightly, discontining support for the old access_token query parameter. All consumers should move to headers [1] instead. Support for the old method ends 2020/07/01. [1] https://developer.github.com/changes/2019-11-05-deprecated-passwords-and-authorizations-api/#authenticating-using-query-parameters Fixes: defunkt/gist#308 Signed-off-by: Robin H. Johnson <robbat2@gentoo.org>
Instead of through query parameter, since that method is being actively deprecated.