FEATURE: add profile_background fields into SSO (#5701)

Add profile_background and card_background fields into Discourse SSO.
This commit is contained in:
Misaka 0x4e21 2018-05-07 08:03:26 +00:00 committed by Régis Hanol
parent 5a57a454fe
commit ff6be3c2e3
15 changed files with 474 additions and 4 deletions

View File

@ -1,5 +1,6 @@
require_dependency 'rate_limiter'
require_dependency 'single_sign_on'
require_dependency 'url_helper'
class SessionController < ApplicationController
class LocalLoginNotAllowed < StandardError; end
@ -54,6 +55,26 @@ class SessionController < ApplicationController
sso.moderator = current_user.moderator?
sso.groups = current_user.groups.pluck(:name).join(",")
sso.avatar_url = Discourse.store.cdn_url UrlHelper.absolute(
"#{Discourse.store.absolute_base_url}/#{Discourse.store.get_path_for_upload(current_user.uploaded_avatar)}"
) unless current_user.uploaded_avatar.nil?
sso.profile_background_url = UrlHelper.absolute upload_cdn_path(
current_user.user_profile.profile_background
) if current_user.user_profile.profile_background.present?
sso.card_background_url = UrlHelper.absolute upload_cdn_path(
current_user.user_profile.card_background
) if current_user.user_profile.card_background.present?
sso.avatar_url = Discourse.store.cdn_url UrlHelper.absolute(
"#{Discourse.store.absolute_base_url}/#{Discourse.store.get_path_for_upload(current_user.uploaded_avatar)}"
) unless current_user.uploaded_avatar.nil?
sso.profile_background_url = UrlHelper.absolute upload_cdn_path(
current_user.user_profile.profile_background
) if current_user.user_profile.profile_background.present?
sso.card_background_url = UrlHelper.absolute upload_cdn_path(
current_user.user_profile.card_background
) if current_user.user_profile.card_background.present?
if sso.return_sso_url.blank?
render plain: "return_sso_url is blank, it must be provided", status: 400
return

View File

@ -0,0 +1,28 @@
module Jobs
class DownloadProfileBackgroundFromUrl < Jobs::Base
sidekiq_options retry: false
def execute(args)
url = args[:url]
user_id = args[:user_id]
raise Discourse::InvalidParameters.new(:url) if url.blank?
raise Discourse::InvalidParameters.new(:user_id) if user_id.blank?
return unless user = User.find_by(id: user_id)
begin
UserProfile.import_url_for_user(
url,
user,
is_card_background: args[:is_card_background],
)
rescue Discourse::InvalidParameters => e
raise e unless e.message == 'url'
end
end
end
end

View File

@ -190,13 +190,31 @@ class DiscourseSingleSignOn < SingleSignOn
)
end
if profile_background_url.present?
Jobs.enqueue(:download_profile_background_from_url,
url: profile_background_url,
user_id: user.id,
is_card_background: false
)
end
if card_background_url.present?
Jobs.enqueue(:download_profile_background_from_url,
url: card_background_url,
user_id: user.id,
is_card_background: true
)
end
user.create_single_sign_on_record!(
last_payload: unsigned_payload,
external_id: external_id,
external_username: username,
external_email: email,
external_name: name,
external_avatar_url: avatar_url
external_avatar_url: avatar_url,
external_profile_background_url: profile_background_url,
external_card_background_url: card_background_url
)
end
end
@ -233,10 +251,36 @@ class DiscourseSingleSignOn < SingleSignOn
end
end
profile_background_missing = user.user_profile.profile_background.blank? || Upload.get_from_url(user.user_profile.profile_background).blank?
if (profile_background_missing || SiteSetting.sso_overrides_profile_background) && profile_background_url.present?
profile_background_changed = sso_record.external_profile_background_url != profile_background_url
if profile_background_changed || profile_background_missing
Jobs.enqueue(:download_profile_background_from_url,
url: profile_background_url,
user_id: user.id,
is_card_background: false
)
end
end
card_background_missing = user.user_profile.card_background.blank? || Upload.get_from_url(user.user_profile.card_background).blank?
if (card_background_missing || SiteSetting.sso_overrides_profile_background) && card_background_url.present?
card_background_changed = sso_record.external_card_background_url != card_background_url
if card_background_changed || card_background_missing
Jobs.enqueue(:download_profile_background_from_url,
url: card_background_url,
user_id: user.id,
is_card_background: true
)
end
end
# change external attributes for sso record
sso_record.external_username = username
sso_record.external_email = email
sso_record.external_name = name
sso_record.external_avatar_url = avatar_url
sso_record.external_profile_background_url = profile_background_url
sso_record.external_card_background_url = card_background_url
end
end

