ylqjgm
/
canvas-lms
mirror of https://github.com/instructure/canvas-lms.git
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

allow suspending pseudonyms 

where suspending means they still show up everywhere, but the user is no longer
allowed to login

closes FOO-2039

test plan:
 * have a regular user with an access token, and an active session
 * (via a separate session or access token) suspend a pseudonym
   via the API as an admin (logins API, set workflow_state to
   suspended)
 * ensure the original user gets logged out when they refresh, and
   that their access token doesn't work
 * but as the admin, you can still see the user

Change-Id: Idc0c61bcc244697e3c89b9beb2edfbe2a504b00e
Reviewed-on: https://gerrit.instructure.com/c/canvas-lms/+/269878
Reviewed-by: Simon Williams <simon@instructure.com>
Tested-by: Service Cloud Jenkins <svc.cloudjenkins@instructure.com>
QA-Review: Cody Cutrer <cody@instructure.com>
Product-Review: Cody Cutrer <cody@instructure.com>
This commit is contained in:
Cody Cutrer 2021-07-22 14:25:06 -06:00
parent 83801331cb
commit 0847dc9670
20 changed files with 294 additions and 59 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

6
app/controllers/communication_channels_controller.rb
View File

@ -337,7 +337,7 @@ class CommunicationChannelsController < ApplicationController
return unless @merge_opportunities.empty?
failed = true
elsif cc.active?
pseudonym = @root_account.pseudonyms.active.where(:user_id => @user).exists?
pseudonym = @root_account.pseudonyms.active_only.where(:user_id => @user).exists?
if @user.pre_registered? && pseudonym
@user.register
return redirect_with_success_flash
@ -347,12 +347,12 @@ class CommunicationChannelsController < ApplicationController
else
# Open registration and admin-created users are pre-registered, and have already claimed a CC, but haven't
# set up a password yet
@pseudonym = @root_account.pseudonyms.active.where(:password_auto_generated => true, :user_id => @user).first if @user.pre_registered? || @user.creation_pending?
@pseudonym = @root_account.pseudonyms.active_only.where(:password_auto_generated => true, :user_id => @user).first if @user.pre_registered? || @user.creation_pending?
# Users implicitly created via course enrollment or account admin creation are creation pending, and don't have a pseudonym yet
@pseudonym ||= @root_account.pseudonyms.build(:user => @user, :unique_id => cc.path) if @user.creation_pending?
# We create the pseudonym with unique_id = cc.path, but if that unique_id is taken, just nil it out and make the user come
# up with something new
@pseudonym.unique_id = '' if @pseudonym && @pseudonym.new_record? && @root_account.pseudonyms.active.by_unique_id(@pseudonym.unique_id).exists?
@pseudonym.unique_id = '' if @pseudonym&.new_record? && @root_account.pseudonyms.active_only.by_unique_id(@pseudonym.unique_id).exists?
# Have to either have a pseudonym to register with, or be looking at merge opportunities
return render :confirm_failed, status: :bad_request if !@pseudonym && @merge_opportunities.empty?

2
app/controllers/profile_controller.rb
View File

@ -207,7 +207,7 @@ class ProfileController < ApplicationController
@other_channels = @channels.select{|c| c.path_type != "email"}
@default_email_channel = @email_channels.first
@default_pseudonym = @user.primary_pseudonym
@pseudonyms = @user.pseudonyms.active
@pseudonyms = @user.pseudonyms.active_only
@password_pseudonyms = @pseudonyms.select{|p| !p.managed_password? }
@context = @user.profile
set_active_tab "profile_settings"

31
app/controllers/pseudonyms_controller.rb
View File

