diff --git a/lib/controllers/authorize_controller.rb b/lib/controllers/authorize_controller.rb index 61e4bae4..8a403ba6 100644 --- a/lib/controllers/authorize_controller.rb +++ b/lib/controllers/authorize_controller.rb @@ -12,10 +12,15 @@ class AuthorizeController < BaseController session[:state] = SecureRandom.hex(32) - redirect_uri = Authorize.construct_redirect_uri(request.referrer) - parameters = %Q(?scope=user:email&client_id=#{ENV.fetch('GITHUB_BASIC_CLIENT_ID')}&redirect_uri=#{redirect_uri}&state=#{session[:state]}) - - redirect URI.join(GITHUB_OAUTH_AUTHORIZE_URL, parameters) + # redirect_uri is already percent-encoded, so build the query by hand to avoid double-encoding. + query = [ + "scope=user:email%20read:org", + "client_id=#{ENV.fetch('GITHUB_BASIC_CLIENT_ID')}", + "redirect_uri=#{Authorize.construct_redirect_uri(request.referrer)}", + "state=#{session[:state]}", + ].join("&") + + redirect URI.join(GITHUB_OAUTH_AUTHORIZE_URL, "?#{query}") end get '/callback*' do diff --git a/test/integration/app_not_logged_in_test.rb b/test/integration/app_not_logged_in_test.rb index 85ec7d28..aabe12a3 100644 --- a/test/integration/app_not_logged_in_test.rb +++ b/test/integration/app_not_logged_in_test.rb @@ -366,6 +366,16 @@ def test_authorize_without_referer assert_equal 400, last_response.status end + def test_authorize_requests_read_org_scope + # Without read:org the membership check 302s and starkast? is always false. + ClimateControl.modify(GITHUB_BASIC_CLIENT_ID: "fake_client_id") do + header "Referer", "http://fake/referer" + get "/authorize" + end + + assert_includes last_response["Location"], "read:org" + end + def test_authorize_callback access_token = "fake_access_token" user = "fake_user"