Use token callback user details (#18)

* Add way to use user details returned in token response

* Add spec

* Apply suggestions from code review

Co-Authored-By: Robin Ward <robin.ward@gmail.com>

config/locales/server.en.yml

@@ -6,6 +6,9 @@ en:
     oauth2_authorize_url: 'Authorization URL for OAuth2'
     oauth2_token_url: 'Token URL for OAuth2'
     oauth2_token_url_method: 'Method used to fetch the Token URL'
+    oauth2_callback_user_id_path: 'Path in the token response to the user id. eg: params.info.uuid'
+    oauth2_callback_user_info_paths: 'Paths in the token response to other user properties. Supported properties are name, username, email, email_verfied and avatar. Format is property:path, eg: name:params.info.name'
+    oauth2_fetch_user_details: "Fetch user JSON for OAuth2"
     oauth2_user_json_url: 'URL to fetch user JSON for OAuth2 (note we replace :id with the id returned by OAuth call and :token with the token id)'
     oauth2_user_json_url_method: 'Method used to fetch the user JSON URL'
     oauth2_json_user_id_path: 'Path in the OAuth2 User JSON to the user id. eg: user.id'
@@ -22,3 +25,6 @@ en:
     oauth2_scope: "When authorizing request this scope"
     oauth2_button_title: "The text for the OAuth2 button"
     oauth2_full_screen_login: "Use main browser window instead of popup for login"
+
+    errors:
+      oauth2_fetch_user_details: "oauth2_callback_user_id_path must be present to disable oauth2_fetch_user_details"

config/settings.yml

@@ -6,13 +6,20 @@ login:
   oauth2_client_secret: ''
   oauth2_authorize_url: ''
   oauth2_token_url: ''
-  oauth2_user_json_url: ''
   oauth2_token_url_method:
     default: 'POST'
     type: enum
     choices:
       - GET
       - POST
+  oauth2_callback_user_id_path: ''
+  oauth2_callback_user_info_paths:
+    type: list
+    default: 'id'
+  oauth2_fetch_user_details:
+    default: true
+    validator: "Oauth2FetchUserDetailsValidator"
+  oauth2_user_json_url: ''
   oauth2_user_json_url_method:
     default: 'GET'
     type: enum

lib/validators/oauth2_basic/oauth2_fetch_user_details_validator.rb

@@ -0,0 +1,16 @@
+# frozen_string_literal: true
+
+class Oauth2FetchUserDetailsValidator
+  def initialize(opts = {})
+    @opts = opts
+  end
+
+  def valid_value?(val)
+    return true if val == "t"
+    SiteSetting.oauth2_callback_user_id_path.length > 0
+  end
+
+  def error_message
+    I18n.t("site_settings.errors.oauth2_fetch_user_details")
+  end
+end

plugin.rb

@@ -12,15 +12,38 @@ enabled_site_setting :oauth2_enabled
 
 class ::OmniAuth::Strategies::Oauth2Basic < ::OmniAuth::Strategies::OAuth2
   option :name, "oauth2_basic"
+
+  uid do
+    if path = SiteSetting.oauth2_callback_user_id_path.split('.')
+      recurse(access_token, [*path]) if path.present?
+    end
+  end
+
   info do
-    {
-      id: access_token['id']
-    }
+    if paths = SiteSetting.oauth2_callback_user_info_paths.split('|')
+      result = Hash.new
+      paths.each do |p|
+        segments = p.split(':')
+        if segments.length == 2
+          key = segments.first
+          path = [*segments.last.split('.')]
+          result[key] = recurse(access_token, path)
+        end
+      end
+      result
+    end
   end
 
   def callback_url
     Discourse.base_url_no_prefix + script_name + callback_path
   end
+
+  def recurse(obj, keys)
+    return nil if !obj
+    k = keys.shift
+    result = obj.respond_to?(k) ? obj.send(k) : obj[k]
+    keys.empty? ? result : recurse(result, keys)
+  end
 end
 
 class OAuth2BasicAuthenticator < ::Auth::OAuth2Authenticator
@@ -112,11 +135,21 @@ class OAuth2BasicAuthenticator < ::Auth::OAuth2Authenticator
   end
 
   def after_authenticate(auth)
-    log("after_authenticate response: \n\ncreds: #{auth['credentials'].to_hash}\ninfo: #{auth['info'].to_hash}\nextra: #{auth['extra'].to_hash}")
+    log("after_authenticate response: \n\ncreds: #{auth['credentials'].to_hash}\nuid: #{auth['uid']}\ninfo: #{auth['info'].to_hash}\nextra: #{auth['extra'].to_hash}")
 
     result = Auth::Result.new
     token = auth['credentials']['token']
-    user_details = fetch_user_details(token, auth['info'][:id])
+
+    user_details = {}
+    user_details[:user_id] = auth['uid'] if auth['uid']
+    ['name', 'username', 'email', 'email_verified', 'avatar'].each do |key|
+      user_details[key.to_sym] = auth['info'][key] if auth['info'][key]
+    end
+
+    if SiteSetting.oauth2_fetch_user_details?
+      fetched_user_details = fetch_user_details(token, auth['uid'])
+      user_details.merge!(fetched_user_details)
+    end
 
     result.name = user_details[:name]
     result.username = user_details[:username]
@@ -171,3 +204,5 @@ register_css <<CSS
   }
 
 CSS