@ -41,6 +41,7 @@ class PseudonymsController < ApplicationController
# provider that this login is associated with
# @response_field authentication_provider_type The type of the authentication
# provider that this login is associated with
# @response_field workflow_state The current status of the login
#
# @example_response
# [
@ -51,7 +52,8 @@ class PseudonymsController < ApplicationController
# "unique_id": "belieber@example.com",
# "user_id": 2,
# "authentication_provider_id": 1,
# "authentication_provider_type": "facebook"
# "authentication_provider_type": "facebook",
# "workflow_state": "active"
# }
# ]
def index
@ -85,7 +87,7 @@ class PseudonymsController < ApplicationController
end
@ccs = CommunicationChannel.email.by_path(email).shard(shards.to_a).active.to_a
if @domain_root_account
@domain_root_account.pseudonyms.active.by_unique_id(email).each do |p|
@domain_root_account.pseudonyms.active_only.by_unique_id(email).each do |p|
cc = p.communication_channel if p.communication_channel && p.user
cc ||= p.user.communication_channel rescue nil
@ccs << cc
@ -139,7 +141,7 @@ class PseudonymsController < ApplicationController
flash[:error] = t 'The link you used has expired. Click "Forgot Password?" to get a new reset-password link.'
redirect_to canvas_login_url
end
@password_pseudonyms = @cc.user.pseudonyms.active.select{|p| p.account.canvas_authentication? }
@password_pseudonyms = @cc.user.pseudonyms.active_only.select{|p| p.account.canvas_authentication? }
js_env :PASSWORD_POLICY => @domain_root_account.password_policy,
:PASSWORD_POLICIES => Hash[@password_pseudonyms.map{ |p| [p.id, p.account.password_policy]}]
end
@ -301,6 +303,9 @@ class PseudonymsController < ApplicationController
# provider, or the type of the provider (in which case, it will find the
# first matching provider).
#
# @argument login[workflow_state] [String, "active"|"suspended"]
# Used to suspend or re-activate a login.
#
# @example_request
# curl https://<canvas>/api/v1/accounts/:account_id/logins/:login_id \
# -H "Authorization: Bearer <ACCESS-TOKEN>" \
@ -315,7 +320,8 @@ class PseudonymsController < ApplicationController
# "created_at": "2020-01-29T19:33:35Z",
# "sis_user_id": null,
# "integration_id": null,
# "authentication_provider_id": null
# "authentication_provider_id": null,
# "workflow_state": "active",
# }
def update
if api_request?
@ -406,7 +412,8 @@ class PseudonymsController < ApplicationController
:password,
:sis_user_id,
:authentication_provider_id,
:integration_id
:integration_id,
:workflow_state,
).blank?
render json: nil, status: :bad_request
return false
@ -449,6 +456,20 @@ class PseudonymsController < ApplicationController
@pseudonym.password = params[:pseudonym][:password]
@pseudonym.password_confirmation = params[:pseudonym][:password_confirmation]
end or return false
# give a 400 instead of a 401 if the workflow_state doesn't make sense
if params[:pseudonym].key?(:workflow_state) && !%w[active suspended].include?(params[:pseudonym][:workflow_state])
@pseudonym.errors.add(:workflow_state, 'invalid workflow_state')
respond_to do |format|
format.html { render(params[:action] == 'edit' ? :edit : :new) }
format.json { render json: @pseudonym.errors, status: :bad_request }
end
return false
end
has_right_if_requests_change(:workflow_state, :delete) do
@pseudonym.workflow_state = params[:pseudonym][:workflow_state]
end or return false
end
private

2
app/controllers/user_observees_controller.rb
View File

@ -141,7 +141,7 @@ class UserObserveesController < ApplicationController
common_root_accounts = common_root_accounts_for(observer, student)
code.destroy
else
observee_pseudonym = @domain_root_account.pseudonyms.active.by_unique_id(params[:observee][:unique_id]).first
observee_pseudonym = @domain_root_account.pseudonyms.active_only.by_unique_id(params[:observee][:unique_id]).first
common_root_accounts = common_root_accounts_for(observer, observee_pseudonym.user) if observee_pseudonym
if observee_pseudonym.nil? || common_root_accounts.empty?

19
app/controllers/users_controller.rb
View File

