more validation of blank password

The LDAP unique_id response stuff doesn't go through the normal
pseudonym method. Also add validation to the controller action, to catch
it even earlier.

fixes CNVS-4447

test plan: the javascript on the normal login page blocks blank
passwords, so the easiest way to test this currently is to use a mobile
browser or change your user-agent to be a mobile browser so that you get
the mobile login screen, which currently allows blank passwords through
(though that'll be fixed at some point too). then login with a username
but no password and verify you get a "No password was given" error
rather than an incorrect username/password error.

Change-Id: Iccd3cccb1f5e2d5ee9f6a71aead74acf6f2e2c13
Reviewed-on: https://gerrit.instructure.com/18389
Tested-by: Jenkins <jenkins@instructure.com>
Reviewed-by: Brian Palmer <brianp@instructure.com>
QA-Review: Brian Palmer <brianp@instructure.com>
This commit is contained in:
Brian Palmer 2013-03-07 12:26:29 -07:00
parent c90011468b
commit 4722d61200
4 changed files with 26 additions and 2 deletions

View File

@ -133,6 +133,10 @@ class PseudonymSessionsController < ApplicationController
# reset the session id cookie to prevent session fixation.
reset_session_for_login
if params[:pseudonym_session].blank? || params[:pseudonym_session][:password].blank?
return unsuccessful_login(t('errors.blank_password', "No password was given"))
end
# Try to use authlogic's built-in login approach first
@pseudonym_session = @domain_root_account.pseudonym_sessions.new(params[:pseudonym_session])
@pseudonym_session.remote_ip = request.remote_ip

View File

@ -365,6 +365,8 @@ class AccountAuthorizationConfig < ActiveRecord::Base
end
def ldap_bind_result(unique_id, password_plaintext)
return nil if password_plaintext.blank?
if self.last_timeout_failure.present?
failure_timeout = self.class.ldap_failure_wait_time.ago
if self.last_timeout_failure >= failure_timeout

View File

@ -67,6 +67,14 @@ describe PseudonymSessionsController do
response.should render_template('new')
end
it "should re-render if no password given" do
user_with_pseudonym(:username => 'jt@instructure.com', :active_all => 1, :password => 'qwerty')
post 'create', :pseudonym_session => { :unique_id => 'jt@instructure.com', :password => ''}
response.status.should == '400 Bad Request'
response.should render_template('new')
flash[:error].should match(/no password/i)
end
it "password auth should work" do
user_with_pseudonym(:username => 'jt@instructure.com', :active_all => 1, :password => 'qwerty')
post 'create', :pseudonym_session => { :unique_id => 'jt@instructure.com', :password => 'qwerty'}

View File

@ -43,14 +43,24 @@ describe AccountAuthorizationConfig do
end
end
end
context "#ldap_bind_result" do
it "should not attempt to bind with a blank password" do
aac = AccountAuthorizationConfig.new
aac.auth_type = 'ldap'
aac.ldap_filter = 'bob'
aac.expects(:ldap_connection).never
aac.ldap_bind_result('test', '')
end
end
it "should replace empty string with nil" do
@account = Account.new
config = @account.account_authorization_configs.build
config.change_password_url = ""
config.change_password_url.should be_nil
end
context "SAML settings" do
before(:each) do
@account = Account.create!(:name => "account")