From 67f3fe14aac8de9f41835a870022d6a074e1cf54 Mon Sep 17 00:00:00 2001 From: mentalstring Date: Tue, 28 Apr 2020 07:06:35 +0100 Subject: [PATCH] FEATURE: support SSO website and location overrides Add location and website + the ability to override using SSO using the `sso_overrides_location` and `sso_overrides_website` site settings. --- .../app/controllers/preferences/profile.js | 10 ++++++ .../app/templates/preferences/profile.hbs | 24 ++++++++------ app/models/discourse_single_sign_on.rb | 5 +++ app/serializers/user_serializer.rb | 10 ++++++ app/serializers/web_hook_user_serializer.rb | 2 ++ app/services/user_updater.rb | 10 ++++-- config/locales/server.en.yml | 2 ++ config/site_settings.yml | 2 ++ lib/single_sign_on.rb | 1 + spec/models/discourse_single_sign_on_spec.rb | 2 ++ spec/services/user_updater_spec.rb | 32 +++++++++++++++++++ test/javascripts/fixtures/user_fixtures.js | 1 + 12 files changed, 89 insertions(+), 12 deletions(-) diff --git a/app/assets/javascripts/discourse/app/controllers/preferences/profile.js b/app/assets/javascripts/discourse/app/controllers/preferences/profile.js index 71992e99fd..7eac6c1067 100644 --- a/app/assets/javascripts/discourse/app/controllers/preferences/profile.js +++ b/app/assets/javascripts/discourse/app/controllers/preferences/profile.js @@ -48,6 +48,16 @@ export default Controller.extend({ return canChangeBio; }, + @discourseComputed("model.can_change_location") + canChangeLocation(canChangeLocation) { + return canChangeLocation; + }, + + @discourseComputed("model.can_change_website") + canChangeWebsite(canChangeWebsite) { + return canChangeWebsite; + }, + actions: { showFeaturedTopicModal() { showModal("feature-topic-on-profile", { diff --git a/app/assets/javascripts/discourse/app/templates/preferences/profile.hbs b/app/assets/javascripts/discourse/app/templates/preferences/profile.hbs index f07bcafb74..fb3d780b0e 100644 --- a/app/assets/javascripts/discourse/app/templates/preferences/profile.hbs +++ b/app/assets/javascripts/discourse/app/templates/preferences/profile.hbs @@ -17,19 +17,23 @@ {{d-button icon="globe" label="user.use_current_timezone" action=(action "useCurrentTimezone") }} -
- -
- {{input type="text" value=model.location class="input-xxlarge" id="edit-location"}} +{{#if model.can_change_location}} +
+ +
+ {{input type="text" value=model.location class="input-xxlarge" id="edit-location"}} +
-
+{{/if}} -
- -
- {{input type="text" value=model.website class="input-xxlarge"}} +{{#if model.can_change_website}} +
+ +
+ {{input type="text" value=model.website class="input-xxlarge"}} +
-
+{{/if}} {{#each userFields as |uf|}}
diff --git a/app/models/discourse_single_sign_on.rb b/app/models/discourse_single_sign_on.rb index 085946cf84..ab8c6c6a25 100644 --- a/app/models/discourse_single_sign_on.rb +++ b/app/models/discourse_single_sign_on.rb @@ -117,6 +117,11 @@ class DiscourseSingleSignOn < SingleSignOn user.user_profile.save! end + if location + user.user_profile.location = location + user.user_profile.save! + end + unless admin.nil? && moderator.nil? Group.refresh_automatic_groups!(:admins, :moderators, :staff) end diff --git a/app/serializers/user_serializer.rb b/app/serializers/user_serializer.rb index 866dc39a8d..352603d755 100644 --- a/app/serializers/user_serializer.rb +++ b/app/serializers/user_serializer.rb @@ -51,6 +51,8 @@ class UserSerializer < UserCardSerializer :ignored_usernames, :mailing_list_posts_per_day, :can_change_bio, + :can_change_location, + :can_change_website, :user_api_keys, :user_auth_tokens @@ -108,6 +110,14 @@ class UserSerializer < UserCardSerializer !(SiteSetting.enable_sso && SiteSetting.sso_overrides_bio) end + def can_change_location + !(SiteSetting.enable_sso && SiteSetting.sso_overrides_location) + end + + def can_change_website + !(SiteSetting.enable_sso && SiteSetting.sso_overrides_website) + end + def user_api_keys keys = object.user_api_keys.where(revoked_at: nil).map do |k| { diff --git a/app/serializers/web_hook_user_serializer.rb b/app/serializers/web_hook_user_serializer.rb index 4fd63a8abe..31d178b1a0 100644 --- a/app/serializers/web_hook_user_serializer.rb +++ b/app/serializers/web_hook_user_serializer.rb @@ -27,6 +27,8 @@ class WebHookUserSerializer < UserSerializer gravatar_avatar_upload_id custom_avatar_upload_id can_change_bio + can_change_location + can_change_website user_api_keys group_users user_auth_tokens diff --git a/app/services/user_updater.rb b/app/services/user_updater.rb index 02db79cb95..b30f5b13c7 100644 --- a/app/services/user_updater.rb +++ b/app/services/user_updater.rb @@ -52,13 +52,19 @@ class UserUpdater def update(attributes = {}) user_profile = user.user_profile - user_profile.location = attributes.fetch(:location) { user_profile.location } user_profile.dismissed_banner_key = attributes[:dismissed_banner_key] if attributes[:dismissed_banner_key].present? - user_profile.website = format_url(attributes.fetch(:website) { user_profile.website }) unless SiteSetting.enable_sso && SiteSetting.sso_overrides_bio user_profile.bio_raw = attributes.fetch(:bio_raw) { user_profile.bio_raw } end + unless SiteSetting.enable_sso && SiteSetting.sso_overrides_location + user_profile.location = attributes.fetch(:location) { user_profile.location } + end + + unless SiteSetting.enable_sso && SiteSetting.sso_overrides_website + user_profile.website = format_url(attributes.fetch(:website) { user_profile.website }) + end + if attributes[:profile_background_upload_url] == "" user_profile.profile_background_upload_id = nil elsif upload = Upload.get_from_url(attributes[:profile_background_upload_url]) diff --git a/config/locales/server.en.yml b/config/locales/server.en.yml index baf137f94b..7a0ebe6f4e 100644 --- a/config/locales/server.en.yml +++ b/config/locales/server.en.yml @@ -1623,6 +1623,8 @@ en: 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." sso_overrides_avatar: "Overrides user avatar with external site avatar from SSO payload. If enabled, users will not be allowed to upload avatars on Discourse." + sso_overrides_location: "Overrides user location with external location from SSO payload and prevent local changes." + sso_overrides_website: "Overrides user website with external location from SSO payload and prevent local changes." sso_overrides_profile_background: "Overrides user profile background with external site avatar from SSO payload." sso_overrides_card_background: "Overrides user card background with external site avatar from SSO payload." sso_not_approved_url: "Redirect unapproved SSO accounts to this URL" diff --git a/config/site_settings.yml b/config/site_settings.yml index 7d9fd0fd86..ed4a4f1b81 100644 --- a/config/site_settings.yml +++ b/config/site_settings.yml @@ -454,6 +454,8 @@ login: default: false client: true sso_overrides_profile_background: false + sso_overrides_location: false + sso_overrides_website: false sso_overrides_card_background: false sso_not_approved_url: "" email_domains_blacklist: diff --git a/lib/single_sign_on.rb b/lib/single_sign_on.rb index a8e8ea7905..e1436ced9f 100644 --- a/lib/single_sign_on.rb +++ b/lib/single_sign_on.rb @@ -27,6 +27,7 @@ class SingleSignOn title username website + location } FIXNUMS = [] diff --git a/spec/models/discourse_single_sign_on_spec.rb b/spec/models/discourse_single_sign_on_spec.rb index 538d18d212..fb318d16bc 100644 --- a/spec/models/discourse_single_sign_on_spec.rb +++ b/spec/models/discourse_single_sign_on_spec.rb @@ -33,6 +33,7 @@ describe DiscourseSingleSignOn do sso.custom_fields["a"] = "Aa" sso.custom_fields["b.b"] = "B.b" sso.website = "https://www.discourse.org/" + sso.location = "Home" sso end @@ -53,6 +54,7 @@ describe DiscourseSingleSignOn do expect(parsed.custom_fields["a"]).to eq "Aa" expect(parsed.custom_fields["b.b"]).to eq "B.b" expect(parsed.website).to eq sso.website + expect(parsed.location).to eq sso.location end it "can do round trip parsing correctly" do diff --git a/spec/services/user_updater_spec.rb b/spec/services/user_updater_spec.rb index c18949ab5e..158c943da6 100644 --- a/spec/services/user_updater_spec.rb +++ b/spec/services/user_updater_spec.rb @@ -249,6 +249,38 @@ describe UserUpdater do end end + context 'when sso overrides location' do + it 'does not change location' do + SiteSetting.sso_url = "https://www.example.com/sso" + SiteSetting.enable_sso = true + SiteSetting.sso_overrides_location = true + + user = Fabricate(:user) + updater = UserUpdater.new(acting_user, user) + + expect(updater.update(location: "new location")).to be_truthy + + user.reload + expect(user.user_profile.location).not_to eq 'new location' + end + end + + context 'when sso overrides website' do + it 'does not change website' do + SiteSetting.sso_url = "https://www.example.com/sso" + SiteSetting.enable_sso = true + SiteSetting.sso_overrides_website = true + + user = Fabricate(:user) + updater = UserUpdater.new(acting_user, user) + + expect(updater.update(website: "https://google.com")).to be_truthy + + user.reload + expect(user.user_profile.website).not_to eq 'https://google.com' + end + end + context 'when updating primary group' do let(:new_group) { Group.create(name: 'new_group') } let(:user) { Fabricate(:user) } diff --git a/test/javascripts/fixtures/user_fixtures.js b/test/javascripts/fixtures/user_fixtures.js index 2bac0bfe34..abf4a39037 100644 --- a/test/javascripts/fixtures/user_fixtures.js +++ b/test/javascripts/fixtures/user_fixtures.js @@ -187,6 +187,7 @@ export default { card_image_badge: "/images/avatar.png", card_image_badge_id: 120, muted_usernames: [], + can_change_location: true, ignored_usernames: [], invited_by: { id: 1,