@ -1898,6 +1898,10 @@ class UsersController < ApplicationController
# Only Available Pronouns set on the root account are allowed
# Adding and changing pronouns must be enabled on the root account.
#
# @argument user[event] [String, "suspend"|"unsuspend"]
# suspends or unsuspends all logins for this user that the calling user
# has permission to
#
# @example_request
#
# curl 'https://<canvas>/api/v1/users/133.json' \
@ -1935,7 +1939,7 @@ class UsersController < ApplicationController
end
if @user.grants_right?(@current_user, :manage_user_details)
managed_attributes.concat([:time_zone, :locale])
managed_attributes.concat([:time_zone, :locale, :event])
end
if @user.grants_right?(@current_user, :update_avatar)
@ -1997,6 +2001,17 @@ class UsersController < ApplicationController
@user.sortable_name_explicitly_set = user_params[:sortable_name].present?
if (event = user_params.delete(:event)) && %w[suspend unsuspend].include?(event) &&
@user != @current_user
@user.pseudonyms.active.shard(@user).each do |p|
next unless p.grants_right?(@current_user, :delete)
next if p.active? && event == 'unsuspend'
next if p.suspended? && event == 'suspend'
p.update!(workflow_state: event == 'suspend' ? 'suspended' : 'active')
end
end
respond_to do |format|
if @user.update(user_params)
@user.avatar_state = (old_avatar_state == :locked ? old_avatar_state : 'approved') if admin_avatar_update
@ -2820,7 +2835,7 @@ class UsersController < ApplicationController
end
if @pseudonym.nil?
@pseudonym = @context.pseudonyms.active.by_unique_id(params[:pseudonym][:unique_id]).first
@pseudonym = @context.pseudonyms.active_only.by_unique_id(params[:pseudonym][:unique_id]).first
# Setting it to nil will cause us to try and create a new one, and give user the login already exists error
@pseudonym = nil if @pseudonym && !['creation_pending', 'pending_approval'].include?(@pseudonym.user.workflow_state)
end

34
app/models/pseudonym.rb
View File