View File

@ -18,6 +18,8 @@ end
# external_email :string
# external_name :string
# external_avatar_url :string(1000)
# external_profile_background_url :string(1000)
# external_card_background_url :string(1000)
#
# Indexes
#

View File

@ -1,3 +1,4 @@
require_dependency 'upload_creator'
class UserProfile < ActiveRecord::Base
# TODO: remove this after Nov 1, 2018
@ -78,6 +79,36 @@ class UserProfile < ActiveRecord::Base
update_columns(bio_cooked: cooked, bio_cooked_version: BAKED_VERSION)
end
def self.import_url_for_user(background_url, user, options = nil)
tempfile = FileHelper.download(
background_url,
max_file_size: SiteSetting.max_image_size_kb.kilobytes,
tmp_file_name: "sso-profile-background",
follow_redirect: true
)
return unless tempfile
ext = FastImage.type(tempfile).to_s
tempfile.rewind
is_card_background = !options || options[:is_card_background]
type = is_card_background ? "card_background" : "profile_background"
upload = UploadCreator.new(tempfile, "external-profile-background." + ext, origin: background_url, type: type).create_for(user.id)
if (is_card_background)
user.user_profile.upload_card_background(upload)
else
user.user_profile.upload_profile_background(upload)
end
rescue Net::ReadTimeout, OpenURI::HTTPError
# skip saving, we are not connected to the net
ensure
tempfile.close! if tempfile && tempfile.respond_to?(:close!)
end
protected
def trigger_badges

View File

@ -3,5 +3,7 @@ class SingleSignOnRecordSerializer < ApplicationSerializer
:last_payload, :created_at,
:updated_at, :external_username,
:external_email, :external_name,
:external_avatar_url
:external_avatar_url,
:external_profile_background_url,
:external_card_background_url
end

View File

@ -1231,6 +1231,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, disabling allow_uploaded_avatars is highly recommended"
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"
sso_allows_all_return_paths: "Do not restrict the domain for return_paths provided by SSO (by default return path must be on current site)"

View File

@ -961,6 +961,8 @@ zh_CN:
sso_overrides_username: "每一次登录时,用 SSO 信息中的外部站点的用户名覆盖本地用户名,并且阻止本地的用户名修改。(警告:因格本地用户名的长度和其他要求,用户名可能会有所差异)"
sso_overrides_name: "每一次登录时,用 SSO 信息中的外部站点的全名覆盖本地全名,并且阻止本地的全名修改。"
sso_overrides_avatar: "用单点登录信息中的外部站点头像覆盖用户头像。如果启用,强烈建议禁用 allow_uploaded_avatars"
sso_overrides_profile_background: "用单点登录信息中的外部站点个人档背景图片覆盖用户头像。"
sso_overrides_card_background: "用单点登录信息中的外部站点用户卡背景图片覆盖用户头像。"
sso_not_approved_url: "重定向未受许可的单点登录账号至这个 URL"
sso_allows_all_return_paths: "不限制 SSO 提供的 return_paths 中的域名(默认情况下返回地址必须位于当前站点)"
allow_new_registrations: "允许新用户注册。取消选择将阻止任何人创建一个新账户。"

View File

@ -348,6 +348,8 @@ login:
sso_overrides_avatar:
default: false
client: true
sso_overrides_profile_background: false
sso_overrides_card_background: false
sso_not_approved_url: ''
email_domains_blacklist:
default: 'mailinator.com'

View File

@ -0,0 +1,6 @@
class AddExternalProfileAndCardBackgroundUrlToSingleSignOnRecord < ActiveRecord::Migration[5.1]
def change
add_column :single_sign_on_records, :external_profile_background_url, :string
add_column :single_sign_on_records, :external_card_background_url, :string
end
end

View File

