From fbc662f41f1fbd457ff4b7f2b10254330b1edd68 Mon Sep 17 00:00:00 2001 From: Nils Caspar Date: Sat, 5 Jan 2013 15:48:39 +0100 Subject: [PATCH 01/20] Added table and model for service whitelist --- db/migrate/20130105152327_create_service_rules.rb | 15 +++++++++++++++ db/schema.rb | 14 +++++++++++++- lib/casino_core/model.rb | 1 + lib/casino_core/model/service_rule.rb | 5 +++++ 4 files changed, 34 insertions(+), 1 deletion(-) create mode 100644 db/migrate/20130105152327_create_service_rules.rb create mode 100644 lib/casino_core/model/service_rule.rb diff --git a/db/migrate/20130105152327_create_service_rules.rb b/db/migrate/20130105152327_create_service_rules.rb new file mode 100644 index 00000000..97978f1e --- /dev/null +++ b/db/migrate/20130105152327_create_service_rules.rb @@ -0,0 +1,15 @@ +class CreateServiceRules < ActiveRecord::Migration + def change + create_table :service_rules do |t| + t.boolean :enabled, null: false, default: true + t.integer :order, null: false, default: 10 + t.string :name, null: false + t.string :url, null: false + t.boolean :regex, null: false, default: false + + t.timestamps + end + + add_index :service_rules, :url, unique: true + end +end diff --git a/db/schema.rb b/db/schema.rb index 4bcec369..3f2ea9af 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -11,7 +11,7 @@ # # It's strongly recommended to check this file into your version control system. -ActiveRecord::Schema.define(:version => 20121231114141) do +ActiveRecord::Schema.define(:version => 20130105152327) do create_table "login_tickets", :force => true do |t| t.string "ticket", :null => false @@ -47,6 +47,18 @@ add_index "proxy_tickets", ["proxy_granting_ticket_id"], :name => "index_proxy_tickets_on_proxy_granting_ticket_id" add_index "proxy_tickets", ["ticket"], :name => "index_proxy_tickets_on_ticket", :unique => true + create_table "service_rules", :force => true do |t| + t.boolean "enabled", :default => true, :null => false + t.integer "order", :default => 10, :null => false + t.string "name", :null => false + t.string "url", :null => false + t.boolean "regex", :default => false, :null => false + t.datetime "created_at", :null => false + t.datetime "updated_at", :null => false + end + + add_index "service_rules", ["url"], :name => "index_service_rules_on_url", :unique => true + create_table "service_tickets", :force => true do |t| t.string "ticket", :null => false t.string "service", :null => false diff --git a/lib/casino_core/model.rb b/lib/casino_core/model.rb index 532e726f..067b8364 100644 --- a/lib/casino_core/model.rb +++ b/lib/casino_core/model.rb @@ -3,6 +3,7 @@ module CASinoCore module Model autoload :LoginTicket, 'casino_core/model/login_ticket.rb' + autoload :ServiceRule, 'casino_core/model/service_rule.rb' autoload :ServiceTicket, 'casino_core/model/service_ticket.rb' autoload :ProxyGrantingTicket, 'casino_core/model/proxy_granting_ticket.rb' autoload :ProxyTicket, 'casino_core/model/proxy_ticket.rb' diff --git a/lib/casino_core/model/service_rule.rb b/lib/casino_core/model/service_rule.rb new file mode 100644 index 00000000..182888df --- /dev/null +++ b/lib/casino_core/model/service_rule.rb @@ -0,0 +1,5 @@ +require 'casino_core/model' + +class CASinoCore::Model::ServiceRule < ActiveRecord::Base + attr_accessible :enabled, :order, :name, :url, :regex +end From e64c2120f6ac94008cea76fdd12382f7a340353f Mon Sep 17 00:00:00 2001 From: Nils Caspar Date: Sat, 5 Jan 2013 16:33:17 +0100 Subject: [PATCH 02/20] Basic functionality if ServiceRule --- lib/casino_core/model/service_rule.rb | 21 +++++++ spec/model/service_rule_spec.rb | 63 +++++++++++++++++++ .../support/factories/service_rule_factory.rb | 16 +++++ 3 files changed, 100 insertions(+) create mode 100644 spec/model/service_rule_spec.rb create mode 100644 spec/support/factories/service_rule_factory.rb diff --git a/lib/casino_core/model/service_rule.rb b/lib/casino_core/model/service_rule.rb index 182888df..ee54a10f 100644 --- a/lib/casino_core/model/service_rule.rb +++ b/lib/casino_core/model/service_rule.rb @@ -2,4 +2,25 @@ class CASinoCore::Model::ServiceRule < ActiveRecord::Base attr_accessible :enabled, :order, :name, :url, :regex + + def self.is_allowed?(service_url) + rules = self.where(enabled: true) + if rules.empty? + true + else + rules.any? { |rule| rule.allows?(service_url) } + end + end + + def allows?(service_url) + if self.regex? + regex = Regexp.new self.url, true + if regex =~ service_url + return true + end + elsif self.url == service_url + return true + end + false + end end diff --git a/spec/model/service_rule_spec.rb b/spec/model/service_rule_spec.rb new file mode 100644 index 00000000..d0fb92c4 --- /dev/null +++ b/spec/model/service_rule_spec.rb @@ -0,0 +1,63 @@ +require 'spec_helper' + +describe CASinoCore::Model::ServiceRule do + context 'with an empty table' do + ['https://www.example.org/', 'http://www.google.com/'].each do |service_url| + it "allows access to #{service_url}" do + described_class.is_allowed?(service_url).should == true + end + end + end + + context 'with a regex rule' do + before(:each) do + FactoryGirl.create :service_rule, :regex, url: '^https://.*$' + end + + ['https://www.example.org/', 'https://www.google.com/'].each do |service_url| + it "allows access to #{service_url}" do + described_class.is_allowed?(service_url).should == true + end + end + + ['http://www.example.org/', 'http://www.google.com/'].each do |service_url| + it "does not allow access to #{service_url}" do + described_class.is_allowed?(service_url).should == false + end + end + end + + context 'with many regex rules' do + before(:each) do + 100.times do |counter| + FactoryGirl.create :service_rule, :regex, url: "^https://www#{counter}.example.com.*$" + end + end + + let(:service_url) { 'https://www111.example.com/bla' } + + it 'does not take too long to check a denied service' do + start = Time.now + described_class.is_allowed?(service_url).should == false + (Time.now - start).should < 0.1 + end + end + + context 'with a non-regex rule' do + before(:each) do + FactoryGirl.create :service_rule, url: 'https://www.google.com/foo' + end + + ['https://www.google.com/foo'].each do |service_url| + it "allows access to #{service_url}" do + described_class.is_allowed?(service_url).should == true + end + end + + ['https://www.example.org/', 'http://www.example.org/', 'https://www.google.com/test'].each do |service_url| + it "does not allow access to #{service_url}" do + described_class.is_allowed?(service_url).should == false + end + end + end +end diff --git a/spec/support/factories/service_rule_factory.rb b/spec/support/factories/service_rule_factory.rb new file mode 100644 index 00000000..37cf6be2 --- /dev/null +++ b/spec/support/factories/service_rule_factory.rb @@ -0,0 +1,16 @@ +require 'factory_girl' + +FactoryGirl.define do + factory :service_rule, class: CASinoCore::Model::ServiceRule do + sequence :order do |n| + n + end + sequence :name do |n| + "Rule #{n}" + end + + trait :regex do + regex true + end + end +end From 2ab5d9ed16f7e1c3265d0d95954fd1c8beed10fa Mon Sep 17 00:00:00 2001 From: Nils Caspar Date: Sat, 5 Jan 2013 16:58:58 +0100 Subject: [PATCH 03/20] Simplified regex --- spec/model/service_rule_spec.rb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/spec/model/service_rule_spec.rb b/spec/model/service_rule_spec.rb index d0fb92c4..5d25e283 100644 --- a/spec/model/service_rule_spec.rb +++ b/spec/model/service_rule_spec.rb @@ -11,7 +11,7 @@ context 'with a regex rule' do before(:each) do - FactoryGirl.create :service_rule, :regex, url: '^https://.*$' + FactoryGirl.create :service_rule, :regex, url: '^https://.*' end ['https://www.example.org/', 'https://www.google.com/'].each do |service_url| @@ -30,7 +30,7 @@ context 'with many regex rules' do before(:each) do 100.times do |counter| - FactoryGirl.create :service_rule, :regex, url: "^https://www#{counter}.example.com.*$" + FactoryGirl.create :service_rule, :regex, url: "^https://www#{counter}.example.com" end end From a746379b5af63c8424b6f38dfcaf70c6e08c8555 Mon Sep 17 00:00:00 2001 From: Nils Caspar Date: Sat, 5 Jan 2013 20:35:46 +0100 Subject: [PATCH 04/20] allowed? instead of is_allowed? --- lib/casino_core/model/service_rule.rb | 2 +- spec/model/service_rule_spec.rb | 80 ++++++++++++++------------- 2 files changed, 42 insertions(+), 40 deletions(-) diff --git a/lib/casino_core/model/service_rule.rb b/lib/casino_core/model/service_rule.rb index ee54a10f..2630181d 100644 --- a/lib/casino_core/model/service_rule.rb +++ b/lib/casino_core/model/service_rule.rb @@ -3,7 +3,7 @@ class CASinoCore::Model::ServiceRule < ActiveRecord::Base attr_accessible :enabled, :order, :name, :url, :regex - def self.is_allowed?(service_url) + def self.allowed?(service_url) rules = self.where(enabled: true) if rules.empty? true diff --git a/spec/model/service_rule_spec.rb b/spec/model/service_rule_spec.rb index 5d25e283..bf4a24d2 100644 --- a/spec/model/service_rule_spec.rb +++ b/spec/model/service_rule_spec.rb @@ -1,62 +1,64 @@ require 'spec_helper' describe CASinoCore::Model::ServiceRule do - context 'with an empty table' do - ['https://www.example.org/', 'http://www.google.com/'].each do |service_url| - it "allows access to #{service_url}" do - described_class.is_allowed?(service_url).should == true + describe '.allowed?' do + context 'with an empty table' do + ['https://www.example.org/', 'http://www.google.com/'].each do |service_url| + it "allows access to #{service_url}" do + described_class.allowed?(service_url).should == true + end end end - end - context 'with a regex rule' do - before(:each) do - FactoryGirl.create :service_rule, :regex, url: '^https://.*' - end + context 'with a regex rule' do + before(:each) do + FactoryGirl.create :service_rule, :regex, url: '^https://.*' + end - ['https://www.example.org/', 'https://www.google.com/'].each do |service_url| - it "allows access to #{service_url}" do - described_class.is_allowed?(service_url).should == true + ['https://www.example.org/', 'https://www.google.com/'].each do |service_url| + it "allows access to #{service_url}" do + described_class.allowed?(service_url).should == true + end end - end - ['http://www.example.org/', 'http://www.google.com/'].each do |service_url| - it "does not allow access to #{service_url}" do - described_class.is_allowed?(service_url).should == false + ['http://www.example.org/', 'http://www.google.com/'].each do |service_url| + it "does not allow access to #{service_url}" do + described_class.allowed?(service_url).should == false + end end end - end - context 'with many regex rules' do - before(:each) do - 100.times do |counter| - FactoryGirl.create :service_rule, :regex, url: "^https://www#{counter}.example.com" + context 'with many regex rules' do + before(:each) do + 100.times do |counter| + FactoryGirl.create :service_rule, :regex, url: "^https://www#{counter}.example.com" + end end - end - let(:service_url) { 'https://www111.example.com/bla' } + let(:service_url) { 'https://www111.example.com/bla' } - it 'does not take too long to check a denied service' do - start = Time.now - described_class.is_allowed?(service_url).should == false - (Time.now - start).should < 0.1 + it 'does not take too long to check a denied service' do + start = Time.now + described_class.allowed?(service_url).should == false + (Time.now - start).should < 0.1 + end end - end - context 'with a non-regex rule' do - before(:each) do - FactoryGirl.create :service_rule, url: 'https://www.google.com/foo' - end + context 'with a non-regex rule' do + before(:each) do + FactoryGirl.create :service_rule, url: 'https://www.google.com/foo' + end - ['https://www.google.com/foo'].each do |service_url| - it "allows access to #{service_url}" do - described_class.is_allowed?(service_url).should == true + ['https://www.google.com/foo'].each do |service_url| + it "allows access to #{service_url}" do + described_class.allowed?(service_url).should == true + end end - end - ['https://www.example.org/', 'http://www.example.org/', 'https://www.google.com/test'].each do |service_url| - it "does not allow access to #{service_url}" do - described_class.is_allowed?(service_url).should == false + ['https://www.example.org/', 'http://www.example.org/', 'https://www.google.com/test'].each do |service_url| + it "does not allow access to #{service_url}" do + described_class.allowed?(service_url).should == false + end end end end From 59c3f8d1bdf4223f943e0c8a2a03931ff6b415d0 Mon Sep 17 00:00:00 2001 From: Nils Caspar Date: Sat, 5 Jan 2013 21:44:15 +0100 Subject: [PATCH 05/20] Check if service is allowed --- lib/casino_core/helper/service_tickets.rb | 10 +++- .../processor/login_credential_requestor.rb | 60 +++++++++++++++---- .../login_credential_requestor_spec.rb | 13 ++++ 3 files changed, 69 insertions(+), 14 deletions(-) diff --git a/lib/casino_core/helper/service_tickets.rb b/lib/casino_core/helper/service_tickets.rb index bf2eca1e..3670fccf 100644 --- a/lib/casino_core/helper/service_tickets.rb +++ b/lib/casino_core/helper/service_tickets.rb @@ -7,10 +7,18 @@ module ServiceTickets include CASinoCore::Helper::Tickets include CASinoCore::Helper::ProxyTickets + class ServiceNotAllowedError < StandardError; end + def acquire_service_ticket(ticket_granting_ticket, service, credentials_supplied = nil) + service_url = clean_service_url(service) + unless CASinoCore::Model::ServiceRule.allowed?(service_url) + message = "#{service_url} is not in the list of allowed URLs" + logger.error message + raise ServiceNotAllowedError, message + end ticket_granting_ticket.service_tickets.create!({ ticket: random_ticket_string('ST'), - service: clean_service_url(service), + service: service_url, issued_from_credentials: !!credentials_supplied }) end diff --git a/lib/casino_core/processor/login_credential_requestor.rb b/lib/casino_core/processor/login_credential_requestor.rb index 44f97921..b87ff4ee 100644 --- a/lib/casino_core/processor/login_credential_requestor.rb +++ b/lib/casino_core/processor/login_credential_requestor.rb @@ -14,27 +14,61 @@ class CASinoCore::Processor::LoginCredentialRequestor < CASinoCore::Processor # The method will call one of the following methods on the listener: # * `#user_logged_in`: The first argument (String) is the URL (if any), the user should be redirected to. # * `#user_not_logged_in`: The first argument is a LoginTicket. It should be stored in a hidden field with name "lt". + # * `#service_not_allowed`: The user tried to access a service that this CAS server is not allowed to serve. # # @param [Hash] params parameters supplied by user # @param [Hash] cookies cookies supplied by user # @param [String] user_agent user-agent delivered by the client def process(params = nil, cookies = nil, user_agent = nil) - params ||= {} - cookies ||= {} - request_env ||= {} - if !params[:renew] && (ticket_granting_ticket = find_valid_ticket_granting_ticket(cookies[:tgt], user_agent)) - service_url_with_ticket = unless params[:service].nil? - acquire_service_ticket(ticket_granting_ticket, params[:service], true).service_with_ticket_url - end - @listener.user_logged_in(service_url_with_ticket) + @params = params || {} + @cookies = cookies || {} + @user_agent = user_agent || {} + if check_service_allowed + handle_allowed_service + end + end + + private + def handle_allowed_service + if !@params[:renew] && (@ticket_granting_ticket = find_valid_ticket_granting_ticket(@cookies[:tgt], @user_agent)) + handle_logged_in else - if params[:gateway] == 'true' && params[:service] - # we actually lie to the listener to simplify things - @listener.user_logged_in(params[:service]) + handle_not_logged_in + end + end + + def handle_logged_in + service_url_with_ticket = unless @params[:service].nil? + acquire_service_ticket(@ticket_granting_ticket, @params[:service], true).service_with_ticket_url + end + @listener.user_logged_in(service_url_with_ticket) + end + + def handle_not_logged_in + if gateway_request? + # we actually lie to the listener to simplify things + @listener.user_logged_in(@params[:service]) + else + login_ticket = acquire_login_ticket + @listener.user_not_logged_in(login_ticket) + end + end + + def check_service_allowed + if @params[:service].nil? + true + else + service_url = clean_service_url(@params[:service]) + if CASinoCore::Model::ServiceRule.allowed?(service_url) + true else - login_ticket = acquire_login_ticket - @listener.user_not_logged_in(login_ticket) + @listener.service_not_allowed(service_url) + false end end end + + def gateway_request? + @params[:gateway] == 'true' && @params[:service] + end end diff --git a/spec/processor/login_credential_requestor_spec.rb b/spec/processor/login_credential_requestor_spec.rb index 08ecdbc1..834318e2 100644 --- a/spec/processor/login_credential_requestor_spec.rb +++ b/spec/processor/login_credential_requestor_spec.rb @@ -5,6 +5,19 @@ let(:listener) { Object.new } let(:processor) { described_class.new(listener) } + context 'with a not allowed service' do + before(:each) do + FactoryGirl.create :service_rule, :regex, url: '^https://.*' + end + let(:service) { 'http://www.example.org/' } + let(:params) { { service: service } } + + it 'calls the #service_not_allowed method on the listener' do + listener.should_receive(:service_not_allowed).with(service) + processor.process(params) + end + end + context 'when logged out' do it 'calls the #user_not_logged_in method on the listener' do listener.should_receive(:user_not_logged_in).with(kind_of(CASinoCore::Model::LoginTicket)) From da7a951d4654e699b31d0d0202938c0cb65399a3 Mon Sep 17 00:00:00 2001 From: Nils Caspar Date: Sun, 6 Jan 2013 09:56:17 +0100 Subject: [PATCH 06/20] Refactoring --- .../processor/login_credential_requestor.rb | 12 ++++-------- 1 file changed, 4 insertions(+), 8 deletions(-) diff --git a/lib/casino_core/processor/login_credential_requestor.rb b/lib/casino_core/processor/login_credential_requestor.rb index b87ff4ee..a53d1906 100644 --- a/lib/casino_core/processor/login_credential_requestor.rb +++ b/lib/casino_core/processor/login_credential_requestor.rb @@ -55,16 +55,12 @@ def handle_not_logged_in end def check_service_allowed - if @params[:service].nil? + service_url = clean_service_url(@params[:service]) unless @params[:service].nil? + if service_url.nil? || CASinoCore::Model::ServiceRule.allowed?(service_url) true else - service_url = clean_service_url(@params[:service]) - if CASinoCore::Model::ServiceRule.allowed?(service_url) - true - else - @listener.service_not_allowed(service_url) - false - end + @listener.service_not_allowed(service_url) + false end end From 1ca58b0f6f3e511fd7114baebaeac00be141ef6d Mon Sep 17 00:00:00 2001 From: Nils Caspar Date: Sun, 6 Jan 2013 09:57:16 +0100 Subject: [PATCH 07/20] Handle not allowed service --- .../processor/login_credential_acceptor.rb | 38 ++++++++++++------- .../login_credential_acceptor_spec.rb | 12 ++++++ 2 files changed, 37 insertions(+), 13 deletions(-) diff --git a/lib/casino_core/processor/login_credential_acceptor.rb b/lib/casino_core/processor/login_credential_acceptor.rb index 2db243ef..ee89db2e 100644 --- a/lib/casino_core/processor/login_credential_acceptor.rb +++ b/lib/casino_core/processor/login_credential_acceptor.rb @@ -22,19 +22,11 @@ class CASinoCore::Processor::LoginCredentialAcceptor < CASinoCore::Processor # @param [Hash] cookies cookies supplied by user # @param [String] user_agent user-agent delivered by the client def process(params = nil, cookies = nil, user_agent = nil) - params ||= {} - cookies ||= {} - if login_ticket_valid?(params[:lt]) - authentication_result = validate_login_credentials(params[:username], params[:password]) - if !authentication_result.nil? - ticket_granting_ticket = acquire_ticket_granting_ticket(authentication_result, user_agent) - url = unless params[:service].nil? - acquire_service_ticket(ticket_granting_ticket, params[:service], true).service_with_ticket_url - end - @listener.user_logged_in(url, ticket_granting_ticket.ticket) - else - @listener.invalid_login_credentials(acquire_login_ticket) - end + @params = params || {} + @cookies = cookies || {} + @user_agent = user_agent + if login_ticket_valid?(@params[:lt]) + authenticate_user else @listener.invalid_login_ticket(acquire_login_ticket) end @@ -56,4 +48,24 @@ def login_ticket_valid?(lt) end end + def authenticate_user + authentication_result = validate_login_credentials(@params[:username], @params[:password]) + if !authentication_result.nil? + user_logged_in(authentication_result) + else + @listener.invalid_login_credentials(acquire_login_ticket) + end + end + + def user_logged_in(authentication_result) + begin + ticket_granting_ticket = acquire_ticket_granting_ticket(authentication_result, @user_agent) + url = unless @params[:service].nil? + acquire_service_ticket(ticket_granting_ticket, @params[:service], true).service_with_ticket_url + end + @listener.user_logged_in(url, ticket_granting_ticket.ticket) + rescue ServiceNotAllowedError => e + @listener.service_not_allowed(clean_service_url @params[:service]) + end + end end diff --git a/spec/processor/login_credential_acceptor_spec.rb b/spec/processor/login_credential_acceptor_spec.rb index ba75e611..44c6bd32 100644 --- a/spec/processor/login_credential_acceptor_spec.rb +++ b/spec/processor/login_credential_acceptor_spec.rb @@ -39,6 +39,18 @@ listener.stub(:user_logged_in) end + context 'with a not allowed service' do + before(:each) do + FactoryGirl.create :service_rule, :regex, url: '^https://.*' + end + let(:service) { 'http://www.example.org/' } + + it 'calls the #service_not_allowed method on the listener' do + listener.should_receive(:service_not_allowed).with(service) + processor.process(login_data) + end + end + context 'when all authenticators raise an error' do before(:each) do CASinoCore::Authenticator::Static.any_instance.stub(:validate) do From 2557d6e057667af86edd0dd6cd75af8aa8564992 Mon Sep 17 00:00:00 2001 From: Nils Caspar Date: Sun, 6 Jan 2013 09:57:51 +0100 Subject: [PATCH 08/20] Refactoring --- lib/casino_core/helper/login_tickets.rb | 15 +++++++++++++++ .../processor/login_credential_acceptor.rb | 15 --------------- 2 files changed, 15 insertions(+), 15 deletions(-) diff --git a/lib/casino_core/helper/login_tickets.rb b/lib/casino_core/helper/login_tickets.rb index e65385cc..44a37a21 100644 --- a/lib/casino_core/helper/login_tickets.rb +++ b/lib/casino_core/helper/login_tickets.rb @@ -9,6 +9,21 @@ def acquire_login_ticket logger.debug "Created login ticket '#{ticket.ticket}'" ticket end + + def login_ticket_valid?(lt) + ticket = CASinoCore::Model::LoginTicket.find_by_ticket lt + if ticket.nil? + logger.info "Login ticket '#{lt}' not found" + false + elsif ticket.created_at < CASinoCore::Settings.login_ticket[:lifetime].seconds.ago + logger.info "Login ticket '#{ticket.ticket}' expired" + false + else + logger.debug "Login ticket '#{ticket.ticket}' successfully validated" + ticket.delete + true + end + end end end end diff --git a/lib/casino_core/processor/login_credential_acceptor.rb b/lib/casino_core/processor/login_credential_acceptor.rb index ee89db2e..e515c5a0 100644 --- a/lib/casino_core/processor/login_credential_acceptor.rb +++ b/lib/casino_core/processor/login_credential_acceptor.rb @@ -33,21 +33,6 @@ def process(params = nil, cookies = nil, user_agent = nil) end private - def login_ticket_valid?(lt) - ticket = CASinoCore::Model::LoginTicket.find_by_ticket lt - if ticket.nil? - logger.info "Login ticket '#{lt}' not found" - false - elsif ticket.created_at < CASinoCore::Settings.login_ticket[:lifetime].seconds.ago - logger.info "Login ticket '#{ticket.ticket}' expired" - false - else - logger.debug "Login ticket '#{ticket.ticket}' successfully validated" - ticket.delete - true - end - end - def authenticate_user authentication_result = validate_login_credentials(@params[:username], @params[:password]) if !authentication_result.nil? From e3c4c0de83932156d93d69a88e4095055e74187f Mon Sep 17 00:00:00 2001 From: Nils Caspar Date: Sun, 6 Jan 2013 10:02:57 +0100 Subject: [PATCH 09/20] Documentation --- lib/casino_core/processor/login_credential_acceptor.rb | 1 + 1 file changed, 1 insertion(+) diff --git a/lib/casino_core/processor/login_credential_acceptor.rb b/lib/casino_core/processor/login_credential_acceptor.rb index e515c5a0..4e607671 100644 --- a/lib/casino_core/processor/login_credential_acceptor.rb +++ b/lib/casino_core/processor/login_credential_acceptor.rb @@ -17,6 +17,7 @@ class CASinoCore::Processor::LoginCredentialAcceptor < CASinoCore::Processor # The second argument (String) is the ticket-granting ticket. It should be stored in a cookie named "tgt". # * `#invalid_login_ticket` and `#invalid_login_credentials`: The first argument is a LoginTicket. # See {CASinoCore::Processor::LoginCredentialRequestor} for details. + # * `#service_not_allowed`: The user tried to access a service that this CAS server is not allowed to serve. # # @param [Hash] params parameters supplied by user # @param [Hash] cookies cookies supplied by user From d01667491b3ce599884fb784d32e26a6796f19ad Mon Sep 17 00:00:00 2001 From: Nils Caspar Date: Sun, 6 Jan 2013 10:03:12 +0100 Subject: [PATCH 10/20] No need for cookies here --- lib/casino_core/processor/login_credential_acceptor.rb | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/lib/casino_core/processor/login_credential_acceptor.rb b/lib/casino_core/processor/login_credential_acceptor.rb index 4e607671..3f0df8bb 100644 --- a/lib/casino_core/processor/login_credential_acceptor.rb +++ b/lib/casino_core/processor/login_credential_acceptor.rb @@ -20,11 +20,9 @@ class CASinoCore::Processor::LoginCredentialAcceptor < CASinoCore::Processor # * `#service_not_allowed`: The user tried to access a service that this CAS server is not allowed to serve. # # @param [Hash] params parameters supplied by user - # @param [Hash] cookies cookies supplied by user # @param [String] user_agent user-agent delivered by the client - def process(params = nil, cookies = nil, user_agent = nil) + def process(params = nil, user_agent = nil) @params = params || {} - @cookies = cookies || {} @user_agent = user_agent if login_ticket_valid?(@params[:lt]) authenticate_user From 596b5f942adf5454f017accc8d6fd84287af9d37 Mon Sep 17 00:00:00 2001 From: Nils Caspar Date: Sun, 6 Jan 2013 18:02:01 +0100 Subject: [PATCH 11/20] Handle ServiceNotAllowedError --- .../processor/api/service_ticket_provider.rb | 13 +++++++++++-- .../processor/api/service_ticket_provider_spec.rb | 15 ++++++++++++++- 2 files changed, 25 insertions(+), 3 deletions(-) diff --git a/lib/casino_core/processor/api/service_ticket_provider.rb b/lib/casino_core/processor/api/service_ticket_provider.rb index 120d2a02..76299fe9 100644 --- a/lib/casino_core/processor/api/service_ticket_provider.rb +++ b/lib/casino_core/processor/api/service_ticket_provider.rb @@ -15,6 +15,7 @@ class CASinoCore::Processor::API::ServiceTicketProvider < CASinoCore::Processor # The service ticket (and nothing else) should be displayed. # * `#invalid_ticket_granting_ticket_via_api`: No argument. The application should respond with status "400 Bad Request" # * `#no_service_provided_via_api`: No argument. The application should respond with status "400 Bad Request" + # * `#service_not_allowed_via_api`: The user tried to access a service that this CAS server is not allowed to serve. # # @param [String] ticket_granting_ticket ticket_granting_ticket supplied by the user in the URL # @param [Hash] parameters parameters supplied by user (`service` in particular) @@ -37,8 +38,12 @@ def fetch_valid_ticket_granting_ticket def handle_ticket_granting_ticket case when (@service_url and @ticket_granting_ticket) - create_service_ticket - callback_granted_service_ticket + begin + create_service_ticket + callback_granted_service_ticket + rescue ServiceNotAllowedError => e + callback_service_not_allowed + end when (@service_url and not @ticket_granting_ticket) callback_invalid_tgt when (not @service_url and @ticket_granting_ticket) @@ -62,4 +67,8 @@ def callback_empty_service @listener.no_service_provided_via_api end + def callback_service_not_allowed + @listener.service_not_allowed_via_api(clean_service_url @service_url) + end + end diff --git a/spec/processor/api/service_ticket_provider_spec.rb b/spec/processor/api/service_ticket_provider_spec.rb index f702a50c..3180ff32 100644 --- a/spec/processor/api/service_ticket_provider_spec.rb +++ b/spec/processor/api/service_ticket_provider_spec.rb @@ -5,7 +5,8 @@ let(:listener) { Object.new } let(:processor) { described_class.new(listener) } - let(:parameters) { { service: 'http://example.org/' } } + let(:service) { 'http://example.org/' } + let(:parameters) { { service: service } } context 'with an invalid ticket-granting ticket' do let(:ticket_granting_ticket) { 'TGT-INVALID' } @@ -21,6 +22,18 @@ let(:ticket) { ticket_granting_ticket.ticket } let(:user_agent) { ticket_granting_ticket.user_agent } + context 'with a not allowed service' do + before(:each) do + FactoryGirl.create :service_rule, :regex, url: '^https://.*' + end + let(:service) { 'http://www.example.org/' } + + it 'calls the #service_not_allowed method on the listener' do + listener.should_receive(:service_not_allowed_via_api).with(service) + processor.process(ticket, parameters, user_agent) + end + end + it 'calls the #granted_service_ticket_via_api method on the listener' do listener.should_receive(:granted_service_ticket_via_api).with(/^ST\-/) processor.process(ticket, parameters, user_agent) From b183561e815c7dcb0223295afc0c3a66c81bca98 Mon Sep 17 00:00:00 2001 From: Nils Caspar Date: Fri, 11 Jan 2013 14:59:09 +0100 Subject: [PATCH 12/20] Rake tasks for service rule management --- casino_core.gemspec | 1 + lib/casino_core/helper/service_tickets.rb | 2 +- lib/casino_core/rake_tasks.rb | 1 + lib/casino_core/tasks/service_rule.rake | 46 +++++++++++++++++++++++ 4 files changed, 49 insertions(+), 1 deletion(-) create mode 100644 lib/casino_core/tasks/service_rule.rake diff --git a/casino_core.gemspec b/casino_core.gemspec index 511af809..c8eda1c4 100644 --- a/casino_core.gemspec +++ b/casino_core.gemspec @@ -29,6 +29,7 @@ Gem::Specification.new do |s| s.add_runtime_dependency 'activerecord', '~> 3.2.9' s.add_runtime_dependency 'addressable', '~> 2.3' + s.add_runtime_dependency 'terminal-table', '~> 1.4' s.add_runtime_dependency 'useragent', '~> 0.4' end diff --git a/lib/casino_core/helper/service_tickets.rb b/lib/casino_core/helper/service_tickets.rb index 3670fccf..c670476e 100644 --- a/lib/casino_core/helper/service_tickets.rb +++ b/lib/casino_core/helper/service_tickets.rb @@ -33,7 +33,7 @@ def clean_service_url(dirty_service) service_uri.query_values = nil end if "#{service_uri.path}".length > 1 - service_uri.path = service_uri.path.gsub(/\/\z/, '') + service_uri.path = service_uri.path.gsub(/\/+\z/, '') end clean_service = service_uri.to_s diff --git a/lib/casino_core/rake_tasks.rb b/lib/casino_core/rake_tasks.rb index 04668c3a..e95e2be6 100644 --- a/lib/casino_core/rake_tasks.rb +++ b/lib/casino_core/rake_tasks.rb @@ -5,6 +5,7 @@ def load_tasks %w( database cleanup + service_rule ).each do |task| load "casino_core/tasks/#{task}.rake" end diff --git a/lib/casino_core/tasks/service_rule.rake b/lib/casino_core/tasks/service_rule.rake new file mode 100644 index 00000000..c84e2352 --- /dev/null +++ b/lib/casino_core/tasks/service_rule.rake @@ -0,0 +1,46 @@ +require 'terminal-table' +require 'casino_core/model' +require 'casino_core/helper/service_tickets' + +namespace :casino_core do + namespace :service_rule do + include CASinoCore::Helper::ServiceTickets + + desc 'Add a service rule (prefix the url parameter with "regex:" to add a regular expression)' + task :add, [:name, :url] => 'casino_core:db:configure_connection' do |task, args| + match = /^regex:(.*)/.match(args[:url]) + if match.nil? + url = clean_service_url(args[:url]) + regex = false + else + url = match[1] + regex = true + end + CASinoCore::Model::ServiceRule.create! name: args[:name], url: url, regex: regex + end + + desc 'Remove a servcice rule.' + task :delete, [:id] => 'casino_core:db:configure_connection' do |task, args| + CASinoCore::Model::ServiceRule.find(args[:id]).destroy + end + + desc 'Delete all servcice rules.' + task :flush => 'casino_core:db:configure_connection' do |task, args| + CASinoCore::Model::ServiceRule.delete_all + end + + desc 'List all service rules.' + task list: 'casino_core:db:configure_connection' do + table = Terminal::Table.new :headings => ['ID', 'Name', 'URL'] do |t| + CASinoCore::Model::ServiceRule.all.each do |service_rule| + url = service_rule.url + if service_rule.regex? + url += " (Regex)" + end + t.add_row [service_rule.id, service_rule.name, url] + end + end + puts table + end + end +end From 90a2b24bc24d633a2602be28eed9e465975a6b71 Mon Sep 17 00:00:00 2001 From: Nils Caspar Date: Fri, 11 Jan 2013 14:59:19 +0100 Subject: [PATCH 13/20] Updated gems --- Gemfile.lock | 18 ++++++++++-------- 1 file changed, 10 insertions(+), 8 deletions(-) diff --git a/Gemfile.lock b/Gemfile.lock index f5bcfac1..a781267b 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -4,26 +4,27 @@ PATH casino_core (1.0.12) activerecord (~> 3.2.9) addressable (~> 2.3) + terminal-table (~> 1.4) useragent (~> 0.4) GEM remote: http://rubygems.org/ specs: - activemodel (3.2.9) - activesupport (= 3.2.9) + activemodel (3.2.11) + activesupport (= 3.2.11) builder (~> 3.0.0) - activerecord (3.2.9) - activemodel (= 3.2.9) - activesupport (= 3.2.9) + activerecord (3.2.11) + activemodel (= 3.2.11) + activesupport (= 3.2.11) arel (~> 3.0.2) tzinfo (~> 0.3.29) - activesupport (3.2.9) + activesupport (3.2.11) i18n (~> 0.6) multi_json (~> 1.0) addressable (2.3.2) arel (3.0.2) builder (3.0.4) - crack (0.3.1) + crack (0.3.2) database_cleaner (0.9.1) diff-lcs (1.1.3) factory_girl (4.1.0) @@ -45,8 +46,9 @@ GEM simplecov-html (~> 0.7.1) simplecov-html (0.7.1) sqlite3 (1.3.6) + terminal-table (1.4.5) tzinfo (0.3.35) - useragent (0.4.15) + useragent (0.4.16) webmock (1.9.0) addressable (>= 2.2.7) crack (>= 0.1.7) From 9a55623042f3530ccc146b4e094e6017e61ac709 Mon Sep 17 00:00:00 2001 From: Nils Caspar Date: Fri, 11 Jan 2013 23:17:43 +0100 Subject: [PATCH 14/20] Don't strip last / in uri path --- lib/casino_core/helper/service_tickets.rb | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/lib/casino_core/helper/service_tickets.rb b/lib/casino_core/helper/service_tickets.rb index c670476e..3e5a51a0 100644 --- a/lib/casino_core/helper/service_tickets.rb +++ b/lib/casino_core/helper/service_tickets.rb @@ -32,9 +32,10 @@ def clean_service_url(dirty_service) if service_uri.query_values.blank? service_uri.query_values = nil end - if "#{service_uri.path}".length > 1 - service_uri.path = service_uri.path.gsub(/\/+\z/, '') - end + + service_uri.path = (service_uri.path || '').gsub(/\/+\z/, '') + service_uri.path = '/' if service_uri.path.blank? + clean_service = service_uri.to_s logger.debug("Cleaned dirty service URL '#{dirty_service}' to '#{clean_service}'") if dirty_service != clean_service From 5b9a319b91c95dcd514fb4f0436b28152c22aa84 Mon Sep 17 00:00:00 2001 From: Nils Caspar Date: Fri, 11 Jan 2013 23:17:58 +0100 Subject: [PATCH 15/20] Some validation --- lib/casino_core/model/service_rule.rb | 2 ++ 1 file changed, 2 insertions(+) diff --git a/lib/casino_core/model/service_rule.rb b/lib/casino_core/model/service_rule.rb index 2630181d..646d88e7 100644 --- a/lib/casino_core/model/service_rule.rb +++ b/lib/casino_core/model/service_rule.rb @@ -2,6 +2,8 @@ class CASinoCore::Model::ServiceRule < ActiveRecord::Base attr_accessible :enabled, :order, :name, :url, :regex + validates :name, presence: true + validates :url, uniqueness: true, presence: true def self.allowed?(service_url) rules = self.where(enabled: true) From d0f9cc1b31d7696e0c99ac4a6104cb49b622f328 Mon Sep 17 00:00:00 2001 From: Nils Caspar Date: Fri, 11 Jan 2013 23:18:19 +0100 Subject: [PATCH 16/20] More output --- lib/casino_core/tasks/service_rule.rake | 22 ++++++++++++++-------- 1 file changed, 14 insertions(+), 8 deletions(-) diff --git a/lib/casino_core/tasks/service_rule.rake b/lib/casino_core/tasks/service_rule.rake index c84e2352..2cf0c4ef 100644 --- a/lib/casino_core/tasks/service_rule.rake +++ b/lib/casino_core/tasks/service_rule.rake @@ -8,36 +8,42 @@ namespace :casino_core do desc 'Add a service rule (prefix the url parameter with "regex:" to add a regular expression)' task :add, [:name, :url] => 'casino_core:db:configure_connection' do |task, args| + service_rule = CASinoCore::Model::ServiceRule.new name: args[:name] match = /^regex:(.*)/.match(args[:url]) if match.nil? - url = clean_service_url(args[:url]) - regex = false + service_rule.url = clean_service_url(args[:url]) else - url = match[1] - regex = true + service_rule.url = match[1] + service_rule.regex = true + end + if !service_rule.save + fail service_rule.errors.full_messages.join("\n") + elsif service_rule.regex && service_rule.url[0] != '^' + puts 'Warning: Potentially unsafe regex! Use ^ to match the beginning of the URL. Example: ^https://' end - CASinoCore::Model::ServiceRule.create! name: args[:name], url: url, regex: regex end desc 'Remove a servcice rule.' task :delete, [:id] => 'casino_core:db:configure_connection' do |task, args| - CASinoCore::Model::ServiceRule.find(args[:id]).destroy + CASinoCore::Model::ServiceRule.find(args[:id]).delete + puts "Successfully deleted service rule ##{args[:id]}." end desc 'Delete all servcice rules.' task :flush => 'casino_core:db:configure_connection' do |task, args| CASinoCore::Model::ServiceRule.delete_all + puts 'Successfully deleted all service rules.' end desc 'List all service rules.' task list: 'casino_core:db:configure_connection' do - table = Terminal::Table.new :headings => ['ID', 'Name', 'URL'] do |t| + table = Terminal::Table.new :headings => ['Enabled', 'ID', 'Name', 'URL'] do |t| CASinoCore::Model::ServiceRule.all.each do |service_rule| url = service_rule.url if service_rule.regex? url += " (Regex)" end - t.add_row [service_rule.id, service_rule.name, url] + t.add_row [service_rule.enabled, service_rule.id, service_rule.name, url] end end puts table From e83a73fc1e1258ca0471a51abafcf052980a0adb Mon Sep 17 00:00:00 2001 From: Nils Caspar Date: Sat, 12 Jan 2013 12:01:31 +0100 Subject: [PATCH 17/20] Fixed Specs --- spec/processor/login_credential_acceptor_spec.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/spec/processor/login_credential_acceptor_spec.rb b/spec/processor/login_credential_acceptor_spec.rb index 44c6bd32..cb73ec88 100644 --- a/spec/processor/login_credential_acceptor_spec.rb +++ b/spec/processor/login_credential_acceptor_spec.rb @@ -83,7 +83,7 @@ let(:service) { 'https://www.example.com' } it 'calls the #user_logged_in method on the listener' do - listener.should_receive(:user_logged_in).with(/^#{service}\?ticket=ST\-/, /^TGC\-/) + listener.should_receive(:user_logged_in).with(/^#{service}\/\?ticket=ST\-/, /^TGC\-/) processor.process(login_data) end From cdb2fb1e4f54e5e3815e3d374e5e53ec2d6ab898 Mon Sep 17 00:00:00 2001 From: Nils Caspar Date: Sat, 2 Feb 2013 14:40:33 +0100 Subject: [PATCH 18/20] Upgrade information --- UPGRADE.md | 10 ++++++++++ 1 file changed, 10 insertions(+) create mode 100644 UPGRADE.md diff --git a/UPGRADE.md b/UPGRADE.md new file mode 100644 index 00000000..ff157ca7 --- /dev/null +++ b/UPGRADE.md @@ -0,0 +1,10 @@ +# Upgrade CASinoCore + +Here is a list of backward-incompatible changes that were introduced. + +## 1.1.0 + +New callbacks: + +* `login_credential_requestor` calls `#service_not_allowed` on the listener, when a service is not in the service whitelist. +* `api/service_ticket_provider` calls `#service_not_allowed_via_api` on the listener, when a service is not in the service whitelist. From 13c1d67117a0e360cefd9f0f484b800ab8bf9e1e Mon Sep 17 00:00:00 2001 From: Nils Caspar Date: Sat, 2 Feb 2013 15:07:33 +0100 Subject: [PATCH 19/20] More Upgrade hints --- UPGRADE.md | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/UPGRADE.md b/UPGRADE.md index ff157ca7..bcd02c90 100644 --- a/UPGRADE.md +++ b/UPGRADE.md @@ -4,6 +4,10 @@ Here is a list of backward-incompatible changes that were introduced. ## 1.1.0 +API changes: + +* `login_credential_acceptor`: The parameters of `#process` changed from `params, cookies, user_agent` to just `params, user_agent` + New callbacks: * `login_credential_requestor` calls `#service_not_allowed` on the listener, when a service is not in the service whitelist. From 212d0773caa8153909eb0f4c6eec8200b1831ba6 Mon Sep 17 00:00:00 2001 From: Nils Caspar Date: Sat, 2 Feb 2013 15:08:12 +0100 Subject: [PATCH 20/20] Removed unused variable --- lib/casino_core/processor/api/service_ticket_provider.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/casino_core/processor/api/service_ticket_provider.rb b/lib/casino_core/processor/api/service_ticket_provider.rb index 76299fe9..29432de9 100644 --- a/lib/casino_core/processor/api/service_ticket_provider.rb +++ b/lib/casino_core/processor/api/service_ticket_provider.rb @@ -41,7 +41,7 @@ def handle_ticket_granting_ticket begin create_service_ticket callback_granted_service_ticket - rescue ServiceNotAllowedError => e + rescue ServiceNotAllowedError callback_service_not_allowed end when (@service_url and not @ticket_granting_ticket)