@ -60,7 +60,7 @@ class Pseudonym < ActiveRecord::Base
alias_method :context, :account
include StickySisFields
are_sis_sticky :unique_id
are_sis_sticky :unique_id, :workflow_state
validates :unique_id, format: { with: /\A[[:print:]]+\z/ },
length: { within: 1..MAX_UNIQUE_ID_LENGTH },
@ -157,7 +157,7 @@ class Pseudonym < ActiveRecord::Base
def self.custom_find_by_unique_id(unique_id)
return unless unique_id
active.by_unique_id(unique_id).where("authentication_provider_id IS NULL OR EXISTS (?)",
active_only.by_unique_id(unique_id).where("authentication_provider_id IS NULL OR EXISTS (?)",
AuthenticationProvider.active.where(auth_type: ['canvas', 'ldap'])
.where("authentication_provider_id=authentication_providers.id"))
.order("authentication_provider_id NULLS LAST").first
@ -165,8 +165,8 @@ class Pseudonym < ActiveRecord::Base
def self.for_auth_configuration(unique_id, aac)
auth_id = aac.try(:auth_provider_filter)
active.by_unique_id(unique_id).where(authentication_provider_id: auth_id).
order("authentication_provider_id NULLS LAST").take
active_only.by_unique_id(unique_id).where(authentication_provider_id: auth_id)
.order("authentication_provider_id NULLS LAST").take
end
def set_password_changed
@ -276,6 +276,7 @@ class Pseudonym < ActiveRecord::Base
workflow do
state :active
state :deleted
state :suspended
end
set_policy do
@ -412,8 +413,9 @@ class Pseudonym < ActiveRecord::Base
end
def valid_arbitrary_credentials?(plaintext_password)
return false if self.deleted?
return false unless active?
return false if plaintext_password.blank?
require 'net/ldap'
res = false
res ||= valid_ldap_credentials?(plaintext_password)
@ -515,7 +517,10 @@ class Pseudonym < ActiveRecord::Base
nil
end
scope :active, -> { where(workflow_state: 'active') }
scope :active, -> { where.not(workflow_state: 'deleted') }
scope :active_only, -> { where(workflow_state: 'active') }
scope :deleted, -> { where(workflow_state: 'deleted') }
def self.serialization_excludes; [:crypted_password, :password_salt, :reset_password_token, :persistence_token, :single_access_token, :perishable_token, :sis_ssha]; end
@ -532,6 +537,7 @@ class Pseudonym < ActiveRecord::Base
# a failed login instead of an error.
raise ImpossibleCredentialsError, "pseudonym cannot have a unique_id of length #{credentials[:unique_id].length}"
end
too_many_attempts = false
begin
associated_shards = associated_shards(credentials[:unique_id])
@ -540,19 +546,21 @@ class Pseudonym < ActiveRecord::Base
# by searching all accounts the slow way
Canvas::Errors.capture(e)
end
pseudonyms = Shard.partition_by_shard(account_ids) do |account_ids|
pseudonyms = Shard.partition_by_shard(account_ids) do |shard_account_ids|
next if GlobalLookups.enabled? && associated_shards && !associated_shards.include?(Shard.current)
active.
by_unique_id(credentials[:unique_id]).
where(:account_id => account_ids).
preload(:user).
select { |p|
active_only
.by_unique_id(credentials[:unique_id])
.where(account_id: shard_account_ids)
.preload(:user)
.select do |p|
valid = p.valid_arbitrary_credentials?(credentials[:password])
too_many_attempts = true if p.audit_login(remote_ip, valid) == :too_many_attempts
valid
}
end
end
return :too_many_attempts if too_many_attempts
pseudonyms
end

64
db/migrate/20210722203513_update_pseudonym_unique_indexes.rb Normal file
View File

@ -0,0 +1,64 @@
# frozen_string_literal: true
#
# Copyright (C) 2021 - present Instructure, Inc.
#
# This file is part of Canvas.
#
# Canvas is free software: you can redistribute it and/or modify it under
# the terms of the GNU Affero General Public License as published by the Free
# Software Foundation, version 3 of the License.
#
# Canvas is distributed in the hope that it will be useful, but WITHOUT ANY
# WARRANTY; without even the implied warranty of MERCHANTABILITY or FITNESS FOR
# A PARTICULAR PURPOSE. See the GNU Affero General Public License for more
# details.
#
# You should have received a copy of the GNU Affero General Public License along
# with this program. If not, see <http://www.gnu.org/licenses/>.
class UpdatePseudonymUniqueIndexes < ActiveRecord::Migration[6.0]
tag :predeploy
disable_ddl_transaction!
def up
add_index :pseudonyms,
"LOWER(unique_id), account_id, authentication_provider_id",
name: 'index_pseudonyms_unique_with_auth_provider',
if_not_exists: true,
algorithm: :concurrently,
unique: true,
where: "workflow_state IN ('active', 'suspended')"
add_index :pseudonyms,
"LOWER(unique_id), account_id",
name: 'index_pseudonyms_unique_without_auth_provider',
if_not_exists: true,
algorithm: :concurrently,
unique: true,
where: "workflow_state IN ('active', 'suspended') AND authentication_provider_id IS NULL"
remove_index :pseudonyms,
name: 'index_pseudonyms_on_unique_id_and_account_id_and_authentication_provider_id',
algorithm: :concurrently,
if_exists: true
remove_index :pseudonyms,
name: 'index_pseudonyms_on_unique_id_and_account_id_no_authentication_provider_id',
algorithm: :concurrently,
if_exists: true
end
def down
execute "CREATE UNIQUE INDEX index_pseudonyms_on_unique_id_and_account_id_and_authentication_provider_id ON #{Pseudonym.quoted_table_name} (LOWER(unique_id), account_id, authentication_provider_id) WHERE workflow_state='active'"
execute "CREATE UNIQUE INDEX index_pseudonyms_on_unique_id_and_account_id_no_authentication_provider_id ON #{Pseudonym.quoted_table_name} (LOWER(unique_id), account_id) WHERE workflow_state='active' AND authentication_provider_id IS NULL"
remove_index :pseudonyms,
name: 'index_pseudonyms_unique_with_auth_provider',
algorithm: :concurrently,
if_exists: true
remove_index :pseudonyms,
name: 'index_pseudonyms_unique_without_auth_provider',
algorithm: :concurrently,
if_exists: true
end
end

2
doc/api/sis_csv.md
View File

@ -277,7 +277,7 @@ recommended to omit this field over using fake email addresses for testing.</td>
<td>status</td>
<td>enum</td>
<td>✓</td>
<td></td>
<td>✓</td>
<td>active, deleted</td>
</tr>
</table>

3
lib/api/v1/pseudonym.rb
View File

@ -27,7 +27,8 @@ module Api::V1::Pseudonym
:sis_user_id,
:integration_id,
:authentication_provider_id,
:created_at].freeze
:created_at,
:workflow_state].freeze
def pseudonym_json(pseudonym, current_user, session)
opts = API_PSEUDONYM_JSON_OPTS