+
+load File.expand_path("../lib/validators/oauth2_basic/oauth2_fetch_user_details_validator.rb", __FILE__)

spec/plugin_spec.rb

@@ -176,4 +176,71 @@ describe OAuth2BasicAuthenticator do
 
     expect(result).to eq 'http://example.com/1.png'
   end
+
+  context 'token_callback' do
+    let(:user) { Fabricate(:user) }
+    let(:strategy) { OmniAuth::Strategies::Oauth2Basic.new({}) }
+    let(:authenticator) { OAuth2BasicAuthenticator.new('oauth2_basic') }
+
+    let(:auth) do
+      {
+        'credentials' => {
+          'token' => 'token'
+        },
+        'uid' => 'e028b1b918853eca7fba208a9d7e9d29a6e93c57',
+        'info' => {
+          "name" => 'Sammy the Shark',
+          "email" => 'sammy@digitalocean.com'
+        },
+        'extra' => {}
+      }
+    end
+
+    let(:access_token) do
+      { "params" =>
+        { "info" =>
+          {
+            "name" => "Sammy the Shark",
+            "email" => "sammy@digitalocean.com",
+            "uuid" => "e028b1b918853eca7fba208a9d7e9d29a6e93c57"
+          }
+        }
+      }
+    end
+
+    before(:each) do
+      SiteSetting.oauth2_callback_user_id_path = 'params.info.uuid'
+      SiteSetting.oauth2_callback_user_info_paths = 'name:params.info.name|email:params.info.email'
+    end
+
+    it 'can retrieve user id from access token callback' do
+      strategy.stubs(:access_token).returns(access_token)
+      expect(strategy.uid).to eq 'e028b1b918853eca7fba208a9d7e9d29a6e93c57'
+    end
+
+    it 'can retrive user properties from access token callback' do
+      strategy.stubs(:access_token).returns(access_token)
+      expect(strategy.info['name']).to eq 'Sammy the Shark'
+      expect(strategy.info['email']).to eq 'sammy@digitalocean.com'
+    end
+
+    it 'does apply user properties from access token callback in after_authenticate' do
+      SiteSetting.oauth2_fetch_user_details = true
+      authenticator.stubs(:fetch_user_details).returns(email: 'sammy@digitalocean.com')
+      result = authenticator.after_authenticate(auth)
+
+      expect(result.extra_data[:oauth2_basic_user_id]).to eq 'e028b1b918853eca7fba208a9d7e9d29a6e93c57'
+      expect(result.name).to eq 'Sammy the Shark'
+      expect(result.email).to eq 'sammy@digitalocean.com'
+    end
+
+    it 'does work if user details are not fetched' do
+      SiteSetting.oauth2_fetch_user_details = false
+      result = authenticator.after_authenticate(auth)
+
+      expect(result.extra_data[:oauth2_basic_user_id]).to eq 'e028b1b918853eca7fba208a9d7e9d29a6e93c57'
+      expect(result.name).to eq 'Sammy the Shark'
+      expect(result.email).to eq 'sammy@digitalocean.com'
+    end
+  end
 end