diff --git a/app/controllers/doorkeeper_applications_controller.rb b/app/controllers/doorkeeper_applications_controller.rb index 19609b565..2108374d0 100644 --- a/app/controllers/doorkeeper_applications_controller.rb +++ b/app/controllers/doorkeeper_applications_controller.rb @@ -24,6 +24,45 @@ def users_with_access @users = query.page(params[:page]).per(100) end + def access_logs + relation = @application.event_logs + .includes(:user) + .where(event_id: EventLog::SUCCESSFUL_USER_APPLICATION_AUTHORIZATION.id) + .order(created_at: :desc) + + unless params[:include_smokey_users] == "true" + smokey_uids = User.where("name LIKE 'Smokey%'").pluck(:uid) + relation = relation.where.not(uid: smokey_uids) + end + + if params[:month].present? + relation = relation.where("DATE_FORMAT(created_at, '%Y-%m')=?", params[:month]) + end + + @logs = relation + .page(params[:page]) + .per(100) + end + + def monthly_access_stats + relation = @application.event_logs + .where(event_id: EventLog::SUCCESSFUL_USER_APPLICATION_AUTHORIZATION.id) + .group("DATE_FORMAT(created_at, '%Y-%m')") + .order(Arel.sql("DATE_FORMAT(created_at, '%Y-%m') DESC")) + + unless params[:include_smokey_users] == "true" + smokey_uids = User.where("name LIKE 'Smokey%'").pluck(:uid) + relation = relation.where.not(uid: smokey_uids) + end + + @monthly_access_stats = relation + .pluck( + Arel.sql("DATE_FORMAT(created_at, '%Y-%m')"), + Arel.sql("COUNT(*)"), + Arel.sql("COUNT(DISTINCT uid)"), + ) + end + private def load_and_authorize_application @@ -44,6 +83,8 @@ def doorkeeper_application_params :home_uri, :supports_push_updates, :api_only, + :include_smokey_users, + :month, ) end end diff --git a/app/models/doorkeeper/application.rb b/app/models/doorkeeper/application.rb index 05625608f..d8dcae546 100644 --- a/app/models/doorkeeper/application.rb +++ b/app/models/doorkeeper/application.rb @@ -2,6 +2,7 @@ class Doorkeeper::Application < ActiveRecord::Base # rubocop:disable Rails/ApplicationRecord has_many :supported_permissions, dependent: :destroy + has_many :event_logs, class_name: "EventLog" default_scope { not_retired.ordered_by_name } diff --git a/app/models/event_log.rb b/app/models/event_log.rb index df7ff6f7b..6fe27f202 100644 --- a/app/models/event_log.rb +++ b/app/models/event_log.rb @@ -66,6 +66,7 @@ class EventLog < ApplicationRecord validates :application_id, presence: { if: proc { |event_log| EVENTS_REQUIRING_APPLICATION.include? event_log.entry } } belongs_to :initiator, class_name: "User" + belongs_to :user, class_name: "User", foreign_key: :uid, primary_key: :uid belongs_to :application, class_name: "Doorkeeper::Application" belongs_to :user_agent diff --git a/app/policies/application_policy.rb b/app/policies/application_policy.rb index d35350575..7ef76f899 100644 --- a/app/policies/application_policy.rb +++ b/app/policies/application_policy.rb @@ -6,4 +6,6 @@ def index? alias_method :update?, :index? alias_method :manage_supported_permissions?, :index? alias_method :users_with_access?, :index? + alias_method :access_logs?, :index? + alias_method :monthly_access_stats?, :index? end diff --git a/app/views/doorkeeper_applications/access_logs.html.erb b/app/views/doorkeeper_applications/access_logs.html.erb new file mode 100644 index 000000000..1cab66296 --- /dev/null +++ b/app/views/doorkeeper_applications/access_logs.html.erb @@ -0,0 +1,94 @@ +<% content_for :title, "#{@application.name} access log" %> + +<% content_for :breadcrumbs, + render("govuk_publishing_components/components/breadcrumbs", { + collapse_on_mobile: true, + breadcrumbs: [ + { + title: "Dashboard", + url: root_path, + }, + { + title: "Applications", + url: doorkeeper_applications_path, + }, + { + title: @application.name, + url: edit_doorkeeper_application_path(@application), + } + ] + }) +%> + +
+ +<%= render "govuk_publishing_components/components/details", { + title: "About this data" +} do %> ++ Signon records a "successful authorization" event whenever a user uses Signon to access one of the publishing + applications. This is a record of all of these events for <%= @application.name %>. +
++ Applications cache authentications for around 20 hours, so if a user clicks an application multiple + times a day, they may only appear in the event log once. +
++ <% if DateTime.current.before? DateTime.new(2025, 11, 1) # This branch can be removed after November 2025 %> + Note that authorization data has only been recorded in the Signon event log since November 2023, so it is not + possible to view events before that date. + <% else %> + Note that data in the event log in Signon is only retained for 2 years, so it is not possible to view events + before that date. + <% end %> +
+<% end %> + +<% if @logs.any? %> + <%= render "components/table", { + caption: pluralize(number_with_delimiter(@logs.total_count), "event"), + caption_classes: "govuk-heading-m", + head: [ + { text: "Time" }, + { text: "Event" }, + ], + rows: @logs.map do |log| + next if log.requires_admin? && !current_user.govuk_admin? + + [ + { text: formatted_date(log), format: "event-log-date" }, + { text: "#{formatted_message(log)} for #{link_to(log.user.name, log.user, class: "govuk-link")}".html_safe }, + ] + end.compact + } %> + + <%= paginate(@logs, theme: "gds") %> +<% else %> + <%= render "govuk_publishing_components/components/notice", { + title: "No activity logged" + } %> +<% end %> diff --git a/app/views/doorkeeper_applications/edit.html.erb b/app/views/doorkeeper_applications/edit.html.erb index 5e12a0cc2..5ad233244 100644 --- a/app/views/doorkeeper_applications/edit.html.erb +++ b/app/views/doorkeeper_applications/edit.html.erb @@ -23,6 +23,14 @@ doorkeeper_application_supported_permissions_path(@application), class: "govuk-link" %> +<%= link_to "View access log", + access_logs_doorkeeper_application_path(@application), + class: "govuk-link" %> + +<%= link_to "View monthly access stats", + monthly_access_stats_doorkeeper_application_path(@application), + class: "govuk-link" %> ++ Signon records a "successful authorization" event whenever a user uses Signon to access one of the publishing + applications. This is a monthly count of all of these events for <%= @application.name %>. +
++ Applications cache authentications for around 20 hours, so if a user clicks an application multiple + times a day, they may only appear in the event log once. +
++ The total authorization count is the total number of events recorded in the log for the month (if a + user accesses the same app multiple times, this number will increase). +
++ The unique users authorization count is the number of distinct users who recorded events in the log for the month + (if a user accesses the same app multiple times in a month, this will only count as one unique user). +
++ <% if DateTime.current.before? DateTime.new(2025, 11, 1) # This branch can be removed after November 2025 %> + Note that authorization data has only been recorded in the Signon event log since November 2023, so it is not + possible to view events before that date. + <% else %> + Note that data in the event log in Signon is only retained for 2 years, so it is not possible to view events + before that date. + <% end %> +
+<% end %> +<% if @monthly_access_stats.any? %> + <%= render "components/table", { + head: [ + { text: "Month" }, + { text: "Total authorization count" }, + { text: "Unique users authorization count" }, + { text: "Access logs" }, + ], + rows: @monthly_access_stats.map do |month, total_count, unique_users_count| + [ + { text: month }, + { text: total_count }, + { text: unique_users_count }, + { text: link_to("#{month} access logs", access_logs_doorkeeper_application_path(@application, month:), class: "govuk-link")}, + ] + end + } %> +<% else %> + <%= render "govuk_publishing_components/components/notice", { + title: "No activity logged" + } %> +<% end %> diff --git a/config/routes.rb b/config/routes.rb index e35bdbec7..5331e19b7 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -92,6 +92,8 @@ resources :doorkeeper_applications, only: %i[index edit update] do member do get :users_with_access + get :access_logs + get :monthly_access_stats end resources :supported_permissions, only: %i[index new create edit update destroy] do get :confirm_destroy, on: :member diff --git a/db/migrate/20240809112119_index_event_logs_columns.rb b/db/migrate/20240809112119_index_event_logs_columns.rb new file mode 100644 index 000000000..815dc9071 --- /dev/null +++ b/db/migrate/20240809112119_index_event_logs_columns.rb @@ -0,0 +1,8 @@ +class IndexEventLogsColumns < ActiveRecord::Migration[7.1] + def change + change_table :event_logs, bulk: true do + add_index :event_logs, :application_id + add_index :event_logs, :event_id + end + end +end diff --git a/db/schema.rb b/db/schema.rb index f8c307a13..7b7f06301 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[7.1].define(version: 2024_04_29_130154) do +ActiveRecord::Schema[7.1].define(version: 2024_08_09_112119) do create_table "batch_invitation_application_permissions", id: :integer, charset: "utf8mb3", force: :cascade do |t| t.integer "batch_invitation_id", null: false t.integer "supported_permission_id", null: false @@ -51,11 +51,13 @@ t.integer "user_agent_id" t.text "user_agent_string" t.string "user_email_string" + t.index ["application_id"], name: "index_event_logs_on_application_id" + t.index ["event_id"], name: "index_event_logs_on_event_id" t.index ["uid", "created_at"], name: "index_event_logs_on_uid_and_created_at" t.index ["user_agent_id"], name: "event_logs_user_agent_id_fk" end - create_table "oauth_access_grants", id: :integer, charset: "utf8mb3", collation: "utf8_unicode_ci", force: :cascade do |t| + create_table "oauth_access_grants", id: :integer, charset: "utf8mb3", collation: "utf8mb3_unicode_ci", force: :cascade do |t| t.integer "resource_owner_id", null: false t.integer "application_id", null: false t.string "token", null: false @@ -69,7 +71,7 @@ t.index ["token"], name: "index_oauth_access_grants_on_token", unique: true end - create_table "oauth_access_tokens", id: :integer, charset: "utf8mb3", collation: "utf8_unicode_ci", force: :cascade do |t| + create_table "oauth_access_tokens", id: :integer, charset: "utf8mb3", collation: "utf8mb3_unicode_ci", force: :cascade do |t| t.integer "resource_owner_id", null: false t.integer "application_id", null: false t.string "token", null: false @@ -154,7 +156,7 @@ t.index ["user_id", "application_id", "supported_permission_id"], name: "index_app_permissions_on_user_and_app_and_supported_permission", unique: true end - create_table "users", id: :integer, charset: "utf8mb3", collation: "utf8_unicode_ci", force: :cascade do |t| + create_table "users", id: :integer, charset: "utf8mb3", collation: "utf8mb3_unicode_ci", force: :cascade do |t| t.string "name", null: false t.string "email", default: "", null: false t.string "encrypted_password", default: "" diff --git a/test/integration/application_access_log_page_test.rb b/test/integration/application_access_log_page_test.rb new file mode 100644 index 000000000..65c4b9b0d --- /dev/null +++ b/test/integration/application_access_log_page_test.rb @@ -0,0 +1,53 @@ +require "test_helper" + +class ApplicationAccessLogPageIntegrationTest < ActionDispatch::IntegrationTest + setup do + @application = create(:application, name: "app-name", description: "app-description") + @user = create(:user, name: "Normal User") + end + + test "users don't have permission to view account access log" do + visit root_path + signin_with(@user) + + visit access_logs_doorkeeper_application_path(@application) + flash = find("div[role='alert']") + assert flash.has_content?("You do not have permission to perform this action.") + end + + context "logged in as an superadmin" do + setup do + visit new_user_session_path + @superadmin = create(:superadmin_user) + signin_with(@superadmin) + end + + should "have permission to view account access log" do + visit access_logs_doorkeeper_application_path(@application) + assert_equal page.title, "app-name access log - GOV.UK Signon" + end + + context "when there are no matching events" do + should "see a message stating that there is no activity logged" do + visit access_logs_doorkeeper_application_path(@application) + assert_text "app-name access log" + assert_text "No activity logged" + end + end + + context "when there are matching events" do + setup do + event_id = 47 + create(:event_log, event_id:, application_id: @application.id, uid: @superadmin.uid) + create(:event_log, event_id:, application_id: @application.id, uid: @user.uid) + end + + should "see a list of events for the application" do + visit access_logs_doorkeeper_application_path(@application) + assert_text "#{@application.name} access log" + assert_text "Successful user application authorization for #{@application.name} for #{@superadmin.name}" + assert_text "Successful user application authorization for #{@application.name} for #{@user.name}" + end + end + end +end diff --git a/test/integration/application_monthly_access_stats_page_test.rb b/test/integration/application_monthly_access_stats_page_test.rb new file mode 100644 index 000000000..897685502 --- /dev/null +++ b/test/integration/application_monthly_access_stats_page_test.rb @@ -0,0 +1,61 @@ +require "test_helper" + +class ApplicationMonthlyAccessStatsPageIntegrationTest < ActionDispatch::IntegrationTest + setup do + @application = create(:application, name: "app-name", description: "app-description") + @user = create(:user, name: "Normal User") + end + + test "users don't have permission to view account access log" do + visit root_path + signin_with(@user) + + visit monthly_access_stats_doorkeeper_application_path(@application) + flash = find("div[role='alert']") + assert flash.has_content?("You do not have permission to perform this action.") + end + + context "logged in as an superadmin" do + setup do + visit new_user_session_path + @superadmin = create(:superadmin_user) + signin_with(@superadmin) + end + + should "have permission to view account access log" do + visit monthly_access_stats_doorkeeper_application_path(@application) + assert_equal page.title, "Monthly access counts to app-name - GOV.UK Signon" + end + + context "when there are no matching events" do + should "see a message stating that there is no activity logged" do + visit monthly_access_stats_doorkeeper_application_path(@application) + assert_text "Monthly access counts to app-name" + assert_text "No activity logged" + end + end + + context "when there are matching events" do + setup do + event_id = EventLog::SUCCESSFUL_USER_APPLICATION_AUTHORIZATION.id + create(:event_log, created_at: Date.new(2020, 1, 1), event_id:, application_id: @application.id, uid: @superadmin.uid) + create(:event_log, created_at: Date.new(2020, 1, 1), event_id:, application_id: @application.id, uid: @superadmin.uid) + create(:event_log, created_at: Date.new(2020, 1, 1), event_id:, application_id: @application.id, uid: @user.uid) + create(:event_log, created_at: Date.new(2020, 2, 1), event_id:, application_id: @application.id, uid: @superadmin.uid) + create(:event_log, created_at: Date.new(2020, 2, 1), event_id:, application_id: @application.id, uid: @user.uid) + end + + should "see a list of events for the application" do + visit monthly_access_stats_doorkeeper_application_path(@application) + assert_text "Monthly access counts to #{@application.name}" + + assert_text "Month Total authorization count Unique users authorization count Access logs" + # Test data has two months - these should be sorted in descending order + # 2020-02 has two events for two different users, so we should get 2 2 + assert_text "2020-02 2 2 2020-02 access logs" + # 2020-01 has three events for two different users, so we should get 3 2 + assert_text "2020-01 3 2 2020-01 access logs" + end + end + end +end