40
lib/authentication_methods.rb
View File

@ -149,9 +149,7 @@ module AuthenticationMethods
if token_string
@access_token = AccessToken.authenticate(token_string)
if !@access_token
raise AccessTokenError
end
raise AccessTokenError unless @access_token
account = access_token_account(@domain_root_account, @access_token)
raise AccessTokenError unless @access_token.authorized_for_account?(account)
@ -160,10 +158,10 @@ module AuthenticationMethods
@real_current_user = @access_token.real_user
@real_current_pseudonym = SisPseudonym.for(@real_current_user, @domain_root_account, type: :implicit, require_sis: false) if @real_current_user
@current_pseudonym = SisPseudonym.for(@current_user, @domain_root_account, type: :implicit, require_sis: false)
@current_pseudonym = nil if (@current_pseudonym&.suspended? && !@real_current_pseudonym) || @real_current_pseudonym&.suspended?
raise AccessTokenError unless @current_user && @current_pseudonym
unless @current_user && @current_pseudonym
raise AccessTokenError
end
validate_scopes
@access_token.used!
@ -190,7 +188,7 @@ module AuthenticationMethods
load_pseudonym_from_jwt
load_pseudonym_from_access_token unless @current_pseudonym.present?
if !@current_pseudonym
unless @current_pseudonym
if @policy_pseudonym_id
@current_pseudonym = Pseudonym.where(id: @policy_pseudonym_id).first
else
@ -211,11 +209,8 @@ module AuthenticationMethods
session_refreshed_at < invalid_before
logger.info "[AUTH] Invalidating session: Session created before user logged out."
destroy_session
@current_pseudonym = nil
if api_request? || request.format.json?
raise LoggedOutError
end
invalidate_session
return
end
if @current_pseudonym &&
@ -223,12 +218,14 @@ module AuthenticationMethods
@current_pseudonym.cas_ticket_expired?(session[:cas_session])
logger.info "[AUTH] Invalidating session: CAS ticket expired - #{session[:cas_session]}."
destroy_session
@current_pseudonym = nil
invalidate_session
return
end
raise LoggedOutError if api_request? || request.format.json?
redirect_to_login
if @current_pseudonym.suspended?
logger.info "[AUTH] Invalidating session: Pseudonym is suspended."
invalidate_session
return
end
end
end
@ -418,4 +415,13 @@ module AuthenticationMethods
reset_session
saved.each_pair { |k, v| session[k] = v }
end
def invalidate_session
destroy_session
@current_pseudonym = nil
raise LoggedOutError if api_request? || request.format.json?
redirect_to_login
end
end

2
lib/sis/user_importer.rb
View File

