diff --git a/Gemfile b/Gemfile index 07e4f84c5..a8f26cd6f 100644 --- a/Gemfile +++ b/Gemfile @@ -7,7 +7,7 @@ gem 'rails', '6.1.7.6' gem 'rake' gem 'pg' -gem 'authlogic' +gem 'devise' gem 'will_paginate' gem 'json' gem 'delayed_job_active_record' @@ -34,6 +34,9 @@ gem 'image_processing' gem 'maxminddb' gem 'redcarpet' gem 'scrypt' +gem 'omniauth' +gem 'omniauth-rails_csrf_protection' +gem 'omniauth-saml' gem 'aws-sdk-codedeploy' gem 'aws-sdk-cloudwatchlogs' diff --git a/Gemfile.lock b/Gemfile.lock index 5fa679a6f..6091084b1 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -64,11 +64,6 @@ GEM public_suffix (>= 2.0.2, < 6.0) appsignal (3.4.13) rack - authlogic (6.4.1) - activemodel (>= 5.2, < 6.2) - activerecord (>= 5.2, < 6.2) - activesupport (>= 5.2, < 6.2) - request_store (~> 1.0) aws-eventstream (1.1.1) aws-partitions (1.441.0) aws-sdk-cloudwatchlogs (1.6.0) @@ -140,6 +135,12 @@ GEM delayed_job_active_record (4.1.6) activerecord (>= 3.0, < 6.2) delayed_job (>= 3.0, < 5) + devise (4.9.3) + bcrypt (~> 3.0) + orm_adapter (~> 0.1) + railties (>= 4.1.0) + responders + warden (~> 1.2.3) diff-lcs (1.4.4) docile (1.3.1) dotenv (2.7.6) @@ -171,6 +172,7 @@ GEM globalid (1.2.1) activesupport (>= 6.1) hashdiff (1.0.1) + hashie (5.0.0) htmlentities (4.3.4) i18n (1.14.1) concurrent-ruby (~> 1.0) @@ -225,7 +227,7 @@ GEM timeout net-smtp (0.4.0) net-protocol - nio4r (2.5.9) + nio4r (2.7.0) nokogiri (1.15.4) mini_portile2 (~> 2.8.2) racc (~> 1.4) @@ -235,15 +237,28 @@ GEM racc (~> 1.4) nokogiri (1.15.4-x86_64-linux) racc (~> 1.4) + omniauth (2.1.1) + hashie (>= 3.4.6) + rack (>= 2.2.3) + rack-protection + omniauth-rails_csrf_protection (1.0.1) + actionpack (>= 4.2) + omniauth (~> 2.0) + omniauth-saml (2.1.0) + omniauth (~> 2.0) + ruby-saml (~> 1.12) + orm_adapter (0.5.0) pg (1.2.3) pry (0.13.1) coderay (~> 1.1) method_source (~> 1.0) public_suffix (5.0.3) - puma (5.6.7) + puma (5.6.8) nio4r (~> 2.0) racc (1.7.1) rack (2.2.8) + rack-protection (3.1.0) + rack (~> 2.2, >= 2.2.4) rack-test (2.1.0) rack (>= 1.3) rails (6.1.7.6) @@ -286,6 +301,9 @@ GEM regexp_parser (2.8.2) request_store (1.5.0) rack (>= 1.4) + responders (3.1.1) + actionpack (>= 5.2) + railties (>= 5.2) rexml (3.2.6) rspec-core (3.10.1) rspec-support (~> 3.10.0) @@ -304,6 +322,9 @@ GEM rspec-mocks (~> 3.10) rspec-support (~> 3.10) rspec-support (3.10.2) + ruby-saml (1.16.0) + nokogiri (>= 1.13.10) + rexml ruby-vips (2.1.4) ffi (~> 1.12) rubyzip (2.3.2) @@ -348,6 +369,8 @@ GEM concurrent-ruby (~> 1.0) uglifier (4.1.8) execjs (>= 0.3.0, < 3) + warden (1.2.9) + rack (>= 2.0.9) webmock (3.18.1) addressable (>= 2.8.0) crack (>= 0.3.2) @@ -371,7 +394,6 @@ PLATFORMS DEPENDENCIES appsignal - authlogic aws-sdk-cloudwatchlogs aws-sdk-codedeploy aws-sdk-s3 @@ -387,6 +409,7 @@ DEPENDENCIES database_cleaner delayed-web delayed_job_active_record + devise dotenv-rails email_spec factory_bot_rails @@ -403,6 +426,9 @@ DEPENDENCIES maxminddb net-http-persistent nokogiri + omniauth + omniauth-rails_csrf_protection + omniauth-saml pg pry puma (< 6) diff --git a/README.md b/README.md index 34c3f5efc..acc8392d0 100644 --- a/README.md +++ b/README.md @@ -12,12 +12,6 @@ We recommend using [Docker Desktop][2] to get setup quickly. If you'd prefer not docker compose run --rm web rake db:setup ``` -### Create an admin user - -``` -docker compose run --rm web rake epets:add_sysadmin_user -``` - ### Load the country list ``` diff --git a/app/assets/stylesheets/petitions/admin/_header.scss b/app/assets/stylesheets/petitions/admin/_header.scss index 25441550f..32d6d888e 100644 --- a/app/assets/stylesheets/petitions/admin/_header.scss +++ b/app/assets/stylesheets/petitions/admin/_header.scss @@ -9,6 +9,7 @@ float: right; } li { + color: $white; display: inline; margin-left: $gutter-one-third; a { diff --git a/app/controllers/admin/admin_controller.rb b/app/controllers/admin/admin_controller.rb index c92caabf4..555eec293 100644 --- a/app/controllers/admin/admin_controller.rb +++ b/app/controllers/admin/admin_controller.rb @@ -26,4 +26,12 @@ def set_appsignal_namespace def set_current_user Admin::Current.user = current_user end + + def after_sign_in_path_for(resource) + admin_root_url + end + + def after_sign_out_path_for(resource) + admin_login_url + end end diff --git a/app/controllers/admin/admin_users_controller.rb b/app/controllers/admin/admin_users_controller.rb index 3af497500..28ee372a3 100644 --- a/app/controllers/admin/admin_users_controller.rb +++ b/app/controllers/admin/admin_users_controller.rb @@ -1,67 +1,7 @@ class Admin::AdminUsersController < Admin::AdminController before_action :require_sysadmin - before_action :find_user, only: %i[edit update destroy] - - rescue_from AdminUser::CannotDeleteCurrentUser do - redirect_to admin_admin_users_url, alert: :user_is_current_user - end - - rescue_from AdminUser::MustBeAtLeastOneAdminUser do - redirect_to admin_admin_users_url, alert: :user_count_is_too_low - end - - rescue_from ActiveRecord::DeleteRestrictionError do - redirect_to admin_admin_users_url, alert: :user_has_moderated_petitions - end def index @users = AdminUser.by_name.paginate(page: params[:page], per_page: 50) end - - def new - @user = AdminUser.new - end - - def create - @user = AdminUser.new(admin_user_params) - - if @user.save - redirect_to admin_admin_users_url, notice: :user_created - else - render :new - end - end - - def edit - end - - def update - if @user.update(admin_user_params) - redirect_to admin_admin_users_url, notice: :user_updated - else - render :edit - end - end - - def destroy - if @user.destroy(current_user: current_user) - redirect_to admin_admin_users_url, notice: :user_deleted - else - redirect_to admin_admin_users_url, alert: :user_not_deleted - end - end - - protected - - def find_user - @user = AdminUser.find(params[:id]) - end - - def admin_user_params - params. - require(:admin_user). - permit(:password, :password_confirmation, :first_name, - :last_name, :role, :email, :force_password_reset, - :account_disabled) - end end diff --git a/app/controllers/admin/archived/government_response_controller.rb b/app/controllers/admin/archived/government_response_controller.rb index c479541cf..79650c65e 100644 --- a/app/controllers/admin/archived/government_response_controller.rb +++ b/app/controllers/admin/archived/government_response_controller.rb @@ -26,6 +26,16 @@ def update end end + def destroy + @government_response.destroy + + if @government_response.destroyed? + redirect_to admin_archived_petition_path(@petition), notice: :government_response_deleted + else + redirect_to admin_archived_petition_government_response_path(@petition), alert: :government_response_not_deleted + end + end + private def fetch_petition diff --git a/app/controllers/admin/government_response_controller.rb b/app/controllers/admin/government_response_controller.rb index ec3c888d6..8f3e5f25d 100644 --- a/app/controllers/admin/government_response_controller.rb +++ b/app/controllers/admin/government_response_controller.rb @@ -26,6 +26,16 @@ def update end end + def destroy + @government_response.destroy + + if @government_response.destroyed? + redirect_to admin_petition_path(@petition), notice: :government_response_deleted + else + redirect_to admin_petition_government_response_path(@petition), alert: :government_response_not_deleted + end + end + private def fetch_petition diff --git a/app/controllers/admin/omniauth_callbacks_controller.rb b/app/controllers/admin/omniauth_callbacks_controller.rb new file mode 100644 index 000000000..78a4fb705 --- /dev/null +++ b/app/controllers/admin/omniauth_callbacks_controller.rb @@ -0,0 +1,45 @@ +class Admin::OmniauthCallbacksController < Devise::OmniauthCallbacksController + skip_before_action :require_admin + skip_before_action :verify_authenticity_token, only: %i[saml] + + rescue_from ActiveRecord::RecordNotFound do + redirect_to admin_login_url, alert: :login_failed + end + + def saml + @user = AdminUser.find_or_create_from!(provider, auth_data) + + if @user.present? + sign_in @user, event: :authentication + + set_flash_message(:notice, :signed_in) + set_refresh_header + + render "admin/admin/index" + else + redirect_to admin_login_url, alert: :invalid_login + end + end + + def failure + redirect_to admin_login_url, alert: :login_failed + end + + private + + def after_omniauth_failure_path_for(scope) + admin_login_url + end + + def auth_data + request.env["omniauth.auth"] + end + + def provider + IdentityProvider.find_by!(name: auth_data.provider) + end + + def set_refresh_header + headers['Refresh'] = "0; url=#{admin_root_url}" + end +end diff --git a/app/controllers/admin/profile_controller.rb b/app/controllers/admin/profile_controller.rb deleted file mode 100644 index d45f59c13..000000000 --- a/app/controllers/admin/profile_controller.rb +++ /dev/null @@ -1,20 +0,0 @@ -class Admin::ProfileController < Admin::AdminController - skip_before_action :check_for_password_change - - def edit - end - - def update - if current_user.update_with_password(admin_user_params) - redirect_to admin_root_url, notice: :password_updated - else - render :edit - end - end - - def admin_user_params - params.require(:admin_user).permit( - :current_password, :password, :password_confirmation - ) - end -end diff --git a/app/controllers/admin/sessions_controller.rb b/app/controllers/admin/sessions_controller.rb new file mode 100644 index 000000000..1df24f9e2 --- /dev/null +++ b/app/controllers/admin/sessions_controller.rb @@ -0,0 +1,52 @@ +class Admin::SessionsController < Devise::SessionsController + skip_before_action :require_admin, except: :continue + prepend_before_action :skip_timeout, only: :status + + helper_method :last_request_at + + def create + if provider? + redirect_to sso_provider_url(provider), status: :temporary_redirect + else + redirect_to admin_login_url, alert: :invalid_login + end + end + + def continue + respond_to do |format| + format.json + end + end + + def status + respond_to do |format| + format.json + end + end + + private + + def email_domain + Mail::Address.new(sign_in_params[:email]).domain + rescue Mail::Field::ParseError + nil + end + + def provider + @provider ||= IdentityProvider.find_by(domain: email_domain) + end + + def provider? + provider.present? + end + + def skip_timeout + request.env['devise.skip_trackable'] = true + end + + def last_request_at + if user_session && user_session.key?("last_request_at") + Time.at(user_session["last_request_at"]).in_time_zone + end + end +end diff --git a/app/controllers/admin/user_sessions_controller.rb b/app/controllers/admin/user_sessions_controller.rb deleted file mode 100644 index 976f31888..000000000 --- a/app/controllers/admin/user_sessions_controller.rb +++ /dev/null @@ -1,46 +0,0 @@ -class Admin::UserSessionsController < Admin::AdminController - skip_before_action :require_admin, only: [:new, :create, :destroy, :status] - skip_before_action :check_for_password_change - - before_action :reset_session, only: [:create, :destroy] - - def new - @user_session = AdminUserSession.new - end - - def create - @user_session = AdminUserSession.new(user_session_params) - - if @user_session.save - redirect_to_target_or_default - elsif @user_session.last_login_attempt? - render :new, alert: :last_login - elsif @user_session.being_brute_force_protected? - render :new, alert: :disabled_login - else - render :new, alert: :invalid_login - end - end - - def destroy - current_session.destroy if logged_in? - redirect_to admin_login_url, notice: :logged_out - end - - def status - end - - def continue - current_user.touch(:last_request_at) - end - - private - - def user_session_params - params.require(:admin_user_session).permit(:email, :password).to_h - end - - def last_request_update_allowed? - action_name != 'status' - end -end diff --git a/app/controllers/concerns/authentication.rb b/app/controllers/concerns/authentication.rb index fa6435912..2ae551c8b 100644 --- a/app/controllers/concerns/authentication.rb +++ b/app/controllers/concerns/authentication.rb @@ -2,32 +2,10 @@ module Authentication extend ActiveSupport::Concern included do - before_action :set_login_timeout - before_action :logout_stale_session - before_action :require_admin - before_action :check_for_password_change - - helper_method :current_user, :current_session, :logged_in? - end - - def current_session - return @current_session if defined?(@current_session) - @current_session = AdminUserSession.find - end - def current_user - return @current_user if defined?(@current_user) - @current_user = current_session && current_session.record - end - - def logged_in? - current_user - end - - def redirect_to_target_or_default - redirect_to(session[:return_to] || admin_root_url) - session[:return_to] = nil + alias_method :logged_in?, :user_signed_in? + helper_method :logged_in? end def require_admin @@ -36,12 +14,6 @@ def require_admin end end - def check_for_password_change - if current_user.has_to_change_password? - redirect_to edit_admin_profile_url(current_user), alert: :change_password - end - end - def require_moderator unless current_user.is_a_moderator? || current_user.is_a_sysadmin? redirect_to admin_root_url, alert: :moderator_required @@ -53,18 +25,4 @@ def require_sysadmin redirect_to admin_root_url, alert: :sysadmin_required end end - - private - - def store_target_location - session[:return_to] = request.fullpath - end - - def set_login_timeout - AdminUser.logged_in_timeout = Site.login_timeout - end - - def logout_stale_session - current_session.destroy if current_session && current_session.stale? - end end diff --git a/app/controllers/concerns/flash_i18n.rb b/app/controllers/concerns/flash_i18n.rb index a74acdc1a..33d6084bf 100644 --- a/app/controllers/concerns/flash_i18n.rb +++ b/app/controllers/concerns/flash_i18n.rb @@ -1,6 +1,4 @@ module FlashI18n - protected - def redirect_to(url, options = {}) self.class._flash_types.each do |flash_type| if options.key?(flash_type) @@ -17,6 +15,8 @@ def redirect_to(url, options = {}) super(url, options) end + protected + def translate_flash(key) if admin_request? scope = :"admin.flash" diff --git a/app/jobs/archive_petition_job.rb b/app/jobs/archive_petition_job.rb index 6165c1de1..b10e73557 100644 --- a/app/jobs/archive_petition_job.rb +++ b/app/jobs/archive_petition_job.rb @@ -75,6 +75,8 @@ def perform(petition) e.subject = email.subject e.body = email.body e.sent_by = email.sent_by + e.email_count = email.email_count + e.emails_enqueued_at = email.emails_enqueued_at e.created_at = email.created_at e.updated_at = email.updated_at end diff --git a/app/jobs/archived/email_petitioners_job.rb b/app/jobs/archived/email_petitioners_job.rb index 7901e6e73..ada6fa7dc 100644 --- a/app/jobs/archived/email_petitioners_job.rb +++ b/app/jobs/archived/email_petitioners_job.rb @@ -12,6 +12,13 @@ class EmailPetitionersJob < ApplicationJob log_exception(exception) end + after_enqueue_send_email_jobs do + email.update_columns( + email_count: petition.signatures.subscribers, + emails_enqueued_at: Time.current + ) + end + def perform(**args) @email = args[:email] super diff --git a/app/jobs/concerns/email_all_petition_signatories.rb b/app/jobs/concerns/email_all_petition_signatories.rb index f8febf1a8..6dc685777 100644 --- a/app/jobs/concerns/email_all_petition_signatories.rb +++ b/app/jobs/concerns/email_all_petition_signatories.rb @@ -13,6 +13,10 @@ module EmailAllPetitionSignatories attr_reader :petition, :requested_at, :scope queue_as :high_priority + + with_options(skip_after_callbacks_if_terminated: true) do + define_callbacks :enqueue_send_email_jobs + end end module ClassMethods @@ -23,6 +27,14 @@ def run_later_tonight(**args) set(wait_until: later_tonight).perform_later(**args.merge(requested_at: requested_at_iso8601)) end + def before_enqueue_send_email_jobs(*filters, &blk) + set_callback(:enqueue_send_email_jobs, :before, *filters, &blk) + end + + def after_enqueue_send_email_jobs(*filters, &blk) + set_callback(:enqueue_send_email_jobs, :after, *filters, &blk) + end + private def requested_at @@ -64,8 +76,10 @@ def perform(**args) # and enqueues a job to do the actual sending def enqueue_send_email_jobs Appsignal.without_instrumentation do - signatures_to_email.find_each do |signature| - email_delivery_job_class.perform_later(**mailer_arguments(signature)) + run_callbacks :enqueue_send_email_jobs do + signatures_to_email.find_each do |signature| + email_delivery_job_class.perform_later(**mailer_arguments(signature)) + end end end end diff --git a/app/jobs/email_petitioners_job.rb b/app/jobs/email_petitioners_job.rb index 764ec6c8e..11aecbd31 100644 --- a/app/jobs/email_petitioners_job.rb +++ b/app/jobs/email_petitioners_job.rb @@ -11,6 +11,13 @@ class EmailPetitionersJob < ApplicationJob log_exception(exception) end + after_enqueue_send_email_jobs do + email.update_columns( + email_count: petition.signatures.subscribers, + emails_enqueued_at: Time.current + ) + end + def perform(**args) @email = args[:email] super diff --git a/app/models/admin_user.rb b/app/models/admin_user.rb index 3ac906f3a..e95e38c22 100644 --- a/app/models/admin_user.rb +++ b/app/models/admin_user.rb @@ -1,27 +1,20 @@ class AdminUser < ActiveRecord::Base - DISABLED_LOGIN_COUNT = 5 SYSADMIN_ROLE = 'sysadmin' MODERATOR_ROLE = 'moderator' REVIEWER_ROLE = 'reviewer' ROLES = [SYSADMIN_ROLE, MODERATOR_ROLE, REVIEWER_ROLE] - PASSWORD_REGEX = /\A.*(?=.*\d)(?=.*[a-z])(?=.*[A-Z])(?=.*[\W_]).*\z/ class CannotDeleteCurrentUser < RuntimeError; end class MustBeAtLeastOneAdminUser < RuntimeError; end - acts_as_authentic do |config| - config.crypto_provider = ::Authlogic::CryptoProviders::SCrypt + devise :trackable, :timeoutable - config.check_passwords_against_database = true - config.ignore_blank_passwords = true - config.logged_in_timeout = Site.login_timeout - config.require_password_confirmation = true - - config.validates_length_of :password, minimum: 8, unless: ->(u) { u.password.blank? } - config.validates_confirmation_of :password, unless: ->(u) { u.password.blank? } - config.validates :email, email: true, uniqueness: { case_sensitive: false } - config.validates_uniqueness_of :email, uniqueness: { case_sensitive: false } - end + # TODO: Drop these columns once rollout of SSO has been completed + self.ignored_columns = %i[ + encrypted_password password_salt + force_password_reset password_changed_at + failed_attempts locked_at + ] with_options dependent: :restrict_with_exception do with_options foreign_key: :moderated_by_id do @@ -31,48 +24,53 @@ class MustBeAtLeastOneAdminUser < RuntimeError; end end # = Validations = - validates_presence_of :email, :first_name, :last_name - validates_presence_of :password, on: :create - validates_format_of :password, with: PASSWORD_REGEX, allow_blank: true - validates_inclusion_of :role, in: ROLES + validates :first_name, :last_name, presence: true + validates :email, presence: true, email: true + validates :email, uniqueness: { case_sensitive: false } + validates :role, presence: true, inclusion: { in: ROLES } # = Callbacks = - before_update if: :crypted_password_changed? do - self.force_password_reset = false - self.password_changed_at = Time.current - end + before_save :reset_persistence_token, unless: :persistence_token? # = Finders = scope :by_name, -> { order(:last_name, :first_name) } scope :by_role, ->(role) { where(role: role).order(:id) } # = Methods = - def current_password - defined?(@current_password) ? @current_password : nil + def self.timeout_in + Site.login_timeout.seconds + end + + def self.find_or_create_from!(provider, auth_data) + find_or_create_by!(email: auth_data.fetch(:uid)) do |user| + user.first_name = auth_data.info.fetch(:first_name) + user.last_name = auth_data.info.fetch(:last_name) + groups = Array.wrap(auth_data.info.fetch(:groups)) + + if (groups & provider.sysadmins).any? + user.role = SYSADMIN_ROLE + elsif (groups & provider.moderators).any? + user.role = MODERATOR_ROLE + elsif (groups & provider.reviewers).any? + user.role = REVIEWER_ROLE + end + end + rescue ActiveRecord::RecordNotUnique => e + find_by!(email: auth_data.fetch(:uid)) + rescue ActiveRecord::RecordInvalid => e + Appsignal.send_exception(e) and return nil end - def current_password=(value) - @current_password = value + def reset_persistence_token + self.persistence_token = SecureRandom.hex(64) end - def update_with_password(attrs) - if attrs[:password].blank? - attrs.delete(:password) - attrs.delete(:password_confirmation) if attrs[:password_confirmation].blank? - end - - self.attributes = attrs - self.valid? - - if current_password.blank? - errors.add(:current_password, :blank) - elsif !valid_password?(current_password) - errors.add(:current_password, :invalid) - elsif current_password == password - errors.add(:password, :taken) - end + def reset_persistence_token! + SecureRandom.hex(64).tap { |token| update_column(:persistence_token, token) } + end - errors.empty? && save(validate: false) + def valid_persistence_token?(token) + persistence_token == token end def destroy(current_user: nil) @@ -109,10 +107,6 @@ def is_a_reviewer? self.role == 'reviewer' end - def has_to_change_password? - self.force_password_reset or (self.password_changed_at and self.password_changed_at < 9.months.ago) - end - def can_take_petitions_down? is_a_sysadmin? || is_a_moderator? end @@ -129,19 +123,11 @@ def can_moderate_petitions? is_a_sysadmin? || is_a_moderator? end - def account_disabled - self.failed_login_count >= DISABLED_LOGIN_COUNT - end - - def account_disabled=(flag) - self.failed_login_count = (flag == "0" or !flag) ? 0 : DISABLED_LOGIN_COUNT - end - - def elapsed_time(now = Time.current) + def elapsed_time(last_request_at, now = Time.current) (now - last_request_at).floor end - def time_remaining(now = Time.current) - [Site.login_timeout - elapsed_time(now), 0].max + def time_remaining(last_request_at, now = Time.current) + [Site.login_timeout - elapsed_time(last_request_at, now), 0].max end end diff --git a/app/models/admin_user_session.rb b/app/models/admin_user_session.rb deleted file mode 100644 index e11841576..000000000 --- a/app/models/admin_user_session.rb +++ /dev/null @@ -1,33 +0,0 @@ -class AdminUserSession < Authlogic::Session::Base - allow_http_basic_auth false - consecutive_failed_logins_limit AdminUser::DISABLED_LOGIN_COUNT - logout_on_timeout true - - before_save do - record.reset_persistence_token! - end - - before_destroy do - controller.reset_session - - if stale? - stale_record.reset_persistence_token! - else - record.reset_persistence_token! - end - end - - def last_login_attempt? - failed_login_count == consecutive_failed_logins_limit - 1 - end - - def time_remaining - record ? record.time_remaining : 0 - end - - private - - def failed_login_count - attempted_record.present? ? attempted_record.failed_login_count : 0 - end -end diff --git a/app/models/archived/government_response.rb b/app/models/archived/government_response.rb index 0e8802da0..97873ff17 100644 --- a/app/models/archived/government_response.rb +++ b/app/models/archived/government_response.rb @@ -13,6 +13,14 @@ class GovernmentResponse < ActiveRecord::Base petition.touch(:government_response_at) unless petition.government_response_at? end + after_destroy do + # This prevents any enqueued email jobs from being sent + petition.set_email_requested_at_for("government_response") + + # This removes the petition from the 'Government response' list + petition.update_columns(government_response_at: nil) + end + def responded_on super || default_responded_on end diff --git a/app/models/archived/signature.rb b/app/models/archived/signature.rb index 1cb34d5e7..6270dc67e 100644 --- a/app/models/archived/signature.rb +++ b/app/models/archived/signature.rb @@ -108,6 +108,10 @@ def subscribed where(notify_by_email: true) end + def subscribers + validated.subscribed.count + end + def validated where(state: VALIDATED_STATE) end diff --git a/app/models/concerns/perishable_token_generator.rb b/app/models/concerns/perishable_token_generator.rb index f591113af..47bf9858c 100644 --- a/app/models/concerns/perishable_token_generator.rb +++ b/app/models/concerns/perishable_token_generator.rb @@ -4,7 +4,7 @@ module PerishableTokenGenerator class_methods do def has_perishable_token(called: 'perishable_token') before_create do - write_attribute(called, Authlogic::Random.friendly_token) + write_attribute(called, SecureRandom.base58(20)) end end end diff --git a/app/models/government_response.rb b/app/models/government_response.rb index af9eaea70..af01a709a 100644 --- a/app/models/government_response.rb +++ b/app/models/government_response.rb @@ -11,6 +11,18 @@ class GovernmentResponse < ActiveRecord::Base petition.touch(:government_response_at) unless petition.government_response_at? end + after_destroy do + unless petition.archived? + Appsignal.increment_counter("petition.responded", -1) + + # This prevents any enqueued email jobs from being sent + petition.set_email_requested_at_for("government_response") + + # This removes the petition from the 'Government response' list + petition.update_columns(government_response_at: nil) + end + end + def responded_on super || default_responded_on end diff --git a/app/models/identity_provider.rb b/app/models/identity_provider.rb new file mode 100644 index 000000000..67025cfb4 --- /dev/null +++ b/app/models/identity_provider.rb @@ -0,0 +1,100 @@ +class IdentityProvider + class ProviderNotFound < ArgumentError; end + + class << self + delegate :each, to: :providers + + def providers + @providers ||= load_providers + end + + def names + providers.map(&:name) + end + + def find_by(domain:) + providers.detect { |provider| provider.domains.include?(domain) } + end + + def find_by!(name:) + providers.detect { |provider| provider.name.to_s == name } || raise_provider_not_found(name) + end + + private + + def load_providers + Rails.application.config_for(:sso).map { |options| IdentityProvider.new(options) } + end + + def raise_provider_not_found(name) + raise ProviderNotFound, "Couldn't find the provider '#{name}'" + end + end + + attr_reader :name, :attribute_statements + attr_reader :assertion_consumer_service_url, :sp_entity_id + attr_reader :idp_sso_service_url, :idp_cert, :domains + attr_reader :sysadmins, :moderators, :reviewers + + def initialize(options) + @name = options.fetch(:name).to_sym + @attribute_statements = options.fetch(:attributes, default_attributes) + @assertion_consumer_service_url = "#{Site.moderate_url}/admin/auth/#{name}/callback" + @sp_entity_id = "#{Site.moderate_url}/admin/auth/#{name}" + @idp_sso_service_url = options.fetch(:idp_sso_service_url) + @idp_cert = options.fetch(:idp_cert, "") + @domains = options.fetch(:domains) + @sysadmins = options.fetch(:sysadmins, []) + @moderators = options.fetch(:moderators, []) + @reviewers = options.fetch(:reviewers, []) + + unless klass_defined? + strategies.const_set(klass, new_klass) + end + end + + def to_param + name.to_s + end + + def config + { + attribute_statements: attribute_statements, + assertion_consumer_service_url: assertion_consumer_service_url, + sp_entity_id: sp_entity_id, + idp_sso_service_url: idp_sso_service_url, + idp_cert: idp_cert + } + end + + private + + def default_attributes + { + email: ["email"], + first_name: ["first_name"], + last_name: ["last_name"], + groups: ["groups"] + } + end + + def strategies + OmniAuth::Strategies + end + + def parent_klass + OmniAuth::Strategies::SAML + end + + def new_klass + Class.new(parent_klass) + end + + def klass + @klass ||= name.to_s.camelize.to_sym + end + + def klass_defined? + strategies.const_defined?(klass, false) + end +end diff --git a/app/models/signature.rb b/app/models/signature.rb index 605f31407..1a0944099 100644 --- a/app/models/signature.rb +++ b/app/models/signature.rb @@ -623,7 +623,7 @@ def validate!(now = Time.current, force: false, request: nil) end unless signed_token? - attributes[:signed_token] = Authlogic::Random.friendly_token + attributes[:signed_token] = SecureRandom.base58(20) end update_columns(attributes) @@ -820,7 +820,7 @@ def generate_uuid end def generate_and_save_signed_token - token = Authlogic::Random.friendly_token + token = SecureRandom.base58(20) retry_lock do if signed_token? diff --git a/app/models/site.rb b/app/models/site.rb index 615d400e2..e56e4084b 100644 --- a/app/models/site.rb +++ b/app/models/site.rb @@ -86,6 +86,14 @@ def moderate_host_with_port instance.moderate_host_with_port end + def moderate_url + if table_exists? + instance.moderate_url + else + default_moderate_url + end + end + def constraints_for_moderation if table_exists? instance.constraints_for_moderation diff --git a/app/views/admin/admin_users/_form.html.erb b/app/views/admin/admin_users/_form.html.erb deleted file mode 100644 index 458be9fab..000000000 --- a/app/views/admin/admin_users/_form.html.erb +++ /dev/null @@ -1,65 +0,0 @@ -<%= form_for [:admin, @user] do |f| -%> - <%= form_row for: [f.object, :first_name] do %> - <%= f.label :first_name, class: 'form-label form-label-inline' %><%= mandatory_field %> - <%= error_messages_for_field f.object, :first_name %> - <%= f.text_field :first_name, tabindex: increment, class: 'form-control', autofocus: 'autofocus' %> - <% end %> - - <%= form_row for: [f.object, :last_name] do %> - <%= f.label :last_name, class: 'form-label form-label-inline' %><%= mandatory_field %> - <%= error_messages_for_field f.object, :last_name %> - <%= f.text_field :last_name, tabindex: increment, class: 'form-control' %> - <% end %> - - <%= form_row for: [f.object, :email] do %> - <%= f.label :email, class: 'form-label form-label-inline' %><%= mandatory_field %> - <%= error_messages_for_field f.object, :email %> - <%= f.email_field :email, tabindex: increment, class: 'form-control' %> - <% end %> - - <%= form_row for: [f.object, :email], class: 'inline' do %> - <%= f.label :role, class: 'form-label' %> - <%= error_messages_for_field f.object, :role %> - <% AdminUser::ROLES.each do |role| %> -
- <%= f.radio_button :role, role %> - <%= f.label :role, role, for: "admin_user_role_#{role}" %> -
- <% end %> - <% end %> - - <%= form_row for: [f.object, :force_password_reset] do %> - <%= error_messages_for_field f.object, :force_password_reset %> -
- <%= f.check_box :force_password_reset, tabindex: increment %> - <%= f.label :force_password_reset %> -
- <% end %> - - <%= form_row for: [f.object, :account_disabled] do %> - <%= error_messages_for_field f.object, :account_disabled %> -
- <%= f.check_box :account_disabled, tabindex: increment %> - <%= f.label :account_disabled %> -
- <% end %> - - <%= field_set_tag do %> - <%= content_tag(:legend, 'Passwords', class: 'form-label-bold') %> - <%= content_tag(:p, 'Note, no password needs to be entered unless you want to change it') unless f.object.new_record? %> - <%= form_row for: [f.object, :password] do %> - <%= f.label :password, class: 'form-label form-label-inline' %><%= mandatory_field if f.object.new_record? %> - <%= error_messages_for_field f.object, :password %> - <%= f.password_field :password, tabindex: increment, class: 'form-control', autocomplete: 'off' %> - <% end %> - - <%= form_row for: [f.object, :password_confirmation] do %> - <%= f.label :password_confirmation, class: 'form-label form-label-inline' %><%= mandatory_field if f.object.new_record? %> - <%= error_messages_for_field f.object, :password_confirmation %> - <%= f.password_field :password_confirmation, tabindex: increment, class: 'form-control', autocomplete: 'off' %> - <% end %> - <% end %> - - - <%= f.submit 'Save', class: "button" %> -<% end -%> diff --git a/app/views/admin/admin_users/edit.html.erb b/app/views/admin/admin_users/edit.html.erb deleted file mode 100644 index 3b6470a30..000000000 --- a/app/views/admin/admin_users/edit.html.erb +++ /dev/null @@ -1,6 +0,0 @@ -

