diff --git a/app/controllers/login/saml_controller.rb b/app/controllers/login/saml_controller.rb index 157e0724e4c..5732ee140dd 100644 --- a/app/controllers/login/saml_controller.rb +++ b/app/controllers/login/saml_controller.rb @@ -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) diff --git a/app/models/authentication_provider.rb b/app/models/authentication_provider.rb index 9e3b7b2e16e..40d30a627fd 100644 --- a/app/models/authentication_provider.rb +++ b/app/models/authentication_provider.rb @@ -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 diff --git a/app/models/pseudonym.rb b/app/models/pseudonym.rb index 82fec23390d..513ab20803e 100644 --- a/app/models/pseudonym.rb +++ b/app/models/pseudonym.rb @@ -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 diff --git a/spec/controllers/login/saml_controller_spec.rb b/spec/controllers/login/saml_controller_spec.rb index f0c7bfef6b5..e6027d56bb7 100644 --- a/spec/controllers/login/saml_controller_spec.rb +++ b/spec/controllers/login/saml_controller_spec.rb @@ -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)