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

Check user access to service on sign in #333

Merged
merged 5 commits into from
Sep 13, 2023
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 .env.development
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,10 @@ DFE_SIGN_IN_CLIENT_ID=checkrecordteacher
DFE_SIGN_IN_REDIRECT_URL=http://check.localhost:3000/check-records/auth/dfe/callback
DFE_SIGN_IN_SECRET=override-locally
DFE_SIGN_IN_ISSUER=https://dev-oidc.signin.education.gov.uk
DFE_SIGN_IN_API_BASE_URL=https://dev-api.signin.education.gov.uk
DFE_SIGN_IN_API_SECRET=override-locally
DFE_SIGN_IN_API_AUDIENCE=signin.education.gov.uk
DFE_SIGN_IN_API_ROLE_CODES=override-locally
GOVUK_NOTIFY_API_KEY=override-locally
HOSTING_DOMAIN=http://localhost:3000
HOSTING_ENVIRONMENT_NAME=local
Expand Down
4 changes: 4 additions & 0 deletions .env.test
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,10 @@ DFE_SIGN_IN_CLIENT_ID=checkrecordteacher
DFE_SIGN_IN_ISSUER=test
DFE_SIGN_IN_REDIRECT_URL=test
DFE_SIGN_IN_SECRET=override-locally
DFE_SIGN_IN_API_BASE_URL=https://dev-api.signin.education.gov.uk
DFE_SIGN_IN_API_SECRET=override-locally
DFE_SIGN_IN_API_AUDIENCE=signin.education.gov.uk
DFE_SIGN_IN_API_ROLE_CODES=override-locally
GOVUK_NOTIFY_API_KEY=override-locally
HOSTING_DOMAIN=http://qualifications.localhost
HOSTING_ENVIRONMENT_NAME=local
Expand Down
1 change: 1 addition & 0 deletions Gemfile
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ gem "govuk_feature_flags",
gem "govuk_markdown"
gem "hashie"
gem "jsbundling-rails"
gem "jwt"
gem "logstash-logger"
gem "mail-notify"
gem "name_of_person"
Expand Down
1 change: 1 addition & 0 deletions Gemfile.lock
Original file line number Diff line number Diff line change
Expand Up @@ -595,6 +595,7 @@ DEPENDENCIES
govuk_markdown
hashie
jsbundling-rails
jwt
launchy
logstash-logger
mail-notify
Expand Down
13 changes: 12 additions & 1 deletion app/controllers/check_records/omniauth_callbacks_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,18 @@ class CheckRecords::OmniauthCallbacksController < ApplicationController
protect_from_forgery except: :dfe_bypass

def dfe
@dsi_user = DsiUser.create_or_update_from_dsi(request.env["omniauth.auth"])
auth = request.env["omniauth.auth"]

unless CheckRecords::DfESignIn.bypass?
role = DfESignInApi::GetUserAccessToService.new(
org_id: auth.extra.raw_info.organisation.id,
user_id: auth.uid,
).call

return redirect_to check_records_not_authorised_path unless role
end

@dsi_user = DsiUser.create_or_update_from_dsi(auth, role)
session[:dsi_user_id] = @dsi_user.id
session[:dsi_user_session_expiry] = 2.hours.from_now.to_i

Expand Down
3 changes: 3 additions & 0 deletions app/controllers/check_records/sign_in_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -5,5 +5,8 @@ class SignInController < CheckRecordsController

def new
end

def not_authorised
end
end
end
35 changes: 35 additions & 0 deletions app/lib/dfe_sign_in_api/client.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,35 @@
require "jwt"

module DfESignInApi
module Client
TIMEOUT_IN_SECONDS = 5

def client
@client ||=
Faraday.new(
url: ENV.fetch("DFE_SIGN_IN_API_BASE_URL"),
request: {
timeout: TIMEOUT_IN_SECONDS
}
) do |faraday|
faraday.request :authorization, "Bearer", jwt
faraday.request :json
faraday.response :json
faraday.adapter Faraday.default_adapter
end
end

private

def jwt
@jwt ||= JWT.encode(
{
iss: ENV.fetch("DFE_SIGN_IN_CLIENT_ID"),
aud: ENV.fetch("DFE_SIGN_IN_API_AUDIENCE"),
},
ENV.fetch("DFE_SIGN_IN_API_SECRET"),
"HS256",
)
end
end
end
34 changes: 34 additions & 0 deletions app/lib/dfe_sign_in_api/get_user_access_to_service.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,34 @@
module DfESignInApi
class GetUserAccessToService
include Client

