mirror of
https://github.com/codeninjasllc/discourse.git
synced 2024-11-27 09:36:19 -05:00
add option to override user attributes from SSO payload
add an external_username attribute for username from SSO payload repair the field name in SingleSignOnRecord migration move setting of external_username for sso to controller add settings toggle to override username/email from SSO payload fix changing of external username after override toggle complete tests and logic for sso override add some extra context to username override option add external_email and external_name to single sign on record add setting for name override from SSO payload complete override with stored external_email and external_name add missing checks to tests remove an unneeded describe block break up a monster method for single sign on fixes for sso attribute override after failed tests
This commit is contained in:
parent
5e6764e9c9
commit
c3eb2025d8
7 changed files with 166 additions and 28 deletions
|
@ -39,31 +39,19 @@ class DiscourseSingleSignOn < SingleSignOn
|
|||
"SSO_NONCE_#{nonce}"
|
||||
end
|
||||
|
||||
|
||||
def lookup_or_create_user
|
||||
sso_record = SingleSignOnRecord.where(external_id: external_id).first
|
||||
if sso_record && sso_record.user
|
||||
|
||||
if sso_record && user = sso_record.user
|
||||
sso_record.last_payload = unsigned_payload
|
||||
sso_record.save
|
||||
else
|
||||
user = User.where(email: Email.downcase(email)).first
|
||||
|
||||
user_params = {
|
||||
email: email,
|
||||
name: User.suggest_name(name || username || email),
|
||||
username: UserNameSuggester.suggest(username || name || email),
|
||||
}
|
||||
|
||||
if user || user = User.create(user_params)
|
||||
if sso_record = user.single_sign_on_record
|
||||
sso_record.last_payload = unsigned_payload
|
||||
sso_record.external_id = external_id
|
||||
sso_record.save!
|
||||
else
|
||||
sso_record = user.create_single_sign_on_record(last_payload: unsigned_payload,
|
||||
external_id: external_id)
|
||||
end
|
||||
end
|
||||
user = match_email_or_create_user
|
||||
sso_record = user.single_sign_on_record
|
||||
end
|
||||
|
||||
# if the user isn't new or it's attached to the SSO record we might be overriding username or email
|
||||
unless user.new_record?
|
||||
change_external_attributes_and_override(sso_record, user)
|
||||
end
|
||||
|
||||
if sso_record && (user = sso_record.user) && !user.active
|
||||
|
@ -71,8 +59,62 @@ class DiscourseSingleSignOn < SingleSignOn
|
|||
user.save
|
||||
user.enqueue_welcome_message('welcome_user')
|
||||
end
|
||||
|
||||
# optionally save the user and sso_record if they have changed
|
||||
user.save!
|
||||
sso_record.save!
|
||||
|
||||
sso_record && sso_record.user
|
||||
end
|
||||
end
|
||||
|
||||
private
|
||||
|
||||
def match_email_or_create_user
|
||||
user = User.where(email: Email.downcase(email)).first
|
||||
|
||||
user_params = {
|
||||
email: email,
|
||||
name: User.suggest_name(name || username || email),
|
||||
username: UserNameSuggester.suggest(username || name || email),
|
||||
}
|
||||
|
||||
if user || user = User.create(user_params)
|
||||
if sso_record = user.single_sign_on_record
|
||||
sso_record.last_payload = unsigned_payload
|
||||
sso_record.external_id = external_id
|
||||
else
|
||||
sso_record = user.create_single_sign_on_record(last_payload: unsigned_payload,
|
||||
external_id: external_id,
|
||||
external_username: username,
|
||||
external_email: email,
|
||||
external_name: name)
|
||||
end
|
||||
end
|
||||
|
||||
user
|
||||
end
|
||||
|
||||
def change_external_attributes_and_override(sso_record, user)
|
||||
if SiteSetting.sso_overrides_email && email != sso_record.external_email
|
||||
# set the user's email to whatever came in the payload
|
||||
user.email = email
|
||||
end
|
||||
|
||||
if SiteSetting.sso_overrides_username && username != sso_record.external_username && user.username != username
|
||||
# we have an external username change, and the user's current username doesn't match
|
||||
# run it through the UserNameSuggester to override it
|
||||
user.username = UserNameSuggester.suggest(username || name || email)
|
||||
end
|
||||
|
||||
if SiteSetting.sso_overrides_name && name != sso_record.external_name && user.name != name
|
||||
# we have an external name change, and the user's current name doesn't match
|
||||
# run it through the name suggester to override it
|
||||
user.name = User.suggest_name(name || username || email)
|
||||
end
|
||||
|
||||
# change external attributes for sso record
|
||||
sso_record.external_username = username
|
||||
sso_record.external_email = email
|
||||
sso_record.external_name = name
|
||||
end
|
||||
end
|
|
@ -676,6 +676,9 @@ en:
|
|||
enable_sso: "Enable single sign on via an external site"
|
||||
sso_url: "URL of single sign on endpoint"
|
||||
sso_secret: "Secret string used to encrypt/decrypt SSO information, be sure it is 10 chars or longer"
|
||||
sso_overrides_email: "Overrides local email with external site email from SSO payload (WARNING: discrepancies can occur due to normalization of local emails)"
|
||||
sso_overrides_username: "Overrides local username with external site username from SSO payload (WARNING: discrepancies can occur due to differences in username length/requirements)"
|
||||
sso_overrides_name: "Overrides local name with external site name from SSO payload (WARNING: discrepancies can occur due to normalization of local names)"
|
||||
|
||||
enable_local_logins: "Enable traditional, local username and password authentication"
|
||||
enable_local_account_create: "Enable creating new local accounts"
|
||||
|
|
|
@ -78,6 +78,15 @@ users:
|
|||
default: ''
|
||||
sso_secret:
|
||||
defalt: ''
|
||||
sso_overrides_email:
|
||||
client: true
|
||||
default: false
|
||||
sso_overrides_username:
|
||||
client: true
|
||||
default: false
|
||||
sso_overrides_name:
|
||||
client: true
|
||||
default: false
|
||||
enable_local_logins:
|
||||
client: true
|
||||
default: true
|
||||
|
|
|
@ -0,0 +1,5 @@
|
|||
class AddExternalUsernameToSingleSignOnRecord < ActiveRecord::Migration
|
||||
def change
|
||||
add_column :single_sign_on_records, :external_username, :string
|
||||
end
|
||||
end
|
|
@ -0,0 +1,6 @@
|
|||
class AddExternalEmailAndExternalNameToSingleSignOnRecord < ActiveRecord::Migration
|
||||
def change
|
||||
add_column :single_sign_on_records, :external_email, :string
|
||||
add_column :single_sign_on_records, :external_name, :string
|
||||
end
|
||||
end
|
|
@ -1,5 +1,5 @@
|
|||
class SingleSignOn
|
||||
ACCESSORS = [:nonce, :name, :username, :email, :about_me, :external_id]
|
||||
ACCESSORS = [:nonce, :name, :username, :email, :about_me, :external_email, :external_username, :external_name, :external_id]
|
||||
FIXNUMS = []
|
||||
NONCE_EXPIRY_TIME = 10.minutes
|
||||
|
||||
|
|
|
@ -35,15 +35,17 @@ describe SessionController do
|
|||
sso = get_sso("/")
|
||||
user = Fabricate(:user)
|
||||
sso.email = user.email
|
||||
sso.external_id = "abc"
|
||||
sso.external_id = 'abc'
|
||||
sso.username = 'sam'
|
||||
|
||||
get :sso_login, Rack::Utils.parse_query(sso.payload)
|
||||
|
||||
response.should redirect_to('/')
|
||||
logged_on_user = Discourse.current_user_provider.new(request.env).current_user
|
||||
logged_on_user.email.should == user.email
|
||||
|
||||
|
||||
logged_on_user.single_sign_on_record.external_id.should == "abc"
|
||||
logged_on_user.single_sign_on_record.external_username.should == 'sam'
|
||||
end
|
||||
|
||||
it 'allows you to create an account' do
|
||||
|
@ -63,11 +65,11 @@ describe SessionController do
|
|||
logged_on_user.username.should == 'sam'
|
||||
|
||||
logged_on_user.single_sign_on_record.external_id.should == "666"
|
||||
logged_on_user.single_sign_on_record.external_username.should == 'sam'
|
||||
logged_on_user.active.should == true
|
||||
end
|
||||
|
||||
|
||||
it 'allows login to existing account with valid nonce' do
|
||||
|
||||
sso = get_sso('/hello/world')
|
||||
sso.external_id = '997'
|
||||
|
||||
|
@ -87,8 +89,79 @@ describe SessionController do
|
|||
# nonce is bad now
|
||||
get :sso_login, Rack::Utils.parse_query(sso.payload)
|
||||
response.code.should == '500'
|
||||
|
||||
end
|
||||
|
||||
describe 'local attribute ovveride from SSO payload' do
|
||||
before do
|
||||
SiteSetting.stubs("sso_overrides_email").returns(true)
|
||||
SiteSetting.stubs("sso_overrides_username").returns(true)
|
||||
SiteSetting.stubs("sso_overrides_name").returns(true)
|
||||
|
||||
@user = Fabricate(:user)
|
||||
|
||||
@sso = get_sso('/hello/world')
|
||||
@sso.external_id = '997'
|
||||
|
||||
@reversed_username = @user.username.reverse
|
||||
@sso.username = @reversed_username
|
||||
|
||||
@sso.email = "#{@reversed_username}@garbage.org"
|
||||
@reversed_name = @user.name.reverse
|
||||
|
||||
@sso.name = @reversed_name
|
||||
|
||||
@suggested_username = UserNameSuggester.suggest(@sso.username || @sso.name || @sso.email)
|
||||
@suggested_name = User.suggest_name(@sso.name || @sso.username || @sso.email)
|
||||
|
||||
@user.create_single_sign_on_record(external_id: '997', last_payload: '')
|
||||
end
|
||||
|
||||
it 'stores the external attributes' do
|
||||
get :sso_login, Rack::Utils.parse_query(@sso.payload)
|
||||
|
||||
@user.single_sign_on_record.reload
|
||||
@user.single_sign_on_record.external_username.should == @sso.username
|
||||
@user.single_sign_on_record.external_email.should == @sso.email
|
||||
@user.single_sign_on_record.external_name.should == @sso.name
|
||||
end
|
||||
|
||||
it 'overrides attributes' do
|
||||
get :sso_login, Rack::Utils.parse_query(@sso.payload)
|
||||
|
||||
logged_on_user = Discourse.current_user_provider.new(request.env).current_user
|
||||
|
||||
logged_on_user.username.should == @suggested_username
|
||||
logged_on_user.email.should == "#{@reversed_username}@garbage.org"
|
||||
logged_on_user.name.should == @suggested_name
|
||||
end
|
||||
|
||||
it 'does not change matching attributes for an existing account' do
|
||||
@sso.username = @user.username
|
||||
@sso.name = @user.name
|
||||
@sso.email = @user.email
|
||||
|
||||
get :sso_login, Rack::Utils.parse_query(@sso.payload)
|
||||
|
||||
logged_on_user = Discourse.current_user_provider.new(request.env).current_user
|
||||
logged_on_user.username.should == @user.username
|
||||
logged_on_user.name.should == @user.name
|
||||
logged_on_user.email.should == @user.email
|
||||
end
|
||||
|
||||
it 'does not change attributes for unchanged external attributes' do
|
||||
@user.single_sign_on_record.external_username = @sso.username
|
||||
@user.single_sign_on_record.external_email = @sso.email
|
||||
@user.single_sign_on_record.external_name = @sso.name
|
||||
@user.single_sign_on_record.save
|
||||
|
||||
get :sso_login, Rack::Utils.parse_query(@sso.payload)
|
||||
|
||||
logged_on_user = Discourse.current_user_provider.new(request.env).current_user
|
||||
logged_on_user.username.should == @user.username
|
||||
logged_on_user.email.should == @user.email
|
||||
logged_on_user.name.should == @user.name
|
||||
end
|
||||
end
|
||||
end
|
||||
|
||||
describe '.create' do
|
||||
|
|
Loading…
Reference in a new issue