diff --git a/CHANGELOG.md b/CHANGELOG.md index 064ad7c..5e45622 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -3,6 +3,10 @@ * Drop support for Rails 6 * Drop support for Ruby 3.0 +* Adds config option that allows apps to change the default behaviour of intercepting 401 requests, by including an initialiser: `GDS::SSO.config { |config| config.intercept_401_responses = false }`. +* Added AuthorisedUserConstraint class so that consumer apps can easily add permission based constraints. +* Adds deprecation warning for GDS::SSO::ControllerMethods::PermissionDeniedException. If your app uses this you can replace it with GDS::SSO::PermissionDeniedError. + # 19.0.0 * We no longer set `ActiveSupport::Cache::NullStore.new` as the default cache. This avoids a deprecation warning when the gem is used in Rails apps. diff --git a/lib/gds-sso.rb b/lib/gds-sso.rb index f255cbe..e5808e4 100644 --- a/lib/gds-sso.rb +++ b/lib/gds-sso.rb @@ -10,10 +10,13 @@ module GDS module SSO - autoload :FailureApp, "gds-sso/failure_app" - autoload :ControllerMethods, "gds-sso/controller_methods" - autoload :User, "gds-sso/user" - autoload :ApiAccess, "gds-sso/api_access" + autoload :FailureApp, "gds-sso/failure_app" + autoload :ControllerMethods, "gds-sso/controller_methods" + autoload :User, "gds-sso/user" + autoload :ApiAccess, "gds-sso/api_access" + autoload :AuthoriseUser, "gds-sso/authorise_user" + autoload :AuthorisedUserConstraint, "gds-sso/authorised_user_constraint" + autoload :PermissionDeniedError, "gds-sso/controller_methods" # User to return as logged in during tests mattr_accessor :test_user @@ -52,6 +55,7 @@ def self.default_strategies config.app_middleware.use Warden::Manager do |config| config.default_strategies(*default_strategies) config.failure_app = GDS::SSO::FailureApp + config.intercept_401 = GDS::SSO::Config.intercept_401_responses end end end diff --git a/lib/gds-sso/authorise_user.rb b/lib/gds-sso/authorise_user.rb new file mode 100644 index 0000000..6f62ce4 --- /dev/null +++ b/lib/gds-sso/authorise_user.rb @@ -0,0 +1,49 @@ +module GDS + module SSO + class AuthoriseUser + def self.call(...) = new(...).call + + def initialize(current_user, permissions) + @current_user = current_user + @permissions = permissions + end + + def call + case permissions + when String + unless current_user.has_permission?(permissions) + raise GDS::SSO::PermissionDeniedError, "Sorry, you don't seem to have the #{permissions} permission for this app." + end + when Hash + raise ArgumentError, "Must be either `any_of` or `all_of`" unless permissions.keys.size == 1 + + if permissions[:any_of] + authorise_user_with_at_least_one_of_permissions!(permissions[:any_of]) + elsif permissions[:all_of] + authorise_user_with_all_permissions!(permissions[:all_of]) + else + raise ArgumentError, "Must be either `any_of` or `all_of`" + end + end + end + + private + + attr_reader :current_user, :permissions + + def authorise_user_with_at_least_one_of_permissions!(permissions) + if permissions.none? { |permission| current_user.has_permission?(permission) } + raise GDS::SSO::PermissionDeniedError, + "Sorry, you don't seem to have any of the permissions: #{permissions.to_sentence} for this app." + end + end + + def authorise_user_with_all_permissions!(permissions) + unless permissions.all? { |permission| current_user.has_permission?(permission) } + raise GDS::SSO::PermissionDeniedError, + "Sorry, you don't seem to have all of the permissions: #{permissions.to_sentence} for this app." + end + end + end + end +end diff --git a/lib/gds-sso/authorised_user_constraint.rb b/lib/gds-sso/authorised_user_constraint.rb new file mode 100644 index 0000000..4c32525 --- /dev/null +++ b/lib/gds-sso/authorised_user_constraint.rb @@ -0,0 +1,21 @@ +module GDS + module SSO + class AuthorisedUserConstraint + def initialize(permissions) + @permissions = permissions + end + + def matches?(request) + warden = request.env["warden"] + warden.authenticate! if !warden.authenticated? || warden.user.remotely_signed_out? + + GDS::SSO::AuthoriseUser.call(warden.user, permissions) + true + end + + private + + attr_reader :permissions + end + end +end diff --git a/lib/gds-sso/config.rb b/lib/gds-sso/config.rb index c591635..ad14420 100644 --- a/lib/gds-sso/config.rb +++ b/lib/gds-sso/config.rb @@ -29,6 +29,9 @@ module Config mattr_accessor :api_only + mattr_accessor :intercept_401_responses + @@intercept_401_responses = true + mattr_accessor :additional_mock_permissions_required mattr_accessor :connection_opts diff --git a/lib/gds-sso/controller_methods.rb b/lib/gds-sso/controller_methods.rb index d87fe79..af2cb3b 100644 --- a/lib/gds-sso/controller_methods.rb +++ b/lib/gds-sso/controller_methods.rb @@ -1,11 +1,19 @@ module GDS module SSO + class PermissionDeniedError < StandardError + end + module ControllerMethods - class PermissionDeniedException < StandardError + # TODO: remove this for the next major release + class PermissionDeniedException < PermissionDeniedError + def initialize(...) + warn "GDS::SSO::ControllerMethods::PermissionDeniedException is deprecated, please replace with GDS::SSO::PermissionDeniedError" + super(...) + end end def self.included(base) - base.rescue_from PermissionDeniedException do |e| + base.rescue_from PermissionDeniedError do |e| if GDS::SSO::Config.api_only render json: { message: e.message }, status: :forbidden else @@ -24,22 +32,7 @@ def authorise_user!(permissions) # Otherwise current_user might be nil, and we'd error out authenticate_user! - case permissions - when String - unless current_user.has_permission?(permissions) - raise PermissionDeniedException, "Sorry, you don't seem to have the #{permissions} permission for this app." - end - when Hash - raise ArgumentError, "Must be either `any_of` or `all_of`" unless permissions.keys.size == 1 - - if permissions[:any_of] - authorise_user_with_at_least_one_of_permissions!(permissions[:any_of]) - elsif permissions[:all_of] - authorise_user_with_all_permissions!(permissions[:all_of]) - else - raise ArgumentError, "Must be either `any_of` or `all_of`" - end - end + GDS::SSO::AuthoriseUser.call(current_user, permissions) end def authenticate_user! @@ -65,22 +58,6 @@ def logout def warden request.env["warden"] end - - private - - def authorise_user_with_at_least_one_of_permissions!(permissions) - if permissions.none? { |permission| current_user.has_permission?(permission) } - raise PermissionDeniedException, - "Sorry, you don't seem to have any of the permissions: #{permissions.to_sentence} for this app." - end - end - - def authorise_user_with_all_permissions!(permissions) - unless permissions.all? { |permission| current_user.has_permission?(permission) } - raise PermissionDeniedException, - "Sorry, you don't seem to have all of the permissions: #{permissions.to_sentence} for this app." - end - end end end end diff --git a/lib/gds-sso/railtie.rb b/lib/gds-sso/railtie.rb index 642631f..c6d6f7d 100644 --- a/lib/gds-sso/railtie.rb +++ b/lib/gds-sso/railtie.rb @@ -1,6 +1,10 @@ module GDS module SSO class Railtie < Rails::Railtie + config.action_dispatch.rescue_responses.merge!( + "GDS::SSO::PermissionDeniedError" => :forbidden, + ) + initializer "gds-sso.initializer" do GDS::SSO.config do |config| config.cache = Rails.cache diff --git a/spec/controller/controller_methods_spec.rb b/spec/controller/controller_methods_spec.rb index 67d63a2..9100fd5 100644 --- a/spec/controller/controller_methods_spec.rb +++ b/spec/controller/controller_methods_spec.rb @@ -1,57 +1,24 @@ require "spec_helper" -RSpec.describe GDS::SSO::ControllerMethods, "#authorise_user!" do - let(:current_user) { double } - let(:expected_error) { GDS::SSO::ControllerMethods::PermissionDeniedException } +RSpec.describe GDS::SSO::ControllerMethods do + describe "#authorise_user!" do + let(:current_user) { double } + let(:expected_error) { GDS::SSO::PermissionDeniedError } - context "with a single string permission argument" do - it "permits users with the required permission" do - allow(current_user).to receive(:has_permission?).with("good").and_return(true) + context "when the user is authorised" do + it "does not raise an error" do + allow(current_user).to receive(:has_permission?).with("good").and_return(true) - expect { ControllerSpy.new(current_user).authorise_user!("good") }.not_to raise_error + expect { ControllerSpy.new(current_user).authorise_user!("good") }.not_to raise_error + end end - it "does not permit the users without the required permission" do - allow(current_user).to receive(:has_permission?).with("good").and_return(false) + context "when the user is not authorised" do + it "raises an error" do + allow(current_user).to receive(:has_permission?).with("bad").and_return(false) - expect { ControllerSpy.new(current_user).authorise_user!("good") }.to raise_error(expected_error) - end - end - - context "with the `all_of` option" do - it "permits users with all of the required permissions" do - allow(current_user).to receive(:has_permission?).with("good").and_return(true) - allow(current_user).to receive(:has_permission?).with("bad").and_return(true) - - expect { ControllerSpy.new(current_user).authorise_user!(all_of: %w[good bad]) }.not_to raise_error - end - - it "does not permit users without all of the required permissions" do - allow(current_user).to receive(:has_permission?).with("good").and_return(false) - allow(current_user).to receive(:has_permission?).with("bad").and_return(true) - - expect { ControllerSpy.new(current_user).authorise_user!(all_of: %w[good bad]) }.to raise_error(expected_error) - end - end - - context "with the `any_of` option" do - it "permits users with any of the required permissions" do - allow(current_user).to receive(:has_permission?).with("good").and_return(true) - allow(current_user).to receive(:has_permission?).with("bad").and_return(false) - - expect { ControllerSpy.new(current_user).authorise_user!(any_of: %w[good bad]) }.not_to raise_error - end - - it "does not permit users without any of the required permissions" do - allow(current_user).to receive(:has_permission?).and_return(false) - - expect { ControllerSpy.new(current_user).authorise_user!(any_of: %w[good bad]) }.to raise_error(expected_error) - end - end - - context "with none of `any_of` or `all_of`" do - it "raises an `ArgumentError`" do - expect { ControllerSpy.new(current_user).authorise_user!(whoops: "bad") }.to raise_error(ArgumentError) + expect { ControllerSpy.new(current_user).authorise_user!("bad") }.to raise_error(expected_error) + end end end end diff --git a/spec/internal/app/controllers/example_controller.rb b/spec/internal/app/controllers/example_controller.rb index 2490f2e..a5fb992 100644 --- a/spec/internal/app/controllers/example_controller.rb +++ b/spec/internal/app/controllers/example_controller.rb @@ -1,7 +1,6 @@ class ExampleController < ApplicationController before_action :authenticate_user!, except: :not_restricted before_action -> { authorise_user!("execute") }, only: :this_requires_execute_permission - def not_restricted render body: "jabberwocky" end @@ -13,4 +12,8 @@ def restricted def this_requires_execute_permission render body: "you have execute permission" end + + def constraint_restricted + render body: "constraint restricted" + end end diff --git a/spec/internal/config/routes.rb b/spec/internal/config/routes.rb index 29bdc67..7b02e42 100644 --- a/spec/internal/config/routes.rb +++ b/spec/internal/config/routes.rb @@ -4,4 +4,8 @@ get "/not-restricted" => "example#not_restricted" get "/restricted" => "example#restricted" get "/this-requires-execute-permission" => "example#this_requires_execute_permission" + + constraints(GDS::SSO::AuthorisedUserConstraint.new("execute")) do + get "/constraint-restricted" => "example#constraint_restricted" + end end diff --git a/spec/spec_helper.rb b/spec/spec_helper.rb index bc97004..07b8419 100644 --- a/spec/spec_helper.rb +++ b/spec/spec_helper.rb @@ -8,6 +8,7 @@ Combustion.initialize! :all do config.cache_store = :null_store + config.action_dispatch.show_exceptions = :all end require "rspec/rails" diff --git a/spec/system/authentication_and_authorisation_spec.rb b/spec/system/authentication_and_authorisation_spec.rb index 92f1be2..2c672cd 100644 --- a/spec/system/authentication_and_authorisation_spec.rb +++ b/spec/system/authentication_and_authorisation_spec.rb @@ -119,6 +119,27 @@ end end + context "when accessing a route that is restricted by the authorised user constraint" do + it "allows access when an authenticated user has correct permissions" do + stub_signon_authenticated(permissions: %w[execute]) + visit "/constraint-restricted" + expect(page).to have_content("constraint restricted") + end + + it "redirects an unauthenticated request to signon" do + visit "/constraint-restricted" + expect(page.response_headers["Location"]).to match("/auth/gds") + visit page.response_headers["Location"] + expect(page.response_headers["Location"]).to match("http://signon/oauth/authorize") + end + + it "restricts access when an authenticated user does not have the correct permissions" do + stub_signon_authenticated(permissions: %w[no-access]) + visit "/constraint-restricted" + expect(page.status_code).to eq(403) + end + end + def stub_signon_authenticated(permissions: []) # visit restricted page to trigger redirect URL to record state attribute visit "/auth/gds" diff --git a/spec/unit/authorise_user_spec.rb b/spec/unit/authorise_user_spec.rb new file mode 100644 index 0000000..a84ab64 --- /dev/null +++ b/spec/unit/authorise_user_spec.rb @@ -0,0 +1,69 @@ +require "spec_helper" +require "gds-sso/authorise_user" + +describe GDS::SSO::AuthoriseUser do + describe "#call" do + let(:current_user) { double } + + context "with a single string permission argument" do + let(:permissions) { "admin" } + let(:expected_error) { GDS::SSO::PermissionDeniedError } + + it "permits users with the required permission" do + allow(current_user).to receive(:has_permission?).with("admin").and_return(true) + + expect { described_class.call(current_user, permissions) }.not_to raise_error + end + + it "does not permit the users without the required permission" do + allow(current_user).to receive(:has_permission?).with("admin").and_return(false) + + expect { described_class.call(current_user, permissions) }.to raise_error(expected_error) + end + end + + context "with the `all_of` option" do + let(:permissions) { { all_of: %w[admin editor] } } + let(:expected_error) { GDS::SSO::PermissionDeniedError } + + it "permits users with all of the required permissions" do + allow(current_user).to receive(:has_permission?).with("admin").and_return(true) + allow(current_user).to receive(:has_permission?).with("editor").and_return(true) + + expect { described_class.call(current_user, permissions) }.not_to raise_error + end + + it "does not permit users without all of the required permissions" do + allow(current_user).to receive(:has_permission?).with("admin").and_return(false) + allow(current_user).to receive(:has_permission?).with("editor").and_return(true) + + expect { described_class.call(current_user, permissions) }.to raise_error(expected_error) + end + end + + context "with the `any_of` option" do + let(:permissions) { { any_of: %w[admin editor] } } + let(:expected_error) { GDS::SSO::PermissionDeniedError } + + it "permits users with any of the required permissions" do + allow(current_user).to receive(:has_permission?).with("admin").and_return(true) + allow(current_user).to receive(:has_permission?).with("editor").and_return(false) + + expect { described_class.call(current_user, permissions) }.not_to raise_error + end + + it "does not permit users without any of the required permissions" do + allow(current_user).to receive(:has_permission?).and_return(false) + + expect { described_class.call(current_user, permissions) }.to raise_error(expected_error) + end + end + + context "with none of `any_of` or `all_of`" do + it "raises an `ArgumentError`" do + expect { described_class.call(current_user, { admin: true }) } + .to raise_error(ArgumentError) + end + end + end +end diff --git a/spec/unit/authorised_user_constraint_spec.rb b/spec/unit/authorised_user_constraint_spec.rb new file mode 100644 index 0000000..e5a652b --- /dev/null +++ b/spec/unit/authorised_user_constraint_spec.rb @@ -0,0 +1,50 @@ +require "spec_helper" +require "gds-sso/authorised_user_constraint" + +describe GDS::SSO::AuthorisedUserConstraint do + before do + allow(GDS::SSO::AuthoriseUser).to receive(:call).and_return(true) + end + + describe "#matches?" do + let(:user) { double("user", remotely_signed_out?: remotely_signed_out) } + let(:warden) do + double( + "warden", + authenticated?: user_authenticated, + user:, + authenticate!: nil, + ) + end + let(:user_authenticated) { true } + let(:remotely_signed_out) { false } + let(:request) { double("request", env: { "warden" => warden }) } + + it "authorises the user" do + expect(GDS::SSO::AuthoriseUser).to receive(:call).with(warden.user, %w[signin]) + expect(warden).not_to receive(:authenticate!) + + described_class.new(%w[signin]).matches?(request) + end + + context "when the user is not authenticated" do + let(:user_authenticated) { false } + + it "authenticates the user" do + expect(warden).to receive(:authenticate!) + + described_class.new(%w[signin]).matches?(request) + end + end + + context "when the user is remotely signed out" do + let(:remotely_signed_out) { true } + + it "authenticates the user" do + expect(warden).to receive(:authenticate!) + + described_class.new(%w[signin]).matches?(request) + end + end + end +end diff --git a/spec/unit/railtie_spec.rb b/spec/unit/railtie_spec.rb index 65a0f6b..c6cdb37 100644 --- a/spec/unit/railtie_spec.rb +++ b/spec/unit/railtie_spec.rb @@ -11,4 +11,23 @@ it "honours API only setting" do expect(GDS::SSO::Config.api_only).to eq false end + + describe "configuring intercept_401_responses" do + it "sets warden intercept_401 to false when the configuration option is set to false" do + allow(GDS::SSO::Config).to receive(:intercept_401_responses).and_return(false) + + expect(warden_manager.config[:intercept_401]).to be(false) + end + + it "sets warden intercept_401 to true when the configuration option is set to true" do + allow(GDS::SSO::Config).to receive(:intercept_401_responses).and_return(true) + + expect(warden_manager.config[:intercept_401]).to be(true) + end + end + + def warden_manager + middleware = Rails.application.config.middleware.find { |m| m.name.include?("Warden::Manager") } + Warden::Manager.new(nil, &middleware.block) + end end