@ -252,7 +252,7 @@ module SIS
pseudo.sis_user_id = user_row.user_id
pseudo.integration_id = user_row.integration_id if user_row.integration_id.present?
pseudo.account = @root_account
pseudo.workflow_state = status_is_active ? 'active' : 'deleted'
pseudo.workflow_state = status_is_active ? 'active' : 'deleted' unless pseudo.stuck_sis_fields.include?(:workflow_state)
if pseudo.new_record? && status_is_active
should_add_account_associations = true
elsif pseudo.workflow_state_changed?

7
spec/apis/auth_spec.rb
View File

@ -667,6 +667,13 @@ describe "API Authentication", type: :request do
expect(JSON.parse(response.body).size).to eq 1
end
it "doesn't allow usage of a suspended pseudonym" do
@pseudonym.update!(workflow_state: 'suspended')
get "/api/v1/courses?access_token=#{@token.full_token}"
expect(response.status).to eq 401
end
it "should allow passing the access token in the authorization header" do
check_used { get "/api/v1/courses", headers: { 'HTTP_AUTHORIZATION' => "Bearer #{@token.full_token}" } }
expect(JSON.parse(response.body).size).to eq 1

3
spec/apis/v1/authentication_audit_api_spec.rb
View File

@ -165,7 +165,8 @@ describe "AuthenticationAudit API", type: :request do
"unique_id" => @pseudonym.unique_id,
"sis_user_id" => nil,
"integration_id" => nil,
"authentication_provider_id" => nil
"authentication_provider_id" => nil,
"workflow_state" => "active",
}]
end
end

38
spec/apis/v1/pseudonyms_api_spec.rb
View File

