diff --git a/config/locales/server.en.yml b/config/locales/server.en.yml index 8bf0904..9e0cb29 100644 --- a/config/locales/server.en.yml +++ b/config/locales/server.en.yml @@ -1,4 +1,7 @@ en: + login: + authenticator_error_fetch_user_details: "Could not retrieve your user details. Do you have an active account?" + site_settings: oauth2_enabled: "Custom OAuth2 is enabled" oauth2_client_id: 'Client ID for custom OAuth2' diff --git a/plugin.rb b/plugin.rb index 973b360..01ab810 100644 --- a/plugin.rb +++ b/plugin.rb @@ -108,30 +108,32 @@ class OAuth2BasicAuthenticator < ::Auth::OAuth2Authenticator log("user_json_url: #{user_json_method} #{user_json_url}") bearer_token = "Bearer #{token}" - user_json_response = - if user_json_method.downcase.to_sym == :post - Net::HTTP - .post_form(URI(user_json_url), 'Authorization' => bearer_token) - .body - else - Excon.get(user_json_url, headers: { 'Authorization' => bearer_token, 'Accept' => 'application/json' }, expects: [200]).body + connection = Excon.new( + user_json_url, + :headers => { 'Authorization' => bearer_token, 'Accept' => 'application/json' } + ) + user_json_response = connection.request(method: user_json_method) + + log("user_json_response: #{user_json_response.inspect}") + + if user_json_response.status == 200 + user_json = JSON.parse(user_json_response.body) + + log("user_json: #{user_json}") + + result = {} + if user_json.present? + json_walk(result, user_json, :user_id) + json_walk(result, user_json, :username) + json_walk(result, user_json, :name) + json_walk(result, user_json, :email) + json_walk(result, user_json, :email_verified) + json_walk(result, user_json, :avatar) end - - user_json = JSON.parse(user_json_response) - - log("user_json: #{user_json}") - - result = {} - if user_json.present? - json_walk(result, user_json, :user_id) - json_walk(result, user_json, :username) - json_walk(result, user_json, :name) - json_walk(result, user_json, :email) - json_walk(result, user_json, :email_verified) - json_walk(result, user_json, :avatar) + result + else + nil end - - result end def after_authenticate(auth) @@ -147,8 +149,13 @@ class OAuth2BasicAuthenticator < ::Auth::OAuth2Authenticator end if SiteSetting.oauth2_fetch_user_details? - fetched_user_details = fetch_user_details(token, auth['uid']) - user_details.merge!(fetched_user_details) + if fetched_user_details = fetch_user_details(token, auth['uid']) + user_details.merge!(fetched_user_details) + else + result.failed = true + result.failed_reason = I18n.t("login.authenticator_error_fetch_user_details") + return result + end end result.name = user_details[:name] diff --git a/spec/plugin_spec.rb b/spec/plugin_spec.rb index 8508741..23a6cac 100644 --- a/spec/plugin_spec.rb +++ b/spec/plugin_spec.rb @@ -90,6 +90,50 @@ describe OAuth2BasicAuthenticator do expect(result.email_valid).to eq(true) end + context "fetch_user_details" do + before(:each) do + SiteSetting.oauth2_fetch_user_details = true + SiteSetting.oauth2_user_json_url = "https://provider.com/user" + SiteSetting.oauth2_user_json_url_method = 'GET' + SiteSetting.oauth2_json_email_path = 'account.email' + end + + let(:success_response) do + { + status: 200, + body: '{"account":{"email":"newemail@example.com"}}' + } + end + + let (:fail_response) do + { + status: 403 + } + end + + it "works" do + stub_request(:get, SiteSetting.oauth2_user_json_url).to_return(success_response) + result = authenticator.after_authenticate(auth) + expect(result.email).to eq("newemail@example.com") + + SiteSetting.oauth2_user_json_url_method = 'POST' + stub_request(:post, SiteSetting.oauth2_user_json_url).to_return(success_response) + result = authenticator.after_authenticate(auth) + expect(result.email).to eq("newemail@example.com") + end + + it "returns an standardised result if the http request fails" do + stub_request(:get, SiteSetting.oauth2_user_json_url).to_return(fail_response) + result = authenticator.after_authenticate(auth) + expect(result.failed).to eq(true) + + SiteSetting.oauth2_user_json_url_method = 'POST' + stub_request(:post, SiteSetting.oauth2_user_json_url).to_return(fail_response) + result = authenticator.after_authenticate(auth) + expect(result.failed).to eq(true) + end + end + context 'avatar downloading' do before { SiteSetting.queue_jobs = true }