Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add constraint and config options #300

Merged
merged 7 commits into from
Apr 29, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
12 changes: 8 additions & 4 deletions lib/gds-sso.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down
49 changes: 49 additions & 0 deletions lib/gds-sso/authorise_user.rb
Original file line number Diff line number Diff line change
@@ -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
21 changes: 21 additions & 0 deletions lib/gds-sso/authorised_user_constraint.rb
Original file line number Diff line number Diff line change
@@ -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
3 changes: 3 additions & 0 deletions lib/gds-sso/config.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
45 changes: 11 additions & 34 deletions lib/gds-sso/controller_methods.rb
Original file line number Diff line number Diff line change
@@ -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
Expand All @@ -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!
Expand All @@ -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
4 changes: 4 additions & 0 deletions lib/gds-sso/railtie.rb
Original file line number Diff line number Diff line change
@@ -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
Expand Down
61 changes: 14 additions & 47 deletions spec/controller/controller_methods_spec.rb
Original file line number Diff line number Diff line change
@@ -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
5 changes: 4 additions & 1 deletion spec/internal/app/controllers/example_controller.rb
Original file line number Diff line number Diff line change
@@ -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
Expand All @@ -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
4 changes: 4 additions & 0 deletions spec/internal/config/routes.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
1 change: 1 addition & 0 deletions spec/spec_helper.rb
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@

Combustion.initialize! :all do
config.cache_store = :null_store
config.action_dispatch.show_exceptions = :all
end

require "rspec/rails"
Expand Down
21 changes: 21 additions & 0 deletions spec/system/authentication_and_authorisation_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down
Loading