@ -47,7 +47,8 @@ describe PseudonymsController, type: :request do
'sis_user_id' => p.sis_user_id,
'unique_id' => p.unique_id,
'user_id' => p.user_id,
'created_at' => p.created_at
'created_at' => p.created_at,
'workflow_state' => 'active',
}
end)
end
@ -91,6 +92,16 @@ describe PseudonymsController, type: :request do
expect(json.count).to eql 2
expect(json.map{|j| j['id']}.include?(to_delete.id)).to be_falsey
end
it "includes suspended pseudonyms" do
to_suspend = @student.pseudonyms.create!(:unique_id => "to-delete@example.com")
to_suspend.update!(workflow_state: 'suspended')
json = api_call(:get, @user_path, @user_path_options)
expect(json.count).to eq 1
expect(json.first['id']).to eq to_suspend.id
expect(json.first['workflow_state']).to eq 'suspended'
end
end
context "An authorized user with an empty query" do
@ -145,7 +156,8 @@ describe PseudonymsController, type: :request do
'integration_id' => nil,
'unique_id' => 'test@example.com',
'user_id' => @student.id,
'created_at' => json['created_at']
'created_at' => json['created_at'],
'workflow_state' => 'active',
})
end
@ -258,11 +270,28 @@ describe PseudonymsController, type: :request do
'integration_id' => nil,
'unique_id' => 'student+new@example.com',
'user_id' => @student.id,
'created_at' => @student.pseudonym.created_at.iso8601
'created_at' => @student.pseudonym.created_at.iso8601,
'workflow_state' => 'active',
})
expect(@student.pseudonym.reload.valid_password?('password123')).to be_truthy
end
it 'can suspend the pseudonym' do
json = api_call(:put, @path, @path_options, { login: { workflow_state: 'suspended' } })
expect(json['workflow_state']).to eq 'suspended'
end
it 'can suspend the pseudonym and alter attributes' do
json = api_call(:put, @path, @path_options, { login: { workflow_state: 'suspended', sis_user_id: 'new-12345' } })
expect(json['workflow_state']).to eq 'suspended'
expect(json['sis_user_id']).to eq 'new-12345'
end
it 'ignores invalid workflow states' do
raw_api_call(:put, @path, @path_options, { login: { workflow_state: 'bogus' } })
expect(response.code).to eql '400'
end
it "should return 400 if the unique_id already exists" do
raw_api_call(:put, @path, @path_options, {
:login => {
@ -408,7 +437,8 @@ describe PseudonymsController, type: :request do
"authentication_provider_id" => nil,
'id' => pseudonym.id,
'user_id' => @student.id,
'created_at' => pseudonym.created_at.iso8601
'created_at' => pseudonym.created_at.iso8601,
'workflow_state' => 'deleted',
})
end

12
spec/apis/v1/users_api_spec.rb
View File

@ -1948,6 +1948,18 @@ describe "Users API", type: :request do
api_call(:put, @path, @path_options, {:user => {:name => "Other Name"}}) # only send in the name
expect(@student.reload.sortable_name).to eq "Name, Other" # should auto sync
end
it "can suspend all pseudonyms" do
api_call(:put, @path, @path_options, { user: { event: "suspend" }})
expect(@student.pseudonym.reload).to be_suspended
end
it "can unsuspend all pseudonyms" do
@student.pseudonym.update!(workflow_state: 'suspended')
api_call(:put, @path, @path_options, { user: { event: "unsuspend" }})
expect(@student.pseudonym.reload).to be_active
end
end
context "non-account-admin user" do

7
spec/controllers/login/canvas_controller_spec.rb
View File

@ -112,6 +112,13 @@ describe Login::CanvasController do
expect(session[:sentinel]).to be_nil
end
it "doesn't allow suspended users" do
@pseudonym.update!(workflow_state: 'suspended')
post 'create', params: {:pseudonym_session => { :unique_id => 'jtfrd@instructure.com', :password => 'qwertyuiop'}}
assert_status(400)
expect(response).to render_template(:new)
end
it "persists the auth provider if the feature flag is enabled" do
Account.default.enable_feature!(:persist_inferred_authentication_providers)
post 'create', params: {:pseudonym_session => { :unique_id => 'jtfrd@instructure.com', :password => 'qwertyuiop'}}

24
spec/controllers/login/cas_controller_spec.rb
View File

@ -82,6 +82,30 @@ describe Login::CasController do
expect(response).to redirect_to(login_url)
end
it "doesn't allow suspended users to login" do
account = account_with_cas(account: Account.default)
user_with_pseudonym(active_all: true, account: account)
@pseudonym.update!(workflow_state: 'suspended')
response_text = <<-RESPONSE_TEXT
<cas:serviceResponse xmlns:cas="http://www.yale.edu/tp/cas">
<cas:authenticationSuccess>
<cas:user>#{@user.email}</cas:user>
</cas:authenticationSuccess>
</cas:serviceResponse>
RESPONSE_TEXT
controller.instance_variable_set(:@domain_root_account, Account.default)
cas_client = controller.client
cas_client.instance_variable_set(:@stub_response, response_text)
def cas_client.request_cas_response(_uri, type, _options={})
type.new(@stub_response, @conf_options)
end
get 'new', params: {:ticket => 'ST-abcd'}
expect(response).to redirect_to(login_url)
end
it "should accept extra attributes" do
account = account_with_cas(account: Account.default)
user_with_pseudonym(active_all: true, account: account)

3
spec/factories/pseudonym_session_factory.rb
View File

@ -33,7 +33,8 @@ module Factories
:global_account_id => '10000000000001',
:sis_user_id => 'U001',
:shard => Shard.default,
:works_for_account? => true
:works_for_account? => true,
:suspended? => false,
)
# at least one thing cares about the id of the pseudonym... using the
# object_id should make it unique (but obviously things will fail if

12
spec/lib/authentication_methods_spec.rb
View File

