federated attributes support for Facebook
refs CNVS-28864 test plan: * configure federated attributes for Facebook (notably name, email, and locale) * ensure on provisioning and login that the attributes are set (particularly locale) Change-Id: Ib83921ec4c1998ce7078a93bcc030ebebeb2c747 Reviewed-on: https://gerrit.instructure.com/81359 Tested-by: Jenkins QA-Review: Jeremy Putnam <jeremyp@instructure.com> Reviewed-by: Ryan Shaw <ryan@instructure.com> Product-Review: Cody Cutrer <cody@instructure.com>
This commit is contained in:
parent
559bde438c
commit
8619b5441d
|
@ -330,6 +330,11 @@ class AccountAuthorizationConfigsController < ApplicationController
|
|||
# The attribute to use to look up the user's login in Canvas. Either
|
||||
# 'id' (the default), or 'email'
|
||||
#
|
||||
# - federated_attributes [Optional]
|
||||
#
|
||||
# See FederatedAttributesConfig. Valid provider attributes are 'email',
|
||||
# 'first_name', 'id', 'last_name', 'locale', and 'name'.
|
||||
#
|
||||
# For GitHub, the additional recognized parameters are:
|
||||
#
|
||||
# - domain [Optional]
|
||||
|
|
|
@ -32,12 +32,14 @@ class Login::Oauth2Controller < Login::OauthBaseController
|
|||
raise ActiveRecord::RecordNotFound unless @aac.is_a?(AccountAuthorizationConfig::Oauth2)
|
||||
|
||||
unique_id = nil
|
||||
provider_attributes = {}
|
||||
return unless timeout_protection do
|
||||
token = @aac.get_token(params[:code], oauth2_login_callback_url)
|
||||
unique_id = @aac.unique_id(token)
|
||||
provider_attributes = @aac.provider_attributes(token)
|
||||
end
|
||||
|
||||
find_pseudonym(unique_id)
|
||||
find_pseudonym(unique_id, provider_attributes)
|
||||
end
|
||||
|
||||
protected
|
||||
|
|
|
@ -61,7 +61,7 @@ class Login::OauthBaseController < ApplicationController
|
|||
false
|
||||
end
|
||||
|
||||
def find_pseudonym(unique_ids)
|
||||
def find_pseudonym(unique_ids, provider_attributes = {})
|
||||
if unique_ids.nil?
|
||||
unknown_user_url = @domain_root_account.unknown_user_url.presence || login_url
|
||||
logger.warn "Received OAuth2 login with no unique_id"
|
||||
|
@ -76,7 +76,11 @@ class Login::OauthBaseController < ApplicationController
|
|||
unique_ids.any? do |unique_id|
|
||||
pseudonym = @domain_root_account.pseudonyms.for_auth_configuration(unique_id, @aac)
|
||||
end
|
||||
pseudonym ||= @aac.provision_user(unique_ids.first) if !unique_ids.empty? && @aac.jit_provisioning?
|
||||
if pseudonym
|
||||
@aac.apply_federated_attributes(pseudonym, provider_attributes)
|
||||
elsif !unique_ids.empty? && @aac.jit_provisioning?
|
||||
pseudonym = @aac.provision_user(unique_ids.first, provider_attributes)
|
||||
end
|
||||
|
||||
if pseudonym
|
||||
# Successful login and we have a user
|
||||
|
|
|
@ -202,7 +202,7 @@ class AccountAuthorizationConfig < ActiveRecord::Base
|
|||
def provision_user(unique_id, provider_attributes = {})
|
||||
User.transaction(requires_new: true) do
|
||||
pseudonym = account.pseudonyms.build
|
||||
pseudonym.user = User.create!(name: unique_id, workflow_state: 'registered')
|
||||
pseudonym.user = User.create!(name: unique_id) { |u| u.workflow_state = 'registered' }
|
||||
pseudonym.authentication_provider = self
|
||||
pseudonym.unique_id = unique_id
|
||||
pseudonym.save!
|
||||
|
@ -242,6 +242,17 @@ class AccountAuthorizationConfig < ActiveRecord::Base
|
|||
cc ||= user.communication_channels.email.new(path: value)
|
||||
cc.workflow_state = 'active'
|
||||
cc.save! if cc.changed?
|
||||
when 'locale'
|
||||
# convert _ to -, be lenient about case, and perform fallbacks
|
||||
value = value.tr('_', '-')
|
||||
lowercase_locales = I18n.available_locales.map(&:to_s).map(&:downcase)
|
||||
while value.include?('-')
|
||||
break if lowercase_locales.include?(value.downcase)
|
||||
value = value.sub(/(?:x-)?-[^-]*$/, '')
|
||||
end
|
||||
if (i = lowercase_locales.index(value.downcase))
|
||||
user.locale = I18n.available_locales[i].to_s
|
||||
end
|
||||
else
|
||||
user.send("#{attribute}=", value)
|
||||
end
|
||||
|
|
|
@ -18,6 +18,7 @@
|
|||
|
||||
class AccountAuthorizationConfig::Facebook < AccountAuthorizationConfig::Oauth2
|
||||
include AccountAuthorizationConfig::PluginSettings
|
||||
|
||||
self.plugin = :facebook
|
||||
plugin_settings :app_id, app_secret: :app_secret_dec
|
||||
|
||||
|
@ -46,18 +47,41 @@ class AccountAuthorizationConfig::Facebook < AccountAuthorizationConfig::Oauth2
|
|||
end
|
||||
validates :login_attribute, inclusion: login_attributes
|
||||
|
||||
def self.recognized_federated_attributes
|
||||
[
|
||||
'email'.freeze,
|
||||
'first_name'.freeze,
|
||||
'id'.freeze,
|
||||
'last_name'.freeze,
|
||||
'locale'.freeze,
|
||||
'name'.freeze,
|
||||
].freeze
|
||||
end
|
||||
|
||||
def login_attribute
|
||||
super || 'id'.freeze
|
||||
end
|
||||
|
||||
def unique_id(token)
|
||||
token.get('me'.freeze).parsed[login_attribute]
|
||||
me(token)[login_attribute]
|
||||
end
|
||||
|
||||
def provider_attributes(token)
|
||||
me(token)
|
||||
end
|
||||
|
||||
protected
|
||||
|
||||
def me(token)
|
||||
# abusing AccessToken#options as a useful place to cache this response
|
||||
token.options[:me] ||= begin
|
||||
attributes = ([login_attribute] + federated_attributes.values.map { |v| v['attribute'] }).uniq
|
||||
token.get("me?fields=#{attributes.join(',')}").parsed
|
||||
end
|
||||
end
|
||||
|
||||
def authorize_options
|
||||
if login_attribute == 'email'.freeze
|
||||
if login_attribute == 'email' || federated_attributes.any? { |(_k, v)| v['attribute'] == 'email' }
|
||||
{ scope: 'email'.freeze }.freeze
|
||||
else
|
||||
{}.freeze
|
||||
|
|
|
@ -39,6 +39,10 @@ class AccountAuthorizationConfig::Oauth < AccountAuthorizationConfig::Delegated
|
|||
@client ||= OAuth::Consumer.new(consumer_key, consumer_secret, consumer_options)
|
||||
end
|
||||
|
||||
def provider_attributes
|
||||
{}
|
||||
end
|
||||
|
||||
protected
|
||||
|
||||
def token_options
|
||||
|
|
|
@ -53,6 +53,10 @@ class AccountAuthorizationConfig::Oauth2 < AccountAuthorizationConfig::Delegated
|
|||
client.auth_code.get_token(code, { redirect_uri: redirect_uri }.merge(token_options))
|
||||
end
|
||||
|
||||
def provider_attributes(token)
|
||||
{}
|
||||
end
|
||||
|
||||
protected
|
||||
|
||||
def client_options
|
||||
|
|
|
@ -84,6 +84,7 @@ describe Login::Oauth2Controller do
|
|||
session[:oauth2_nonce] = 'bob'
|
||||
aac.any_instantiation.expects(:get_token).returns('token')
|
||||
aac.any_instantiation.expects(:unique_id).with('token').returns('user')
|
||||
aac.any_instantiation.expects(:provider_attributes).with('token').returns({})
|
||||
user_with_pseudonym(username: 'user', active_all: 1)
|
||||
@pseudonym.authentication_provider = aac
|
||||
@pseudonym.save!
|
||||
|
@ -99,6 +100,7 @@ describe Login::Oauth2Controller do
|
|||
it "redirects to login if no user found" do
|
||||
aac.any_instantiation.expects(:get_token).returns('token')
|
||||
aac.any_instantiation.expects(:unique_id).with('token').returns('user')
|
||||
aac.any_instantiation.expects(:provider_attributes).with('token').returns({})
|
||||
|
||||
session[:oauth2_nonce] = 'bob'
|
||||
jwt = Canvas::Security.create_jwt(aac_id: aac.global_id, nonce: 'bob')
|
||||
|
@ -111,6 +113,7 @@ describe Login::Oauth2Controller do
|
|||
it "redirects to login if no user information returned" do
|
||||
aac.any_instantiation.expects(:get_token).returns('token')
|
||||
aac.any_instantiation.expects(:unique_id).with('token').returns(nil)
|
||||
aac.any_instantiation.expects(:provider_attributes).with('token').returns({})
|
||||
|
||||
session[:oauth2_nonce] = 'bob'
|
||||
jwt = Canvas::Security.create_jwt(aac_id: aac.global_id, nonce: 'bob')
|
||||
|
@ -131,6 +134,7 @@ describe Login::Oauth2Controller do
|
|||
aac.update_attribute(:jit_provisioning, true)
|
||||
aac.any_instantiation.expects(:get_token).returns('token')
|
||||
aac.any_instantiation.expects(:unique_id).with('token').returns('user')
|
||||
aac.any_instantiation.expects(:provider_attributes).with('token').returns({})
|
||||
|
||||
session[:oauth2_nonce] = 'bob'
|
||||
jwt = Canvas::Security.create_jwt(aac_id: aac.global_id, nonce: 'bob')
|
||||
|
|
|
@ -271,5 +271,25 @@ describe AccountAuthorizationConfig do
|
|||
expect(@user.sortable_name).to eq 'Cutrer, Cody'
|
||||
expect(@user.time_zone.tzinfo.name).to eq 'America/New_York'
|
||||
end
|
||||
|
||||
context 'locale' do
|
||||
it 'translates _ to -' do
|
||||
aac.apply_federated_attributes(@pseudonym, 'locale' => 'en_GB')
|
||||
@user.reload
|
||||
expect(@user.locale).to eq 'en-GB'
|
||||
end
|
||||
|
||||
it 'follows fallbacks' do
|
||||
aac.apply_federated_attributes(@pseudonym, 'locale' => 'en-US')
|
||||
@user.reload
|
||||
expect(@user.locale).to eq 'en'
|
||||
end
|
||||
|
||||
it 'is case insensitive' do
|
||||
aac.apply_federated_attributes(@pseudonym, 'locale' => 'en-gb')
|
||||
@user.reload
|
||||
expect(@user.locale).to eq 'en-GB'
|
||||
end
|
||||
end
|
||||
end
|
||||
end
|
||||
|
|
Loading…
Reference in New Issue