block logins for pseudonyms that are in a suspended state
we offer JIT provisioning for a multitude of auth providers. I'm specifically targeting SAML as that's where most of our reports are coming from. If in the future we want to update more, we can in regards to recognizing and dealing with pseudonyms that have been suspended this change gives us the flexibility to include pseudonyms involved in our SSO authentication flow that have been suspended, bypass the logic for JIT provisioning (thus not duplicating users and setting federated attributes that don't pertain to the user in question) and block the login as no user found with pseudonym; our fallback when pseudonym == nil fixes FOO-2717 flag = none test plan: • set up a SAML IDP to interface with Canvas as a SP and enabled JIT provisioning with the sis_user_id attribute being collected • ensure your IDP is sending the federated attribute that you have set in the auth provider config for sis_user_id • have a user associated with the SAML auth provider • suspend the user • attempt to log in as that user via /login/saml and authenticate • verify the login is rejected with an error stating a user was not found with provided unique id • verify no additional user was provisioned through JIT Change-Id: I129ba60a86f90c98416e60b5c76325561e2bbad3 Reviewed-on: https://gerrit.instructure.com/c/canvas-lms/+/310836 Tested-by: Service Cloud Jenkins <svc.cloudjenkins@instructure.com> Reviewed-by: Jeremy Stanley <jeremy@instructure.com> QA-Review: August Thornton <august@instructure.com> Product-Review: Jesse Poulos <jpoulos@instructure.com>
This commit is contained in:
parent
c420bec585
commit
dbbe3f0941
|
@ -125,14 +125,16 @@ class Login::SamlController < ApplicationController
|
|||
|
||||
reset_session_for_login
|
||||
|
||||
pseudonym = @domain_root_account.pseudonyms.for_auth_configuration(unique_id, aac)
|
||||
pseudonym =
|
||||
@domain_root_account.pseudonyms.for_auth_configuration(unique_id, aac, include_suspended: true)
|
||||
|
||||
if !pseudonym && aac.jit_provisioning?
|
||||
pseudonym = aac.provision_user(unique_id, provider_attributes)
|
||||
elsif pseudonym
|
||||
elsif pseudonym && !pseudonym.suspended?
|
||||
aac.apply_federated_attributes(pseudonym, provider_attributes)
|
||||
end
|
||||
|
||||
if pseudonym && (user = pseudonym.login_assertions_for_user)
|
||||
if pseudonym && !pseudonym.suspended? && (user = pseudonym.login_assertions_for_user)
|
||||
# Successful login and we have a user
|
||||
@domain_root_account.pseudonyms.scoping do
|
||||
PseudonymSession.create!(pseudonym, false)
|
||||
|
|
|
@ -270,7 +270,7 @@ class AuthenticationProvider < ActiveRecord::Base
|
|||
end
|
||||
rescue ActiveRecord::RecordNotUnique
|
||||
self.class.uncached do
|
||||
pseudonyms.active.by_unique_id(unique_id).take!
|
||||
pseudonyms.active_only.by_unique_id(unique_id).take!
|
||||
end
|
||||
end
|
||||
|
||||
|
|
|
@ -178,10 +178,11 @@ class Pseudonym < ActiveRecord::Base
|
|||
.order("authentication_provider_id NULLS LAST").first
|
||||
end
|
||||
|
||||
def self.for_auth_configuration(unique_id, aac)
|
||||
def self.for_auth_configuration(unique_id, aac, include_suspended: false)
|
||||
auth_id = aac.try(:auth_provider_filter)
|
||||
active_only.by_unique_id(unique_id).where(authentication_provider_id: auth_id)
|
||||
.order("authentication_provider_id NULLS LAST").take
|
||||
scope = include_suspended ? active : active_only
|
||||
scope.by_unique_id(unique_id).where(authentication_provider_id: auth_id)
|
||||
.order("authentication_provider_id NULLS LAST").take
|
||||
end
|
||||
|
||||
def audit_log_update
|
||||
|
|
|
@ -324,6 +324,36 @@ describe Login::SamlController do
|
|||
expect(p.user.short_name).to eq "Cody Cutrer"
|
||||
end
|
||||
|
||||
it "skips JIT user provisioning for suspended pseudonyms" do
|
||||
account = account_with_saml
|
||||
user = user_with_pseudonym(active_all: 1, account: account)
|
||||
user.pseudonyms.last.update!(workflow_state: "suspended")
|
||||
|
||||
ap = account.authentication_providers.first
|
||||
ap.update_attribute(:jit_provisioning, true)
|
||||
ap.federated_attributes = { "sis_user_id" => "urn:oid" }
|
||||
ap.save!
|
||||
|
||||
response = SAML2::Response.new
|
||||
response.issuer = SAML2::NameID.new("saml_entity")
|
||||
response.assertions << (assertion = SAML2::Assertion.new)
|
||||
assertion.subject = SAML2::Subject.new
|
||||
assertion.subject.name_id = SAML2::NameID.new(@pseudonym.unique_id)
|
||||
assertion.statements << SAML2::AttributeStatement.new(
|
||||
[SAML2::Attribute.create("urn:oid", "some_unique_id")]
|
||||
)
|
||||
allow(SAML2::Bindings::HTTP_POST).to receive(:decode).and_return(
|
||||
[response, nil]
|
||||
)
|
||||
allow_any_instance_of(SAML2::Entity).to receive(:valid_response?)
|
||||
allow(LoadAccount).to receive(:default_domain_root_account).and_return(account)
|
||||
|
||||
post :create, params: { SAMLResponse: "foo" }
|
||||
expect(response).to redirect_to(login_url)
|
||||
expect(flash[:delegated_message]).to_not be_nil
|
||||
expect(session[:saml_unique_id]).to be_nil
|
||||
end
|
||||
|
||||
it "updates federated attributes" do
|
||||
account = account_with_saml
|
||||
user_with_pseudonym(active_all: 1, account: account)
|
||||
|
|
Loading…
Reference in New Issue