attr_reader :org_id, :user_id

def initialize(org_id:, user_id:)
@org_id = org_id
@user_id = user_id
end

def call
response = client.get(endpoint)

if response.success? && response.body.key?("roles")
response.body["roles"].find { |role| authorised_role_codes.include?(role["code"]) }
end
end

private

def endpoint
"/services/#{service_id}/organisations/#{org_id}/users/#{user_id}"
end

def service_id
ENV["DFE_SIGN_IN_CLIENT_ID"]
end

def authorised_role_codes
ENV.fetch("DFE_SIGN_IN_API_ROLE_CODES").split(",")
end
end
end
12 changes: 11 additions & 1 deletion app/models/dsi_user.rb
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,9 @@ class DsiUser < ApplicationRecord
encrypts :first_name, :last_name

has_many :search_logs
has_many :dsi_user_sessions, dependent: :destroy

def self.create_or_update_from_dsi(dsi_payload)
def self.create_or_update_from_dsi(dsi_payload, role = nil)
dsi_user = find_or_initialize_by(email: dsi_payload.info.fetch(:email))

dsi_user.update!(
Expand All @@ -13,6 +14,15 @@ def self.create_or_update_from_dsi(dsi_payload)
uid: dsi_payload.uid
)

if role.present?
dsi_user.dsi_user_sessions.create!(
role_id: role["id"],
role_code: role["code"],
organisation_id: dsi_payload.extra.raw_info.organisation.id,
organisation_name: dsi_payload.extra.raw_info.organisation.name,
)
end

dsi_user
end
end
3 changes: 3 additions & 0 deletions app/models/dsi_user_session.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
class DsiUserSession < ApplicationRecord
belongs_to :dsi_user
end
5 changes: 5 additions & 0 deletions app/views/check_records/sign_in/not_authorised.html.erb
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
<h1 class="govuk-heading-l">Authorisation required</h1>
<p class="govuk-body">
You are not authorised to access this service.
</p>

9 changes: 9 additions & 0 deletions config/analytics.yml
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,15 @@ shared:
- uid
- created_at
- updated_at
:dsi_user_sessions:
- id
- dsi_user_id
- role_id
- role_code
- organisation_id
- organisation_name
- created_at
- updated_at
:feature_flags_features:
- id
- name
Expand Down
1 change: 1 addition & 0 deletions config/routes/check_records.rb
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
get "/privacy", to: "static#privacy"

get "/sign-in", to: "sign_in#new"
get "/not-authorised", to: "sign_in#not_authorised"
get "/sign-out", to: "sign_out#new"

get "/auth/dfe/callback", to: "omniauth_callbacks#dfe"
Expand Down
12 changes: 12 additions & 0 deletions db/migrate/20230907092835_create_dsi_user_sessions.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
class CreateDsiUserSessions < ActiveRecord::Migration[7.0]
def change
create_table :dsi_user_sessions do |t|
t.belongs_to :dsi_user
t.string :role_id
t.string :role_code
t.string :organisation_id
t.string :organisation_name
t.timestamps
end
end
end
11 changes: 11 additions & 0 deletions db/schema.rb
Original file line number Diff line number Diff line change
Expand Up @@ -90,6 +90,17 @@
create_table "data_migrations", primary_key: "version", id: :string, force: :cascade do |t|
end

create_table "dsi_user_sessions", force: :cascade do |t|
t.bigint "dsi_user_id"
t.string "role_id"
t.string "role_code"
t.string "organisation_id"
t.string "organisation_name"
t.datetime "created_at", null: false
t.datetime "updated_at", null: false
t.index ["dsi_user_id"], name: "index_dsi_user_sessions_on_dsi_user_id"
end

create_table "dsi_users", force: :cascade do |t|
t.string "email", limit: 510, null: false
t.string "first_name", limit: 510
Expand Down
39 changes: 39 additions & 0 deletions spec/lib/dfe_sign_in_api/get_user_access_to_service_spec.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,39 @@
require "rails_helper"

RSpec.describe DfESignInApi::GetUserAccessToService do
describe "#call" do
let(:org_id) { "123" }
let(:user_id) { "456" }
let(:role_id) { "789" }
let(:role_code) { ENV.fetch("DFE_SIGN_IN_API_ROLE_CODES").split(",").first }
let(:endpoint) do
"#{ENV.fetch("DFE_SIGN_IN_API_BASE_URL")}/services/checkrecordteacher/organisations/#{org_id}/users/#{user_id}"
end

