From afaeb20f270a0b75f232bd90059317533ede06ee Mon Sep 17 00:00:00 2001 From: Sam Date: Tue, 10 Apr 2018 13:17:23 +1000 Subject: [PATCH] FEATURE: Add option to have sso synchronize group membership In some cases add_groups and remove_groups is too much work, some sites may wish to simply synchronize group membership based on a list. When sso_overrides_groups is on all not automatic group membership is sourced from SSO. Note if you omit to specify groups, they will be cleared out. --- app/models/discourse_single_sign_on.rb | 22 +++++++++++++++ config/locales/server.en.yml | 1 + config/site_settings.yml | 1 + lib/single_sign_on.rb | 2 -- spec/models/discourse_single_sign_on_spec.rb | 28 ++++++++++++++++++++ 5 files changed, 52 insertions(+), 2 deletions(-) diff --git a/app/models/discourse_single_sign_on.rb b/app/models/discourse_single_sign_on.rb index f25a7164317..40eacadbb69 100644 --- a/app/models/discourse_single_sign_on.rb +++ b/app/models/discourse_single_sign_on.rb @@ -106,7 +106,29 @@ class DiscourseSingleSignOn < SingleSignOn private + def synchronize_groups(user) + names = (groups || "").split(",").map(&:downcase) + ids = Group.where('LOWER(NAME) in (?) AND NOT automatic', names).pluck(:id) + + group_users = GroupUser + .where('group_id IN (SELECT id FROM groups WHERE NOT automatic)') + .where(user_id: user.id) + + group_users.where('group_id NOT IN (?)', ids).destroy_all + + ids -= group_users.where('group_id IN (?)', ids).pluck(:group_id) + + ids.each do |group_id| + GroupUser.create(group_id: group_id, user_id: user.id) + end + end + def apply_group_rules(user) + if SiteSetting.sso_overrides_groups + synchronize_groups(user) + return + end + if add_groups split = add_groups.split(",").map(&:downcase) if split.length > 0 diff --git a/config/locales/server.en.yml b/config/locales/server.en.yml index c6bde6648b7..89a9d05f484 100644 --- a/config/locales/server.en.yml +++ b/config/locales/server.en.yml @@ -1185,6 +1185,7 @@ en: sso_url: "URL of single sign on endpoint (must include http:// or https://)" sso_secret: "Secret string used to cryptographically authenticate SSO information, be sure it is 10 characters or longer" sso_overrides_bio: "Overrides user bio in user profile and prevents user from changing it" + sso_overrides_groups: "Synchronize all manual group membership with groups specified in the groups sso attribute (WARNING: if you do not specify groups all manual group membership will be cleared for user)" sso_overrides_email: "Overrides local email with external site email from SSO payload on every login, and prevent local changes. (WARNING: discrepancies can occur due to normalization of local emails)" sso_overrides_username: "Overrides local username with external site username from SSO payload on every login, and prevent local changes. (WARNING: discrepancies can occur due to differences in username length/requirements)" sso_overrides_name: "Overrides local full name with external site full name from SSO payload on every login, and prevent local changes." diff --git a/config/site_settings.yml b/config/site_settings.yml index 2521003002f..2a32f406f13 100644 --- a/config/site_settings.yml +++ b/config/site_settings.yml @@ -320,6 +320,7 @@ login: default: '' regex: '^https?:\/\/.+[^\/]$' sso_secret: '' + sso_overrides_groups: false sso_overrides_bio: false sso_overrides_email: default: false diff --git a/lib/single_sign_on.rb b/lib/single_sign_on.rb index ba8a9035946..a0fdf257052 100644 --- a/lib/single_sign_on.rb +++ b/lib/single_sign_on.rb @@ -4,7 +4,6 @@ class SingleSignOn :add_groups, :remove_groups, :groups] FIXNUMS = [] BOOLS = [:avatar_force_update, :admin, :moderator, :require_activation, :suppress_welcome_message] - ARRAYS = [:groups] NONCE_EXPIRY_TIME = 10.minutes attr_accessor(*ACCESSORS) @@ -41,7 +40,6 @@ class SingleSignOn if BOOLS.include? k val = ["true", "false"].include?(val) ? val == "true" : nil end - val = Array(val) if ARRAYS.include?(k) && !val.nil? sso.send("#{k}=", val) end diff --git a/spec/models/discourse_single_sign_on_spec.rb b/spec/models/discourse_single_sign_on_spec.rb index 6f3e7ce5dfa..373c2518d2c 100644 --- a/spec/models/discourse_single_sign_on_spec.rb +++ b/spec/models/discourse_single_sign_on_spec.rb @@ -121,6 +121,34 @@ describe DiscourseSingleSignOn do expect(admin_group.users.where('users.id = ?', user.id).exists?).to eq(true) end + it "can force a list of groups with the groups attribute" do + user = Fabricate(:user) + group1 = Fabricate(:group, name: 'group1') + group2 = Fabricate(:group, name: 'group2') + + sso = DiscourseSingleSignOn.new + sso.username = "bobsky" + sso.name = "Bob" + sso.email = user.email + sso.external_id = "A" + + sso.groups = "#{group2.name.capitalize},group4,badname,trust_level_4" + sso.lookup_or_create_user(ip_address) + + SiteSetting.sso_overrides_groups = true + + group1.reload + expect(group1.usernames).to eq("") + expect(group2.usernames).to eq("") + + group1.add(user) + group1.save + + sso.lookup_or_create_user(ip_address) + expect(group1.usernames).to eq("") + expect(group2.usernames).to eq(user.username) + end + it "can specify groups" do user = Fabricate(:user)