Skip to content

Commit

Permalink
Implment terms and conditions acceptance
Browse files Browse the repository at this point in the history
  • Loading branch information
richardpattinson committed Jul 11, 2024
1 parent 0f397f6 commit 2aa2a15
Show file tree
Hide file tree
Showing 15 changed files with 370 additions and 4 deletions.
7 changes: 7 additions & 0 deletions app/controllers/application_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ class ApplicationController < ActionController::Base
before_action :http_basic_authenticate, unless: -> { FeatureFlags::FeatureFlag.active?(:service_open) }
before_action :authenticate_dsi_user!
before_action :handle_expired_session!
before_action :enforce_terms_and_conditions_acceptance!

def http_basic_authenticate
valid_credentials = [
Expand Down Expand Up @@ -63,4 +64,10 @@ def trigger_request_event

DfE::Analytics::SendEvents.do([request_event.as_json])
end

def enforce_terms_and_conditions_acceptance!
if current_dsi_user&.acceptance_required?
redirect_to terms_and_conditions_path
end
end
end
2 changes: 2 additions & 0 deletions app/controllers/sign_in_controller.rb
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
class SignInController < ApplicationController
skip_before_action :authenticate_dsi_user!
skip_before_action :handle_expired_session!
skip_before_action :enforce_terms_and_conditions_acceptance!

before_action :reset_session
before_action :handle_failed_sign_in, if: -> { params[:oauth_failure] == "true" }

Expand Down
2 changes: 2 additions & 0 deletions app/controllers/sign_out_controller.rb
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
class SignOutController < ApplicationController
skip_before_action :handle_expired_session!
skip_before_action :enforce_terms_and_conditions_acceptance!

before_action :reset_session

def new
Expand Down
13 changes: 13 additions & 0 deletions app/controllers/terms_and_conditions_controller.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
# frozen_string_literal: true

class TermsAndConditionsController < ApplicationController
skip_before_action :enforce_terms_and_conditions_acceptance!

def show
end

def update
current_dsi_user.accept_terms!
redirect_to root_path, notice: "Terms and conditions accepted"
end
end
21 changes: 21 additions & 0 deletions app/models/dsi_user.rb
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,8 @@ class DsiUser < ApplicationRecord
encrypts :first_name, :last_name
encrypts :email, deterministic: true

CURRENT_TERMS_AND_CONDITIONS_VERSION = "1.0".freeze

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

Expand All @@ -26,6 +28,25 @@ def self.create_or_update_from_dsi(dsi_payload, role = nil)
dsi_user
end

def accept_terms!
update!(
terms_and_conditions_version_accepted: CURRENT_TERMS_AND_CONDITIONS_VERSION,
terms_and_conditions_accepted_at: Time.zone.now
)
end

def acceptance_required?
!current_version_accepted || acceptance_expired
end

def current_version_accepted
terms_and_conditions_version_accepted == CURRENT_TERMS_AND_CONDITIONS_VERSION
end

def acceptance_expired
terms_and_conditions_accepted_at < 12.months.ago
end

def internal?
DfESignIn.bypass? || Role.enabled.internal.pluck(:code).include?(current_session&.role_code)
end
Expand Down
3 changes: 3 additions & 0 deletions app/views/layouts/base.html.erb
Original file line number Diff line number Diff line change
Expand Up @@ -83,6 +83,9 @@
<li class="govuk-footer__inline-list-item">
<%= govuk_link_to("Privacy notice", "https://www.gov.uk/government/publications/privacy-information-education-providers-workforce-including-teachers", class: "govuk-footer__link") %>
</li>
<li class="govuk-footer__inline-list-item">
<%= govuk_link_to("Terms and conditions", "/terms-and-conditions", class: "govuk-footer__link") %>
</li>
</ul>

<svg aria-hidden="true" focusable="false" class="govuk-footer__licence-logo" xmlns="http://www.w3.org/2000/svg" viewBox="0 0 483.2 195.7" height="17" width="41">
Expand Down
211 changes: 211 additions & 0 deletions app/views/terms_and_conditions/show.html.erb

Large diffs are not rendered by default.

2 changes: 2 additions & 0 deletions config/analytics.yml
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,8 @@ shared:
- uid
- created_at
- updated_at
- terms_and_conditions_version_accepted
- terms_and_conditions_accepted_at
:dsi_user_sessions:
- id
- dsi_user_id
Expand Down
3 changes: 3 additions & 0 deletions config/routes.rb
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,9 @@
get "/auth/dfe/callback", to: "omniauth_callbacks#dfe"
post "/auth/developer/callback", to: "omniauth_callbacks#dfe_bypass"

get "/terms-and-conditions", to: "terms_and_conditions#show"
patch "/terms-and-conditions" => "terms_and_conditions#update"

scope "/feedback" do
get "/", to: "feedbacks#new", as: :feedbacks
post "/", to: "feedbacks#create"
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
class AddTermsAndConditionsFieldsToDsiUser < ActiveRecord::Migration[7.1]
def change
change_table :dsi_users, bulk: true do |f|
f.string :terms_and_conditions_version_accepted
f.datetime :terms_and_conditions_accepted_at
end
end
end
4 changes: 3 additions & 1 deletion db/schema.rb

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

10 changes: 9 additions & 1 deletion spec/support/system/authentication_steps.rb
Original file line number Diff line number Diff line change
@@ -1,15 +1,17 @@
module AuthenticationSteps
def when_i_sign_in_via_dsi(authorised: true, orgs: [organisation])
def when_i_sign_in_via_dsi(authorised: true, orgs: [organisation], accept_terms_and_conditions: true)
given_dsi_auth_is_mocked(authorised:, orgs:)
when_i_visit_the_sign_in_page
and_click_the_dsi_sign_in_button
and_i_accept_the_terms_and_conditions(accept_terms_and_conditions)
end
alias_method :and_i_am_signed_in_via_dsi, :when_i_sign_in_via_dsi

def when_i_sign_in_as_an_internal_user_via_dsi
given_dsi_auth_is_mocked(authorised: true, internal: true)
when_i_visit_the_sign_in_page
and_click_the_dsi_sign_in_button
and_i_accept_the_terms_and_conditions(true)
end
alias_method :and_i_am_signed_in_as_an_internal_user_via_dsi, :when_i_sign_in_as_an_internal_user_via_dsi

Expand Down Expand Up @@ -109,4 +111,10 @@ def organisation(status: "Open")
"status" => { "id" => 1, "name" => status },
}
end

