Handle fetch user details failure (#20)

* handle failure in get_user_details request

* add spec

* improve spec

* return nil on failure

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'

plugin.rb

@@ -108,16 +108,16 @@ class OAuth2BasicAuthenticator < ::Auth::OAuth2Authenticator
     log("user_json_url: #{user_json_method} #{user_json_url}")
 
     bearer_token = "Bearer #{token}"
-    user_json_response =
+    connection = Excon.new(
-      if user_json_method.downcase.to_sym == :post
+      user_json_url,
-        Net::HTTP
+      :headers => { 'Authorization' => bearer_token, 'Accept' => 'application/json' }
-          .post_form(URI(user_json_url), 'Authorization' => bearer_token)
+    )
-          .body
+    user_json_response = connection.request(method: user_json_method)
-      else
-        Excon.get(user_json_url, headers: { 'Authorization' => bearer_token, 'Accept' => 'application/json' }, expects: [200]).body
-      end
 
-    user_json = JSON.parse(user_json_response)
+    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}")
 
@@ -130,8 +130,10 @@ class OAuth2BasicAuthenticator < ::Auth::OAuth2Authenticator
         json_walk(result, user_json, :email_verified)
         json_walk(result, user_json, :avatar)
       end
-
       result
+    else
+      nil
+    end
   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'])
+      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]

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 }