From 2966828d5617699d14b61da477c8943e4c69c884 Mon Sep 17 00:00:00 2001 From: Michael Walker Date: Thu, 22 Oct 2020 09:23:42 +0100 Subject: [PATCH 1/2] Record DeanonymiseToken uses as "data activities" Currently we use access grants for showing when data has been shared, but an access grant only shows when a token was granted, not when it was used. Tokens can be used long after they were created, and re-used if they get refreshed. So it's better to show token use than token creation. --- .../api/v1/deanonymise_token_controller.rb | 7 +++++++ app/models/data_activity.rb | 6 ++++++ app/models/user.rb | 3 +++ db/migrate/20201022082056_create_data_activity.rb | 15 +++++++++++++++ db/schema.rb | 15 ++++++++++++++- 5 files changed, 45 insertions(+), 1 deletion(-) create mode 100644 app/models/data_activity.rb create mode 100644 db/migrate/20201022082056_create_data_activity.rb diff --git a/app/controllers/api/v1/deanonymise_token_controller.rb b/app/controllers/api/v1/deanonymise_token_controller.rb index e8757d3a..2e4a5b0f 100644 --- a/app/controllers/api/v1/deanonymise_token_controller.rb +++ b/app/controllers/api/v1/deanonymise_token_controller.rb @@ -13,6 +13,13 @@ def show elsif token.expired? head 410 else + DataActivity.create!( + user_id: token.resource_owner_id, + oauth_application_id: token.application_id, + token: token.token, + scopes: token.scopes.to_a.join(" "), + ) + render json: { true_subject_identifier: token.resource_owner_id, pairwise_subject_identifier: Doorkeeper::OpenidConnect::UserInfo.new(token).claims[:sub], diff --git a/app/models/data_activity.rb b/app/models/data_activity.rb new file mode 100644 index 00000000..fcc67c29 --- /dev/null +++ b/app/models/data_activity.rb @@ -0,0 +1,6 @@ +class DataActivity < ApplicationRecord + belongs_to :user + + belongs_to :oauth_application, + class_name: "Doorkeeper::Application" +end diff --git a/app/models/user.rb b/app/models/user.rb index 577d3b05..f1714f3a 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -28,6 +28,9 @@ def send_devise_notification(notification, *args) foreign_key: :resource_owner_id, dependent: :destroy + has_many :data_activities, + dependent: :destroy + has_many :security_activities, dependent: :destroy diff --git a/db/migrate/20201022082056_create_data_activity.rb b/db/migrate/20201022082056_create_data_activity.rb new file mode 100644 index 00000000..e5dc21ac --- /dev/null +++ b/db/migrate/20201022082056_create_data_activity.rb @@ -0,0 +1,15 @@ +class CreateDataActivity < ActiveRecord::Migration[6.0] + def change + create_table :data_activities do |t| + t.references :user, null: false + t.references :oauth_application, null: false + t.string :token, null: false + t.string :scopes, null: false + + t.timestamps null: false + end + + add_foreign_key :data_activities, :users, column: :user_id + add_foreign_key :data_activities, :oauth_applications, column: :oauth_application_id + end +end diff --git a/db/schema.rb b/db/schema.rb index ef914373..20486214 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: 2020_10_22_075518) do +ActiveRecord::Schema.define(version: 2020_10_22_082056) do # These are extensions that must be enabled in order to support this database enable_extension "pgcrypto" @@ -23,6 +23,17 @@ t.index ["application_uid"], name: "index_application_keys_on_application_uid" end + create_table "data_activities", force: :cascade do |t| + t.bigint "user_id", null: false + t.bigint "oauth_application_id", null: false + t.string "token", null: false + t.string "scopes", null: false + t.datetime "created_at", precision: 6, null: false + t.datetime "updated_at", precision: 6, null: false + t.index ["oauth_application_id"], name: "index_data_activities_on_oauth_application_id" + t.index ["user_id"], name: "index_data_activities_on_user_id" + end + create_table "email_subscriptions", force: :cascade do |t| t.bigint "user_id", null: false t.string "topic_slug", null: false @@ -151,6 +162,8 @@ t.index ["unlock_token"], name: "index_users_on_unlock_token", unique: true end + add_foreign_key "data_activities", "oauth_applications" + add_foreign_key "data_activities", "users" add_foreign_key "email_subscriptions", "users" add_foreign_key "login_states", "users" add_foreign_key "oauth_access_grants", "oauth_applications", column: "application_id" From 071b7502dac2a2fe36080a497590c39f639cb58b Mon Sep 17 00:00:00 2001 From: Michael Walker Date: Thu, 22 Oct 2020 09:45:30 +0100 Subject: [PATCH 2/2] Display DataActivities on the security page --- app/controllers/security_controller.rb | 12 ++++++------ spec/factories/activities.rb | 3 +++ spec/requests/data_exchange_spec.rb | 16 ++++++++++++---- 3 files changed, 21 insertions(+), 10 deletions(-) diff --git a/app/controllers/security_controller.rb b/app/controllers/security_controller.rb index a4617627..9260d047 100644 --- a/app/controllers/security_controller.rb +++ b/app/controllers/security_controller.rb @@ -4,21 +4,21 @@ class SecurityController < ApplicationController def show @activity = current_user.security_activities.order(created_at: :desc) @data_exchanges = current_user - .access_grants + .data_activities .order(created_at: :desc) - .map { |g| grant_to_exchange(g) } + .map { |a| activity_to_exchange(a) } .compact end private - def grant_to_exchange(grant) - scopes = grant.scopes.map(&:to_sym) - ScopeDefinition.new.hidden_scopes + def activity_to_exchange(activity) + scopes = activity.scopes.split(" ").map(&:to_sym) - ScopeDefinition.new.hidden_scopes return if scopes.empty? { - application_name: grant.application.name, - created_at: grant.created_at, + application_name: activity.oauth_application.name, + created_at: activity.created_at, scopes: scopes, } end diff --git a/spec/factories/activities.rb b/spec/factories/activities.rb index 0597c9fd..c23e2b4e 100644 --- a/spec/factories/activities.rb +++ b/spec/factories/activities.rb @@ -5,4 +5,7 @@ client_id { "MyString" } ip_address { "MyString" } end + + factory :data_activity do + end end diff --git a/spec/requests/data_exchange_spec.rb b/spec/requests/data_exchange_spec.rb index 5085448b..2c3bd43f 100644 --- a/spec/requests/data_exchange_spec.rb +++ b/spec/requests/data_exchange_spec.rb @@ -12,15 +12,23 @@ ) end - let!(:access_grant) do + let(:token) do FactoryBot.create( - :oauth_access_grant, + :oauth_access_token, resource_owner_id: user.id, application_id: application.id, + scopes: application.scopes, + ) + end + + let!(:activity) do + FactoryBot.create( + :data_activity, + user_id: user.id, + oauth_application_id: application.id, created_at: Time.zone.now, scopes: "openid email transition_checker", - redirect_uri: "https://www.gov.uk", - expires_in: 600, + token: token.token, ) end