@ -1,7 +1,7 @@
class SingleSignOn
ACCESSORS = [:nonce, :name, :username, :email, :avatar_url, :avatar_force_update, :require_activation,
:bio, :external_id, :return_sso_url, :admin, :moderator, :suppress_welcome_message, :title,
:add_groups, :remove_groups, :groups]
:add_groups, :remove_groups, :groups, :profile_background_url, :card_background_url]
FIXNUMS = []
BOOLS = [:avatar_force_update, :admin, :moderator, :require_activation, :suppress_welcome_message]
NONCE_EXPIRY_TIME = 10.minutes

View File

@ -28,8 +28,9 @@ describe SessionController do
end
end
describe '#sso_login' do
let(:logo_fixture) { "http://#{Discourse.current_hostname}/uploads/logo.png" }
describe '#sso_login' do
before do
@sso_url = "http://somesite.com/discourse_sso"
@sso_secret = "shjkfdhsfkjh"
@ -294,6 +295,11 @@ describe SessionController do
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
@ -307,7 +313,15 @@ describe SessionController do
@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
@ -334,6 +348,14 @@ describe SessionController do
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
@ -353,6 +375,70 @@ describe SessionController do
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.amazonaws.com/).to_return(status: 200, body: "", headers: {})
@user.create_user_avatar!
upload = Fabricate(:upload, url: "//test.s3.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.amazonaws.com/something",
card_background: "//test.s3.amazonaws.com/something"
)
@user.reload
@user.user_avatar.reload
@user.user_profile.reload
log_in_user(@user)
stub_request(:get, "http://cdn.com/something/else").to_return(
body: lambda { |request| File.new(Rails.root + 'spec/fixtures/images/logo.png') }
)
get :sso_provider, params: Rack::Utils.parse_query(@sso.payload)
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)
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
@ -414,6 +500,11 @@ describe SessionController do
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") }
)
SiteSetting.enable_sso_provider = true
SiteSetting.enable_sso = false
SiteSetting.enable_local_logins = true
@ -425,6 +516,14 @@ describe SessionController do
@sso.return_sso_url = "http://somewhere.over.rainbow/sso"
@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)
@user.reload
@user.user_avatar.reload
@user.user_profile.reload
EmailToken.update_all(confirmed: true)
end
@ -450,6 +549,15 @@ describe SessionController do
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
@ -469,6 +577,14 @@ describe SessionController do
expect(sso2.external_id).to eq(@user.id.to_s)
expect(sso2.admin).to eq(true)
expect(sso2.moderator).to eq(false)
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
end

View File

@ -0,0 +1,16 @@
require 'rails_helper'
RSpec.describe Jobs::DownloadProfileBackgroundFromUrl do
let(:user) { Fabricate(:user) }
describe 'when url is invalid' do
it 'should not raise any error' do
expect do
described_class.new.execute(
url: '/assets/something/nice.jpg',
user_id: user.id
)
end.to_not raise_error
end
end
end

View File

