diff options
| -rw-r--r-- | app/controllers/projects/git_http_controller.rb | 6 | ||||
| -rw-r--r-- | app/controllers/sessions_controller.rb | 5 | ||||
| -rw-r--r-- | app/models/user.rb | 2 | ||||
| -rw-r--r-- | app/models/user_activity.rb | 19 | ||||
| -rw-r--r-- | app/services/event_create_service.rb | 2 | ||||
| -rw-r--r-- | app/services/users/activity_service.rb | 26 | ||||
| -rw-r--r-- | db/migrate/20161007073613_create_user_activities.rb | 27 | ||||
| -rw-r--r-- | db/schema.rb | 8 | ||||
| -rw-r--r-- | lib/api/internal.rb | 12 | ||||
| -rw-r--r-- | spec/controllers/sessions_controller_spec.rb | 6 | ||||
| -rw-r--r-- | spec/factories/user_activities.rb | 6 | ||||
| -rw-r--r-- | spec/requests/api/internal_spec.rb | 13 | ||||
| -rw-r--r-- | spec/requests/git_http_spec.rb | 6 | ||||
| -rw-r--r-- | spec/services/event_create_service_spec.rb | 13 | ||||
| -rw-r--r-- | spec/services/users/activity_service_spec.rb | 26 |
15 files changed, 177 insertions, 0 deletions
diff --git a/app/controllers/projects/git_http_controller.rb b/app/controllers/projects/git_http_controller.rb index 37f6f637ff0..10adddb4636 100644 --- a/app/controllers/projects/git_http_controller.rb +++ b/app/controllers/projects/git_http_controller.rb @@ -5,6 +5,8 @@ class Projects::GitHttpController < Projects::GitHttpClientController # GET /foo/bar.git/info/refs?service=git-receive-pack (git push) def info_refs if upload_pack? && upload_pack_allowed? + log_user_activity + render_ok elsif receive_pack? && receive_pack_allowed? render_ok @@ -106,4 +108,8 @@ class Projects::GitHttpController < Projects::GitHttpClientController def access_klass @access_klass ||= wiki? ? Gitlab::GitAccessWiki : Gitlab::GitAccess end + + def log_user_activity + Users::ActivityService.new(user, 'pull').execute + end end diff --git a/app/controllers/sessions_controller.rb b/app/controllers/sessions_controller.rb index d3091a4f8e9..8c6ba4915cd 100644 --- a/app/controllers/sessions_controller.rb +++ b/app/controllers/sessions_controller.rb @@ -35,6 +35,7 @@ class SessionsController < Devise::SessionsController # hide the signed-in notification flash[:notice] = nil log_audit_event(current_user, with: authentication_method) + log_user_activity(current_user) end end @@ -123,6 +124,10 @@ class SessionsController < Devise::SessionsController for_authentication.security_event end + def log_user_activity(user) + Users::ActivityService.new(user, 'login').execute + end + def load_recaptcha Gitlab::Recaptcha.load_configurations! end diff --git a/app/models/user.rb b/app/models/user.rb index 457ba05fb04..1dde5c89699 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -101,6 +101,7 @@ class User < ActiveRecord::Base has_many :assigned_issues, dependent: :nullify, foreign_key: :assignee_id, class_name: "Issue" has_many :assigned_merge_requests, dependent: :nullify, foreign_key: :assignee_id, class_name: "MergeRequest" + has_one :user_activity, dependent: :destroy # Issues that a user owns are expected to be moved to the "ghost" user before # the user is destroyed. If the user owns any issues during deletion, this @@ -158,6 +159,7 @@ class User < ActiveRecord::Base alias_attribute :private_token, :authentication_token delegate :path, to: :namespace, allow_nil: true, prefix: true + delegate :last_activity_at, to: :user_activity, allow_nil: true state_machine :state, initial: :active do event :block do diff --git a/app/models/user_activity.rb b/app/models/user_activity.rb new file mode 100644 index 00000000000..c5fdbff0feb --- /dev/null +++ b/app/models/user_activity.rb @@ -0,0 +1,19 @@ +class UserActivity < ActiveRecord::Base + belongs_to :user, inverse_of: :user_activity + + validates :user, uniqueness: true, presence: true + validates :last_activity_at, presence: true + + # Updated version of http://apidock.com/rails/ActiveRecord/Timestamp/touch + # That accepts a new record. + def touch + current_time = current_time_from_proper_timezone + + if persisted? + update_column(:last_activity_at, current_time) + else + self.last_activity_at = current_time + save!(validate: false) + end + end +end diff --git a/app/services/event_create_service.rb b/app/services/event_create_service.rb index e24cc66e0fe..0f3a485a3fd 100644 --- a/app/services/event_create_service.rb +++ b/app/services/event_create_service.rb @@ -72,6 +72,8 @@ class EventCreateService def push(project, current_user, push_data) create_event(project, current_user, Event::PUSHED, data: push_data) + + Users::ActivityService.new(current_user, 'push').execute end private diff --git a/app/services/users/activity_service.rb b/app/services/users/activity_service.rb new file mode 100644 index 00000000000..b81f947cd01 --- /dev/null +++ b/app/services/users/activity_service.rb @@ -0,0 +1,26 @@ +module Users + class ActivityService + def initialize(author, activity) + @author = author.respond_to?(:user) ? author.user : author + @activity = activity + end + + def execute + return unless @author && @author.is_a?(User) + + record_activity + end + + private + + def record_activity + user_activity.touch + + Rails.logger.debug("Recorded activity: #{@activity} for User ID: #{@author.id} (username: #{@author.username}") + end + + def user_activity + UserActivity.find_or_initialize_by(user: @author) + end + end +end diff --git a/db/migrate/20161007073613_create_user_activities.rb b/db/migrate/20161007073613_create_user_activities.rb new file mode 100644 index 00000000000..4239cebd8f6 --- /dev/null +++ b/db/migrate/20161007073613_create_user_activities.rb @@ -0,0 +1,27 @@ +class CreateUserActivities < ActiveRecord::Migration + # Set this constant to true if this migration requires downtime. + DOWNTIME = true + + # When a migration requires downtime you **must** uncomment the following + # constant and define a short and easy to understand explanation as to why the + # migration requires downtime. + DOWNTIME_REASON = 'Adding foreign key' + + # When using the methods "add_concurrent_index" or "add_column_with_default" + # you must disable the use of transactions as these methods can not run in an + # existing transaction. When using "add_concurrent_index" make sure that this + # method is the _only_ method called in the migration, any other changes + # should go in a separate migration. This ensures that upon failure _only_ the + # index creation fails and can be retried or reverted easily. + # + # To disable transactions uncomment the following line and remove these + # comments: + # disable_ddl_transaction! + + def change + create_table :user_activities do |t| + t.belongs_to :user, index: { unique: true }, foreign_key: { on_delete: :cascade } + t.datetime :last_activity_at, null: false + end + end +end diff --git a/db/schema.rb b/db/schema.rb index 1c592dd5d6d..3ed28a02de2 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -1230,6 +1230,13 @@ ActiveRecord::Schema.define(version: 20170408033905) do add_index "uploads", ["model_id", "model_type"], name: "index_uploads_on_model_id_and_model_type", using: :btree add_index "uploads", ["path"], name: "index_uploads_on_path", using: :btree + create_table "user_activities", force: :cascade do |t| + t.integer "user_id" + t.datetime "last_activity_at", null: false + end + + add_index "user_activities", ["user_id"], name: "index_user_activities_on_user_id", unique: true, using: :btree + create_table "user_agent_details", force: :cascade do |t| t.string "user_agent", null: false t.string "ip_address", null: false @@ -1389,4 +1396,5 @@ ActiveRecord::Schema.define(version: 20170408033905) do add_foreign_key "timelogs", "merge_requests", name: "fk_timelogs_merge_requests_merge_request_id", on_delete: :cascade add_foreign_key "trending_projects", "projects", on_delete: :cascade add_foreign_key "u2f_registrations", "users" + add_foreign_key "user_activities", "users", on_delete: :cascade end diff --git a/lib/api/internal.rb b/lib/api/internal.rb index 215bc03d0e9..d374d183dcd 100644 --- a/lib/api/internal.rb +++ b/lib/api/internal.rb @@ -15,6 +15,16 @@ module API # project - project path with namespace # action - git action (git-upload-pack or git-receive-pack) # changes - changes as "oldrev newrev ref", see Gitlab::ChangesList + helpers do + def log_user_activity(actor) + commands = Gitlab::GitAccess::DOWNLOAD_COMMANDS + + Gitlab::GitAccess::PUSH_COMMANDS + + Gitlab::GitAccess::GIT_ANNEX_COMMANDS + + ::Users::ActivityService.new(actor, 'Git SSH').execute if commands.include?(params[:action]) + end + end + post "/allowed" do status 200 @@ -40,6 +50,8 @@ module API response = { status: access_status.status, message: access_status.message } if access_status.status + log_user_activity(actor) + # Return the repository full path so that gitlab-shell has it when # handling ssh commands response[:repository_path] = diff --git a/spec/controllers/sessions_controller_spec.rb b/spec/controllers/sessions_controller_spec.rb index 9c16a7bc08b..3459f30ef42 100644 --- a/spec/controllers/sessions_controller_spec.rb +++ b/spec/controllers/sessions_controller_spec.rb @@ -37,6 +37,12 @@ describe SessionsController do subject.sign_out user end end + + it 'updates the user activity' do + expect do + post(:create, user: { login: user.username, password: user.password }) + end.to change { user.reload.last_activity_at }.from(nil) + end end end diff --git a/spec/factories/user_activities.rb b/spec/factories/user_activities.rb new file mode 100644 index 00000000000..32ad8c6a3b2 --- /dev/null +++ b/spec/factories/user_activities.rb @@ -0,0 +1,6 @@ +FactoryGirl.define do + factory :user_activity do + last_activity_at { Time.now } + user + end +end diff --git a/spec/requests/api/internal_spec.rb b/spec/requests/api/internal_spec.rb index 4be67df5a00..63f566da7a8 100644 --- a/spec/requests/api/internal_spec.rb +++ b/spec/requests/api/internal_spec.rb @@ -151,6 +151,11 @@ describe API::Internal, api: true do context "access granted" do before do project.team << [user, :developer] + Timecop.freeze + end + + after do + Timecop.return end context 'with env passed as a JSON' do @@ -176,6 +181,7 @@ describe API::Internal, api: true do expect(response).to have_http_status(200) expect(json_response["status"]).to be_truthy expect(json_response["repository_path"]).to eq(project.wiki.repository.path_to_repo) + expect(key.user.reload.last_activity_at.to_i).to eq(Time.now.to_i) end end @@ -186,6 +192,7 @@ describe API::Internal, api: true do expect(response).to have_http_status(200) expect(json_response["status"]).to be_truthy expect(json_response["repository_path"]).to eq(project.wiki.repository.path_to_repo) + expect(key.user.reload.last_activity_at.to_i).to eq(Time.now.to_i) end end @@ -196,6 +203,7 @@ describe API::Internal, api: true do expect(response).to have_http_status(200) expect(json_response["status"]).to be_truthy expect(json_response["repository_path"]).to eq(project.repository.path_to_repo) + expect(key.user.reload.last_activity_at.to_i).to eq(Time.now.to_i) end end @@ -206,6 +214,7 @@ describe API::Internal, api: true do expect(response).to have_http_status(200) expect(json_response["status"]).to be_truthy expect(json_response["repository_path"]).to eq(project.repository.path_to_repo) + expect(key.user.reload.last_activity_at.to_i).to eq(Time.now.to_i) end context 'project as /namespace/project' do @@ -241,6 +250,7 @@ describe API::Internal, api: true do expect(response).to have_http_status(200) expect(json_response["status"]).to be_falsey + expect(key.user.reload.last_activity_at).to be_nil end end @@ -250,6 +260,7 @@ describe API::Internal, api: true do expect(response).to have_http_status(200) expect(json_response["status"]).to be_falsey + expect(key.user.reload.last_activity_at).to be_nil end end end @@ -267,6 +278,7 @@ describe API::Internal, api: true do expect(response).to have_http_status(200) expect(json_response["status"]).to be_falsey + expect(key.user.reload.last_activity_at).to be_nil end end @@ -276,6 +288,7 @@ describe API::Internal, api: true do expect(response).to have_http_status(200) expect(json_response["status"]).to be_falsey + expect(key.user.reload.last_activity_at).to be_nil end end end diff --git a/spec/requests/git_http_spec.rb b/spec/requests/git_http_spec.rb index 006d6a6af1c..9f2857ce2e7 100644 --- a/spec/requests/git_http_spec.rb +++ b/spec/requests/git_http_spec.rb @@ -157,6 +157,12 @@ describe 'Git HTTP requests', lib: true do expect(response).to have_http_status(:ok) end end + + it 'updates the user last activity' do + download(path, env) do |response| + expect(user.reload.last_activity_at).not_to be_nil + end + end end context 'but only project members are allowed' do diff --git a/spec/services/event_create_service_spec.rb b/spec/services/event_create_service_spec.rb index f2c2009bcbf..54e5c0b236b 100644 --- a/spec/services/event_create_service_spec.rb +++ b/spec/services/event_create_service_spec.rb @@ -129,4 +129,17 @@ describe EventCreateService, services: true do it { expect { subject }.to change { Event.count }.from(0).to(1) } end end + + describe '#push' do + let(:project) { create(:empty_project) } + let(:user) { create(:user) } + + it 'creates a new event' do + expect { service.push(project, user, {}) }.to change { Event.count } + end + + it 'updates user last activity' do + expect { service.push(project, user, {}) }.to change { user.last_activity_at } + end + end end diff --git a/spec/services/users/activity_service_spec.rb b/spec/services/users/activity_service_spec.rb new file mode 100644 index 00000000000..68399118579 --- /dev/null +++ b/spec/services/users/activity_service_spec.rb @@ -0,0 +1,26 @@ +require 'spec_helper' + +describe Users::ActivityService, services: true do + let(:user) { create(:user) } + subject(:service) { described_class.new(user, 'type') } + + describe '#execute' do + context 'when last activity is nil' do + it 'sets the last activity timestamp' do + service.execute + + expect(user.last_activity_at).not_to be_nil + end + end + + context 'when activity_at is not nil' do + it 'updates the activity multiple times' do + activity = create(:user_activity, user: user) + + Timecop.travel(activity.last_activity_at + 1.minute) do + expect { service.execute }.to change { user.reload.last_activity_at } + end + end + end + end +end |