Edit user

-
-
- <%= render :partial => 'form' %> -
-
diff --git a/app/views/admin/admin_users/index.html.erb b/app/views/admin/admin_users/index.html.erb index cff2fb66e..906fbe783 100644 --- a/app/views/admin/admin_users/index.html.erb +++ b/app/views/admin/admin_users/index.html.erb @@ -1,26 +1,16 @@ -<%= link_to 'New user', new_admin_admin_user_path, class: "button" %> - - - + <%- @users.each do |user| -%> - + - - + <%- end -%> <%= will_paginate(@users) if @users.any? %> diff --git a/app/views/admin/admin_users/new.html.erb b/app/views/admin/admin_users/new.html.erb deleted file mode 100644 index 85463f352..000000000 --- a/app/views/admin/admin_users/new.html.erb +++ /dev/null @@ -1,4 +0,0 @@ -

New user

-
- <%= render :partial => 'form' %> -
\ No newline at end of file diff --git a/app/views/admin/archived/government_response/_petition_action_government_response.html.erb b/app/views/admin/archived/government_response/_petition_action_government_response.html.erb index 0193f41fd..600412287 100644 --- a/app/views/admin/archived/government_response/_petition_action_government_response.html.erb +++ b/app/views/admin/archived/government_response/_petition_action_government_response.html.erb @@ -21,8 +21,11 @@ <% end %> <%= email_petitioners_with_count_submit_button(f, petition) %> - <%= f.submit "Save", name: 'save', class: 'button-secondary' %> - <%= link_to 'Cancel', admin_archived_petition_path(@petition), class: 'button-secondary' %> + <% if @government_response.persisted? %> + <%= link_to "Delete", admin_archived_petition_government_response_path(@petition), class: "button-warning", method: :delete, data: { confirm: "Are you sure you want to delete the government response?" } %> + <% end %> + <%= f.submit "Save", name: "save", class: "button-secondary" %> + <%= link_to "Cancel", admin_archived_petition_path(@petition), class: "button-secondary" %> <% end %> <%= javascript_include_tag 'character-counter' %> diff --git a/app/views/admin/archived/petition_emails/_petition_action_email_petitioners.html.erb b/app/views/admin/archived/petition_emails/_petition_action_email_petitioners.html.erb index 42ef9b56f..f7dee5716 100644 --- a/app/views/admin/archived/petition_emails/_petition_action_email_petitioners.html.erb +++ b/app/views/admin/archived/petition_emails/_petition_action_email_petitioners.html.erb @@ -5,6 +5,8 @@ + + @@ -14,6 +16,8 @@ + <%= button_to 'Delete', admin_archived_petition_email_path(petition, email), method: :delete, class: 'button-warning', data: { confirm: 'Delete other parliamentary business?' } %> diff --git a/app/views/admin/government_response/_petition_action_government_response.html.erb b/app/views/admin/government_response/_petition_action_government_response.html.erb index 442a8480c..0cddcc753 100644 --- a/app/views/admin/government_response/_petition_action_government_response.html.erb +++ b/app/views/admin/government_response/_petition_action_government_response.html.erb @@ -21,8 +21,11 @@ <% end %> <%= email_petitioners_with_count_submit_button(f, petition, disabled: @petition.editing_disabled?) %> - <%= f.submit "Save", name: 'save', class: 'button-secondary', disabled: @petition.editing_disabled? %> - <%= link_to 'Cancel', admin_petition_path(@petition), class: 'button-secondary' %> + <% if @government_response.persisted? %> + <%= link_to "Delete", admin_petition_government_response_path(@petition), class: "button-warning", method: :delete, data: { confirm: "Are you sure you want to delete the government response?" } %> + <% end %> + <%= f.submit "Save", name: "save", class: "button-secondary", disabled: @petition.editing_disabled? %> + <%= link_to "Cancel", admin_petition_path(@petition), class: "button-secondary" %> <% end %> <%= javascript_include_tag 'character-counter' %> diff --git a/app/views/admin/petition_emails/_petition_action_email_petitioners.html.erb b/app/views/admin/petition_emails/_petition_action_email_petitioners.html.erb index 6e975e224..fa5d90405 100644 --- a/app/views/admin/petition_emails/_petition_action_email_petitioners.html.erb +++ b/app/views/admin/petition_emails/_petition_action_email_petitioners.html.erb @@ -5,6 +5,8 @@ + + @@ -12,6 +14,8 @@ <% petition.emails.select(&:persisted?).each do |email| %> + +
Name Email RoleDisabledLast login
<%= link_to user.name, edit_admin_admin_user_path(user) %><%= user.name%> <%= mail_to user.email %> <%= user.role %><%= user.account_disabled ? 'Yes' : '' %> - <% if user != current_user %> - <%= cms_delete_link user, :url => admin_admin_user_path(user) %> - <% else %> -   - <% end %> - <%= short_date_time_format(user.current_sign_in_at) %>
SubjectCountSent Actions
<%= email.subject %> <%= button_to 'Edit', edit_admin_archived_petition_email_path(petition, email), method: :get, class: 'button' %> + <%= number_with_delimiter(email.email_count) || "–" %><%= date_format(email.emails_enqueued_at) || "–" %>
SubjectCountSent Actions
<%= email.subject %><%= number_with_delimiter(email.email_count) || "–" %><%= date_format(email.emails_enqueued_at) || "–" %> <%= button_to 'Edit', edit_admin_petition_email_path(petition, email), method: :get, class: 'button', disabled: @petition.editing_disabled? %> <%= button_to 'Delete', admin_petition_email_path(petition, email), method: :delete, class: 'button-warning', disabled: @petition.editing_disabled?, data: { confirm: 'Delete other parliamentary business?' } %> diff --git a/app/views/admin/profile/edit.html.erb b/app/views/admin/profile/edit.html.erb deleted file mode 100644 index 0182b5982..000000000 --- a/app/views/admin/profile/edit.html.erb +++ /dev/null @@ -1,26 +0,0 @@ -

Change your password

-
-
- <%= form_for [:admin, current_user], url: admin_profile_path(current_user) do |f| -%> - <%= form_row for: [f.object, :current_password] do %> - <%= f.label :current_password, class: 'form-label' %> - <%= error_messages_for_field f.object, :current_password %> - <%= f.password_field :current_password, tabindex: increment, class: 'form-control', autofocus: 'autofocus', autocomplete: 'off' %> - <% end %> - - <%= form_row for: [f.object, :password] do %> - <%= f.label :password, 'New password', class: 'form-label' %> - <%= error_messages_for_field f.object, :password %> - <%= f.password_field :password, tabindex: increment, class: 'form-control', autocomplete: 'off' %> - <% end %> - - <%= form_row for: [f.object, :password_confirmation] do %> - <%= f.label :password_confirmation, class: 'form-label' %> - <%= error_messages_for_field f.object, :password_confirmation %> - <%= f.password_field :password_confirmation, tabindex: increment, class: 'form-control', autocomplete: 'off' %> - <% end %> - - <%= f.submit 'Save', class: "button" %> - <% end -%> -
-
diff --git a/app/views/admin/sessions/continue.json.jbuilder b/app/views/admin/sessions/continue.json.jbuilder new file mode 100644 index 000000000..857102f0b --- /dev/null +++ b/app/views/admin/sessions/continue.json.jbuilder @@ -0,0 +1 @@ +json.last_request_at last_request_at diff --git a/app/views/admin/sessions/new.html.erb b/app/views/admin/sessions/new.html.erb new file mode 100644 index 000000000..3f80f9479 --- /dev/null +++ b/app/views/admin/sessions/new.html.erb @@ -0,0 +1,13 @@ +

Sign in