@ -584,4 +584,167 @@ describe DiscourseSingleSignOn do
end
end
end
context 'when sso_overrides_profile_background is not enabled' do
it "correctly handles provided profile_background_urls" do
sso = DiscourseSingleSignOn.new
sso.external_id = 666
sso.email = "sam@sam.com"
sso.name = "sam"
sso.username = "sam"
sso.profile_background_url = "http://awesome.com/image.png"
sso.suppress_welcome_message = true
FileHelper.stubs(:download).returns(file_from_fixtures("logo.png"))
user = sso.lookup_or_create_user(ip_address)
user.reload
user.user_profile.reload
profile_background = user.user_profile.profile_background
# initial creation ...
expect(profile_background).to_not eq(nil)
expect(profile_background).to_not eq('')
FileHelper.stubs(:download) { raise "should not be called" }
sso.profile_background_url = "https://some.new/avatar.png"
user = sso.lookup_or_create_user(ip_address)
user.reload
user.user_profile.reload
# profile_background updated but no override specified ...
expect(user.user_profile.profile_background).to eq(profile_background)
end
end
context 'when sso_overrides_profile_background is enabled' do
let!(:sso_record) { Fabricate(:single_sign_on_record, external_profile_background_url: "http://example.com/an_image.png") }
let!(:sso) {
sso = DiscourseSingleSignOn.new
sso.username = "test"
sso.name = "test"
sso.email = sso_record.user.email
sso.external_id = sso_record.external_id
sso
}
let(:logo) { file_from_fixtures("logo.png") }
before do
SiteSetting.sso_overrides_profile_background = true
end
it "deal with no profile_background_url passed for an existing user with a profile_background" do
Sidekiq::Testing.inline! do
# Deliberately not setting profile_background_url so it should not update
sso_record.user.user_profile.update_columns(profile_background: '')
user = sso.lookup_or_create_user(ip_address)
user.reload
user.user_profile.reload
expect(user).to_not be_nil
expect(user.user_profile.profile_background).to eq('')
end
end
it "deal with a profile_background_url passed for an existing user with a profile_background" do
Sidekiq::Testing.inline! do
FileHelper.stubs(:download).returns(logo)
sso_record.user.user_profile.update_columns(profile_background: '')
sso.profile_background_url = "http://example.com/a_different_image.png"
user = sso.lookup_or_create_user(ip_address)
user.reload
user.user_profile.reload
expect(user).to_not be_nil
expect(user.user_profile.profile_background).to_not eq('')
end
end
end
context 'when sso_overrides_card_background is not enabled' do
it "correctly handles provided card_background_urls" do
sso = DiscourseSingleSignOn.new
sso.external_id = 666
sso.email = "sam@sam.com"
sso.name = "sam"
sso.username = "sam"
sso.card_background_url = "http://awesome.com/image.png"
sso.suppress_welcome_message = true
FileHelper.stubs(:download).returns(file_from_fixtures("logo.png"))
user = sso.lookup_or_create_user(ip_address)
user.reload
user.user_profile.reload
card_background = user.user_profile.card_background
# initial creation ...
expect(card_background).to_not eq(nil)
expect(card_background).to_not eq('')
FileHelper.stubs(:download) { raise "should not be called" }
sso.card_background_url = "https://some.new/avatar.png"
user = sso.lookup_or_create_user(ip_address)
user.reload
user.user_profile.reload
# card_background updated but no override specified ...
expect(user.user_profile.card_background).to eq(card_background)
end
end
context 'when sso_overrides_card_background is enabled' do
let!(:sso_record) { Fabricate(:single_sign_on_record, external_card_background_url: "http://example.com/an_image.png") }
let!(:sso) {
sso = DiscourseSingleSignOn.new
sso.username = "test"
sso.name = "test"
sso.email = sso_record.user.email
sso.external_id = sso_record.external_id
sso
}
let(:logo) { file_from_fixtures("logo.png") }
before do
SiteSetting.sso_overrides_card_background = true
end
it "deal with no card_background_url passed for an existing user with a card_background" do
Sidekiq::Testing.inline! do
# Deliberately not setting card_background_url so it should not update
sso_record.user.user_profile.update_columns(card_background: '')
user = sso.lookup_or_create_user(ip_address)
user.reload
user.user_profile.reload
expect(user).to_not be_nil
expect(user.user_profile.card_background).to eq('')
end
end
it "deal with a card_background_url passed for an existing user with a card_background_url" do
Sidekiq::Testing.inline! do
FileHelper.stubs(:download).returns(logo)
sso_record.user.user_profile.update_columns(card_background: '')
sso.card_background_url = "http://example.com/a_different_image.png"
user = sso.lookup_or_create_user(ip_address)
user.reload
user.user_profile.reload
expect(user).to_not be_nil
expect(user.user_profile.card_background).to_not eq('')
end
end
end
end

View File

@ -202,4 +202,39 @@ describe UserProfile do
end
end
end
context '.import_url_for_user' do
let(:user) { Fabricate(:user) }
before do
stub_request(:any, "thisfakesomething.something.com")
.to_return(body: "abc", status: 404, headers: { 'Content-Length' => 3 })
end
describe 'when profile_background_url returns an invalid status code' do
it 'should not do anything' do
url = "http://thisfakesomething.something.com/"
UserProfile.import_url_for_user(url, user, is_card_background: false)
user.reload
expect(user.user_profile.profile_background).to eq(nil)
end
end
describe 'when card_background_url returns an invalid status code' do
it 'should not do anything' do
url = "http://thisfakesomething.something.com/"
UserProfile.import_url_for_user(url, user, is_card_background: true)
user.reload
expect(user.user_profile.card_background).to eq(nil)
end
end
end
end