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

Implemnt terms and conditions acceptance #361

Merged
merged 1 commit into from
Jul 12, 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
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
Loading