From 27e732a58d92b051ce36ca6886a119581c335a59 Mon Sep 17 00:00:00 2001 From: Maja Komel Date: Mon, 15 Oct 2018 07:03:53 +0200 Subject: [PATCH] FEATURE: allow multiple secrets for Discourse SSO provider This splits off the logic between SSO keys used incoming vs outgoing, it allows to far better restrict who is allowed to log in using a site. This allows for better auditing of the SSO provider feature --- .../admin/components/secret-value-list.js.es6 | 87 +++++ .../admin/mixins/setting-component.js.es6 | 3 +- .../components/secret-value-list.hbs | 22 ++ .../components/site-settings/secret-list.hbs | 3 + .../stylesheets/common/admin/admin_base.scss | 54 ++- app/controllers/session_controller.rb | 2 +- app/services/wildcard_domain_checker.rb | 10 + config/locales/server.en.yml | 6 + config/site_settings.yml | 7 + ...d_sso_provider_secrets_to_site_settings.rb | 12 + lib/single_sign_on.rb | 27 +- lib/site_setting_extension.rb | 9 +- spec/requests/session_controller_spec.rb | 349 +++++++----------- spec/services/wildcard_domain_checker_spec.rb | 35 ++ .../components/secret-value-list-test.js.es6 | 63 ++++ 15 files changed, 459 insertions(+), 230 deletions(-) create mode 100644 app/assets/javascripts/admin/components/secret-value-list.js.es6 create mode 100644 app/assets/javascripts/admin/templates/components/secret-value-list.hbs create mode 100644 app/assets/javascripts/admin/templates/components/site-settings/secret-list.hbs create mode 100644 app/services/wildcard_domain_checker.rb create mode 100644 db/migrate/20181005084357_add_sso_provider_secrets_to_site_settings.rb create mode 100644 spec/services/wildcard_domain_checker_spec.rb create mode 100644 test/javascripts/components/secret-value-list-test.js.es6 diff --git a/app/assets/javascripts/admin/components/secret-value-list.js.es6 b/app/assets/javascripts/admin/components/secret-value-list.js.es6 new file mode 100644 index 00000000000..939ba45c9e7 --- /dev/null +++ b/app/assets/javascripts/admin/components/secret-value-list.js.es6 @@ -0,0 +1,87 @@ +import { on } from "ember-addons/ember-computed-decorators"; + +export default Ember.Component.extend({ + classNameBindings: [":value-list", ":secret-value-list"], + inputInvalidKey: Ember.computed.empty("newKey"), + inputInvalidSecret: Ember.computed.empty("newSecret"), + inputDelimiter: null, + collection: null, + values: null, + + @on("didReceiveAttrs") + _setupCollection() { + const values = this.get("values"); + + this.set( + "collection", + this._splitValues(values, this.get("inputDelimiter") || "\n") + ); + }, + + actions: { + changeKey(index, newValue) { + this._replaceValue(index, newValue, "key"); + }, + + changeSecret(index, newValue) { + this._replaceValue(index, newValue, "secret"); + }, + + addValue() { + if (this.get("inputInvalidKey") || this.get("inputInvalidSecret")) return; + this._addValue(this.get("newKey"), this.get("newSecret")); + this.setProperties({ newKey: "", newSecret: "" }); + }, + + removeValue(value) { + this._removeValue(value); + } + }, + + _addValue(value, secret) { + this.get("collection").addObject({ key: value, secret: secret }); + this._saveValues(); + }, + + _removeValue(value) { + const collection = this.get("collection"); + collection.removeObject(value); + this._saveValues(); + }, + + _replaceValue(index, newValue, keyName) { + let item = this.get("collection")[index]; + Ember.set(item, keyName, newValue); + + this._saveValues(); + }, + + _saveValues() { + this.set( + "values", + this.get("collection") + .map(function(elem) { + return `${elem.key}|${elem.secret}`; + }) + .join("\n") + ); + }, + + _splitValues(values, delimiter) { + if (values && values.length) { + const keys = ["key", "secret"]; + var res = []; + values.split(delimiter).forEach(function(str) { + var object = {}; + str.split("|").forEach(function(a, i) { + object[keys[i]] = a; + }); + res.push(object); + }); + + return res; + } else { + return []; + } + } +}); diff --git a/app/assets/javascripts/admin/mixins/setting-component.js.es6 b/app/assets/javascripts/admin/mixins/setting-component.js.es6 index 532abe8f20c..9408bfbceba 100644 --- a/app/assets/javascripts/admin/mixins/setting-component.js.es6 +++ b/app/assets/javascripts/admin/mixins/setting-component.js.es6 @@ -11,7 +11,8 @@ const CUSTOM_TYPES = [ "value_list", "category", "uploaded_image_list", - "compact_list" + "compact_list", + "secret_list" ]; export default Ember.Mixin.create({ diff --git a/app/assets/javascripts/admin/templates/components/secret-value-list.hbs b/app/assets/javascripts/admin/templates/components/secret-value-list.hbs new file mode 100644 index 00000000000..7058504facc --- /dev/null +++ b/app/assets/javascripts/admin/templates/components/secret-value-list.hbs @@ -0,0 +1,22 @@ +{{#if collection}} +
+ {{#each collection as |value index|}} +
+ {{d-button action="removeValue" + actionParam=value + icon="times" + class="remove-value-btn btn-small"}} + {{input value=value.key class="value-input" focus-out=(action "changeKey" index)}} + {{input value=value.secret class="value-input" focus-out=(action "changeSecret" index) type="password"}} +
+ {{/each}} +
+{{/if}} + +
+ {{text-field value=newKey class="new-value-input key" placeholder=setting.placeholder.key}} + {{input type="password" value=newSecret class="new-value-input secret" placeholder=setting.placeholder.value}} + {{d-button action="addValue" + icon="plus" + class="add-value-btn btn-small"}} +
diff --git a/app/assets/javascripts/admin/templates/components/site-settings/secret-list.hbs b/app/assets/javascripts/admin/templates/components/site-settings/secret-list.hbs new file mode 100644 index 00000000000..1e71c18c638 --- /dev/null +++ b/app/assets/javascripts/admin/templates/components/site-settings/secret-list.hbs @@ -0,0 +1,3 @@ +{{secret-value-list setting=setting values=value}} +{{setting-validation-message message=validationMessage}} +
{{{unbound setting.description}}}
diff --git a/app/assets/stylesheets/common/admin/admin_base.scss b/app/assets/stylesheets/common/admin/admin_base.scss index 69269d1f12c..24721a26be6 100644 --- a/app/assets/stylesheets/common/admin/admin_base.scss +++ b/app/assets/stylesheets/common/admin/admin_base.scss @@ -865,6 +865,17 @@ table#user-badges { } } +@mixin value-btn { + width: 29px; + border: 1px solid $primary-low; + outline: none; + padding: 0; + + &:focus { + border-color: $tertiary; + } +} + .value-list { .value { padding: 0.125em 0; @@ -891,15 +902,8 @@ table#user-badges { } .remove-value-btn { + @include value-btn; margin-right: 0.25em; - width: 29px; - border: 1px solid $primary-low; - outline: none; - padding: 0; - - &:focus { - border-color: $tertiary; - } } } .values { @@ -907,6 +911,40 @@ table#user-badges { } } +.secret-value-list { + .value { + flex-flow: row wrap; + margin-left: -0.25em; + margin-top: -0.125em; + .new-value-input { + flex: 1; + } + .value-input, + .new-value-input { + margin-top: 0.125em; + &:last-of-type { + margin-left: 0.25em; + } + } + .remove-value-btn { + margin-left: 0.25em; + margin-top: 0.125em; + } + .add-value-btn { + @include value-btn; + margin-left: 0.25em; + margin-top: 0.125em; + } + &:last-of-type { + .new-value-input { + &:first-of-type { + margin-left: 0.25em; + } + } + } + } +} + // Mobile view text-inputs need some padding .mobile-view .admin-contents { input[type="text"] { diff --git a/app/controllers/session_controller.rb b/app/controllers/session_controller.rb index b523943e8d9..af1e761a65f 100644 --- a/app/controllers/session_controller.rb +++ b/app/controllers/session_controller.rb @@ -46,7 +46,7 @@ class SessionController < ApplicationController payload ||= request.query_string if SiteSetting.enable_sso_provider - sso = SingleSignOn.parse(payload, SiteSetting.sso_secret) + sso = SingleSignOn.parse(payload) if sso.return_sso_url.blank? render plain: "return_sso_url is blank, it must be provided", status: 400 diff --git a/app/services/wildcard_domain_checker.rb b/app/services/wildcard_domain_checker.rb new file mode 100644 index 00000000000..3f91811f07e --- /dev/null +++ b/app/services/wildcard_domain_checker.rb @@ -0,0 +1,10 @@ +module WildcardDomainChecker + + def self.check_domain(domain, external_domain) + escaped_domain = domain[0] == "*" ? Regexp.escape(domain).sub("\\*", '\S*') : Regexp.escape(domain) + domain_regex = Regexp.new("^#{escaped_domain}$", 'i') + + external_domain.match(domain_regex) + end + +end diff --git a/config/locales/server.en.yml b/config/locales/server.en.yml index 6a5e45265a3..2606cee026f 100644 --- a/config/locales/server.en.yml +++ b/config/locales/server.en.yml @@ -1319,6 +1319,7 @@ en: enable_sso_provider: "Implement Discourse SSO provider protocol at the /session/sso_provider endpoint, requires sso_secret to be set" 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_provider_secrets: "A list of domain-secret pairs that are using Discourse as a SSO provider. Make sure SSO secret is 10 characters or longer. Wildcard symbol * can be used to match any domain or only a part of it (e.g. *.example.com)." 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)" @@ -1862,6 +1863,11 @@ en: max_username_length_exists: "You cannot set the maximum username length below the longest username (%{username})." max_username_length_range: "You cannot set the maximum below the minimum." + placeholder: + sso_provider_secrets: + key: "www.example.com" + value: "SSO secret" + search: within_post: "#%{post_number} by %{username}" types: diff --git a/config/site_settings.yml b/config/site_settings.yml index 1a882a9c1ef..c98dc0610d8 100644 --- a/config/site_settings.yml +++ b/config/site_settings.yml @@ -341,6 +341,13 @@ login: sso_secret: default: '' secret: true + sso_provider_secrets: + default: '' + type: list + list_type: secret + placeholder: + key: "sso_provider.key_placeholder" + value: "sso_provider.value_placeholder" sso_overrides_groups: false sso_overrides_bio: false sso_overrides_email: diff --git a/db/migrate/20181005084357_add_sso_provider_secrets_to_site_settings.rb b/db/migrate/20181005084357_add_sso_provider_secrets_to_site_settings.rb new file mode 100644 index 00000000000..4e95023b430 --- /dev/null +++ b/db/migrate/20181005084357_add_sso_provider_secrets_to_site_settings.rb @@ -0,0 +1,12 @@ +class AddSsoProviderSecretsToSiteSettings < ActiveRecord::Migration[5.2] + def up + return unless SiteSetting.enable_sso_provider && SiteSetting.sso_secret.present? + sso_secret = SiteSetting.sso_secret + execute "INSERT INTO site_settings(name, data_type, value, created_at, updated_at) + VALUES ('sso_provider_secrets', 8, '*|#{sso_secret}', now(), now())" + end + + def down + execute "DELETE FROM site_settings WHERE name = 'sso_provider_secrets'" + end +end diff --git a/lib/single_sign_on.rb b/lib/single_sign_on.rb index 6900b633b2b..f5a59e26a63 100644 --- a/lib/single_sign_on.rb +++ b/lib/single_sign_on.rb @@ -50,9 +50,14 @@ class SingleSignOn def self.parse(payload, sso_secret = nil) sso = new - sso.sso_secret = sso_secret if sso_secret parsed = Rack::Utils.parse_query(payload) + decoded = Base64.decode64(parsed["sso"]) + decoded_hash = Rack::Utils.parse_query(decoded) + + return_sso_url = decoded_hash['return_sso_url'] + sso.sso_secret = sso_secret || (provider_secret(return_sso_url) if return_sso_url) + if sso.sign(parsed["sso"]) != parsed["sig"] diags = "\n\nsso: #{parsed["sso"]}\n\nsig: #{parsed["sig"]}\n\nexpected sig: #{sso.sign(parsed["sso"])}" if parsed["sso"] =~ /[^a-zA-Z0-9=\r\n\/+]/m @@ -83,6 +88,17 @@ class SingleSignOn sso end + def self.provider_secret(return_sso_url) + provider_secrets = SiteSetting.sso_provider_secrets.split(/[\|,\n]/) + provider_secrets_hash = Hash[*provider_secrets] + return_url_host = URI.parse(return_sso_url).host + + secret = provider_secrets_hash.select do |domain, _| + WildcardDomainChecker.check_domain(domain, return_url_host) + end + secret.present? ? secret.values.first : nil + end + def diagnostics SingleSignOn::ACCESSORS.map { |a| "#{a}: #{send(a)}" }.join("\n") end @@ -99,8 +115,9 @@ class SingleSignOn @custom_fields ||= {} end - def sign(payload) - OpenSSL::HMAC.hexdigest("sha256", sso_secret, payload) + def sign(payload, provider_secret = nil) + secret = provider_secret || sso_secret + OpenSSL::HMAC.hexdigest("sha256", secret, payload) end def to_url(base_url = nil) @@ -108,9 +125,9 @@ class SingleSignOn "#{base}#{base.include?('?') ? '&' : '?'}#{payload}" end - def payload + def payload(provider_secret = nil) payload = Base64.strict_encode64(unsigned_payload) - "sso=#{CGI::escape(payload)}&sig=#{sign(payload)}" + "sso=#{CGI::escape(payload)}&sig=#{sign(payload, provider_secret)}" end def unsigned_payload diff --git a/lib/site_setting_extension.rb b/lib/site_setting_extension.rb index 490cc26b6d5..9122da94d91 100644 --- a/lib/site_setting_extension.rb +++ b/lib/site_setting_extension.rb @@ -220,7 +220,8 @@ module SiteSettingExtension value: value.to_s, category: categories[s], preview: previews[s], - secret: secret_settings.include?(s) + secret: secret_settings.include?(s), + placeholder: placeholder(s) }.merge(type_supervisor.type_hash(s)) opts @@ -231,6 +232,12 @@ module SiteSettingExtension I18n.t("site_settings.#{setting}") end + def placeholder(setting) + if !I18n.t("site_settings.placeholder.#{setting}", default: "").empty? + I18n.t("site_settings.placeholder.#{setting}") + end + end + def self.client_settings_cache_key # NOTE: we use the git version in the key to ensure # that we don't end up caching the incorrect version diff --git a/spec/requests/session_controller_spec.rb b/spec/requests/session_controller_spec.rb index 8a540829d03..2f8b8a4bf75 100644 --- a/spec/requests/session_controller_spec.rb +++ b/spec/requests/session_controller_spec.rb @@ -521,153 +521,6 @@ RSpec.describe SessionController do expect(response.status).to eq(419) end - describe 'can act as an SSO provider' do - before do - stub_request(:any, /#{Discourse.current_hostname}\/uploads/).to_return( - status: 200, - body: lambda { |request| file_from_fixtures("logo.png") } - ) - - SiteSetting.enable_sso_provider = true - SiteSetting.enable_sso = false - SiteSetting.enable_local_logins = true - SiteSetting.sso_secret = "topsecret" - - @sso = SingleSignOn.new - @sso.nonce = "mynonce" - @sso.sso_secret = SiteSetting.sso_secret - @sso.return_sso_url = "http://somewhere.over.rainbow/sso" - - @user = Fabricate(:user, password: "myfrogs123ADMIN", active: true, admin: true) - group = Fabricate(:group) - group.add(@user) - - @user.create_user_avatar! - UserAvatar.import_url_for_user(logo_fixture, @user) - UserProfile.import_url_for_user(logo_fixture, @user, is_card_background: false) - UserProfile.import_url_for_user(logo_fixture, @user, is_card_background: true) - - @user.reload - @user.user_avatar.reload - @user.user_profile.reload - EmailToken.update_all(confirmed: true) - end - - it "successfully logs in and redirects user to return_sso_url when the user is not logged in" do - get "/session/sso_provider", params: Rack::Utils.parse_query(@sso.payload), headers: headers - - expect(response).to redirect_to("/login") - - post "/session.json", - params: { login: @user.username, password: "myfrogs123ADMIN" }, xhr: true - location = response.cookies["sso_destination_url"] - # javascript code will handle redirection of user to return_sso_url - expect(location).to match(/^http:\/\/somewhere.over.rainbow\/sso/) - - payload = location.split("?")[1] - sso2 = SingleSignOn.parse(payload, "topsecret") - - expect(sso2.email).to eq(@user.email) - expect(sso2.name).to eq(@user.name) - expect(sso2.username).to eq(@user.username) - expect(sso2.external_id).to eq(@user.id.to_s) - expect(sso2.admin).to eq(true) - expect(sso2.moderator).to eq(false) - expect(sso2.groups).to eq(@user.groups.pluck(:name).join(",")) - - expect(sso2.avatar_url.blank?).to_not eq(true) - expect(sso2.profile_background_url.blank?).to_not eq(true) - expect(sso2.card_background_url.blank?).to_not eq(true) - - expect(sso2.avatar_url).to start_with(Discourse.base_url) - expect(sso2.profile_background_url).to start_with(Discourse.base_url) - expect(sso2.card_background_url).to start_with(Discourse.base_url) - end - - it "successfully redirects user to return_sso_url when the user is logged in" do - sign_in(@user) - - get "/session/sso_provider", params: Rack::Utils.parse_query(@sso.payload), headers: headers - - location = response.header["Location"] - expect(location).to match(/^http:\/\/somewhere.over.rainbow\/sso/) - - payload = location.split("?")[1] - sso2 = SingleSignOn.parse(payload, "topsecret") - - expect(sso2.email).to eq(@user.email) - expect(sso2.name).to eq(@user.name) - expect(sso2.username).to eq(@user.username) - expect(sso2.external_id).to eq(@user.id.to_s) - expect(sso2.admin).to eq(true) - expect(sso2.moderator).to eq(false) - expect(sso2.groups).to eq(@user.groups.pluck(:name).join(",")) - - expect(sso2.avatar_url.blank?).to_not eq(true) - expect(sso2.profile_background_url.blank?).to_not eq(true) - expect(sso2.card_background_url.blank?).to_not eq(true) - - expect(sso2.avatar_url).to start_with(Discourse.base_url) - expect(sso2.profile_background_url).to start_with(Discourse.base_url) - expect(sso2.card_background_url).to start_with(Discourse.base_url) - end - - it 'handles non local content correctly' do - SiteSetting.avatar_sizes = "100|49" - SiteSetting.enable_s3_uploads = true - SiteSetting.s3_access_key_id = "XXX" - SiteSetting.s3_secret_access_key = "XXX" - SiteSetting.s3_upload_bucket = "test" - SiteSetting.s3_cdn_url = "http://cdn.com" - - stub_request(:any, /test.s3.dualstack.us-east-1.amazonaws.com/).to_return(status: 200, body: "", headers: {}) - - @user.create_user_avatar! - upload = Fabricate(:upload, url: "//test.s3.dualstack.us-east-1.amazonaws.com/something") - - Fabricate(:optimized_image, - sha1: SecureRandom.hex << "A" * 8, - upload: upload, - width: 98, - height: 98, - url: "//test.s3.amazonaws.com/something/else" - ) - - @user.update_columns(uploaded_avatar_id: upload.id) - @user.user_profile.update_columns( - profile_background: "//test.s3.dualstack.us-east-1.amazonaws.com/something", - card_background: "//test.s3.dualstack.us-east-1.amazonaws.com/something" - ) - - @user.reload - @user.user_avatar.reload - @user.user_profile.reload - - sign_in(@user) - - stub_request(:get, "http://cdn.com/something/else").to_return( - body: lambda { |request| File.new(Rails.root + 'spec/fixtures/images/logo.png') } - ) - - get "/session/sso_provider", params: Rack::Utils.parse_query(@sso.payload), headers: headers - - location = response.header["Location"] - # javascript code will handle redirection of user to return_sso_url - expect(location).to match(/^http:\/\/somewhere.over.rainbow\/sso/) - - payload = location.split("?")[1] - sso2 = SingleSignOn.parse(payload, "topsecret") - - expect(sso2.avatar_url.blank?).to_not eq(true) - expect(sso2.profile_background_url.blank?).to_not eq(true) - expect(sso2.card_background_url.blank?).to_not eq(true) - - expect(sso2.avatar_url).to start_with("#{SiteSetting.s3_cdn_url}/original") - expect(sso2.profile_background_url).to start_with(SiteSetting.s3_cdn_url) - expect(sso2.card_background_url).to start_with(SiteSetting.s3_cdn_url) - end - end - describe 'local attribute override from SSO payload' do before do SiteSetting.email_editable = false @@ -724,91 +577,159 @@ RSpec.describe SessionController do end describe '#sso_provider' do - before do - stub_request(:any, /#{Discourse.current_hostname}\/uploads/).to_return( - status: 200, - body: lambda { |request| file_from_fixtures("logo.png") } - ) + let(:headers) { { host: Discourse.current_hostname } } - SiteSetting.enable_sso_provider = true - SiteSetting.enable_sso = false - SiteSetting.enable_local_logins = true - SiteSetting.sso_secret = "topsecret" + describe 'can act as an SSO provider' do + before do + stub_request(:any, /#{Discourse.current_hostname}\/uploads/).to_return( + status: 200, + body: lambda { |request| file_from_fixtures("logo.png") } + ) - @sso = SingleSignOn.new - @sso.nonce = "mynonce" - @sso.sso_secret = SiteSetting.sso_secret - @sso.return_sso_url = "http://somewhere.over.rainbow/sso" + SiteSetting.enable_sso_provider = true + SiteSetting.enable_sso = false + SiteSetting.enable_local_logins = true + SiteSetting.sso_provider_secrets = "www.random.site|secretForRandomSite\nsomewhere.over.rainbow|secretForOverRainbow" - @user = Fabricate(:user, password: "myfrogs123ADMIN", active: true, admin: true) - @user.create_user_avatar! - UserAvatar.import_url_for_user(logo_fixture, @user) - UserProfile.import_url_for_user(logo_fixture, @user, is_card_background: false) - UserProfile.import_url_for_user(logo_fixture, @user, is_card_background: true) + @sso = SingleSignOn.new + @sso.nonce = "mynonce" + @sso.return_sso_url = "http://somewhere.over.rainbow/sso" - @user.reload - @user.user_avatar.reload - @user.user_profile.reload - EmailToken.update_all(confirmed: true) - end + @user = Fabricate(:user, password: "myfrogs123ADMIN", active: true, admin: true) + group = Fabricate(:group) + group.add(@user) - it "successfully logs in and redirects user to return_sso_url when the user is not logged in" do - get "/session/sso_provider", params: Rack::Utils.parse_query(@sso.payload) - expect(response).to redirect_to("/login") + @user.create_user_avatar! + UserAvatar.import_url_for_user(logo_fixture, @user) + UserProfile.import_url_for_user(logo_fixture, @user, is_card_background: false) + UserProfile.import_url_for_user(logo_fixture, @user, is_card_background: true) - post "/session.json", - params: { login: @user.username, password: "myfrogs123ADMIN" }, - xhr: true + @user.reload + @user.user_avatar.reload + @user.user_profile.reload + EmailToken.update_all(confirmed: true) + end - location = response.cookies["sso_destination_url"] - # javascript code will handle redirection of user to return_sso_url - expect(location).to match(/^http:\/\/somewhere.over.rainbow\/sso/) + it "successfully logs in and redirects user to return_sso_url when the user is not logged in" do + get "/session/sso_provider", params: Rack::Utils.parse_query(@sso.payload("secretForOverRainbow")) - payload = location.split("?")[1] - sso2 = SingleSignOn.parse(payload, "topsecret") + expect(response).to redirect_to("/login") - expect(sso2.email).to eq(@user.email) - expect(sso2.name).to eq(@user.name) - expect(sso2.username).to eq(@user.username) - expect(sso2.external_id).to eq(@user.id.to_s) - expect(sso2.admin).to eq(true) - expect(sso2.moderator).to eq(false) - expect(sso2.groups).to eq(@user.groups.pluck(:name).join(",")) + post "/session.json", + params: { login: @user.username, password: "myfrogs123ADMIN" }, xhr: true, headers: headers - expect(sso2.avatar_url.blank?).to_not eq(true) - expect(sso2.profile_background_url.blank?).to_not eq(true) - expect(sso2.card_background_url.blank?).to_not eq(true) + location = response.cookies["sso_destination_url"] + # javascript code will handle redirection of user to return_sso_url + expect(location).to match(/^http:\/\/somewhere.over.rainbow\/sso/) - expect(sso2.avatar_url).to start_with(Discourse.base_url) - expect(sso2.profile_background_url).to start_with(Discourse.base_url) - expect(sso2.card_background_url).to start_with(Discourse.base_url) - end + payload = location.split("?")[1] + sso2 = SingleSignOn.parse(payload) - it "successfully redirects user to return_sso_url when the user is logged in" do - sign_in(@user) + expect(sso2.email).to eq(@user.email) + expect(sso2.name).to eq(@user.name) + expect(sso2.username).to eq(@user.username) + expect(sso2.external_id).to eq(@user.id.to_s) + expect(sso2.admin).to eq(true) + expect(sso2.moderator).to eq(false) + expect(sso2.groups).to eq(@user.groups.pluck(:name).join(",")) - get "/session/sso_provider", params: Rack::Utils.parse_query(@sso.payload) + expect(sso2.avatar_url.blank?).to_not eq(true) + expect(sso2.profile_background_url.blank?).to_not eq(true) + expect(sso2.card_background_url.blank?).to_not eq(true) - location = response.header["Location"] - expect(location).to match(/^http:\/\/somewhere.over.rainbow\/sso/) + expect(sso2.avatar_url).to start_with(Discourse.base_url) + expect(sso2.profile_background_url).to start_with(Discourse.base_url) + expect(sso2.card_background_url).to start_with(Discourse.base_url) + end - payload = location.split("?")[1] - sso2 = SingleSignOn.parse(payload, "topsecret") + it "it fails to log in if secret is wrong" do + get "/session/sso_provider", params: Rack::Utils.parse_query(@sso.payload("secretForRandomSite")) - expect(sso2.email).to eq(@user.email) - expect(sso2.name).to eq(@user.name) - expect(sso2.username).to eq(@user.username) - expect(sso2.external_id).to eq(@user.id.to_s) - expect(sso2.admin).to eq(true) - expect(sso2.moderator).to eq(false) + expect(response.status).to eq(500) + end - expect(sso2.avatar_url.blank?).to_not eq(true) - expect(sso2.profile_background_url.blank?).to_not eq(true) - expect(sso2.card_background_url.blank?).to_not eq(true) + it "successfully redirects user to return_sso_url when the user is logged in" do + sign_in(@user) - expect(sso2.avatar_url).to start_with("#{Discourse.store.absolute_base_url}/original") - expect(sso2.profile_background_url).to start_with(Discourse.base_url) - expect(sso2.card_background_url).to start_with(Discourse.base_url) + get "/session/sso_provider", params: Rack::Utils.parse_query(@sso.payload("secretForOverRainbow")) + + location = response.header["Location"] + expect(location).to match(/^http:\/\/somewhere.over.rainbow\/sso/) + + payload = location.split("?")[1] + sso2 = SingleSignOn.parse(payload) + + expect(sso2.email).to eq(@user.email) + expect(sso2.name).to eq(@user.name) + expect(sso2.username).to eq(@user.username) + expect(sso2.external_id).to eq(@user.id.to_s) + expect(sso2.admin).to eq(true) + expect(sso2.moderator).to eq(false) + expect(sso2.groups).to eq(@user.groups.pluck(:name).join(",")) + + expect(sso2.avatar_url.blank?).to_not eq(true) + expect(sso2.profile_background_url.blank?).to_not eq(true) + expect(sso2.card_background_url.blank?).to_not eq(true) + + expect(sso2.avatar_url).to start_with(Discourse.base_url) + expect(sso2.profile_background_url).to start_with(Discourse.base_url) + expect(sso2.card_background_url).to start_with(Discourse.base_url) + end + + it 'handles non local content correctly' do + SiteSetting.avatar_sizes = "100|49" + SiteSetting.enable_s3_uploads = true + SiteSetting.s3_access_key_id = "XXX" + SiteSetting.s3_secret_access_key = "XXX" + SiteSetting.s3_upload_bucket = "test" + SiteSetting.s3_cdn_url = "http://cdn.com" + + stub_request(:any, /test.s3.dualstack.us-east-1.amazonaws.com/).to_return(status: 200, body: "", headers: { referer: "fgdfds" }) + + @user.create_user_avatar! + upload = Fabricate(:upload, url: "//test.s3.dualstack.us-east-1.amazonaws.com/something") + + Fabricate(:optimized_image, + sha1: SecureRandom.hex << "A" * 8, + upload: upload, + width: 98, + height: 98, + url: "//test.s3.amazonaws.com/something/else" + ) + + @user.update_columns(uploaded_avatar_id: upload.id) + @user.user_profile.update_columns( + profile_background: "//test.s3.dualstack.us-east-1.amazonaws.com/something", + card_background: "//test.s3.dualstack.us-east-1.amazonaws.com/something" + ) + + @user.reload + @user.user_avatar.reload + @user.user_profile.reload + + sign_in(@user) + + stub_request(:get, "http://cdn.com/something/else").to_return( + body: lambda { |request| File.new(Rails.root + 'spec/fixtures/images/logo.png') } + ) + + get "/session/sso_provider", params: Rack::Utils.parse_query(@sso.payload("secretForOverRainbow")) + + location = response.header["Location"] + # javascript code will handle redirection of user to return_sso_url + expect(location).to match(/^http:\/\/somewhere.over.rainbow\/sso/) + + payload = location.split("?")[1] + sso2 = SingleSignOn.parse(payload) + + expect(sso2.avatar_url.blank?).to_not eq(true) + expect(sso2.profile_background_url.blank?).to_not eq(true) + expect(sso2.card_background_url.blank?).to_not eq(true) + + expect(sso2.avatar_url).to start_with("#{SiteSetting.s3_cdn_url}/original") + expect(sso2.profile_background_url).to start_with(SiteSetting.s3_cdn_url) + expect(sso2.card_background_url).to start_with(SiteSetting.s3_cdn_url) + end end end diff --git a/spec/services/wildcard_domain_checker_spec.rb b/spec/services/wildcard_domain_checker_spec.rb new file mode 100644 index 00000000000..806ca99246c --- /dev/null +++ b/spec/services/wildcard_domain_checker_spec.rb @@ -0,0 +1,35 @@ +require 'rails_helper' + +describe WildcardDomainChecker do + + describe 'check_domain' do + context 'valid domain' do + it 'returns correct domain' do + result1 = WildcardDomainChecker.check_domain('*.discourse.org', 'anything.is.possible.discourse.org') + expect(result1[0]).to eq('anything.is.possible.discourse.org') + + result2 = WildcardDomainChecker.check_domain('www.discourse.org', 'www.discourse.org') + expect(result2[0]).to eq('www.discourse.org') + + result3 = WildcardDomainChecker.check_domain('*', 'hello.discourse.org') + expect(result3[0]).to eq('hello.discourse.org') + end + end + + context 'invalid domain' do + it "doesn't return the domain" do + result1 = WildcardDomainChecker.check_domain('*.discourse.org', 'bad-domain.discourse.org.evil.com') + expect(result1).to eq(nil) + + result2 = WildcardDomainChecker.check_domain('www.discourse.org', 'www.discourse.org.evil.com') + expect(result2).to eq(nil) + + result3 = WildcardDomainChecker.check_domain('www.discourse.org', 'www.www.discourse.org') + expect(result3).to eq(nil) + + result4 = WildcardDomainChecker.check_domain('www.*.discourse.org', 'www.www.discourse.org') + expect(result4).to eq(nil) + end + end + end +end diff --git a/test/javascripts/components/secret-value-list-test.js.es6 b/test/javascripts/components/secret-value-list-test.js.es6 new file mode 100644 index 00000000000..1f602cbdbde --- /dev/null +++ b/test/javascripts/components/secret-value-list-test.js.es6 @@ -0,0 +1,63 @@ +import componentTest from "helpers/component-test"; +moduleForComponent("secret-value-list", { integration: true }); + +componentTest("adding a value", { + template: "{{secret-value-list values=values}}", + + async test(assert) { + this.set("values", "firstKey|FirstValue\nsecondKey|secondValue"); + + await fillIn(".new-value-input.key", "thirdKey"); + await click(".add-value-btn"); + + assert.ok( + find(".values .value").length === 2, + "it doesn't add the value to the list if secret is missing" + ); + + await fillIn(".new-value-input.key", ""); + await fillIn(".new-value-input.secret", "thirdValue"); + await click(".add-value-btn"); + + assert.ok( + find(".values .value").length === 2, + "it doesn't add the value to the list if key is missing" + ); + + await fillIn(".new-value-input.key", "thirdKey"); + await fillIn(".new-value-input.secret", "thirdValue"); + await click(".add-value-btn"); + + assert.ok( + find(".values .value").length === 3, + "it adds the value to the list of values" + ); + + assert.deepEqual( + this.get("values"), + "firstKey|FirstValue\nsecondKey|secondValue\nthirdKey|thirdValue", + "it adds the value to the list of values" + ); + } +}); + +componentTest("removing a value", { + template: "{{secret-value-list values=values}}", + + async test(assert) { + this.set("values", "firstKey|FirstValue\nsecondKey|secondValue"); + + await click(".values .value[data-index='0'] .remove-value-btn"); + + assert.ok( + find(".values .value").length === 1, + "it removes the value from the list of values" + ); + + assert.equal( + this.get("values"), + "secondKey|secondValue", + "it removes the expected value" + ); + } +});