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

Improve app authentication #1345

Closed
wants to merge 4 commits into from
Closed
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
7 changes: 1 addition & 6 deletions app/controllers/application_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -3,10 +3,5 @@ class ApplicationController < ActionController::Base
# For APIs, you may want to use :null_session instead.
protect_from_forgery with: :exception

include GDS::SSO::ControllerMethods
before_action :authenticate_user!

def admin_user?
current_user.permissions.include?("admin")
end
include Authentication
end
17 changes: 17 additions & 0 deletions app/controllers/concerns/authentication.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
# Requires a user to be logged in through GDS SSO for any action, and keeps track of the current
# user through `Current`.
module Authentication
extend ActiveSupport::Concern

included do
include GDS::SSO::ControllerMethods

before_action :authenticate_user!, :set_current
end

private

def set_current
Current.user = current_user
end
end
2 changes: 1 addition & 1 deletion app/controllers/recommended_links_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,6 @@ def set_recommended_link
def recommended_link_params
params
.expect(recommended_link: %i[link title description keywords comment])
.merge(user_id: current_user.id)
.merge(user_id: Current.user.id)
end
end
4 changes: 2 additions & 2 deletions app/helpers/application_helper.rb
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
module ApplicationHelper
def navigation_items
return [] unless current_user
return [] unless Current.user

[
{
Expand All @@ -9,7 +9,7 @@ def navigation_items
active: controller.controller_name == "recommended_links",
},
{
text: current_user.name,
text: Current.user.name,
href: Plek.new.external_url_for("signon"),
},
{
Expand Down
3 changes: 3 additions & 0 deletions app/models/current.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
class Current < ActiveSupport::CurrentAttributes
attribute :user
end
6 changes: 6 additions & 0 deletions app/models/user.rb
Original file line number Diff line number Diff line change
@@ -1,5 +1,11 @@
class User < ApplicationRecord
ADMIN_PERMISSION_KEY = "admin".freeze

include GDS::SSO::User

serialize :permissions, type: Array

def admin?
permissions.include?(ADMIN_PERMISSION_KEY)
end
end
21 changes: 21 additions & 0 deletions spec/models/user_spec.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
require "gds-sso/lint/user_spec"

RSpec.describe User, type: :model do
it_behaves_like "a gds-sso user class"

describe "#admin?" do
subject(:user) { build_stubbed(:user, permissions:) }

context "when the user has the `admin` permission" do
let(:permissions) { %w[admin and others] }

it { is_expected.to be_admin }
end

context "when the user does not have the `admin` permission" do
let(:permissions) { %w[blah] }

it { is_expected.not_to be_admin }
end
end
end
38 changes: 38 additions & 0 deletions spec/requests/authentication_spec.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,38 @@
class FakeController < ApplicationController
def hello
render plain: "Hello, world!"
end
end

RSpec.describe "Authentication", type: :request do
before(:all) do
Rails.application.routes.draw do
get "hello" => "fake#hello"
end
end

after(:all) do
Rails.application.reload_routes!
end

context "when the user is authenticated" do
include_context "with an SSO authenticated user"

it "allows the request to proceed" do
get hello_path

expect(response).to have_http_status(:ok)
expect(response.body).to eq("Hello, world!")
end
end

context "when the user is unauthenticated" do
include_context "without an SSO authenticated user"

it "redirects to the GDS SSO page" do
get hello_path

expect(response).to redirect_to("http://www.example.com/auth/gds")
end
end
end
10 changes: 2 additions & 8 deletions spec/spec_helper.rb
Original file line number Diff line number Diff line change
Expand Up @@ -31,14 +31,8 @@

config.include FactoryBot::Syntax::Methods

config.before(:all, type: :system) do
@user = create(:user)
GDS::SSO.test_user = @user
end

config.after(:all, type: :system) do
@user.destroy!
end
config.include SharedContexts::Authentication
config.include_context "with an SSO authenticated user", type: :system

config.before(:each, type: :system) do
driven_by :rack_test
Expand Down
25 changes: 25 additions & 0 deletions spec/support/shared_contexts/authentication.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
module SharedContexts
module Authentication
RSpec.shared_context "with an SSO authenticated user" do
let(:sso_user) { create(:user) }

before do
GDS::SSO.test_user = sso_user
end

after do
GDS::SSO.test_user = nil
end
end

RSpec.shared_context "without an SSO authenticated user" do
before do
# Unfortunately gds-sso assumes that a nil `test_user` is undesirable, and loads the first
# user from the database. Pretending to set this environment variable is the only way to
# force an unauthenticated state.
allow(ENV).to receive(:[]).and_call_original
allow(ENV).to receive(:[]).with("GDS_SSO_MOCK_INVALID").and_return("1")
end
end
end
end
Loading