@ -26,7 +26,7 @@ describe AuthenticationMethods do
include Canvas::RequestForgeryProtection
include AuthenticationMethods
attr_accessor :redirects, :params, :session, :request, :render_hash
attr_accessor :redirects, :params, :session, :request, :render_hash, :fix_ms_office_redirects
def initialize(request:, root_account: Account.default, params: {})
@request = request
@ -102,6 +102,16 @@ describe AuthenticationMethods do
expect(@controller.instance_variable_get(:@current_pseudonym)).to be_nil
end
it "destroys the session if the pseudonym was suspended" do
@pseudonym.reload
@pseudonym.update!(workflow_state: 'suspended')
expect(@controller).to receive(:destroy_session).once
expect(@controller.send(:load_user)).to be_nil
expect(@controller.instance_variable_get(:@current_user)).to be_nil
expect(@controller.instance_variable_get(:@current_pseudonym)).to be_nil
end
it "should not destroy session if user was logged out in the future" do
Timecop.freeze(5.minutes.from_now) do
@user.stamp_logout_time!

40
spec/models/pseudonym_spec.rb
View File

@ -361,14 +361,14 @@ describe Pseudonym do
it "should only query the pertinent shard" do
expect(Pseudonym).to receive(:associated_shards).with('abc').and_return([@shard1])
expect(Pseudonym).to receive(:active).once.and_return(Pseudonym.none)
expect(Pseudonym).to receive(:active_only).once.and_return(Pseudonym.none)
allow(GlobalLookups).to receive(:enabled?).and_return(true)
Pseudonym.authenticate({ unique_id: 'abc', password: 'def' }, [Account.default.id, account2])
end
it "should query all pertinent shards" do
expect(Pseudonym).to receive(:associated_shards).with('abc').and_return([Shard.default, @shard1])
expect(Pseudonym).to receive(:active).twice.and_return(Pseudonym.none)
expect(Pseudonym).to receive(:active_only).twice.and_return(Pseudonym.none)
allow(GlobalLookups).to receive(:enabled?).and_return(true)
Pseudonym.authenticate({ unique_id: 'abc', password: 'def' }, [Account.default.id, account2])
end
@ -721,13 +721,25 @@ describe Pseudonym do
end
describe ".find_all_by_arbtrary_credentials" do
it "doesn't choke on if global lookups is down" do
let_once(:p) do
u = User.create!
p = u.pseudonyms.create!(unique_id: 'a', account: Account.default, password: 'abcdefgh', password_confirmation: 'abcdefgh')
u.pseudonyms.create!(unique_id: 'a', account: Account.default, password: 'abcdefgh', password_confirmation: 'abcdefgh')
end
it "finds a valid pseudonym" do
expect(Pseudonym.find_all_by_arbitrary_credentials(
{ unique_id: 'a', password: 'abcdefgh' },
[Account.default.id], '127.0.0.1'
)).to eq [p]
end
it "doesn't choke on if global lookups is down" do
expect(GlobalLookups).to receive(:enabled?).and_return(true)
expect(Pseudonym).to receive(:associated_shards).and_raise("an error")
expect(Pseudonym.find_all_by_arbitrary_credentials({ unique_id: 'a', password: 'abcdefgh' },
[Account.default.id], '127.0.0.1')).to eq [p]
expect(Pseudonym.find_all_by_arbitrary_credentials(
{ unique_id: 'a', password: 'abcdefgh' },
[Account.default.id], '127.0.0.1'
)).to eq [p]
end
it "throws an error if your credentials are absurd" do
@ -736,5 +748,21 @@ describe Pseudonym do
creds = { unique_id: unique_id, password: 'foobar' }
expect{ Pseudonym.find_all_by_arbitrary_credentials(creds, [Account.default.id], '127.0.0.1') }.to raise_error(ImpossibleCredentialsError)
end
it "doesn't find deleted pseudonyms" do
p.update!(workflow_state: 'deleted')
expect(Pseudonym.find_all_by_arbitrary_credentials(
{ unique_id: 'a', password: 'abcdefgh' },
[Account.default.id], '127.0.0.1'
)).to eq []
end
it "doesn't find suspended pseudonyms" do
p.update!(workflow_state: 'suspended')
expect(Pseudonym.find_all_by_arbitrary_credentials(
{ unique_id: 'a', password: 'abcdefgh' },
[Account.default.id], '127.0.0.1'
)).to eq []
end
end
end