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) diff --git a/UPGRADE.md b/UPGRADE.md new file mode 100644 index 00000000..bcd02c90 --- /dev/null +++ b/UPGRADE.md @@ -0,0 +1,14 @@ +# Upgrade CASinoCore + +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. +* `api/service_ticket_provider` calls `#service_not_allowed_via_api` on the listener, when a service is not in the service whitelist. 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/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/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/helper/service_tickets.rb b/lib/casino_core/helper/service_tickets.rb index bf2eca1e..3e5a51a0 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 @@ -24,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 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..646d88e7 --- /dev/null +++ b/lib/casino_core/model/service_rule.rb @@ -0,0 +1,28 @@ +require 'casino_core/model' + +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) + 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/lib/casino_core/processor/api/service_ticket_provider.rb b/lib/casino_core/processor/api/service_ticket_provider.rb index 120d2a02..29432de9 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 + 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/lib/casino_core/processor/login_credential_acceptor.rb b/lib/casino_core/processor/login_credential_acceptor.rb index 2db243ef..3f0df8bb 100644 --- a/lib/casino_core/processor/login_credential_acceptor.rb +++ b/lib/casino_core/processor/login_credential_acceptor.rb @@ -17,43 +17,39 @@ 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 # @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 + def process(params = nil, user_agent = nil) + @params = params || {} + @user_agent = user_agent + if login_ticket_valid?(@params[:lt]) + authenticate_user else @listener.invalid_login_ticket(acquire_login_ticket) end 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 + def authenticate_user + authentication_result = validate_login_credentials(@params[:username], @params[:password]) + if !authentication_result.nil? + user_logged_in(authentication_result) else - logger.debug "Login ticket '#{ticket.ticket}' successfully validated" - ticket.delete - true + @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/lib/casino_core/processor/login_credential_requestor.rb b/lib/casino_core/processor/login_credential_requestor.rb index 44f97921..a53d1906 100644 --- a/lib/casino_core/processor/login_credential_requestor.rb +++ b/lib/casino_core/processor/login_credential_requestor.rb @@ -14,27 +14,57 @@ 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 + 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 + service_url = clean_service_url(@params[:service]) unless @params[:service].nil? + if service_url.nil? || CASinoCore::Model::ServiceRule.allowed?(service_url) + true else - if params[:gateway] == 'true' && params[:service] - # 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 + @listener.service_not_allowed(service_url) + false end end + + def gateway_request? + @params[:gateway] == 'true' && @params[:service] + end end 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..2cf0c4ef --- /dev/null +++ b/lib/casino_core/tasks/service_rule.rake @@ -0,0 +1,52 @@ +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| + service_rule = CASinoCore::Model::ServiceRule.new name: args[:name] + match = /^regex:(.*)/.match(args[:url]) + if match.nil? + service_rule.url = clean_service_url(args[:url]) + else + 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 + end + + desc 'Remove a servcice rule.' + task :delete, [:id] => 'casino_core:db:configure_connection' do |task, args| + 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 => ['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.enabled, service_rule.id, service_rule.name, url] + end + end + puts table + end + end +end diff --git a/spec/model/service_rule_spec.rb b/spec/model/service_rule_spec.rb new file mode 100644 index 00000000..bf4a24d2 --- /dev/null +++ b/spec/model/service_rule_spec.rb @@ -0,0 +1,65 @@ +require 'spec_helper' + +describe CASinoCore::Model::ServiceRule do + 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 + + 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.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.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.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.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.allowed?(service_url).should == false + end + end + end + 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) diff --git a/spec/processor/login_credential_acceptor_spec.rb b/spec/processor/login_credential_acceptor_spec.rb index ba75e611..cb73ec88 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 @@ -71,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 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)) 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