+
+
+ <%= form_for(@user, as: :user, url: admin_login_path, authenticity_token: form_authenticity_token) do |form| %> +
+ <%= form.label :email, "Email", class: "form-label" %> + <%= form.text_field :email, class: "form-control", type: "email", autofocus: "autofocus" %> +
+ + <%= form.submit "Sign in", class: "button" %> + <% end %> +
+
diff --git a/app/views/admin/sessions/status.json.jbuilder b/app/views/admin/sessions/status.json.jbuilder new file mode 100644 index 000000000..804614345 --- /dev/null +++ b/app/views/admin/sessions/status.json.jbuilder @@ -0,0 +1,5 @@ +if current_user + json.time_remaining current_user.time_remaining(last_request_at) +else + json.time_remaining 0 +end diff --git a/app/views/admin/shared/_header_user_actions.html.erb b/app/views/admin/shared/_header_user_actions.html.erb index 41cbe6bd8..d4024db58 100644 --- a/app/views/admin/shared/_header_user_actions.html.erb +++ b/app/views/admin/shared/_header_user_actions.html.erb @@ -1,6 +1,6 @@ <% if current_user %>
    -
  • <%= link_to current_user.pretty_name, edit_admin_profile_path(current_user) %>
    • +
  • <%= current_user.pretty_name %>
    • <% if current_user.is_a_sysadmin? %>
  • <%= link_to "Site", edit_admin_site_path %>
  • <%= link_to "Parliament", admin_parliament_path %>
    • diff --git a/app/views/admin/user_sessions/continue.json.jbuilder b/app/views/admin/user_sessions/continue.json.jbuilder deleted file mode 100644 index f60d49986..000000000 --- a/app/views/admin/user_sessions/continue.json.jbuilder +++ /dev/null @@ -1 +0,0 @@ -json.last_request_at current_user.last_request_at diff --git a/app/views/admin/user_sessions/new.html.erb b/app/views/admin/user_sessions/new.html.erb deleted file mode 100644 index 4554fe2b3..000000000 --- a/app/views/admin/user_sessions/new.html.erb +++ /dev/null @@ -1,16 +0,0 @@ -

    Sign in

    -
    -
    - <%= form_for @user_session do |f| %> -
    - <%= f.label :email, "Email", class: "form-label" %> - <%= f.text_field :email, :class => 'form-control', :type => 'email', :autofocus => 'autofocus' %> -
    -
    - <%= f.label :password, "Password", class: "form-label" %> - <%= f.password_field :password, :class => 'form-control' %> -
    - <%= f.submit 'Sign in', :class => 'button' %> - <% end %> -
    -
    diff --git a/app/views/admin/user_sessions/status.json.jbuilder b/app/views/admin/user_sessions/status.json.jbuilder deleted file mode 100644 index 618c77b4c..000000000 --- a/app/views/admin/user_sessions/status.json.jbuilder +++ /dev/null @@ -1,5 +0,0 @@ -if current_session - json.time_remaining current_session.time_remaining -else - json.time_remaining 0 -end diff --git a/config/brakeman.ignore b/config/brakeman.ignore index 3c5131577..59e71e92b 100644 --- a/config/brakeman.ignore +++ b/config/brakeman.ignore @@ -128,26 +128,6 @@ "user_input": null, "confidence": "Medium", "note": "" - }, - { - "warning_type": "Mass Assignment", - "warning_code": 105, - "fingerprint": "f523941f55e48c9af1dda1b71dd47c35e8cd6bce092c90e04bc940d257bf44e8", - "check_name": "PermitAttributes", - "message": "Potentially dangerous key allowed for mass assignment", - "file": "app/controllers/admin/admin_users_controller.rb", - "line": 64, - "link": "https://brakemanscanner.org/docs/warning_types/mass_assignment/", - "code": "params.require(:admin_user).permit(:password, :password_confirmation, :first_name, :last_name, :role, :email, :force_password_reset, :account_disabled)", - "render_path": null, - "location": { - "type": "method", - "class": "Admin::AdminUsersController", - "method": "admin_user_params" - }, - "user_input": ":role", - "confidence": "Medium", - "note": "" } ], "updated": "2023-03-30 00:48:36 +0100", diff --git a/config/initializers/delayed_web.rb b/config/initializers/delayed_web.rb index 582345de3..d0ace5844 100644 --- a/config/initializers/delayed_web.rb +++ b/config/initializers/delayed_web.rb @@ -1,19 +1,31 @@ # Tell Delayed::Web that we're using ActiveRecord as our backend. Rails.application.config.to_prepare do Delayed::Web::Job.backend = 'active_record' -end -# Authenticate our delayed job web interface -Delayed::Web::ApplicationController.class_eval do - include Authentication, FlashI18n + # Authenticate our delayed job web interface + Delayed::Web::ApplicationController.class_eval do + include FlashI18n - def admin_request? - true - end + before_action :require_admin + + def admin_request? + true + end + + protected + + def admin_login_url + main_app.admin_login_url + end - protected + def current_user + @current_user ||= warden.authenticate(scope: :user) + end - def admin_login_url - main_app.admin_login_url + def require_admin + unless current_user + redirect_to admin_login_url, alert: :admin_required + end + end end end diff --git a/config/initializers/devise.rb b/config/initializers/devise.rb new file mode 100644 index 000000000..204e689f1 --- /dev/null +++ b/config/initializers/devise.rb @@ -0,0 +1,61 @@ +Devise.setup do |config| + # ==> Controller configuration + config.parent_controller = 'Admin::AdminController' + + # ==> Mailer Configuration + config.mailer_sender = 'petitionscommittee@parliament.uk' + config.mailer = 'Devise::Mailer' + config.parent_mailer = 'ActionMailer::Base' + + # ==> ORM configuration + require 'devise/orm/active_record' + + # ==> Configuration for any authentication mechanism + config.authentication_keys = [:email] + config.case_insensitive_keys = [:email] + config.strip_whitespace_keys = [:email] + + # ==> Configuration for :timeoutable + # This is overidden on the AdminUser model to use Site.login_timeout + config.timeout_in = 24.hours + + # ==> Navigation configuration + config.sign_out_via = :get + + # ==> Warden configuration + # Reset the token after logging in so that other sessions are logged out + Warden::Manager.after_set_user except: :fetch do |user, warden, options| + if warden.authenticated?(:user) + session = warden.session(:user) + session['persistence_token'] = user.reset_persistence_token! + end + end + + # Logout the user if the token doesn't match what's in the session + Warden::Manager.after_set_user only: :fetch do |user, warden, options| + if warden.authenticated?(:user) && options[:store] != false + session = warden.session(:user) + + unless user.valid_persistence_token?(session['persistence_token']) + warden.raw_session.clear + warden.logout(:user) + + throw :warden, scope: :user, message: :other_login + end + end + end + + # The devise failure app redirects to self when a session times out + # which causes issues with flash messages, etc. so change the message + # in the options hash to trigger the alternative path. + Warden::Manager.before_failure do |env, options| + if options[:message] == :timeout + options[:message] = :timedout + end + end + + # Reset the token after logging out + Warden::Manager.before_logout do |user, warden, options| + user && user.reset_persistence_token! + end +end diff --git a/config/initializers/omniauth.rb b/config/initializers/omniauth.rb new file mode 100644 index 000000000..22bf0a143 --- /dev/null +++ b/config/initializers/omniauth.rb @@ -0,0 +1,10 @@ +Rails.application.config.middleware.use OmniAuth::Builder do + configure do |config| + config.path_prefix = '/admin/auth' + config.logger = Rails.logger + end + + IdentityProvider.each do |idp| + provider idp.name, idp.config + end +end diff --git a/config/locales/activerecord.en-GB.yml b/config/locales/activerecord.en-GB.yml index 18bda7f12..30fb9e9c2 100644 --- a/config/locales/activerecord.en-GB.yml +++ b/config/locales/activerecord.en-GB.yml @@ -36,13 +36,6 @@ en-GB: models: admin_user: attributes: - current_password: - invalid: "Current password is incorrect" - password: - invalid: "Password must contain at least one digit, a lower and upper case letter and a special character" - taken: "Password is the same as the current password" - password_confirmation: - confirmation: "Password confirmation doesn't match password" role: inclusion: "Role '%{value}' is invalid" diff --git a/config/locales/admin.en-GB.yml b/config/locales/admin.en-GB.yml index afed6feeb..abbaf34a5 100644 --- a/config/locales/admin.en-GB.yml +++ b/config/locales/admin.en-GB.yml @@ -54,6 +54,8 @@ en-GB: email_resent_to_creator: "Resent the email to the petition creator and forwarded a copy to the feedback address" email_sent_overnight: "Email will be sent overnight" enqueued_petition_statistics_update: "Updating the petition statistics - please wait a few minutes and then refresh this page" + government_response_deleted: "Deleted government response successfully" + government_response_not_deleted: "Unable to delete government response - please contact support" government_response_updated: "Updated government response successfully" failed_tasks: "There was a problem starting the tasks - please contact support" invalidation_cant_be_cancelled: "Can't cancel invalidations that have completed" @@ -71,9 +73,8 @@ en-GB: invalidation_removed: "Invalidation removed successfully" invalidation_started: "Enqueued the invalidation %{summary}" invalidation_updated: "Invalidation updated successfully" - last_login: "You have one more attempt before your account is temporarily disabled" - disabled_login: "Failed login limit exceeded, your account has been temporarily disabled" - invalid_login: "Invalid email/password combination" + invalid_login: "Invalid login details" + login_failed: "There was a problem logging in - please contact support" logged_out: "You have been logged out" moderator_required: "You must be logged in as a moderator or system administrator to view this page" moderation_delay_sent: "An email has been sent to creators that moderation has been delayed" diff --git a/config/locales/devise.en-GB.yml b/config/locales/devise.en-GB.yml new file mode 100644 index 000000000..cd2e5af47 --- /dev/null +++ b/config/locales/devise.en-GB.yml @@ -0,0 +1,18 @@ +# Additional translations at https://github.com/heartcombo/devise/wiki/I18n + +en: + devise: + failure: + already_authenticated: "You are already logged in" + invalid: "Invalid email/password combination" + locked: "Failed login limit exceeded, your account has been temporarily disabled" + last_attempt: "You have one more attempt before your account is temporarily disabled" + other_login: "You have been logged out" + not_found_in_database: "Invalid email/password combination" + timedout: "Your session expired. Please log in again to continue" + omniauth_callbacks: + signed_in: "Logged in successfully" + sessions: + signed_in: "Logged in successfully" + signed_out: "You have been logged out" + already_signed_out: "You have been logged out" diff --git a/config/routes.rb b/config/routes.rb index 697461023..e50ae9697 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -130,9 +130,8 @@ resource :parliament, only: %i[show update] resource :search, only: %i[show] - resources :admin_users + resources :admin_users, only: %[index] resources :profile, only: %i[edit update] - resources :user_sessions, only: %i[create] resources :invalidations, except: %i[show] do post :cancel, :count, :start, on: :member @@ -152,7 +151,6 @@ scope only: %i[show update] do resource :debate_outcome, path: 'debate-outcome' - resource :government_response, path: 'government-response', controller: 'government_response' resource :notes resource :details, controller: 'petition_details' resource :schedule_debate, path: 'schedule-debate', controller: 'schedule_debate' @@ -163,6 +161,10 @@ resource :removal, controller: 'petition_removals' end + scope only: %i[show update destroy] do + resource :government_response, path: 'government-response', controller: 'government_response' + end + resources :signatures, only: %i[index destroy] do post :validate, :invalidate, on: :member post :subscribe, :unsubscribe, on: :member @@ -213,7 +215,6 @@ scope only: %i[show update] do resource :debate_outcome, path: 'debate-outcome' - resource :government_response, path: 'government-response', controller: 'government_response' resource :notes resource :details, controller: 'petition_details' resource :schedule_debate, path: 'schedule-debate', controller: 'schedule_debate' @@ -222,6 +223,10 @@ resource :topics, controller: 'petition_topics' resource :removal, controller: 'petition_removals' end + + scope only: %i[show update destroy] do + resource :government_response, path: 'government-response', controller: 'government_response' + end end resources :signatures, only: %i[index destroy] do @@ -239,15 +244,32 @@ get '/', action: 'index', as: :stats post '/', action: 'create', as: nil end + end + + devise_for :users, class_name: 'AdminUser', module: 'admin', skip: %i[sessions] - controller 'user_sessions' do - get '/logout', action: 'destroy' - get '/login', action: 'new' - get '/continue', action: 'continue' - get '/status', action: 'status' + as :user do + controller 'admin/sessions' do + get '/admin/login', action: 'new' + post '/admin/login', action: 'create', as: nil + get '/admin/logout', action: 'destroy' + get '/admin/continue', action: 'continue' + get '/admin/status', action: 'status' + end + + controller 'admin/omniauth_callbacks' do + get '/admin/auth/failure', action: 'failure' + + scope '/admin/auth/:provider', via: %i[get post] do + match '/', action: 'passthru', as: :sso_provider + match '/callback', action: 'saml', as: :sso_provider_callback + end end end end + # Devise needs a `new_user_session_url` helper for its failure app + direct(:new_user_session) { route_for(:admin_login) } + get 'ping', to: 'ping#ping' end diff --git a/config/sso.yml b/config/sso.yml new file mode 100644 index 000000000..e774e8c3e --- /dev/null +++ b/config/sso.yml @@ -0,0 +1,13 @@ +development: [] + +test: + - name: "example" + idp_sso_service_url: "http://localhost:3000/sso" + domains: + - "example.com" + sysadmins: + - "sysadmins" + moderators: + - "moderators" + reviewers: + - "reviewers" diff --git a/db/migrate/20231215151430_authlogic_to_devise.rb b/db/migrate/20231215151430_authlogic_to_devise.rb new file mode 100644 index 000000000..af9d78d10 --- /dev/null +++ b/db/migrate/20231215151430_authlogic_to_devise.rb @@ -0,0 +1,27 @@ +class AuthlogicToDevise < ActiveRecord::Migration[6.1] + def up + rename_column :admin_users, :crypted_password, :encrypted_password + rename_column :admin_users, :login_count, :sign_in_count + rename_column :admin_users, :current_login_at, :current_sign_in_at + rename_column :admin_users, :last_login_at, :last_sign_in_at + rename_column :admin_users, :current_login_ip, :current_sign_in_ip + rename_column :admin_users, :last_login_ip, :last_sign_in_ip + rename_column :admin_users, :failed_login_count, :failed_attempts + + add_column :admin_users, :locked_at, :timestamp + remove_column :admin_users, :last_request_at + end + + def down + rename_column :admin_users, :encrypted_password, :crypted_password + rename_column :admin_users, :sign_in_count, :login_count + rename_column :admin_users, :current_sign_in_at, :current_login_at + rename_column :admin_users, :last_sign_in_at, :last_login_at + rename_column :admin_users, :current_sign_in_ip, :current_login_ip + rename_column :admin_users, :last_sign_in_ip, :last_login_ip + rename_column :admin_users, :failed_attempts, :failed_login_count + + remove_column :admin_users, :locked_at + add_column :admin_users, :last_request_at, :timestamp + end +end diff --git a/db/migrate/20240117134550_add_email_count_to_other_parliamentary_business.rb b/db/migrate/20240117134550_add_email_count_to_other_parliamentary_business.rb new file mode 100644 index 000000000..f8989f9fc --- /dev/null +++ b/db/migrate/20240117134550_add_email_count_to_other_parliamentary_business.rb @@ -0,0 +1,6 @@ +class AddEmailCountToOtherParliamentaryBusiness < ActiveRecord::Migration[6.1] + def change + add_column :petition_emails, :email_count, :integer + add_column :petition_emails, :emails_enqueued_at, :datetime + end +end diff --git a/db/migrate/20240117150804_add_email_count_to_other_parliamentary_business_archived.rb b/db/migrate/20240117150804_add_email_count_to_other_parliamentary_business_archived.rb new file mode 100644 index 000000000..bee2afa38 --- /dev/null +++ b/db/migrate/20240117150804_add_email_count_to_other_parliamentary_business_archived.rb @@ -0,0 +1,6 @@ +class AddEmailCountToOtherParliamentaryBusinessArchived < ActiveRecord::Migration[6.1] + def change + add_column :archived_petition_emails, :email_count, :integer + add_column :archived_petition_emails, :emails_enqueued_at, :datetime + end +end diff --git a/db/schema.rb b/db/schema.rb index 6fa87c42f..b7caf0033 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -10,7 +10,7 @@ # # It's strongly recommended that you check this file into your version control system. -ActiveRecord::Schema.define(version: 2023_03_27_145255) do +ActiveRecord::Schema.define(version: 2024_01_17_150804) do # These are extensions that must be enabled in order to support this database enable_extension "intarray" @@ -47,14 +47,14 @@ create_table "admin_users", id: :serial, force: :cascade do |t| t.string "email", limit: 255, null: false t.string "persistence_token", limit: 255 - t.string "crypted_password", limit: 255 + t.string "encrypted_password", limit: 255 t.string "password_salt", limit: 255 - t.integer "login_count", default: 0 - t.integer "failed_login_count", default: 0 - t.datetime "current_login_at" - t.datetime "last_login_at" - t.string "current_login_ip", limit: 255 - t.string "last_login_ip", limit: 255 + t.integer "sign_in_count", default: 0 + t.integer "failed_attempts", default: 0 + t.datetime "current_sign_in_at" + t.datetime "last_sign_in_at" + t.string "current_sign_in_ip", limit: 255 + t.string "last_sign_in_ip", limit: 255 t.string "first_name", limit: 255 t.string "last_name", limit: 255 t.string "role", limit: 10, null: false @@ -62,7 +62,7 @@ t.datetime "password_changed_at" t.datetime "created_at" t.datetime "updated_at" - t.datetime "last_request_at" + t.datetime "locked_at" t.index ["email"], name: "index_admin_users_on_email", unique: true t.index ["last_name", "first_name"], name: "index_admin_users_on_last_name_and_first_name" end @@ -112,6 +112,8 @@ t.string "sent_by" t.datetime "created_at", null: false t.datetime "updated_at", null: false + t.integer "email_count" + t.datetime "emails_enqueued_at" t.index ["petition_id"], name: "index_archived_petition_emails_on_petition_id" end @@ -475,6 +477,8 @@ t.string "sent_by" t.datetime "created_at", null: false t.datetime "updated_at", null: false + t.integer "email_count" + t.datetime "emails_enqueued_at" t.index ["petition_id"], name: "index_petition_emails_on_petition_id" end diff --git a/docker/mailcatcher/Dockerfile b/docker/mailcatcher/Dockerfile index d29cadecd..4481d6817 100644 --- a/docker/mailcatcher/Dockerfile +++ b/docker/mailcatcher/Dockerfile @@ -2,7 +2,8 @@ FROM ruby:2.7 ARG VERSION=0.8.2 -RUN gem install mailcatcher -v $VERSION +RUN gem install sqlite3 -v 1.6.9 && \ + gem install mailcatcher -v $VERSION EXPOSE 1025 1080 diff --git a/features/admin/admin_access.feature b/features/admin/admin_access.feature index 3a7c1d88f..1c44c96c0 100644 --- a/features/admin/admin_access.feature +++ b/features/admin/admin_access.feature @@ -15,11 +15,11 @@ Feature: Restricted access to the admin console And I should be connected to the server via an ssl connection And the markup should be valid + @javascript Scenario: Login and logout to the admin console as a sysadmin - Given a sysadmin user exists with first_name: "John", last_name: "Admin", email: "admin@example.com", password: "Letmein1!", password_confirmation: "Letmein1!" + Given a sysadmin SSO user exists When I go to the admin login page - And I fill in "Email" with "admin@example.com" - And I fill in "Password" with "Letmein1!" + And I fill in "Email" with "sysadmin@example.com" And I press "Sign in" Then I should be on the admin home page And I should be connected to the server via an ssl connection @@ -29,74 +29,54 @@ Feature: Restricted access to the admin console And I follow "Logout" And I should be on the admin login page - Scenario: Login and logout to the admin console as a moderator user - Given a moderator user exists with first_name: "John", last_name: "Moderator", email: "admin@example.com", password: "Letmein1!", password_confirmation: "Letmein1!" + @javascript + Scenario: Login and logout to the admin console as a moderator + Given a moderator SSO user exists When I go to the admin login page - And I fill in "Email" with "admin@example.com" - And I fill in "Password" with "Letmein1!" + And I fill in "Email" with "moderator@example.com" And I press "Sign in" Then I should be on the admin home page - And I should see "John Moderator" + And I should be connected to the server via an ssl connection + And the markup should be valid And I should not see "Users" + And I should see "John Moderator" And I follow "Logout" And I should be on the admin login page - Scenario: Invalid login - Given I go to the admin login page - And I fill in "Email" with "admin@example.com" - And I fill in "Password" with "letmein1" + @javascript + Scenario: Login and logout to the admin console as a reviewer + Given a reviewer SSO user exists + When I go to the admin login page + And I fill in "Email" with "reviewer@example.com" And I press "Sign in" - Then I should see "Invalid email/password combination" - And should not see "Logout" + Then I should be on the admin home page + And I should be connected to the server via an ssl connection + And the markup should be valid + And I should not see "Users" + And I should see "John Reviewer" + And I follow "Logout" + And I should be on the admin login page - Scenario: 5 failed logins disables an account - Given a moderator user exists with email: "admin@example.com", password: "Letmein1!", password_confirmation: "Letmein1!" - And I go to the admin login page - And I try the password "wrong trousers" 3 times in a row - And I fill in "Email" with "admin@example.com" - And I fill in "Password" with "wrong trousers" - And I press "Sign in" - Then I should see "You have one more attempt before your account is temporarily disabled" - And should not see "Logout" - And I fill in "Email" with "admin@example.com" - And I fill in "Password" with "wrong trousers" + Scenario: Invalid domain + When I go to the admin login page + And I fill in "Email" with "admin@example.org" And I press "Sign in" - Then I should see "Failed login limit exceeded, your account has been temporarily disabled" + Then I should see "Invalid login details" And should not see "Logout" - Scenario: 5 failed logins with an email address containing a wildcard does not disable an account - Given a moderator user exists with email: "admin@example.com", password: "Letmein1!", password_confirmation: "Letmein1!" - And I go to the admin login page - And I try the password "wrong trousers" 5 times in a row for the email "admin%" - And I fill in "Email" with "admin@example.com" - And I fill in "Password" with "Letmein1!" - And I press "Sign in" - Then I should be on the admin home page - - Scenario: Login as a user who hasn't changed their password for over 9 months - Given a moderator user exists with email: "admin@example.com", password: "Letmein1!", password_confirmation: "Letmein1!" - And the password was changed 10 months ago + Scenario: Invalid login + Given an invalid SSO login When I go to the admin login page And I fill in "Email" with "admin@example.com" - And I fill in "Password" with "Letmein1!" And I press "Sign in" - Then I should be on the admin edit profile page for "admin@example.com" - And I should be connected to the server via an ssl connection - And I fill in "Current password" with "Letmein1!" - And I fill in "New password" with "Letmeout1!" - And I fill in "Password confirmation" with "Letmeout1!" - And I press "Save" - Then I should be on the admin home page + Then I should see "There was a problem logging in - please contact support" + And should not see "Logout" - Scenario: Login as a user who is logging in for the first time - Given a moderator user exists with email: "admin@example.com", password: "Letmein1!", password_confirmation: "Letmein1!", force_password_reset: true + Scenario: Valid login but no role + Given a valid SSO login with no roles When I go to the admin login page - And I fill in "Email" with "admin@example.com" - And I fill in "Password" with "Letmein1!" + And I fill in "Email" with "norole@example.com" And I press "Sign in" - Then I should be on the admin edit profile page for "admin@example.com" - And I fill in "Current password" with "Letmein1!" - And I fill in "New password" with "Letmeout1!" - And I fill in "Password confirmation" with "Letmeout1!" - And I press "Save" - Then I should be on the admin home page + Then I should see "Invalid login details" + And should not see "Logout" + diff --git a/features/admin/admin_users.feature b/features/admin/admin_users.feature new file mode 100644 index 000000000..de406a8b8 --- /dev/null +++ b/features/admin/admin_users.feature @@ -0,0 +1,35 @@ +@admin +Feature: Admin users index and crud + As a sysadmin + I can see the admin users index and crud admin users + + Background: + Given the time is "Sun, 17 Dec 2023 08:28:41 +0000" + And I am logged in as a sysadmin with the email "muddy@example.com", first_name "Sys", last_name "Admin" + And a moderator user exists with email: "naomi@example.com", first_name: "Naomi", last_name: "Campbell" + + Scenario: Accessing the admin users index + When I go to the admin home page + And I follow "Users" + Then I should be on the admin users index page + And I should be connected to the server via an ssl connection + + Scenario: Ordering of the users index + Given a moderator user exists with email: "derek@example.com", first_name: "Derek", last_name: "Jacobi", current_sign_in_at: "2023-12-11 14:27:14" + And a moderator user exists with email: "helen@example.com", first_name: "Helen", last_name: "Hunt", current_sign_in_at: "2023-12-15 10:09:31" + When I go to the admin users index page + Then I should see the following admin user table: + | Name | Email | Role | Last login | + | Admin, Sys | muddy@example.com | sysadmin | 08:28am on 17 December 2023 | + | Campbell, Naomi | naomi@example.com | moderator | | + | Hunt, Helen | helen@example.com | moderator | 10:09am on 15 December 2023 | + | Jacobi, Derek | derek@example.com | moderator | 14:27pm on 11 December 2023 | + And the markup should be valid + + Scenario: Pagination of the users index + Given 50 moderator users exist + When I go to the admin users index page + And I follow "Next" + Then I should see 2 rows in the admin user table + And I follow "Previous" + Then I should see 50 rows in the admin user table diff --git a/features/admin/admin_users_crud.feature b/features/admin/admin_users_crud.feature deleted file mode 100644 index c3c79ac11..000000000 --- a/features/admin/admin_users_crud.feature +++ /dev/null @@ -1,98 +0,0 @@ -@admin -Feature: Admin users index and crud - As a sysadmin - I can see the admin users index and crud admin users - - Background: - Given I am logged in as a sysadmin with the email "muddy@fox.com", first_name "Sys", last_name "Admin" - And a moderator user exists with email: "naomi@example.com", first_name: "Naomi", last_name: "Campbell" - - Scenario: Accessing the admin users index - When I go to the admin home page - And I follow "Users" - Then I should be on the admin users index page - And I should be connected to the server via an ssl connection - - Scenario: Ordering of the users index - Given a moderator user exists with email: "derek@example.com", first_name: "Derek", last_name: "Jacobi" - And a moderator user exists with email: "helen@example.com", first_name: "Helen", last_name: "Hunt", failed_login_count: 5 - When I go to the admin users index page - Then I should see the following admin user table: - | Name | Email | Role | Disabled | - | Admin, Sys | muddy@fox.com | sysadmin | | - | Campbell, Naomi | naomi@example.com | moderator | | - | Hunt, Helen | helen@example.com | moderator | Yes | - | Jacobi, Derek | derek@example.com | moderator | | - And the markup should be valid - - Scenario: Pagination of the users index - Given 50 moderator users exist - When I go to the admin users index page - And I follow "Next" - Then I should see 2 rows in the admin user table - And I follow "Previous" - Then I should see 50 rows in the admin user table - - Scenario: Inspecting the new user form - When I go to the admin users index page - And I follow "New user" - Then I should be on the admin new user page - And I should be connected to the server via an ssl connection - And I should see a "First name" text field - And I should see a "Last name" text field - And I should see a "Email" email field - And I should see a "sysadmin" radio field - And I should see a "moderator" radio field - And I should see a "Force password reset" checkbox field - And I should see a "Account disabled" checkbox field - And I should see a "Password" password field - And I should see a "Password confirmation" password field - - Scenario: Creating a new user - When I go to the admin users index page - And I follow "New user" - And I fill in "First name" with "Derek" - And I fill in "Last name" with "Jacobi" - And I fill in "Email" with "derek@example.com" - And I choose "sysadmin" - And I fill in "Password" with "Letmein1!" - And I fill in "Password confirmation" with "Letmein1!" - And I press "Save" - Then I should be on the admin users index page - And I should see "derek@example.com" - When I follow "Jacobi, Derek" - Then the "sysadmin" radio button should be chosen - And the "Account disabled" checkbox should not be checked - - Scenario: Updating a user - When I go to the admin users index page - And I follow "Campbell, Naomi" - And I fill in "First name" with "Nolene" - And I fill in "Email" with "helen@example.com" - And I check "Account disabled" - And I press "Save" - Then I should be on the admin users index page - And I should see "helen@example.com" - When I follow "Campbell, Nolene" - And the "Email" field should contain "helen@example.com" - And the "Account disabled" checkbox should be checked - And a moderator user should exist with email: "helen@example.com", failed_login_count: "5" - And I should be connected to the server via an ssl connection - - Scenario: Enabling a user's disabled account - Given a moderator user exists with email: "derek@example.com", first_name: "Derek", last_name: "Jacobi", failed_login_count: "5" - When I go to the admin users index page - And I follow "Jacobi, Derek" - And the "Account disabled" checkbox should be checked - And I uncheck "Account disabled" - And I press "Save" - Then I should be on the admin users index page - When I follow "Jacobi, Derek" - And the "Account disabled" checkbox should not be checked - And a moderator user should exist with email: "derek@example.com", failed_login_count: "0" - - Scenario: Deleting a user - When I go to the admin users index page - And I follow "Delete" for "naomi@example.com" - Then a admin user should not exist with email: "naomi@example.com" - And the row with the name "naomi@example.com" is not listed diff --git a/features/admin/all_petitions.feature b/features/admin/all_petitions.feature index 65fabca5b..7b9e44ed6 100644 --- a/features/admin/all_petitions.feature +++ b/features/admin/all_petitions.feature @@ -108,19 +108,19 @@ Feature: A moderator user views all petitions | My open petition with scheduled debate date | Scenario: A sysadmin can view all petitions - Given I am logged in as a sysadmin + Given I log out and login back in as a sysadmin And an open petition exists with action: "Simply the best" When I view all petitions Then I should see "Simply the best" Scenario: A moderator user can view all petitions - Given I am logged in as a moderator + Given I log out and login back in as a moderator And an open petition exists with action: "Simply the best" When I view all petitions Then I should see "Simply the best" Scenario: A moderator can download petitions as CSV - Given I am logged in as a moderator + Given I log out and login back in as a moderator And the time is "2015-08-21 06:00:00" When I view all petitions And I follow "Download CSV" diff --git a/features/admin/anonymize_petitions.feature b/features/admin/anonymize_petitions.feature index 46fc4ad31..5457e8f16 100644 --- a/features/admin/anonymize_petitions.feature +++ b/features/admin/anonymize_petitions.feature @@ -4,7 +4,7 @@ Feature: An admin anonymizes petitions I want to anonymize all petitions 6 months after parliament closes in accordance with our privacy policy Background: - Given I am logged in as a sysadmin with the email "muddy@fox.com", first_name "Sys", last_name "Admin" + Given I am logged in as a sysadmin with the email "muddy@example.com", first_name "Sys", last_name "Admin" @javascript Scenario: Admin anonymizes petitions 6 months after parliament closes diff --git a/features/admin/profile.feature b/features/admin/profile.feature deleted file mode 100644 index b9430d02c..000000000 --- a/features/admin/profile.feature +++ /dev/null @@ -1,42 +0,0 @@ -@admin -Feature: Moderator users password change - As a an moderator - I can change my password - - Background: - Given I am logged in as a moderator named "John Moderator" with the password "Letmein1!" - - Scenario: Accessing the Profile page - When I go to the admin home page - And I follow "John Moderator" - Then I should be on the admin edit profile page - And I should see a "Current password" password field - And I should see a "New password" password field - And I should see a "Password confirmation" password field - And I should be connected to the server via an ssl connection - And the markup should be valid - - Scenario: Changing password successfully - When I go to the admin edit profile page - And I fill in "Current password" with "Letmein1!" - And I fill in "New password" with "Letmeout1!" - And I fill in "Password confirmation" with "Letmeout1!" - And I press "Save" - Then I should be on the admin home page - And I should see "Password was successfully updated" - - Scenario: Incorrect current password - When I go to the admin edit profile page - And I fill in "Current password" with "wrong password" - And I fill in "New password" with "Letmeout1!" - And I fill in "Password confirmation" with "Letmeout1!" - And I press "Save" - Then I should see "Current password is incorrect" - - Scenario: Invalid new password - When I go to the admin edit profile page - And I fill in "Current password" with "Letmein1!" - And I fill in "New password" with "12345678" - And I fill in "Password confirmation" with "12345678" - And I press "Save" - Then I should see "Password must contain at least one digit, a lower and upper case letter and a special character" diff --git a/features/step_definitions/authentication_steps.rb b/features/step_definitions/authentication_steps.rb index e3cdbd1c5..fc0cf7299 100644 --- a/features/step_definitions/authentication_steps.rb +++ b/features/step_definitions/authentication_steps.rb @@ -1,44 +1,76 @@ Given /^I am logged in as a sysadmin$/ do - @user = FactoryBot.create(:sysadmin_user) + @user = FactoryBot.build(:sysadmin_sso_user) step "the admin user is logged in" end Given /^I am logged in as a moderator$/ do - @user = FactoryBot.create(:moderator_user) + @user = FactoryBot.build(:moderator_sso_user) step "the admin user is logged in" end -Given /^I am logged in as a moderator named "([^\"]*)"$/ do |name| - first_name, last_name = name.split - @user = FactoryBot.create(:moderator_user, first_name: first_name, last_name: last_name) - step "the admin user is logged in" +Given(/^I log out and login back in as a sysadmin$/) do + @user = FactoryBot.build(:sysadmin_sso_user) + + steps %q[ + the admin user is logged out + the admin user is logged in + ] end -Given /^I am logged in as a moderator with the password "([^\"]*)"$/ do |password| - @user = FactoryBot.create(:moderator_user, :password => password, :password_confirmation => password) - step "the admin user is logged in" +Given(/^I log out and login back in as a moderator$/) do + @user = FactoryBot.build(:moderator_sso_user) + + steps %q[ + the admin user is logged out + the admin user is logged in + ] end -Given /^I am logged in as a moderator named "([^\"]*)" with the password "([^\"]*)"$/ do |name, password| +Given /^I am logged in as a moderator named "([^\"]*)"$/ do |name| first_name, last_name = name.split - @user = FactoryBot.create(:moderator_user, first_name: first_name, last_name: last_name, :password => password, :password_confirmation => password) + @user = FactoryBot.build(:moderator_sso_user, first_name: first_name, last_name: last_name) step "the admin user is logged in" end Given /^I am logged in as a sysadmin named "([^\"]*)"$/ do |name| first_name, last_name = name.split - @user = FactoryBot.create(:sysadmin_user, first_name: first_name, last_name: last_name) + @user = FactoryBot.build(:sysadmin_sso_user, first_name: first_name, last_name: last_name) step "the admin user is logged in" end Given /^I am logged in as a sysadmin with the email "([^\"]*)", first_name "([^\"]*)", last_name "([^\"]*)"$/ do |email, first_name, last_name| - @user = FactoryBot.create(:sysadmin_user, :email => email, :first_name => first_name, :last_name => last_name) + @user = FactoryBot.build(:sysadmin_sso_user, email: email, first_name: first_name, last_name: last_name) step "the admin user is logged in" end Given /^the admin user is logged in$/ do + OmniAuth.config.mock_auth[:example] = @user + visit admin_login_url - fill_in("Email", :with => @user.email) - fill_in("Password", :with => "Letmein1!") + fill_in("Email", with: @user.uid) click_button("Sign in") end + +Given /^the admin user is logged out$/ do + visit admin_logout_url +end + +Given /^a sysadmin SSO user exists$/ do + OmniAuth.config.mock_auth[:example] = FactoryBot.build(:sysadmin_sso_user) +end + +Given /^a moderator SSO user exists$/ do + OmniAuth.config.mock_auth[:example] = FactoryBot.build(:moderator_sso_user) +end + +Given /^a reviewer SSO user exists$/ do + OmniAuth.config.mock_auth[:example] = FactoryBot.build(:reviewer_sso_user) +end + +Given(/^a valid SSO login with no roles$/) do + OmniAuth.config.mock_auth[:example] = FactoryBot.build(:norole_sso_user) +end + +Given /^an invalid SSO login$/ do + OmniAuth.config.mock_auth[:example] = :invalid_credentials +end diff --git a/features/step_definitions/pickle_steps.rb b/features/step_definitions/pickle_steps.rb index c7d58b89b..f8ee725a9 100644 --- a/features/step_definitions/pickle_steps.rb +++ b/features/step_definitions/pickle_steps.rb @@ -1,25 +1,13 @@ -Given(/^a sysadmin user exists with first_name: "([^"]*)", last_name: "([^"]*)", email: "([^"]*)", password: "([^"]*)", password_confirmation: "([^"]*)"$/) do |first_name, last_name, email, password, password_confirmation| - @user = FactoryBot.create(:sysadmin_user, first_name: first_name, last_name: last_name, email: email, password: password, password_confirmation: password_confirmation) -end - -Given(/^a moderator user exists with first_name: "([^"]*)", last_name: "([^"]*)", email: "([^"]*)", password: "([^"]*)", password_confirmation: "([^"]*)"$/) do |first_name, last_name, email, password, password_confirmation| - @user = FactoryBot.create(:moderator_user, first_name: first_name, last_name: last_name, email: email, password: password, password_confirmation: password_confirmation) -end - -Given(/^a moderator user exists with email: "([^"]*)", password: "([^"]*)", password_confirmation: "([^"]*)"$/) do |email, password, password_confirmation| - @user = FactoryBot.create(:moderator_user, email: email, password: password, password_confirmation: password_confirmation) -end - -Given(/^a moderator user exists with email: "([^"]*)", password: "([^"]*)", password_confirmation: "([^"]*)", force_password_reset: true$/) do |email, password, password_confirmation| - @user = FactoryBot.create(:moderator_user, email: email, password: password, password_confirmation: password_confirmation, force_password_reset: true) +Given(/^a sysadmin user exists with email: "([^"]*)", first_name: "([^"]*)", last_name: "([^"]*)"$/) do |email, first_name, last_name| + @user = FactoryBot.create(:sysadmin_user, first_name: first_name, last_name: last_name, email: email) end Given(/^a moderator user exists with email: "([^"]*)", first_name: "([^"]*)", last_name: "([^"]*)"$/) do |email, first_name, last_name| @user = FactoryBot.create(:moderator_user, first_name: first_name, last_name: last_name, email: email) end -Given(/^a moderator user exists with email: "([^"]*)", first_name: "([^"]*)", last_name: "([^"]*)", failed_login_count: (\d+)$/) do |email, first_name, last_name, failed_login_count| - @user = FactoryBot.create(:moderator_user, first_name: first_name, last_name: last_name, email: email, failed_login_count: failed_login_count) +Given(/^a moderator user exists with email: "([^"]*)", first_name: "([^"]*)", last_name: "([^"]*)", current_sign_in_at: "([^"]*)"$/) do |email, first_name, last_name, current_sign_in_at| + @user = FactoryBot.create(:moderator_user, first_name: first_name, last_name: last_name, email: email, current_sign_in_at: current_sign_in_at) end Given(/^(\d+) moderator users exist$/) do |number| @@ -34,14 +22,6 @@ end end -When(/^a moderator user should exist with email: "([^"]*)", failed_login_count: "([^"]*)"$/) do |email, failed_login_count| - expect(AdminUser.where(email: email, failed_login_count: failed_login_count)).to exist -end - -Given(/^a moderator user exists with email: "([^"]*)", first_name: "([^"]*)", last_name: "([^"]*)", failed_login_count: "([^"]*)"$/) do |email, first_name, last_name, failed_login_count| - @user = FactoryBot.create(:moderator_user, email: email, first_name: first_name, last_name: last_name, failed_login_count: failed_login_count) -end - Then(/^a admin user should not exist with email: "([^"]*)"$/) do |email| expect(AdminUser.where(email: email)).not_to exist end diff --git a/features/support/hooks.rb b/features/support/hooks.rb index 103d1aadf..1d3405adc 100644 --- a/features/support/hooks.rb +++ b/features/support/hooks.rb @@ -83,3 +83,16 @@ Before do Rails.application.env_config['action_dispatch.show_detailed_exceptions'] = false end + +Before do + OmniAuth.config.test_mode = true + + OmniAuth.config.on_failure = Proc.new { |env| + OmniAuth::FailureEndpoint.new(env).redirect_to_failure + } +end + +After do + OmniAuth.config.mock_auth[:example] = nil + OmniAuth.config.test_mode = false +end diff --git a/features/support/paths.rb b/features/support/paths.rb index 250e0f80c..10e44ef57 100644 --- a/features/support/paths.rb +++ b/features/support/paths.rb @@ -110,12 +110,6 @@ def admin_url(admin_page) when /^new user page$/ new_admin_admin_user_url - when /^edit profile page$/ - edit_admin_profile_url(@user) - - when /^edit profile page for "([^\"]*)"$/ - edit_admin_profile_url(AdminUser.find_by(email: $1)) - when /^debate outcomes form page for "([^\"]*)"$/ admin_petition_debate_outcome_url(Petition.find_by(action: $1)) diff --git a/lib/package_builder/scripts/after_install.sh b/lib/package_builder/scripts/after_install.sh index edf20d66e..2e3d18ebc 100644 --- a/lib/package_builder/scripts/after_install.sh +++ b/lib/package_builder/scripts/after_install.sh @@ -5,6 +5,7 @@ set -o pipefail chown -R deploy:deploy /home/deploy/epetitions/releases/<%= release %> su - deploy <<'EOF' +ln -nfs /home/deploy/epetitions/shared/config/sso.yml /home/deploy/epetitions/releases/<%= release %>/config/sso.yml ln -nfs /home/deploy/epetitions/shared/tmp /home/deploy/epetitions/releases/<%= release %>/tmp ln -nfs /home/deploy/epetitions/shared/log /home/deploy/epetitions/releases/<%= release %>/log ln -nfs /home/deploy/epetitions/shared/bundle /home/deploy/epetitions/releases/<%= release %>/vendor/bundle diff --git a/lib/tasks/epets.rake b/lib/tasks/epets.rake index 79ae580b6..c0457e1be 100644 --- a/lib/tasks/epets.rake +++ b/lib/tasks/epets.rake @@ -6,7 +6,6 @@ namespace :epets do if AdminUser.find_by(email: 'admin@example.com').nil? admin = AdminUser.new(:first_name => 'Cool', :last_name => 'Admin', :email => 'admin@example.com') admin.role = 'sysadmin' - admin.password = admin.password_confirmation = 'Letmein1!' admin.save! end end diff --git a/spec/controllers/admin/admin_users_controller_spec.rb b/spec/controllers/admin/admin_users_controller_spec.rb index f65cae87a..de055aec5 100644 --- a/spec/controllers/admin/admin_users_controller_spec.rb +++ b/spec/controllers/admin/admin_users_controller_spec.rb @@ -8,13 +8,6 @@ expect(response).to redirect_to("https://moderate.petition.parliament.uk/admin/login") end end - - describe "GET 'new'" do - it "should redirect to the login page" do - get :new - expect(response).to redirect_to("https://moderate.petition.parliament.uk/admin/login") - end - end end context "logged in as moderator user" do @@ -31,19 +24,6 @@ end end - context "logged in as sysadmin but need to reset password" do - let(:user) { FactoryBot.create(:sysadmin_user, :force_password_reset => true) } - before :each do - login_as(user) - end - - it "should redirect to edit profile page" do - expect(user.has_to_change_password?).to be_truthy - get :index - expect(response).to redirect_to("https://moderate.petition.parliament.uk/admin/profile/#{user.id}/edit") - end - end - describe "logged in as sysadmin user" do let(:user) { FactoryBot.create(:sysadmin_user, :first_name => 'Sys', :last_name => 'Admin') } before :each do @@ -68,160 +48,5 @@ expect(assigns[:users]).to eq([user, @user4, @user2, @user1, @user3]) end end - - describe "GET 'new'" do - it "should be successful" do - get :new - expect(response).to be_successful - end - - it "should assign a new user" do - get :new - expect(assigns[:user]).to be_a(AdminUser) - expect(assigns[:user]).to be_new_record - end - end - - describe "POST 'create'" do - def do_create - post :create, params: { admin_user: admin_user_attributes } - end - - describe "with valid params" do - let(:admin_user_attributes) do - { - :first_name => 'John', - :last_name => 'Kennedy', - :role => 'moderator', - :email => 'admin@example.com', - :password => 'Letmein1!', - :password_confirmation => 'Letmein1!' - } - end - - it "should create a new user" do - expect do - do_create - end.to change(AdminUser, :count).by(1) - end - - it "should redirect to the index" do - do_create - expect(response).to redirect_to(:action => :index) - end - end - - describe "with invalid params" do - let(:admin_user_attributes) do - { - :email => 'admin@example.com' - } - end - - it "should not create a new user" do - expect { - do_create - }.not_to change(AdminUser, :count) - end - - it "should re-render the new template" do - do_create - expect(response).to render_template('new') - end - end - end - - describe "GET 'edit'" do - let(:edit_user) { FactoryBot.create(:moderator_user) } - - def do_get - get :edit, params: { id: edit_user.to_param } - end - - it "should be successful" do - do_get - expect(response).to be_successful - end - - it "should assign the user" do - do_get - expect(assigns[:user]).to be_a(AdminUser) - expect(assigns[:user]).to eq edit_user - end - end - - describe "PUT 'update'" do - let(:edit_user) { FactoryBot.create(:moderator_user, :email => "admin@example.com", :failed_login_count => 5) } - - def do_update - patch :update, params: { - id: edit_user.to_param, - admin_user: admin_user_attributes - } - end - - describe "with valid params" do - let(:admin_user_attributes) do - { - :email => "another_admin@example.com", - :account_disabled => '0' - } - end - - it "should update the user and redirect to the index" do - do_update - edit_user.reload - expect(edit_user.email).to eq("another_admin@example.com") - expect(response).to redirect_to(:action => :index) - end - - it "should reset the failed login count to 0" do - do_update - edit_user.reload - expect(edit_user.failed_login_count).to eq(0) - end - end - - describe "with invalid params" do - let(:admin_user_attributes) do - { - :email => "bademailaddress" - } - end - - it "should not update the user" do - do_update - edit_user.reload - expect(edit_user.email).to eq("admin@example.com") - expect(response).to render_template('edit') - end - end - end - - describe "DELETE 'destroy'" do - let(:delete_user) { FactoryBot.create(:moderator_user, :email => 'admin@example.com') } - - it "deletes the requested user" do - delete :destroy, params: { id: delete_user.to_param } - expect(AdminUser.exists?(delete_user.id)).to be_falsey - expect(response).to redirect_to(:action => :index) - end - - it "will not let you delete yourself" do - delete :destroy, params: { id: user.to_param } - expect(AdminUser.exists?(user.id)).to be_truthy - expect(response).to redirect_to(:action => :index) - expect(flash[:alert]).to eq "You are not allowed to delete yourself!" - end - - it "will not let you delete users that have moderated petitions" do - petition = FactoryBot.create(:open_petition, moderated_by: delete_user) - - delete :destroy, params: { id: delete_user.to_param } - expect(AdminUser.exists?(user.id)).to be_truthy - expect(response).to redirect_to(:action => :index) - expect(flash[:alert]).to eq "The user has moderated petitions so you can only deactivate their account" - end - end end end diff --git a/spec/controllers/admin/archived/debate_outcomes_controller_spec.rb b/spec/controllers/admin/archived/debate_outcomes_controller_spec.rb index 5fa0ec367..01eeb1583 100644 --- a/spec/controllers/admin/archived/debate_outcomes_controller_spec.rb +++ b/spec/controllers/admin/archived/debate_outcomes_controller_spec.rb @@ -41,25 +41,6 @@ end end - context 'logged in as moderator user but need to reset password' do - let(:user) { FactoryBot.create(:moderator_user, force_password_reset: true) } - before { login_as(user) } - - describe 'GET /show' do - it 'redirects to edit profile page' do - get :show, params: { petition_id: petition.id } - expect(response).to redirect_to("https://moderate.petition.parliament.uk/admin/profile/#{user.id}/edit") - end - end - - describe 'PATCH /update' do - it 'redirects to edit profile page' do - patch :update, params: { petition_id: petition.id } - expect(response).to redirect_to("https://moderate.petition.parliament.uk/admin/profile/#{user.id}/edit") - end - end - end - context "logged in as moderator user" do let(:user) { FactoryBot.create(:moderator_user) } before { login_as(user) } diff --git a/spec/controllers/admin/archived/government_response_controller_spec.rb b/spec/controllers/admin/archived/government_response_controller_spec.rb index 7725d89a4..decfeca3a 100644 --- a/spec/controllers/admin/archived/government_response_controller_spec.rb +++ b/spec/controllers/admin/archived/government_response_controller_spec.rb @@ -42,25 +42,6 @@ end end - context 'logged in as moderator user but need to reset password' do - let(:user) { FactoryBot.create(:moderator_user, force_password_reset: true) } - before { login_as(user) } - - describe 'GET /show' do - it 'redirects to edit profile page' do - get :show, params: { petition_id: petition.id } - expect(response).to redirect_to("https://moderate.petition.parliament.uk/admin/profile/#{user.id}/edit") - end - end - - describe 'PATCH /update' do - it 'redirects to edit profile page' do - patch :update, params: { petition_id: petition.id } - expect(response).to redirect_to("https://moderate.petition.parliament.uk/admin/profile/#{user.id}/edit") - end - end - end - context "logged in as moderator user" do let(:user) { FactoryBot.create(:moderator_user) } before { login_as(user) } @@ -610,5 +591,78 @@ def do_patch(overrides = {}) end end end + + describe 'DELETE /destroy' do + before do + expect(Archived::Petition).to receive_message_chain(:moderated, :find).with(petition.to_param).and_return(petition) + end + + context "when the petition has a government response" do + let!(:petition) { FactoryBot.create(:archived_petition, :response) } + + before do + expect(petition).to receive(:government_response).and_return(government_response) + end + + context "when the government response is succcessfully deleted" do + it "redirects to the petition page and displays a notice" do + delete :destroy, params: { petition_id: petition.id } + + expect(response).to redirect_to("https://moderate.petition.parliament.uk/admin/archived/petitions/#{petition.id}") + expect(controller).to set_flash[:notice].to("Deleted government response successfully") + end + + it "updates the email_requested_at timestamp for 'government_response'" do + email_requested_at = 1.hour.ago + petition.set_email_requested_at_for("government_response", to: email_requested_at) + + expect { + delete :destroy, params: { petition_id: petition.id } + }.to change { + petition.get_email_requested_at_for("government_response") + }.from(email_requested_at).to(be_within(1.second).of(Time.current)) + end + + it "updates the government_response_at timestamp to be nil" do + expect { + delete :destroy, params: { petition_id: petition.id } + }.to change { + petition.government_response_at + }.from(be_present).to(be_nil) + end + end + + context "when the government response is not successfully deleted" do + before do + expect(government_response).to receive(:destroy).and_return(false) + expect(government_response).to receive(:destroyed?).and_return(false) + end + + it "redirects to the government response page and displays an alert" do + delete :destroy, params: { petition_id: petition.id } + + expect(response).to redirect_to("https://moderate.petition.parliament.uk/admin/archived/petitions/#{petition.id}/government-response") + expect(controller).to set_flash[:alert].to("Unable to delete government response - please contact support") + end + end + end + + context "when the petition has no government response" do + let!(:petition) { FactoryBot.create(:archived_petition) } + let(:new_government_response) { petition.build_government_response } + + before do + expect(petition).to receive(:government_response).and_return(nil) + expect(petition).to receive(:build_government_response).and_return(new_government_response) + end + + it "redirects to the petition page and displays a notice" do + delete :destroy, params: { petition_id: petition.id } + + expect(response).to redirect_to("https://moderate.petition.parliament.uk/admin/archived/petitions/#{petition.id}") + expect(controller).to set_flash[:notice].to("Deleted government response successfully") + end + end + end end end diff --git a/spec/controllers/admin/archived/locks_controller_spec.rb b/spec/controllers/admin/archived/locks_controller_spec.rb index 06c630e4c..54500efd7 100644 --- a/spec/controllers/admin/archived/locks_controller_spec.rb +++ b/spec/controllers/admin/archived/locks_controller_spec.rb @@ -20,28 +20,6 @@ end end - context "when logged in as a moderator requiring a password reset" do - let(:moderator) { FactoryBot.create(:moderator_user, force_password_reset: true) } - before { login_as(moderator) } - - [ - ["GET", "/admin/archived/petitions/:petition_id/lock.json", :show, { petition_id: "100000" }], - ["POST", "/admin/archived/petitions/:petition_id/lock.json", :create, { petition_id: "100000" }], - ["PATCH", "/admin/archived/petitions/:petition_id/lock.json", :update, { petition_id: "100000" }], - ["DELETE", "/admin/archived/petitions/:petition_id/lock.json", :destroy, { petition_id: "100000" }] - ].each do |method, path, action, params| - - describe "#{method} #{path}" do - before { process action, method: method, params: params, format: :json } - - it "redirects to the admin profile page" do - expect(response).to redirect_to("https://moderate.petition.parliament.uk/admin/profile/#{moderator.id}/edit") - end - end - - end - end - context "when logged in as a moderator" do let(:moderator) { FactoryBot.create(:moderator_user) } let(:petition) { FactoryBot.create(:archived_petition) } diff --git a/spec/controllers/admin/archived/notes_controller_spec.rb b/spec/controllers/admin/archived/notes_controller_spec.rb index c6cbb0057..3a9210851 100644 --- a/spec/controllers/admin/archived/notes_controller_spec.rb +++ b/spec/controllers/admin/archived/notes_controller_spec.rb @@ -20,25 +20,6 @@ end end - context 'logged in as moderator user but need to reset password' do - let(:user) { FactoryBot.create(:moderator_user, force_password_reset: true) } - before { login_as(user) } - - describe 'GET /show' do - it 'redirects to edit profile page' do - get :show, params: { petition_id: petition.id } - expect(response).to redirect_to("https://moderate.petition.parliament.uk/admin/profile/#{user.id}/edit") - end - end - - describe 'PATCH /update' do - it 'redirects to edit profile page' do - patch :update, params: { petition_id: petition.id } - expect(response).to redirect_to("https://moderate.petition.parliament.uk/admin/profile/#{user.id}/edit") - end - end - end - context "logged in as moderator user" do let(:user) { FactoryBot.create(:moderator_user) } before { login_as(user) } diff --git a/spec/controllers/admin/archived/petition_departments_controller_spec.rb b/spec/controllers/admin/archived/petition_departments_controller_spec.rb index d846b1df7..f0fc1f629 100644 --- a/spec/controllers/admin/archived/petition_departments_controller_spec.rb +++ b/spec/controllers/admin/archived/petition_departments_controller_spec.rb @@ -20,25 +20,6 @@ end end - context "when logged in as moderator user but need to reset password" do - let(:user) { FactoryBot.create(:moderator_user, force_password_reset: true) } - before { login_as(user) } - - describe "GET /admin/archived/petitions/:petition_id/departments" do - it "redirects to the edit profile page" do - get :show, params: { petition_id: petition.id } - expect(response).to redirect_to("https://moderate.petition.parliament.uk/admin/profile/#{user.id}/edit") - end - end - - describe "PATCH /admin/archived/petitions/:petition_id/departments" do - it "redirects to the edit profile page" do - patch :update, params: { petition_id: petition.id } - expect(response).to redirect_to("https://moderate.petition.parliament.uk/admin/profile/#{user.id}/edit") - end - end - end - context "when logged in as moderator user" do let(:user) { FactoryBot.create(:moderator_user) } before { login_as(user) } diff --git a/spec/controllers/admin/archived/petition_details_controller_spec.rb b/spec/controllers/admin/archived/petition_details_controller_spec.rb index d25704269..62fbd9243 100644 --- a/spec/controllers/admin/archived/petition_details_controller_spec.rb +++ b/spec/controllers/admin/archived/petition_details_controller_spec.rb @@ -20,25 +20,6 @@ end end - context 'logged in as moderator user but need to reset password' do - let(:user) { FactoryBot.create(:moderator_user, force_password_reset: true) } - before { login_as(user) } - - describe 'GET #show' do - it 'redirects to edit profile page' do - get :show, params: { petition_id: petition.id } - expect(response).to redirect_to("https://moderate.petition.parliament.uk/admin/profile/#{user.id}/edit") - end - end - - describe 'PATCH #update' do - it 'redirects to edit profile page' do - patch :update, params: { petition_id: petition.id } - expect(response).to redirect_to("https://moderate.petition.parliament.uk/admin/profile/#{user.id}/edit") - end - end - end - context 'logged in as moderator user' do let(:user) { FactoryBot.create(:moderator_user) } before { login_as(user) } diff --git a/spec/controllers/admin/archived/petition_emails_controller_spec.rb b/spec/controllers/admin/archived/petition_emails_controller_spec.rb index 18f1b8830..ffb892f1e 100644 --- a/spec/controllers/admin/archived/petition_emails_controller_spec.rb +++ b/spec/controllers/admin/archived/petition_emails_controller_spec.rb @@ -90,48 +90,6 @@ end end - context 'logged in as moderator user but need to reset password' do - let(:email) { FactoryBot.create(:archived_petition_email, petition: petition) } - let(:user) { FactoryBot.create(:moderator_user, force_password_reset: true) } - - before { login_as(user) } - - describe 'GET /new' do - it 'redirects to edit profile page' do - get :new, params: { petition_id: petition.id } - expect(response).to redirect_to("https://moderate.petition.parliament.uk/admin/profile/#{user.id}/edit") - end - end - - describe 'POST /' do - it 'redirects to edit profile page' do - post :create, params: { petition_id: petition.id } - expect(response).to redirect_to("https://moderate.petition.parliament.uk/admin/profile/#{user.id}/edit") - end - end - - describe 'GET /:id/edit' do - it 'redirects to the login page' do - get :edit, params: { petition_id: petition.id, id: email.id } - expect(response).to redirect_to("https://moderate.petition.parliament.uk/admin/profile/#{user.id}/edit") - end - end - - describe 'PATCH /:id' do - it 'redirects to the login page' do - patch :update, params: { petition_id: petition.id, id: email.id } - expect(response).to redirect_to("https://moderate.petition.parliament.uk/admin/profile/#{user.id}/edit") - end - end - - describe 'DELETE /:id' do - it 'redirects to the login page' do - patch :destroy, params: { petition_id: petition.id, id: email.id } - expect(response).to redirect_to("https://moderate.petition.parliament.uk/admin/profile/#{user.id}/edit") - end - end - end - context "logged in as moderator user" do let(:user) { FactoryBot.create(:moderator_user) } before { login_as(user) } diff --git a/spec/controllers/admin/archived/petition_tags_controller_spec.rb b/spec/controllers/admin/archived/petition_tags_controller_spec.rb index 052df7696..bb7f9f17c 100644 --- a/spec/controllers/admin/archived/petition_tags_controller_spec.rb +++ b/spec/controllers/admin/archived/petition_tags_controller_spec.rb @@ -20,25 +20,6 @@ end end - context "when logged in as moderator user but need to reset password" do - let(:user) { FactoryBot.create(:moderator_user, force_password_reset: true) } - before { login_as(user) } - - describe "GET /admin/archived/petitions/:petition_id/tags" do - it "redirects to the edit profile page" do - get :show, params: { petition_id: petition.id } - expect(response).to redirect_to("https://moderate.petition.parliament.uk/admin/profile/#{user.id}/edit") - end - end - - describe "PATCH /admin/archived/petitions/:petition_id/tags" do - it "redirects to the edit profile page" do - patch :update, params: { petition_id: petition.id } - expect(response).to redirect_to("https://moderate.petition.parliament.uk/admin/profile/#{user.id}/edit") - end - end - end - context "when logged in as moderator user" do let(:user) { FactoryBot.create(:moderator_user) } before { login_as(user) } diff --git a/spec/controllers/admin/archived/petition_topics_controller_spec.rb b/spec/controllers/admin/archived/petition_topics_controller_spec.rb index ef1763f61..dd04af12f 100644 --- a/spec/controllers/admin/archived/petition_topics_controller_spec.rb +++ b/spec/controllers/admin/archived/petition_topics_controller_spec.rb @@ -20,25 +20,6 @@ end end - context "when logged in as moderator user but need to reset password" do - let(:user) { FactoryBot.create(:moderator_user, force_password_reset: true) } - before { login_as(user) } - - describe "GET /admin/archived/petitions/:petition_id/topics" do - it "redirects to the edit profile page" do - get :show, params: { petition_id: petition.id } - expect(response).to redirect_to("https://moderate.petition.parliament.uk/admin/profile/#{user.id}/edit") - end - end - - describe "PATCH /admin/archived/petitions/:petition_id/topics" do - it "redirects to the edit profile page" do - patch :update, params: { petition_id: petition.id } - expect(response).to redirect_to("https://moderate.petition.parliament.uk/admin/profile/#{user.id}/edit") - end - end - end - context "when logged in as moderator user" do let(:user) { FactoryBot.create(:moderator_user) } before { login_as(user) } diff --git a/spec/controllers/admin/archived/petitions_controller_spec.rb b/spec/controllers/admin/archived/petitions_controller_spec.rb index bf3f7a527..44727edaf 100644 --- a/spec/controllers/admin/archived/petitions_controller_spec.rb +++ b/spec/controllers/admin/archived/petitions_controller_spec.rb @@ -21,25 +21,6 @@ end end - context "when logged in as a moderator but need to reset password" do - let(:user) { FactoryBot.create(:moderator_user, force_password_reset: true) } - before { login_as(user) } - - describe "GET /admin/archived/petitions" do - it "redirects to the edit profile page" do - get :index - expect(response).to redirect_to("https://moderate.petition.parliament.uk/admin/profile/#{user.id}/edit") - end - end - - describe "GET /admin/archived/petitions/:id" do - it "redirects to the edit profile page" do - get :show, params: { id: "100000" } - expect(response).to redirect_to("https://moderate.petition.parliament.uk/admin/profile/#{user.id}/edit") - end - end - end - context "when logged in as a moderator" do let(:user) { FactoryBot.create(:moderator_user) } before { login_as(user) } diff --git a/spec/controllers/admin/archived/schedule_debate_controller_spec.rb b/spec/controllers/admin/archived/schedule_debate_controller_spec.rb index 03599b847..a7b9ad8fa 100644 --- a/spec/controllers/admin/archived/schedule_debate_controller_spec.rb +++ b/spec/controllers/admin/archived/schedule_debate_controller_spec.rb @@ -41,25 +41,6 @@ end end - context 'logged in as moderator user but need to reset password' do - let(:user) { FactoryBot.create(:moderator_user, force_password_reset: true) } - before { login_as(user) } - - describe 'GET /show' do - it 'redirects to edit profile page' do - get :show, params: { petition_id: petition.id } - expect(response).to redirect_to("https://moderate.petition.parliament.uk/admin/profile/#{user.id}/edit") - end - end - - describe 'PATCH /update' do - it 'redirects to edit profile page' do - patch :update, params: { petition_id: petition.id } - expect(response).to redirect_to("https://moderate.petition.parliament.uk/admin/profile/#{user.id}/edit") - end - end - end - context "logged in as moderator user" do let(:user) { FactoryBot.create(:moderator_user) } before { login_as(user) } diff --git a/spec/controllers/admin/archived/signatures_controller_spec.rb b/spec/controllers/admin/archived/signatures_controller_spec.rb index f0ced78f7..0543c445b 100644 --- a/spec/controllers/admin/archived/signatures_controller_spec.rb +++ b/spec/controllers/admin/archived/signatures_controller_spec.rb @@ -20,25 +20,6 @@ end end - context "logged in as moderator user but need to reset password" do - let(:user) { FactoryBot.create(:moderator_user, force_password_reset: true) } - before { login_as(user) } - - describe "GET /admin/archived/signatures" do - it "redirects to the login page" do - get :index, params: { q: "Alice" } - expect(response).to redirect_to("https://moderate.petition.parliament.uk/admin/profile/#{user.id}/edit") - end - end - - describe "DELETE /admin/archived/signatures/:id" do - it "redirects to edit profile page" do - delete :destroy, params: { id: signature.id } - expect(response).to redirect_to("https://moderate.petition.parliament.uk/admin/profile/#{user.id}/edit") - end - end - end - context "logged in as moderator user" do let(:user) { FactoryBot.create(:moderator_user) } let(:token) { SecureRandom.base64(32) } diff --git a/spec/controllers/admin/debate_outcomes_controller_spec.rb b/spec/controllers/admin/debate_outcomes_controller_spec.rb index 985e988f2..aa1f8d518 100644 --- a/spec/controllers/admin/debate_outcomes_controller_spec.rb +++ b/spec/controllers/admin/debate_outcomes_controller_spec.rb @@ -40,25 +40,6 @@ end end - context 'logged in as moderator user but need to reset password' do - let(:user) { FactoryBot.create(:moderator_user, force_password_reset: true) } - before { login_as(user) } - - describe 'GET /show' do - it 'redirects to edit profile page' do - get :show, params: { petition_id: petition.id } - expect(response).to redirect_to("https://moderate.petition.parliament.uk/admin/profile/#{user.id}/edit") - end - end - - describe 'PATCH /update' do - it 'redirects to edit profile page' do - patch :update, params: { petition_id: petition.id } - expect(response).to redirect_to("https://moderate.petition.parliament.uk/admin/profile/#{user.id}/edit") - end - end - end - context "logged in as moderator user" do let(:user) { FactoryBot.create(:moderator_user) } before { login_as(user) } diff --git a/spec/controllers/admin/government_response_controller_spec.rb b/spec/controllers/admin/government_response_controller_spec.rb index 978bfeb8b..344ee3f58 100644 --- a/spec/controllers/admin/government_response_controller_spec.rb +++ b/spec/controllers/admin/government_response_controller_spec.rb @@ -41,25 +41,6 @@ end end - context 'logged in as moderator user but need to reset password' do - let(:user) { FactoryBot.create(:moderator_user, force_password_reset: true) } - before { login_as(user) } - - describe 'GET /show' do - it 'redirects to edit profile page' do - get :show, params: { petition_id: petition.id } - expect(response).to redirect_to("https://moderate.petition.parliament.uk/admin/profile/#{user.id}/edit") - end - end - - describe 'PATCH /update' do - it 'redirects to edit profile page' do - patch :update, params: { petition_id: petition.id } - expect(response).to redirect_to("https://moderate.petition.parliament.uk/admin/profile/#{user.id}/edit") - end - end - end - context "logged in as moderator user" do let(:user) { FactoryBot.create(:moderator_user) } before { login_as(user) } @@ -642,5 +623,82 @@ def do_patch(overrides = {}) end end end + + describe 'DELETE /destroy' do + before do + expect(Petition).to receive_message_chain(:moderated, :find).with(petition.to_param).and_return(petition) + end + + context "when the petition has a government response" do + let!(:petition) { FactoryBot.create(:responded_petition) } + + before do + expect(petition).to receive(:government_response).and_return(government_response) + end + + context "when the government response is succcessfully deleted" do + before do + expect(Appsignal).to receive(:increment_counter).with("petition.responded", -1) + end + + it "redirects to the petition page and displays a notice" do + delete :destroy, params: { petition_id: petition.id } + + expect(response).to redirect_to("https://moderate.petition.parliament.uk/admin/petitions/#{petition.id}") + expect(controller).to set_flash[:notice].to("Deleted government response successfully") + end + + it "updates the email_requested_at timestamp for 'government_response'" do + email_requested_at = 1.hour.ago + petition.set_email_requested_at_for("government_response", to: email_requested_at) + + expect { + delete :destroy, params: { petition_id: petition.id } + }.to change { + petition.get_email_requested_at_for("government_response") + }.from(email_requested_at).to(be_within(1.second).of(Time.current)) + end + + it "updates the government_response_at timestamp to be nil" do + expect { + delete :destroy, params: { petition_id: petition.id } + }.to change { + petition.government_response_at + }.from(be_present).to(be_nil) + end + end + + context "when the government response is not successfully deleted" do + before do + expect(government_response).to receive(:destroy).and_return(false) + expect(government_response).to receive(:destroyed?).and_return(false) + end + + it "redirects to the government response page and displays an alert" do + delete :destroy, params: { petition_id: petition.id } + + expect(response).to redirect_to("https://moderate.petition.parliament.uk/admin/petitions/#{petition.id}/government-response") + expect(controller).to set_flash[:alert].to("Unable to delete government response - please contact support") + end + end + end + + context "when the petition has no government response" do + let!(:petition) { FactoryBot.create(:open_petition) } + let(:new_government_response) { petition.build_government_response } + + before do + expect(petition).to receive(:government_response).and_return(nil) + expect(petition).to receive(:build_government_response).and_return(new_government_response) + end + + it "redirects to the petition page and displays a notice" do + delete :destroy, params: { petition_id: petition.id } + + expect(response).to redirect_to("https://moderate.petition.parliament.uk/admin/petitions/#{petition.id}") + expect(controller).to set_flash[:notice].to("Deleted government response successfully") + end + end + end end end diff --git a/spec/controllers/admin/locks_controller_spec.rb b/spec/controllers/admin/locks_controller_spec.rb index f6f12ac79..44289725c 100644 --- a/spec/controllers/admin/locks_controller_spec.rb +++ b/spec/controllers/admin/locks_controller_spec.rb @@ -20,28 +20,6 @@ end end - context "when logged in as a moderator requiring a password reset" do - let(:moderator) { FactoryBot.create(:moderator_user, force_password_reset: true) } - before { login_as(moderator) } - - [ - ["GET", "/admin/petitions/:petition_id/lock.json", :show, { petition_id: "100000" }], - ["POST", "/admin/petitions/:petition_id/lock.json", :create, { petition_id: "100000" }], - ["PATCH", "/admin/petitions/:petition_id/lock.json", :update, { petition_id: "100000" }], - ["DELETE", "/admin/petitions/:petition_id/lock.json", :destroy, { petition_id: "100000" }] - ].each do |method, path, action, params| - - describe "#{method} #{path}" do - before { process action, method: method, params: params, format: :json } - - it "redirects to the admin profile page" do - expect(response).to redirect_to("https://moderate.petition.parliament.uk/admin/profile/#{moderator.id}/edit") - end - end - - end - end - context "when logged in as a moderator" do let(:moderator) { FactoryBot.create(:moderator_user) } let(:petition) { FactoryBot.create(:petition) } diff --git a/spec/controllers/admin/moderation_delays_controller_spec.rb b/spec/controllers/admin/moderation_delays_controller_spec.rb index a4adb43b7..94157bc54 100644 --- a/spec/controllers/admin/moderation_delays_controller_spec.rb +++ b/spec/controllers/admin/moderation_delays_controller_spec.rb @@ -18,26 +18,6 @@ end end - context "when logged in as a moderator requiring a password reset" do - let(:moderator) { FactoryBot.create(:moderator_user, force_password_reset: true) } - before { login_as(moderator) } - - [ - ["GET", "/admin/moderation-delay/new", :new, {}], - ["POST", "/admin/moderation-delay", :create, {}] - ].each do |method, path, action, params| - - describe "#{method} #{path}" do - before { process action, method: method, params: params } - - it "redirects to the admin profile page" do - expect(response).to redirect_to("https://moderate.petition.parliament.uk/admin/profile/#{moderator.id}/edit") - end - end - - end - end - context "when logged in as a moderator" do let(:moderator) { FactoryBot.create(:moderator_user) } before { login_as(moderator) } diff --git a/spec/controllers/admin/notes_controller_spec.rb b/spec/controllers/admin/notes_controller_spec.rb index d15492bde..0000fce9a 100644 --- a/spec/controllers/admin/notes_controller_spec.rb +++ b/spec/controllers/admin/notes_controller_spec.rb @@ -20,25 +20,6 @@ end end - context 'logged in as moderator user but need to reset password' do - let(:user) { FactoryBot.create(:moderator_user, force_password_reset: true) } - before { login_as(user) } - - describe 'GET /show' do - it 'redirects to edit profile page' do - get :show, params: { petition_id: petition.id } - expect(response).to redirect_to("https://moderate.petition.parliament.uk/admin/profile/#{user.id}/edit") - end - end - - describe 'PATCH /update' do - it 'redirects to edit profile page' do - patch :update, params: { petition_id: petition.id } - expect(response).to redirect_to("https://moderate.petition.parliament.uk/admin/profile/#{user.id}/edit") - end - end - end - context "logged in as moderator user" do let(:user) { FactoryBot.create(:moderator_user) } before { login_as(user) } diff --git a/spec/controllers/admin/petition_departments_controller_spec.rb b/spec/controllers/admin/petition_departments_controller_spec.rb index c85b0ac3d..bd6ddfd81 100644 --- a/spec/controllers/admin/petition_departments_controller_spec.rb +++ b/spec/controllers/admin/petition_departments_controller_spec.rb @@ -19,25 +19,6 @@ end end - context "when logged in as moderator user but need to reset password" do - let(:user) { FactoryBot.create(:moderator_user, force_password_reset: true) } - before { login_as(user) } - - describe "GET /admin/petitions/:petition_id/departments" do - it "redirects to the edit profile page" do - get :show, params: { petition_id: petition.id } - expect(response).to redirect_to("https://moderate.petition.parliament.uk/admin/profile/#{user.id}/edit") - end - end - - describe "PATCH /admin/petitions/:petition_id/departments" do - it "redirects to the edit profile page" do - patch :update, params: { petition_id: petition.id } - expect(response).to redirect_to("https://moderate.petition.parliament.uk/admin/profile/#{user.id}/edit") - end - end - end - context "when logged in as moderator user" do let(:user) { FactoryBot.create(:moderator_user) } before { login_as(user) } diff --git a/spec/controllers/admin/petition_details_controller_spec.rb b/spec/controllers/admin/petition_details_controller_spec.rb index d6578ff47..9444b088c 100644 --- a/spec/controllers/admin/petition_details_controller_spec.rb +++ b/spec/controllers/admin/petition_details_controller_spec.rb @@ -20,25 +20,6 @@ end end - context 'logged in as moderator user but need to reset password' do - let(:user) { FactoryBot.create(:moderator_user, force_password_reset: true) } - before { login_as(user) } - - describe 'GET #show' do - it 'redirects to edit profile page' do - get :show, params: { petition_id: petition.id } - expect(response).to redirect_to("https://moderate.petition.parliament.uk/admin/profile/#{user.id}/edit") - end - end - - describe 'PATCH #update' do - it 'redirects to edit profile page' do - patch :update, params: { petition_id: petition.id } - expect(response).to redirect_to("https://moderate.petition.parliament.uk/admin/profile/#{user.id}/edit") - end - end - end - context 'logged in as moderator user' do let(:user) { FactoryBot.create(:moderator_user) } before { login_as(user) } diff --git a/spec/controllers/admin/petition_emails_controller_spec.rb b/spec/controllers/admin/petition_emails_controller_spec.rb index e171426af..4335d2d76 100644 --- a/spec/controllers/admin/petition_emails_controller_spec.rb +++ b/spec/controllers/admin/petition_emails_controller_spec.rb @@ -89,48 +89,6 @@ end end - context 'logged in as moderator user but need to reset password' do - let(:email) { FactoryBot.create(:petition_email, petition: petition) } - let(:user) { FactoryBot.create(:moderator_user, force_password_reset: true) } - - before { login_as(user) } - - describe 'GET /new' do - it 'redirects to edit profile page' do - get :new, params: { petition_id: petition.id } - expect(response).to redirect_to("https://moderate.petition.parliament.uk/admin/profile/#{user.id}/edit") - end - end - - describe 'POST /' do - it 'redirects to edit profile page' do - post :create, params: { petition_id: petition.id } - expect(response).to redirect_to("https://moderate.petition.parliament.uk/admin/profile/#{user.id}/edit") - end - end - - describe 'GET /:id/edit' do - it 'redirects to the login page' do - get :edit, params: { petition_id: petition.id, id: email.id } - expect(response).to redirect_to("https://moderate.petition.parliament.uk/admin/profile/#{user.id}/edit") - end - end - - describe 'PATCH /:id' do - it 'redirects to the login page' do - patch :update, params: { petition_id: petition.id, id: email.id } - expect(response).to redirect_to("https://moderate.petition.parliament.uk/admin/profile/#{user.id}/edit") - end - end - - describe 'DELETE /:id' do - it 'redirects to the login page' do - patch :destroy, params: { petition_id: petition.id, id: email.id } - expect(response).to redirect_to("https://moderate.petition.parliament.uk/admin/profile/#{user.id}/edit") - end - end - end - context "logged in as moderator user" do let(:user) { FactoryBot.create(:moderator_user) } before { login_as(user) } diff --git a/spec/controllers/admin/petition_tags_controller_spec.rb b/spec/controllers/admin/petition_tags_controller_spec.rb index 709a78741..46f8acbd9 100644 --- a/spec/controllers/admin/petition_tags_controller_spec.rb +++ b/spec/controllers/admin/petition_tags_controller_spec.rb @@ -19,25 +19,6 @@ end end - context "when logged in as moderator user but need to reset password" do - let(:user) { FactoryBot.create(:moderator_user, force_password_reset: true) } - before { login_as(user) } - - describe "GET /admin/petitions/:petition_id/tags" do - it "redirects to the edit profile page" do - get :show, params: { petition_id: petition.id } - expect(response).to redirect_to("https://moderate.petition.parliament.uk/admin/profile/#{user.id}/edit") - end - end - - describe "PATCH /admin/petitions/:petition_id/tags" do - it "redirects to the edit profile page" do - patch :update, params: { petition_id: petition.id } - expect(response).to redirect_to("https://moderate.petition.parliament.uk/admin/profile/#{user.id}/edit") - end - end - end - context "when logged in as moderator user" do let(:user) { FactoryBot.create(:moderator_user) } before { login_as(user) } diff --git a/spec/controllers/admin/petition_topics_controller_spec.rb b/spec/controllers/admin/petition_topics_controller_spec.rb index 9c5c39cb0..cedc49a75 100644 --- a/spec/controllers/admin/petition_topics_controller_spec.rb +++ b/spec/controllers/admin/petition_topics_controller_spec.rb @@ -19,25 +19,6 @@ end end - context "when logged in as moderator user but need to reset password" do - let(:user) { FactoryBot.create(:moderator_user, force_password_reset: true) } - before { login_as(user) } - - describe "GET /admin/petitions/:petition_id/topics" do - it "redirects to the edit profile page" do - get :show, params: { petition_id: petition.id } - expect(response).to redirect_to("https://moderate.petition.parliament.uk/admin/profile/#{user.id}/edit") - end - end - - describe "PATCH /admin/petitions/:petition_id/topics" do - it "redirects to the edit profile page" do - patch :update, params: { petition_id: petition.id } - expect(response).to redirect_to("https://moderate.petition.parliament.uk/admin/profile/#{user.id}/edit") - end - end - end - context "when logged in as moderator user" do let(:user) { FactoryBot.create(:moderator_user) } before { login_as(user) } diff --git a/spec/controllers/admin/petitions_controller_spec.rb b/spec/controllers/admin/petitions_controller_spec.rb index 0e56c7249..8fd4ba438 100644 --- a/spec/controllers/admin/petitions_controller_spec.rb +++ b/spec/controllers/admin/petitions_controller_spec.rb @@ -17,25 +17,6 @@ end end - context "when logged in as a moderator but need to reset password" do - let(:user) { FactoryBot.create(:moderator_user, force_password_reset: true) } - before { login_as(user) } - - describe "GET /admin/petitions" do - it "redirects to the edit profile page" do - get :index - expect(response).to redirect_to("https://moderate.petition.parliament.uk/admin/profile/#{user.id}/edit") - end - end - - describe "GET /admin/petitions/:id" do - it "redirects to the edit profile page" do - get :show, params: { id: "100000" } - expect(response).to redirect_to("https://moderate.petition.parliament.uk/admin/profile/#{user.id}/edit") - end - end - end - context "when logged in as a moderator" do let(:user) { FactoryBot.create(:moderator_user) } before { login_as(user) } diff --git a/spec/controllers/admin/profile_controller_spec.rb b/spec/controllers/admin/profile_controller_spec.rb deleted file mode 100644 index 2cdd7d488..000000000 --- a/spec/controllers/admin/profile_controller_spec.rb +++ /dev/null @@ -1,116 +0,0 @@ -require 'rails_helper' - -RSpec.describe Admin::ProfileController, type: :controller, admin: true do - before :each do - @user = FactoryBot.create(:sysadmin_user, :password => 'Letmein1!', - :password_confirmation => 'Letmein1!', :force_password_reset => true) - end - - context "not logged in" do - describe "GET 'edit'" do - it "should redirect to the login page" do - get 'edit', params: { id: @user.id } - expect(response).to redirect_to("https://moderate.petition.parliament.uk/admin/login") - end - end - end - - context "logged in but need to reset password" do - before :each do - login_as(@user) - end - - it "should render successfully" do - expect(@user.has_to_change_password?).to be_truthy - get :edit, params: { id: 50000 } - expect(response).to be_successful - end - end - - context "logged in" do - before :each do - login_as(@user) - end - - describe "GET 'edit'" do - it "should render successfully" do - get :edit, params: { id: 50000 } - expect(response).to be_successful - end - end - - describe "GET 'update'" do - def do_patch(current_password, new_password, password_confirmation) - admin_user_attributes = { - current_password: current_password, - password: new_password, - password_confirmation: password_confirmation - } - patch :update, params: { id: 50000, admin_user: admin_user_attributes } - end - - context "successful password change" do - it "should update password" do - do_patch('Letmein1!', 'Letmeout1!', 'Letmeout1!') - @user.reload - expect(@user.valid_password?('Letmeout1!')).to be_truthy - end - - it "should update password_changed_at to current time" do - do_patch('Letmein1!', 'Letmeout1!', 'Letmeout1!') - @user.reload - expect(@user.password_changed_at).to be_within(1.second).of(Time.current) - end - - it "should set force_password_reset to false" do - expect(@user.force_password_reset).to be_truthy - do_patch('Letmein1!', 'Letmeout1!', 'Letmeout1!') - @user.reload - expect(@user.force_password_reset).to be_falsey - end - - it "should redirect" do - do_patch('Letmein1!', 'Letmeout1!', 'Letmeout1!') - expect(response).to redirect_to("https://moderate.petition.parliament.uk/admin") - end - end - - context "unsuccessful password change" do - it "should have current password incorrect" do - do_patch('Letmeout1!', 'Letmein1!', 'Letmein1!') - expect(@user.valid_password?('Letmeout1!')).to be_falsey - expect(@user.valid_password?('Letmein1!')).to be_truthy - expect(assigns[:current_user].errors[:current_password]).not_to be_blank - end - - it "should not update password_changed_at" do - do_patch('Letmeout1!', 'Letmein1!', 'Letmein1!') - @user.reload - expect(@user.password_changed_at).not_to be_within(1.second).of(Time.current) - expect(@user.valid_password?('Letmein1!')).to be_truthy - end - - it "should have current password and new password are the same" do - do_patch('Letmein1!', 'Letmein1!', 'Letmein1!') - expect(assigns[:current_user].errors[:password]).not_to be_blank - @user.reload - expect(@user.valid_password?('Letmein1!')).to be_truthy - end - - it "should have new password as invalid" do - do_patch('Letmein1!', 'aB1defgh', 'aB1defgh') - expect(assigns[:current_user].errors[:password]).not_to be_blank - @user.reload - expect(@user.valid_password?('Letmein1!')).to be_truthy - end - - it "should have password as invalid when confirmation is different" do - do_patch('Letmein1!', 'aB1!efgh', 'aB1defgh') - expect(assigns[:current_user].errors[:password_confirmation]).not_to be_blank - @user.reload - expect(@user.valid_password?('Letmein1!')).to be_truthy - end - end - end - end -end diff --git a/spec/controllers/admin/schedule_debate_controller_spec.rb b/spec/controllers/admin/schedule_debate_controller_spec.rb index 2f191f2b9..9252e5ba2 100644 --- a/spec/controllers/admin/schedule_debate_controller_spec.rb +++ b/spec/controllers/admin/schedule_debate_controller_spec.rb @@ -41,25 +41,6 @@ end end - context 'logged in as moderator user but need to reset password' do - let(:user) { FactoryBot.create(:moderator_user, force_password_reset: true) } - before { login_as(user) } - - describe 'GET /show' do - it 'redirects to edit profile page' do - get :show, params: { petition_id: petition.id } - expect(response).to redirect_to("https://moderate.petition.parliament.uk/admin/profile/#{user.id}/edit") - end - end - - describe 'PATCH /update' do - it 'redirects to edit profile page' do - patch :update, params: { petition_id: petition.id } - expect(response).to redirect_to("https://moderate.petition.parliament.uk/admin/profile/#{user.id}/edit") - end - end - end - context "logged in as moderator user" do let(:user) { FactoryBot.create(:moderator_user) } before { login_as(user) } diff --git a/spec/controllers/admin/searches_controller_spec.rb b/spec/controllers/admin/searches_controller_spec.rb index 6c1ba092f..a3ddde258 100644 --- a/spec/controllers/admin/searches_controller_spec.rb +++ b/spec/controllers/admin/searches_controller_spec.rb @@ -10,18 +10,6 @@ end end - context "when logged in as a moderator but need to reset password" do - let(:user) { FactoryBot.create(:moderator_user, force_password_reset: true) } - before { login_as(user) } - - describe "GET /admin/search" do - it "redirects to the edit profile page" do - get :show, params: { type: "petition", q: "foo" } - expect(response).to redirect_to("https://moderate.petition.parliament.uk/admin/profile/#{user.id}/edit") - end - end - end - context "when logged in as a moderator" do let(:user) { FactoryBot.create(:moderator_user) } before { login_as(user) } diff --git a/spec/controllers/admin/signatures_controller_spec.rb b/spec/controllers/admin/signatures_controller_spec.rb index 7f1342f75..1faa8d651 100644 --- a/spec/controllers/admin/signatures_controller_spec.rb +++ b/spec/controllers/admin/signatures_controller_spec.rb @@ -20,25 +20,6 @@ end end - context "logged in as moderator user but need to reset password" do - let(:user) { FactoryBot.create(:moderator_user, force_password_reset: true) } - before { login_as(user) } - - describe "GET /admin/signatures" do - it "redirects to the login page" do - get :index, params: { q: "Alice" } - expect(response).to redirect_to("https://moderate.petition.parliament.uk/admin/profile/#{user.id}/edit") - end - end - - describe "DELETE /admin/signatures/:id" do - it "redirects to edit profile page" do - delete :destroy, params: { id: signature.id } - expect(response).to redirect_to("https://moderate.petition.parliament.uk/admin/profile/#{user.id}/edit") - end - end - end - context "logged in as moderator user" do let(:user) { FactoryBot.create(:moderator_user) } let(:token) { SecureRandom.base64(32) } diff --git a/spec/controllers/admin/take_down_controller_spec.rb b/spec/controllers/admin/take_down_controller_spec.rb index c073a5a03..e323d89b4 100644 --- a/spec/controllers/admin/take_down_controller_spec.rb +++ b/spec/controllers/admin/take_down_controller_spec.rb @@ -48,25 +48,6 @@ end end - context 'logged in as moderator user but need to reset password' do - let(:user) { FactoryBot.create(:moderator_user, force_password_reset: true) } - before { login_as(user) } - - describe 'GET /show' do - it 'redirects to edit profile page' do - get :show, params: { petition_id: petition.id } - expect(response).to redirect_to("https://moderate.petition.parliament.uk/admin/profile/#{user.id}/edit") - end - end - - describe 'PATCH /update' do - it 'redirects to edit profile page' do - patch :update, params: { petition_id: petition.id } - expect(response).to redirect_to("https://moderate.petition.parliament.uk/admin/profile/#{user.id}/edit") - end - end - end - context "logged in as moderator user" do let(:user) { FactoryBot.create(:moderator_user) } before { login_as(user) } diff --git a/spec/controllers/admin/trending_domains_controller_spec.rb b/spec/controllers/admin/trending_domains_controller_spec.rb index 80a341ad7..16f01cdad 100644 --- a/spec/controllers/admin/trending_domains_controller_spec.rb +++ b/spec/controllers/admin/trending_domains_controller_spec.rb @@ -13,24 +13,6 @@ end end - context "when logged in as moderator user but need to reset password" do - let(:user) { FactoryBot.create(:moderator_user, force_password_reset: true) } - - before do - login_as(user) - end - - describe "GET /admin/petitions/200000/trending-domains" do - before do - get :index, params: { petition_id: "200000" } - end - - it "redirects to edit profile page" do - expect(response).to redirect_to("https://moderate.petition.parliament.uk/admin/profile/#{user.id}/edit") - end - end - end - context "when logged in as moderator user" do let(:user) { FactoryBot.create(:moderator_user) } diff --git a/spec/controllers/admin/trending_ips_controller_spec.rb b/spec/controllers/admin/trending_ips_controller_spec.rb index b9c3c0479..25c1712e8 100644 --- a/spec/controllers/admin/trending_ips_controller_spec.rb +++ b/spec/controllers/admin/trending_ips_controller_spec.rb @@ -13,24 +13,6 @@ end end - context "when logged in as moderator user but need to reset password" do - let(:user) { FactoryBot.create(:moderator_user, force_password_reset: true) } - - before do - login_as(user) - end - - describe "GET /admin/petitions/200000/trending-ips" do - before do - get :index, params: { petition_id: "200000" } - end - - it "redirects to edit profile page" do - expect(response).to redirect_to("https://moderate.petition.parliament.uk/admin/profile/#{user.id}/edit") - end - end - end - context "when logged in as moderator user" do let(:user) { FactoryBot.create(:moderator_user) } diff --git a/spec/factories.rb b/spec/factories.rb index c4d03081a..7ba8c74db 100644 --- a/spec/factories.rb +++ b/spec/factories.rb @@ -4,11 +4,8 @@ FactoryBot.define do factory :admin_user do sequence(:email) { |n| "admin#{n}@example.com" } - password { "Letmein1!" } - password_confirmation { "Letmein1!" } sequence(:first_name) { |n| "AdminUser#{n}" } sequence(:last_name) { |n| "AdminUser#{n}" } - force_password_reset { false } end factory :sysadmin_user, :parent => :admin_user do @@ -80,6 +77,10 @@ opened_at { 2.years.ago } closed_at { 1.year.ago } + trait :creator do + association :creator, :validated, :creator, factory: :archived_signature + end + after(:build) do |petition, evaluator| petition.parliament ||= Parliament.archived.first || FactoryBot.create(:parliament, :archived) @@ -219,7 +220,7 @@ location_code { "GB" } ip_address { Faker::Internet.public_ip_v4_address } state { Archived::Signature::VALIDATED_STATE } - unsubscribe_token { Authlogic::Random.friendly_token } + unsubscribe_token { SecureRandom.base58(20) } notify_by_email { true } trait :creator do @@ -258,6 +259,7 @@ creator_name { nil } creator_email { nil } creator_attributes { {} } + email_attributes { nil } sponsors_signed { nil } sponsor_count { Site.minimum_number_of_sponsors } increment { true } @@ -294,6 +296,10 @@ if evaluator.admin_notes petition.build_note details: evaluator.admin_notes end + + if evaluator.email_attributes + petition.emails << build(:petition_email, petition: petition, **evaluator.email_attributes) + end end after(:create) do |petition, evaluator| @@ -317,6 +323,11 @@ end end + trait :archived do + archived_at { 3.day.ago } + archiving_started_at { 4.days.ago } + end + trait :with_additional_details do additional_details { "Petition additional details" } end @@ -971,4 +982,82 @@ ) end end + + factory :sso_user, class: Hash do + transient do + email { "admin@example.com" } + first_name { "Sys" } + last_name { "Admin" } + groups { %w[sysadmins] } + end + + provider { "example" } + uid { email } + + info do + { + name: "#{first_name} #{last_name}", + email: email, + first_name: first_name, + last_name: last_name, + groups: groups + } + end + + credentials do + {} + end + + extra do + { + raw_info: { + name: "#{first_name} #{last_name}", + email: email, + first_name: first_name, + last_name: last_name, + groups: groups + } + } + end + + skip_create + + initialize_with { OmniAuth::AuthHash.new(attributes) } + end + + factory :sysadmin_sso_user, parent: :sso_user do + transient do + email { "sysadmin@example.com" } + first_name { "John" } + last_name { "Admin" } + groups { %w[sysadmins] } + end + end + + factory :moderator_sso_user, parent: :sso_user do + transient do + email { "moderator@example.com" } + first_name { "John" } + last_name { "Moderator" } + groups { %w[moderators] } + end + end + + factory :reviewer_sso_user, parent: :sso_user do + transient do + email { "reviewer@example.com" } + first_name { "John" } + last_name { "Reviewer" } + groups { %w[reviewers] } + end + end + + factory :norole_sso_user, parent: :sso_user do + transient do + email { "norole@example.com" } + first_name { "No" } + last_name { "Role" } + groups { %w[] } + end + end end diff --git a/spec/jobs/archive_petition_job_spec.rb b/spec/jobs/archive_petition_job_spec.rb index 2ca51344c..cc9403835 100644 --- a/spec/jobs/archive_petition_job_spec.rb +++ b/spec/jobs/archive_petition_job_spec.rb @@ -287,7 +287,7 @@ end end - context "with a petition that has an government_response email scheduled" do + context "with a petition that has a government_response email scheduled" do let(:petition) do FactoryBot.create(:closed_petition, :email_requested, email_requested_for_government_response_at: Time.current) end @@ -297,7 +297,7 @@ end end - context "with a petition that has an debate_scheduled email scheduled" do + context "with a petition that has a debate_scheduled email scheduled" do let(:petition) do FactoryBot.create(:closed_petition, :email_requested, email_requested_for_debate_scheduled_at: Time.current) end @@ -307,7 +307,7 @@ end end - context "with a petition that has an debate_outcome email scheduled" do + context "with a petition that has a debate_outcome email scheduled" do let(:petition) do FactoryBot.create(:closed_petition, :email_requested, email_requested_for_debate_outcome_at: Time.current) end @@ -317,7 +317,7 @@ end end - context "with a petition that has an petition_email email scheduled" do + context "with a petition that has a petition_email email scheduled" do let(:petition) do FactoryBot.create(:closed_petition, :email_requested, email_requested_for_petition_email_at: Time.current) end @@ -327,7 +327,24 @@ end end - context "with a petition that has an petition_mailshot email scheduled" do + context "with a petition that has a petition_email email that has been sent" do + let(:petition) do + FactoryBot.create(:closed_petition, email_attributes: { email_count: 216, emails_enqueued_at: 6.months.ago }) + end + + let(:email) { petition.emails.first } + let(:archived_email) { archived_petition.emails.first } + + it "copies the email count to the archived petition email" do + expect(archived_email.email_count).to eq(email.email_count) + end + + it "copies the emails enqueued timestamp to the archived petition email" do + expect(archived_email.emails_enqueued_at).to be_usec_precise_with(email.emails_enqueued_at) + end + end + + context "with a petition that has a petition_mailshot email scheduled" do let(:petition) do FactoryBot.create(:closed_petition, :email_requested, email_requested_for_petition_mailshot_at: Time.current) end diff --git a/spec/jobs/archived/email_petitioners_job_spec.rb b/spec/jobs/archived/email_petitioners_job_spec.rb index 990a5d974..8cd4e569f 100644 --- a/spec/jobs/archived/email_petitioners_job_spec.rb +++ b/spec/jobs/archived/email_petitioners_job_spec.rb @@ -3,7 +3,7 @@ RSpec.describe Archived::EmailPetitionersJob, type: :job do let(:email_requested_at) { Time.current } - let(:petition) { FactoryBot.create(:archived_petition) } + let(:petition) { FactoryBot.create(:archived_petition, :creator) } let(:signature) { FactoryBot.create(:archived_signature, petition: petition) } let(:email) { FactoryBot.create(:archived_petition_email, petition: petition) } let(:arguments) { { petition: petition, email: email } } @@ -13,8 +13,32 @@ allow(petition).to receive_message_chain(:signatures_to_email_for, :find_each).and_yield(signature) end - it_behaves_like "job to enqueue signatory mailing jobs" + it_behaves_like "job to enqueue signatory mailing jobs", -> { + subject.perform(petition: petition, email: email, requested_at: requested_at_as_string) + } + context "when sending other parliamentary business emails" do + it "records the number of emails sent" do + expect { + perform_enqueued_jobs { + described_class.run_later_tonight(**arguments) + } + }.to change { + email.reload.email_count + }.from(nil).to(2) + end + + it "records when the emails were enqueued by" do + expect { + perform_enqueued_jobs { + described_class.run_later_tonight(**arguments) + } + }.to change { + email.reload.emails_enqueued_at + }.from(nil).to(be_within(1.second).of(Time.current)) + end + end + context "when the petition email has been deleted" do before do email.destroy diff --git a/spec/jobs/delete_petition_job_spec.rb b/spec/jobs/delete_petition_job_spec.rb index 5cf363d5c..a15cd8846 100644 --- a/spec/jobs/delete_petition_job_spec.rb +++ b/spec/jobs/delete_petition_job_spec.rb @@ -7,7 +7,7 @@ end context "with a stopped petition" do - let!(:petition) { FactoryBot.create(:stopped_petition) } + let!(:petition) { FactoryBot.create(:stopped_petition, :archived) } it "destroys the petition" do expect { @@ -19,7 +19,7 @@ end context "with a closed petition" do - let!(:petition) { FactoryBot.create(:validated_petition, sponsors_signed: true, state: "closed", closed_at: 4.weeks.ago) } + let!(:petition) { FactoryBot.create(:closed_petition, :archived, sponsors_signed: true, closed_at: 4.weeks.ago) } let!(:country_petition_journal) { FactoryBot.create(:country_petition_journal, petition: petition) } let!(:constituency_petition_journal) { FactoryBot.create(:constituency_petition_journal, petition: petition) } @@ -136,7 +136,7 @@ end context "with a rejected petition" do - let!(:petition) { FactoryBot.create(:rejected_petition) } + let!(:petition) { FactoryBot.create(:rejected_petition, :archived) } it "destroys the petition" do expect { @@ -157,7 +157,7 @@ context "with a hidden petition" do let!(:user) { FactoryBot.create(:moderator_user) } - let!(:petition) { FactoryBot.create(:rejected_petition, rejection_code: "libellous", moderated_by: user) } + let!(:petition) { FactoryBot.create(:rejected_petition, :archived, rejection_code: "libellous", moderated_by: user) } it "destroys the petition" do expect { diff --git a/spec/jobs/email_petitioners_job_spec.rb b/spec/jobs/email_petitioners_job_spec.rb index 1180b5285..b76ac18a4 100644 --- a/spec/jobs/email_petitioners_job_spec.rb +++ b/spec/jobs/email_petitioners_job_spec.rb @@ -13,7 +13,31 @@ allow(petition).to receive_message_chain(:signatures_to_email_for, :find_each).and_yield(signature) end - it_behaves_like "job to enqueue signatory mailing jobs" + it_behaves_like "job to enqueue signatory mailing jobs", -> { + subject.perform(petition: petition, email: email, requested_at: requested_at_as_string) + } + + context "when sending other parliamentary business emails" do + it "records the number of emails sent" do + expect { + perform_enqueued_jobs { + described_class.run_later_tonight(**arguments) + } + }.to change { + email.reload.email_count + }.from(nil).to(2) + end + + it "records when the emails were enqueued by" do + expect { + perform_enqueued_jobs { + described_class.run_later_tonight(**arguments) + } + }.to change { + email.reload.emails_enqueued_at + }.from(nil).to(be_within(1.second).of(Time.current)) + end + end context "when the petition email has been deleted" do before do diff --git a/spec/jobs/shared_examples.rb b/spec/jobs/shared_examples.rb index 8be28c967..196669a42 100644 --- a/spec/jobs/shared_examples.rb +++ b/spec/jobs/shared_examples.rb @@ -1,9 +1,11 @@ -RSpec.shared_examples_for "job to enqueue signatory mailing jobs" do +RSpec.shared_examples_for "job to enqueue signatory mailing jobs" do |work_proc| let(:requested_at) { Time.current.change(usec: 0) } let(:requested_at_as_string) { requested_at.getutc.iso8601(6) } - def do_work - subject.perform(petition: petition, requested_at: requested_at_as_string) + unless work_proc.present? + work_proc = -> { + subject.perform(petition: petition, requested_at: requested_at_as_string) + } end describe '.run_later_tonight' do @@ -47,18 +49,18 @@ def do_work context "when the petition has not been updated" do it "enqueues a job to send an email to each signatory" do - do_work + instance_exec(&work_proc) expect(enqueued_jobs.size).to eq(1) end it "the job is the correct type for the type of notification" do - do_work + instance_exec(&work_proc) job = enqueued_jobs.first expect(job[:job]).to eq(subject.email_delivery_job_class) end it "the job has the expected arguments" do - do_work + instance_exec(&work_proc) args = enqueued_jobs.first[:args].first expect(args["timestamp_name"]).to eq(subject.timestamp_name) @@ -72,7 +74,7 @@ def do_work end it "does not enqueue any jobs to send emails" do - do_work + instance_exec(&work_proc) expect(enqueued_jobs).to be_empty end end diff --git a/spec/models/admin_user_session_spec.rb b/spec/models/admin_user_session_spec.rb deleted file mode 100644 index 65117cf53..000000000 --- a/spec/models/admin_user_session_spec.rb +++ /dev/null @@ -1,51 +0,0 @@ -require 'rails_helper' - -RSpec.describe AdminUserSession do - describe "#last_login_attempt?" do - before do - Authlogic::Session::Base.controller = Authlogic::TestCase::MockController.new - FactoryBot.create(:moderator_user, email: email) - end - - let(:email) { "admin@petition.parliament.uk" } - let(:params) { { email: email, password: "password" } } - let(:user_session) { described_class.new(params) } - let(:user) { AdminUser.find_by!(email: email) } - - context "when there are no failed login attempts" do - before do - user.update_columns(failed_login_count: 0) - user_session.save - end - - it "returns false" do - expect(user_session.attempted_record).to be_present - expect(user_session.last_login_attempt?).to be false - end - end - - context "when there are 3 failed login attempts" do - before do - user.update_columns(failed_login_count: 3) - user_session.save - end - - it "returns true" do - expect(user_session.attempted_record).to be_present - expect(user_session.last_login_attempt?).to be true - end - end - - context "when there are 4 failed login attempts" do - before do - user.update_columns(failed_login_count: 4) - user_session.save - end - - it "returns false" do - expect(user_session.attempted_record).to be_present - expect(user_session.last_login_attempt?).to be false - end - end - end -end diff --git a/spec/models/admin_user_spec.rb b/spec/models/admin_user_spec.rb index d7790b125..62c4a9ff4 100644 --- a/spec/models/admin_user_spec.rb +++ b/spec/models/admin_user_spec.rb @@ -4,20 +4,21 @@ describe "schema" do it { is_expected.to have_db_column(:email).of_type(:string).with_options(null: false) } it { is_expected.to have_db_column(:persistence_token).of_type(:string) } - it { is_expected.to have_db_column(:crypted_password).of_type(:string) } - it { is_expected.to have_db_column(:password_salt).of_type(:string) } - it { is_expected.to have_db_column(:login_count).of_type(:integer).with_options(default: 0) } - it { is_expected.to have_db_column(:failed_login_count).of_type(:integer).with_options(default: 0) } - it { is_expected.to have_db_column(:current_login_at).of_type(:datetime) } - it { is_expected.to have_db_column(:last_login_at).of_type(:datetime) } - it { is_expected.to have_db_column(:current_login_ip).of_type(:string) } - it { is_expected.to have_db_column(:last_login_ip).of_type(:string) } + it { is_expected.to have_db_column(:sign_in_count).of_type(:integer).with_options(default: 0) } + it { is_expected.to have_db_column(:current_sign_in_at).of_type(:datetime) } + it { is_expected.to have_db_column(:last_sign_in_at).of_type(:datetime) } + it { is_expected.to have_db_column(:current_sign_in_ip).of_type(:string) } + it { is_expected.to have_db_column(:last_sign_in_ip).of_type(:string) } it { is_expected.to have_db_column(:role).of_type(:string).with_options(limit: 10, null: false) } - it { is_expected.to have_db_column(:force_password_reset).of_type(:boolean).with_options(default: true) } - it { is_expected.to have_db_column(:password_changed_at).of_type(:datetime) } it { is_expected.to have_db_column(:created_at).of_type(:datetime) } it { is_expected.to have_db_column(:updated_at).of_type(:datetime) } - it { is_expected.to have_db_column(:last_request_at).of_type(:datetime) } + + it { is_expected.not_to have_db_column(:encrypted_password).of_type(:string) } + it { is_expected.not_to have_db_column(:password_salt).of_type(:string) } + it { is_expected.not_to have_db_column(:failed_attempts).of_type(:integer).with_options(default: 0) } + it { is_expected.not_to have_db_column(:force_password_reset).of_type(:boolean).with_options(default: true) } + it { is_expected.not_to have_db_column(:password_changed_at).of_type(:datetime) } + it { is_expected.not_to have_db_column(:locked_at).of_type(:datetime) } end describe "indexes" do @@ -25,23 +26,10 @@ it { is_expected.to have_db_index([:last_name, :first_name]) } end - describe "behaviours" do - it { expect(AdminUser.respond_to?(:acts_as_authentic)).to be_truthy } - end - - describe "defaults" do - it "force_password_reset should default to true" do - u = AdminUser.new - expect(u.force_password_reset).to be_truthy - end - end - describe "validations" do - it { is_expected.to validate_presence_of(:password) } it { is_expected.to validate_presence_of(:email) } it { is_expected.to validate_presence_of(:first_name) } it { is_expected.to validate_presence_of(:last_name) } - it { is_expected.to validate_length_of(:password).is_at_least(8) } it { is_expected.to allow_value("oliver@opsb.co.uk").for(:email)} it { is_expected.not_to allow_value("jimbo").for(:email) } @@ -50,20 +38,6 @@ is_expected.to validate_uniqueness_of(:email).case_insensitive end - it "should only allow passwords with a digit, lower and upper case alpha and a special char" do - ['Letmein1!', 'Letmein1_', '1Ab*aaaa'].each do |email| - u = FactoryBot.build(:moderator_user, :password => email, :password_confirmation => email) - expect(u).to be_valid - end - end - - it "should not allow passwords without a digit, lower and upper case alpha and a special char" do - ['Letmein1', 'hell$0123', '^%ttttFFFFF', 'KJDL_3444'].each do |email| - u = FactoryBot.build(:moderator_user, :password => email, :password_confirmation => email) - expect(u).not_to be_valid - end - end - it "should allow sysadmin role" do u = FactoryBot.build(:admin_user, :role => 'sysadmin') expect(u).to be_valid @@ -100,177 +74,6 @@ end describe "instance methods" do - describe "#update_with_password" do - let!(:user) do - FactoryBot.create( - :sysadmin_user, - password: "Testing!23", - password_confirmation: "Testing!23", - password_changed_at: nil, - force_password_reset: true - ) - end - - let(:params) do - { - current_password: current_password, password: password, - password_confirmation: password_confirmation - } - end - - let(:current_password) { "Testing!23" } - let(:password) { "NewP4ssword!" } - let(:password_confirmation) { "NewP4ssword!" } - - context "when the new password is valid" do - it "returns true" do - expect(user.update_with_password(params)).to be_truthy - end - - it "changes the crypted_password field" do - expect { - user.update_with_password(params) - }.to change { - user.reload.crypted_password - } - end - - it "sets the timestamp for when the password was changed" do - expect { - user.update_with_password(params) - }.to change { - user.reload.password_changed_at - }.from(nil).to(be_within(1.second).of(Time.current)) - end - - it "clears the force password reset flag" do - expect { - user.update_with_password(params) - }.to change { - user.reload.force_password_reset - }.from(true).to(false) - end - end - - context "when the current password is missing" do - let(:current_password) { "" } - - it "returns false" do - expect(user.update_with_password(params)).to be_falsey - end - - it "adds an error" do - user.update_with_password(params) - expect(user.errors[:current_password]).to eq(["Current password can't be blank"]) - end - - it "doesn't clear the force password reset flag" do - user.update_with_password(params) - expect(user.force_password_reset).to be true - end - - it "doesn't set the password_changed_at timestamp" do - user.update_with_password(params) - expect(user.password_changed_at).to be_nil - end - end - - context "when the current password is incorrect" do - let(:current_password) { "L3tme!n" } - - it "returns false" do - expect(user.update_with_password(params)).to be_falsey - end - - it "adds an error" do - user.update_with_password(params) - expect(user.errors[:current_password]).to eq(["Current password is incorrect"]) - end - - it "doesn't clear the force password reset flag" do - user.update_with_password(params) - expect(user.force_password_reset).to be true - end - - it "doesn't set the password_changed_at timestamp" do - user.update_with_password(params) - expect(user.password_changed_at).to be_nil - end - end - - context "when the new password is the same as the old password" do - let(:password) { current_password } - let(:password_confirmation) { current_password } - - it "returns false" do - expect(user.update_with_password(params)).to be_falsey - end - - it "adds an error" do - user.update_with_password(params) - expect(user.errors[:password]).to eq(["Password is the same as the current password"]) - end - - it "doesn't clear the force password reset flag" do - user.update_with_password(params) - expect(user.force_password_reset).to be true - end - - it "doesn't set the password_changed_at timestamp" do - user.update_with_password(params) - expect(user.password_changed_at).to be_nil - end - end - - context "when the new password is invalid" do - let(:password) { "password" } - let(:password_confirmation) { "password" } - - it "returns false" do - expect(user.update_with_password(params)).to be_falsey - end - - it "adds an error" do - user.update_with_password(params) - expect(user.errors[:password]).to eq(["Password must contain at least one digit, a lower and upper case letter and a special character"]) - end - - it "doesn't clear the force password reset flag" do - user.update_with_password(params) - expect(user.force_password_reset).to be true - end - - it "doesn't set the password_changed_at timestamp" do - user.update_with_password(params) - expect(user.password_changed_at).to be_nil - end - end - - context "when the new password doesn't match the confirmation" do - let(:password) { "L3tme!n1" } - let(:password_confirmation) { "L3tme!n2" } - - it "returns false" do - expect(user.update_with_password(params)).to be_falsey - end - - it "adds an error" do - user.update_with_password(params) - expect(user.errors[:password_confirmation]).to eq(["Password confirmation doesn't match password"]) - end - - it "doesn't clear the force password reset flag" do - user.update_with_password(params) - expect(user.force_password_reset).to be true - end - - it "doesn't set the password_changed_at timestamp" do - user.update_with_password(params) - expect(user.password_changed_at).to be_nil - end - end - end - describe "#destroy" do context "when there is no current user and there is more than one" do let!(:user_1) { FactoryBot.create(:sysadmin_user) } @@ -342,28 +145,6 @@ end end - describe "#has_to_change_password?" do - it "should be true when force_reset_password is true" do - user = FactoryBot.create(:moderator_user, :force_password_reset => true) - expect(user.has_to_change_password?).to be_truthy - end - - it "should be false when force_reset_password is false" do - user = FactoryBot.create(:moderator_user, :force_password_reset => false) - expect(user.has_to_change_password?).to be_falsey - end - - it "should be true when password was last changed over 9 months ago" do - user = FactoryBot.create(:moderator_user, :force_password_reset => false, :password_changed_at => 9.months.ago - 1.minute) - expect(user.has_to_change_password?).to be_truthy - end - - it "should be false when password was last changed less than 9 months ago" do - user = FactoryBot.create(:moderator_user, :force_password_reset => false, :password_changed_at => 9.months.ago + 1.minute) - expect(user.has_to_change_password?).to be_falsey - end - end - describe "#can_take_petitions_down?" do it "is true if the user is a sysadmin" do user = FactoryBot.create(:admin_user, :role => 'sysadmin') @@ -376,52 +157,21 @@ end end - describe "#account_disabled" do - it "should return true when user has tried to login 5 times unsuccessfully" do - user = FactoryBot.create(:moderator_user) - user.failed_login_count = 5 - expect(user.account_disabled).to be_truthy - end - - it "should return true when user has tried to login 6 times unsuccessfully" do - user = FactoryBot.create(:moderator_user) - user.failed_login_count = 6 - expect(user.account_disabled).to be_truthy - end - - it "should return false when user has tried to login 4 times unsuccessfully" do - user = FactoryBot.create(:moderator_user) - user.failed_login_count = 4 - expect(user.account_disabled).to be_falsey - end - end - - describe "#account_disabled=" do - it "should set the failed login count to 5 when true" do - u = FactoryBot.create(:moderator_user) - u.account_disabled = true - expect(u.failed_login_count).to eq(5) - end - - it "should set the failed login count to 0 when false" do - u = FactoryBot.create(:moderator_user) - u.failed_login_count = 5 - u.account_disabled = false - expect(u.failed_login_count).to eq(0) - end - end - describe "#elapsed_time" do it "returns the number of seconds since the last request" do - user = FactoryBot.build(:admin_user, last_request_at: 60.seconds.ago) - expect(user.elapsed_time).to eq(60) + user = FactoryBot.build(:admin_user) + expect(user.elapsed_time(60.seconds.ago)).to eq(60) end end describe "#time_remaining" do - it "returns the number of seconds since the last request" do - user = FactoryBot.build(:admin_user, last_request_at: 60.seconds.ago) - expect(user.elapsed_time).to eq(60) + before do + allow(Site).to receive(:login_timeout).and_return(300) + end + + it "returns the number of seconds remaining until the user is logged out" do + user = FactoryBot.build(:admin_user) + expect(user.time_remaining(60.seconds.ago)).to eq(240) end end end diff --git a/spec/models/signature_spec.rb b/spec/models/signature_spec.rb index 6df799492..5974dd3d9 100644 --- a/spec/models/signature_spec.rb +++ b/spec/models/signature_spec.rb @@ -2086,7 +2086,7 @@ end context "when another process has updated the column" do - let(:token) { Authlogic::Random.friendly_token } + let(:token) { SecureRandom.base58(20) } before do signature.update_column(:signed_token, nil) diff --git a/spec/requests/admin_user_persistence_token_spec.rb b/spec/requests/admin_user_persistence_token_spec.rb index 9327ab238..5dac1d99e 100644 --- a/spec/requests/admin_user_persistence_token_spec.rb +++ b/spec/requests/admin_user_persistence_token_spec.rb @@ -5,18 +5,17 @@ { first_name: "System", last_name: "Administrator", - email: "admin@petition.parliament.uk", - password: "L3tme1n!", - password_confirmation: "L3tme1n!" + email: "admin@example.com" } end let(:login_params) do - { email: "admin@petition.parliament.uk", password: "L3tme1n!" } + { email: "admin@example.com" } end before do - FactoryBot.create(:sysadmin_user, user_attributes) + sso_user = FactoryBot.create(:sysadmin_sso_user, **user_attributes) + OmniAuth.config.mock_auth[:example] = sso_user end def new_browser @@ -30,44 +29,74 @@ def new_browser context "when a new session is created" do it "logs out existing sessions" do s1 = new_browser - s1.post "/admin/user_sessions", params: { admin_user_session: login_params } + s1.post "/admin/login", params: { user: login_params } + + expect(s1.response.status).to eq(307) + expect(s1.response.location).to eq("https://moderate.petition.parliament.uk/admin/auth/example") + + s1.follow_redirect!(params: s1.request.POST) expect(s1.response.status).to eq(302) - expect(s1.response.headers["Location"]).to eq("https://moderate.petition.parliament.uk/admin") + expect(s1.response.location).to eq("https://moderate.petition.parliament.uk/admin/auth/example/callback") + + s1.follow_redirect! + + expect(s1.response.status).to eq(200) + expect(s1.response).to have_header("Refresh", "0; url=https://moderate.petition.parliament.uk/admin") s2 = new_browser - s2.post "/admin/user_sessions", params: { admin_user_session: login_params } + s2.post "/admin/login", params: { user: login_params } + + expect(s2.response.status).to eq(307) + expect(s2.response.location).to eq("https://moderate.petition.parliament.uk/admin/auth/example") + + s2.follow_redirect!(params: s2.request.POST) expect(s2.response.status).to eq(302) - expect(s2.response.headers["Location"]).to eq("https://moderate.petition.parliament.uk/admin") + expect(s2.response.location).to eq("https://moderate.petition.parliament.uk/admin/auth/example/callback") + + s2.follow_redirect! + + expect(s2.response.status).to eq(200) + expect(s2.response).to have_header("Refresh", "0; url=https://moderate.petition.parliament.uk/admin") s1.get("/admin") expect(s1.response.status).to eq(302) - expect(s1.response.headers["Location"]).to eq("https://moderate.petition.parliament.uk/admin/login") + expect(s1.response.location).to eq("https://moderate.petition.parliament.uk/admin/login") end end context "when a session is destroyed" do it "resets the persistence token" do s1 = new_browser - s1.post "/admin/user_sessions", params: { admin_user_session: login_params } + s1.post "/admin/login", params: { user: login_params } + + expect(s1.response.status).to eq(307) + expect(s1.response.location).to eq("https://moderate.petition.parliament.uk/admin/auth/example") + + s1.follow_redirect!(params: s1.request.POST) expect(s1.response.status).to eq(302) - expect(s1.response.headers["Location"]).to eq("https://moderate.petition.parliament.uk/admin") + expect(s1.response.location).to eq("https://moderate.petition.parliament.uk/admin/auth/example/callback") + + s1.follow_redirect! + + expect(s1.response.status).to eq(200) + expect(s1.response).to have_header("Refresh", "0; url=https://moderate.petition.parliament.uk/admin") s2 = new_browser - s2.cookies["admin_user_credentials"] = s1.cookies["admin_user_credentials"] + s2.cookies["_epets_session"] = s1.cookies["_epets_session"] s1.get("/admin/logout") expect(s1.response.status).to eq(302) - expect(s1.response.headers["Location"]).to eq("https://moderate.petition.parliament.uk/admin/login") + expect(s1.response.location).to eq("https://moderate.petition.parliament.uk/admin/login") s2.get("/admin") expect(s2.response.status).to eq(302) - expect(s2.response.headers["Location"]).to eq("https://moderate.petition.parliament.uk/admin/login") + expect(s2.response.location).to eq("https://moderate.petition.parliament.uk/admin/login") end end @@ -81,8 +110,20 @@ def new_browser Site.instance.update(login_timeout: 600) travel_to 5.minutes.ago do - post "/admin/user_sessions", params: { admin_user_session: login_params } - expect(response).to redirect_to("/admin") + post "/admin/login", params: { user: login_params } + + expect(response.status).to eq(307) + expect(response.location).to eq("https://moderate.petition.parliament.uk/admin/auth/example") + + follow_redirect!(params: request.POST) + + expect(response.status).to eq(302) + expect(response.location).to eq("https://moderate.petition.parliament.uk/admin/auth/example/callback") + + follow_redirect! + + expect(response.status).to eq(200) + expect(response).to have_header("Refresh", "0; url=https://moderate.petition.parliament.uk/admin") end get "/admin" diff --git a/spec/requests/bad_requests_spec.rb b/spec/requests/bad_requests_spec.rb index 67ec515d7..df982a87e 100644 --- a/spec/requests/bad_requests_spec.rb +++ b/spec/requests/bad_requests_spec.rb @@ -26,9 +26,7 @@ { first_name: "System", last_name: "Administrator", - email: "admin@petition.parliament.uk", - password: "L3tme1n!", - password_confirmation: "L3tme1n!" + email: "admin@petition.parliament.uk" } end diff --git a/spec/requests/csrf_token_spec.rb b/spec/requests/csrf_token_spec.rb index 5ed37b7f6..4e94edbab 100644 --- a/spec/requests/csrf_token_spec.rb +++ b/spec/requests/csrf_token_spec.rb @@ -5,14 +5,17 @@ { first_name: "System", last_name: "Administrator", - email: "admin@petition.parliament.uk", - password: "L3tme1n!", - password_confirmation: "L3tme1n!" + email: "admin@example.com" } end let(:login_params) do - { email: "admin@petition.parliament.uk", password: "L3tme1n!" } + { email: "admin@example.com" } + end + + before do + sso_user = FactoryBot.create(:sysadmin_sso_user, **user_attributes) + OmniAuth.config.mock_auth[:example] = sso_user end let(:encrypted_csrf_token) do @@ -33,10 +36,27 @@ get "/admin/login" end + def do_login + post "/admin/login", params: { user: login_params, authenticity_token: authenticity_token } + + expect(response.status).to eq(307) + expect(response.location).to eq("https://moderate.petition.parliament.uk/admin/auth/example") + + follow_redirect!(params: request.POST) + + expect(response.status).to eq(302) + expect(response.location).to eq("https://moderate.petition.parliament.uk/admin/auth/example/callback") + + follow_redirect! + + expect(response.status).to eq(200) + expect(response).to have_header("Refresh", "0; url=https://moderate.petition.parliament.uk/admin") + end + context "when a new session is created" do it "resets the csrf token" do expect { - post "/admin/user_sessions", params: { admin_user_session: login_params, authenticity_token: authenticity_token } + do_login }.to change { session[:_csrf_token] } @@ -44,12 +64,16 @@ end context "when a session is destroyed" do + before do + do_login + end + it "resets the csrf token" do expect { get "/admin/logout" }.to change { session[:_csrf_token] - } + }.from(be_present).to(be_nil) end end @@ -58,8 +82,7 @@ Site.instance.update(login_timeout: 600) travel_to 5.minutes.ago do - post "/admin/user_sessions", params: { admin_user_session: login_params, authenticity_token: authenticity_token } - expect(response).to redirect_to("/admin") + do_login end get "/admin" @@ -71,7 +94,7 @@ expect(response).to redirect_to("/admin/login") }.to change { session[:_csrf_token] - } + }.from(be_present).to(be_nil) end Site.instance.update(login_timeout: 1800) diff --git a/spec/requests/login_timeout_spec.rb b/spec/requests/login_timeout_spec.rb index d39ae6c64..89ff55fec 100644 --- a/spec/requests/login_timeout_spec.rb +++ b/spec/requests/login_timeout_spec.rb @@ -5,17 +5,18 @@ { first_name: "System", last_name: "Administrator", - email: "admin@petition.parliament.uk", - password: "L3tme1n!", - password_confirmation: "L3tme1n!" + email: "admin@example.com" } end let(:login_params) do - { email: "admin@petition.parliament.uk", password: "L3tme1n!" } + { email: "admin@example.com" } end - let!(:user) { FactoryBot.create(:sysadmin_user, user_attributes) } + before do + sso_user = FactoryBot.create(:sysadmin_sso_user, **user_attributes) + OmniAuth.config.mock_auth[:example] = sso_user + end before do host! "moderate.petition.parliament.uk" @@ -26,8 +27,20 @@ Site.instance.update(login_timeout: 600) travel_to 2.minutes.ago do - post "/admin/user_sessions", params: { admin_user_session: login_params } - expect(response).to redirect_to("/admin") + post "/admin/login", params: { user: login_params } + + expect(response.status).to eq(307) + expect(response.location).to eq("https://moderate.petition.parliament.uk/admin/auth/example") + + follow_redirect!(params: request.POST) + + expect(response.status).to eq(302) + expect(response.location).to eq("https://moderate.petition.parliament.uk/admin/auth/example/callback") + + follow_redirect! + + expect(response.status).to eq(200) + expect(response).to have_header("Refresh", "0; url=https://moderate.petition.parliament.uk/admin") end get "/admin" diff --git a/spec/routing/admin/petitions/government_response_spec.rb b/spec/routing/admin/petitions/government_response_spec.rb index 372647a07..436cc20c0 100644 --- a/spec/routing/admin/petitions/government_response_spec.rb +++ b/spec/routing/admin/petitions/government_response_spec.rb @@ -21,7 +21,7 @@ expect(patch("/admin/petitions/1/government-response")).to route_to('admin/government_response#update', petition_id: '1') end - it "doesn't route DELETE /admin/petitions/1/government-response" do - expect(delete("/admin/petitions/1/government-response")).not_to be_routable + it "routes DELETE /admin/petitions/1/government-response to admin/government_response#destroy" do + expect(delete("/admin/petitions/1/government-response")).to route_to('admin/government_response#destroy', petition_id: '1') end end diff --git a/spec/routing/domain_constraints_spec.rb b/spec/routing/domain_constraints_spec.rb index b3386554a..be1cce0fb 100644 --- a/spec/routing/domain_constraints_spec.rb +++ b/spec/routing/domain_constraints_spec.rb @@ -30,7 +30,7 @@ context "and making a request for an admin path" do it "is routable" do - expect(get("/admin/login")).to route_to("admin/user_sessions#new") + expect(get("/admin/login")).to route_to("admin/sessions#new") end end diff --git a/spec/support/authentication.rb b/spec/support/authentication.rb index 3f737e20f..83ee60779 100644 --- a/spec/support/authentication.rb +++ b/spec/support/authentication.rb @@ -1,10 +1,14 @@ -require 'authlogic/test_case' - RSpec.configure do |config| mod = Module.new do def login_as(user) - activate_authlogic - AdminUserSession.create(user) + sign_in(user, scope: :user) + + # The devise sign_in test helper bypasses the hooks that set + # the token in the session so we have to set them manually. + session["warden.user.user.session"] = { + "persistence_token" => user.persistence_token, + "last_request_at" => Time.current.to_i + } end def http_authentication(username, password) @@ -12,6 +16,6 @@ def http_authentication(username, password) end end + config.include(Devise::Test::ControllerHelpers, type: :controller) config.include(mod, type: :controller) - config.include(Authlogic::TestCase, type: :controller) end diff --git a/spec/support/omniauth.rb b/spec/support/omniauth.rb new file mode 100644 index 000000000..841ce48f0 --- /dev/null +++ b/spec/support/omniauth.rb @@ -0,0 +1,16 @@ +RSpec.configure do |config| + config.around(:each, type: :request) do |example| + OmniAuth.config.test_mode = true + existing_failure_proc = OmniAuth.config.on_failure + + OmniAuth.config.on_failure = Proc.new { |env| + OmniAuth::FailureEndpoint.new(env).redirect_to_failure + } + + example.run + ensure + OmniAuth.config.mock_auth[:example] = nil + OmniAuth.config.on_failure = existing_failure_proc + OmniAuth.config.test_mode = false + end +end