def and_i_accept_the_terms_and_conditions(accept)
if accept
click_on "Accept"
end
end
end
2 changes: 1 addition & 1 deletion spec/system/unauthorised_user_signs_in_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@

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

Expand Down
84 changes: 84 additions & 0 deletions spec/system/user_accepts_terms_and_conditions.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,84 @@
# frozen_string_literal: true

require "rails_helper"

RSpec.describe "Terms and conditions acceptance", type: :system do
include AuthenticationSteps

scenario "User accepts terms and conditions", test: :with_stubbed_auth do
given_the_service_is_open
when_i_sign_in_via_dsi(accept_terms_and_conditions: false)
then_i_am_signed_in
and_i_am_redirected_to_the_terms_and_conditions_page

when_i_sign_out
then_i_am_redirected_to_the_sign_in_page
when_i_sign_in_via_dsi(accept_terms_and_conditions: false)
then_i_am_redirected_to_the_terms_and_conditions_page

when_i_go_to_the_root
then_i_am_redirected_to_the_terms_and_conditions_page

when_i_click_accept
then_i_am_taken_to_the_root
and_i_see_the_successful_notification
and_the_dsi_user_has_been_updated

when_13_months_has_passed
when_i_go_to_the_root
when_i_sign_in_via_dsi(accept_terms_and_conditions: false)
then_i_am_redirected_to_the_terms_and_conditions_page
when_i_click_accept
then_i_am_taken_to_the_root
and_i_see_the_successful_notification
end

private

def then_i_am_signed_in
within("header") { expect(page).to have_content "Sign out" }
expect(DsiUser.count).to eq 1
expect(DsiUserSession.count).to eq 1
end

def and_i_am_redirected_to_the_terms_and_conditions_page
expect(page).to have_current_path("/terms-and-conditions")
end
alias_method(
:then_i_am_redirected_to_the_terms_and_conditions_page,
:and_i_am_redirected_to_the_terms_and_conditions_page,
)

def when_i_click_accept
click_on "Accept"
end

def then_i_am_taken_to_the_root
expect(page).to have_current_path("/search")
end

def and_i_see_the_successful_notification
expect(page).to have_content "Terms and conditions accepted"
end

def and_the_dsi_user_has_been_updated
expect(DsiUser.first.terms_and_conditions_version_accepted).to eq("1.0")
expect(DsiUser.first.terms_and_conditions_accepted_at).to_not be(nil)
end

def when_13_months_has_passed
travel_to 13.months.from_now
end

def when_i_go_to_the_root
visit root_path
end

def when_i_sign_out
click_on "Sign out"
end

def then_i_am_redirected_to_the_sign_in_page
expect(page).to have_current_path(sign_in_path)
end
end
2 changes: 1 addition & 1 deletion spec/system/user_belonging_to_closed_org_signs_in_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@

scenario "User belonging to closed organisation signs in via DfE Sign In", test: :with_stubbed_auth do
given_the_service_is_open
when_i_sign_in_via_dsi(orgs: [organisation(status: "Closed")])
when_i_sign_in_via_dsi(orgs: [organisation(status: "Closed")], accept_terms_and_conditions: false)
then_i_am_not_authorised
end

Expand Down

0 comments on commit 2aa2a15

Please sign in to comment.