From a634ff896d05972a336d647d6c8882124610708a Mon Sep 17 00:00:00 2001 From: Angus McLeod Date: Sat, 6 Jul 2019 00:27:07 +1000 Subject: [PATCH] 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 --- config/locales/server.en.yml | 6 ++ config/settings.yml | 9 ++- .../oauth2_fetch_user_details_validator.rb | 16 +++++ plugin.rb | 45 +++++++++++-- spec/plugin_spec.rb | 67 +++++++++++++++++++ 5 files changed, 137 insertions(+), 6 deletions(-) create mode 100644 lib/validators/oauth2_basic/oauth2_fetch_user_details_validator.rb diff --git a/config/locales/server.en.yml b/config/locales/server.en.yml index bc8d5bb..6b06970 100644 --- a/config/locales/server.en.yml +++ b/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" diff --git a/config/settings.yml b/config/settings.yml index 1ce06e6..50470e7 100644 --- a/config/settings.yml +++ b/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 diff --git a/lib/validators/oauth2_basic/oauth2_fetch_user_details_validator.rb b/lib/validators/oauth2_basic/oauth2_fetch_user_details_validator.rb new file mode 100644 index 0000000..3e38f2e --- /dev/null +++ b/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 diff --git a/plugin.rb b/plugin.rb index 63d0466..973b360 100644 --- a/plugin.rb +++ b/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 < { + '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