subject { described_class.new(org_id:, user_id:).call }

context "when the user is authorised" do
before do
stub_request(:get, endpoint)
.to_return_json(
status: 200,
body: { "roles" => [{ "id" => role_id, "code" => role_code }] },
)
end

it { is_expected.to eq({ "id" => role_id, "code" => role_code }) }
end

context "when the user is not authorised" do
before do
stub_request(:get, endpoint)
.to_return_json(
status: 200,
body: { "roles" => [{ "id" => role_id, "code" => "Unauthorised_Role" }] },
)
end

it { is_expected.to be_nil }
end
end
end
27 changes: 26 additions & 1 deletion spec/models/dsi_user_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,18 @@
RSpec.describe DsiUser, type: :model do
describe ".create_or_update_from_dsi" do
let(:dsi_payload) do
OmniAuth::AuthHash.new(uid: "123456", info: { email:, first_name: "John", last_name: "Doe" })
OmniAuth::AuthHash.new(
uid: "123456",
info: { email:, first_name: "John", last_name: "Doe" },
extra: {
raw_info: {
organisation: {
id: "09876",
name: "Test Org",
}
}
}
)
end
let(:email) { "[email protected]" }

Expand Down Expand Up @@ -32,5 +43,19 @@
expect(dsi_user.last_name).to eq dsi_payload.info.last_name
end
end

context "when the user has a role" do
it "creates a session record" do
role = { "id" => "123", "code" => "TestRole_code" }
described_class.create_or_update_from_dsi(dsi_payload, role)

dsi_user_session = DsiUser.first.dsi_user_sessions.first
expect(dsi_user_session).to be_present
expect(dsi_user_session.role_id).to eq "123"
expect(dsi_user_session.role_code).to eq "TestRole_code"
expect(dsi_user_session.organisation_id).to eq "09876"
expect(dsi_user_session.organisation_name).to eq "Test Org"
end
end
end
end
29 changes: 26 additions & 3 deletions spec/support/system/check_records/authentication_steps.rb
Original file line number Diff line number Diff line change
@@ -1,13 +1,13 @@
module CheckRecords
module AuthenticationSteps
def when_i_sign_in_via_dsi
given_dsi_auth_is_mocked
def when_i_sign_in_via_dsi(authorised: true)
given_dsi_auth_is_mocked(authorised:)
when_i_visit_the_sign_in_page
and_click_the_dsi_sign_in_button
end
alias_method :and_i_am_signed_in_via_dsi, :when_i_sign_in_via_dsi

def given_dsi_auth_is_mocked
def given_dsi_auth_is_mocked(authorised:)
OmniAuth.config.mock_auth[:dfe] = OmniAuth::AuthHash.new(
{
provider: "dfe",
Expand All @@ -16,9 +16,24 @@ def given_dsi_auth_is_mocked
email: "[email protected]",
first_name: "Test",
last_name: "User"
},
extra: {
raw_info: {
organisation: {
id: org_id,
}
}
}
}
)

stub_request(
:get,
"#{ENV.fetch("DFE_SIGN_IN_API_BASE_URL")}/services/checkrecordteacher/organisations/#{org_id}/users/123456",
).to_return_json(
status: 200,
body: { "roles" => [{ "code" => (authorised ? role_code : "Unauthorised_Role") }] },
)
end

def when_i_visit_the_sign_in_page
Expand All @@ -28,5 +43,13 @@ def when_i_visit_the_sign_in_page
def and_click_the_dsi_sign_in_button
click_button "Sign in"
end

def org_id
"12345678-1234-1234-1234-123456789012"
end

def role_code
ENV.fetch("DFE_SIGN_IN_API_ROLE_CODES").split(",").first
end
end
end
21 changes: 21 additions & 0 deletions spec/system/check_records/unauthorised_user_signs_in_spec.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
# frozen_string_literal: true

require "rails_helper"

RSpec.describe "DSI authentication", host: :check_records do
include AuthorizationSteps
include CheckRecords::AuthenticationSteps

scenario "Unauthorised user signs in via DfE Sign In", test: :with_stubbed_auth do
when_i_am_authorized_with_basic_auth
when_i_sign_in_via_dsi(authorised: false)
then_i_am_redirected_to_the_unauthorised_page
end

private

def then_i_am_redirected_to_the_unauthorised_page
expect(page).to have_current_path("/check-records/not-authorised")
expect(page).to have_content("You are not authorised to access this service")
end
end