diff --git a/app/controllers/application_controller.rb b/app/controllers/application_controller.rb index 754f5ed..5520089 100644 --- a/app/controllers/application_controller.rb +++ b/app/controllers/application_controller.rb @@ -28,4 +28,10 @@ class ApplicationController < ActionController::Base def after_sign_out_path_for(resource_or_scope) root_path end + + # Write a message into the audit log in the database + def auditlog(message, level: 'info', screenshot: nil, package: nil) + Log.log(message, ip: request.remote_ip, user: current_user, + screenshot: screenshot, package: package) + end end diff --git a/app/controllers/packages_controller.rb b/app/controllers/packages_controller.rb index 0a8942b..785595b 100644 --- a/app/controllers/packages_controller.rb +++ b/app/controllers/packages_controller.rb @@ -56,7 +56,6 @@ class PackagesController < ApplicationController files=[files] if files.class != Array files.each do |img| - # Log.log "Uploaded img=#{img.path}" new_screenshot = @package.screenshots.new(simage: img) # Check if the image was valid @@ -70,7 +69,8 @@ class PackagesController < ApplicationController # Check that this screenshot is not a duplicate for this package if @package.screenshots.where(image_fingerprint: new_screenshot.image_fingerprint).any? - Log.log "Duplicate image with fingerprint #{new_screenshot.image_fingerprint} found. Rejecting." + auditlog "Duplicate image with fingerprint #{new_screenshot.image_fingerprint} found. Rejecting.", + package: @package all_errors << "Your file #{img.original_filename} is a duplicate. Sorry." else # TODO: Can the upload get approved automatically? @@ -79,28 +79,26 @@ class PackagesController < ApplicationController # The user won't know that an account is created. # But this makes it easier to track who screenshots belong to. unless user_signed_in? - sign_in User.create_pseudo_user + sign_in User.create_pseudo_user + auditlog "New pseudo user for anonymous upload created and logged in.", + package: @package, screenshot: new_screenshot end new_screenshot.user = current_user new_screenshot.approve! if can?(:approve, new_screenshot) # Pre-render screenshot in different sizes new_screenshot.simage_derivatives! - Log.log "Derivatives created" new_screenshot.save! - Log.log "Screenshot #{new_screenshot.id} uploaded successfully. " + \ - "ip=#{new_screenshot.uploaderip}. "+ \ - #"user-hash=#{new_screenshot.uploaderhash}. "+ \ - "user-name=#{current_user} "+ \ - "image-fingerprint=#{new_screenshot.image_fingerprint} "+ \ - "image-path=#{img.path}" + auditlog "Screenshot uploaded successfully.", + screenshot: new_screenshot, package: @package @valid_images.push new_screenshot end else errors = new_screenshot.errors[:simage] - Log.log "Screenshot #{img.original_filename} invalid (#{errors})." + auditlog "Screenshot #{img.original_filename} invalid (#{errors}).", + package: @package #@invalid_images.push img.original_filename #raise all_errors << "Your file #{img.original_filename} #{errors.join(' and ')}." @@ -113,7 +111,7 @@ class PackagesController < ApplicationController # # Redirect back to upload form if all uploads were invalid # unless @valid_images.any? - # Log.log "No valid images uploaded. Back to upload form." + # auditlog "No valid images uploaded. Back to upload form." # redirect_to(upload_path, error: all_errors) and return # end @@ -162,10 +160,12 @@ class PackagesController < ApplicationController new_screenshot.uploaderip = session[:ip] new_screenshot.version = @package.version new_screenshot.save - Log.log "Screenshot #{new_screenshot.id} uploaded successfully from legacy upload form." + auditlog "Screenshot #{new_screenshot.id} uploaded successfully from legacy upload form.", + screenshot: new_screenshot, package: @package redirect_to package_path(params[:packagename]) else - Log.log "Screenshot upload rejected. Errors: #{new_screenshot.errors.to_a}" + auditlog "Screenshot upload rejected. Errors: #{new_screenshot.errors.to_a}", + package: @package head :not_acceptable end end @@ -177,7 +177,8 @@ class PackagesController < ApplicationController # Check if the user is allowed to change this screenshot # - Is this the user's own screenshot? (anonymous) if (@screenshot.user == current_user) || (can? :destroy, @screenshot) - logger.debug "User #{current_user} deletes screenshot #{@screenshot}" + auditlog "Screenshot #{@screenshot} deleted", + package: @screenshot.package @screenshot.destroy flash['notice'] = "Screenshot deleted." redirect_back(fallback_location: package_path(name: @screenshot.package.name)) @@ -191,6 +192,8 @@ class PackagesController < ApplicationController if can? :approve, @screenshot @screenshot.approve! + auditlog "Screenshot approved", + package: @screenshot.package, screenshot: @screenshot flash['notice'] = "Screenshot approved." redirect_back(fallback_location: package_path(name: @screenshot.package.name)) else diff --git a/app/controllers/users/my_sessions_controller.rb b/app/controllers/users/my_sessions_controller.rb index 63a6a48..432742b 100644 --- a/app/controllers/users/my_sessions_controller.rb +++ b/app/controllers/users/my_sessions_controller.rb @@ -3,19 +3,6 @@ class Users::MySessionsController < Devise::SessionsController # Hook after successful login def after_login - Log.log "User #{current_user.email} logged in.", request.remote_ip - old_screenshots = session[:uploaded_screenshots] - if old_screenshots.to_a.length > 0 - old_screenshots.each do |id| - next unless id # sometimes we had 'nil' here - ss = Screenshot.find(id) - ss.user = @user - ss.save! - end - # The message is probably confusing. Just add the screenshots. - # flash[:info] = "#{old_screenshots.to_a.length} uploads have been added to your account" - Log.log "Anonymously uploaded screenshots #{old_screenshots} added to #{current_user.email}.", request.remote_ip - session[:uploaded_screenshots] = nil - end + auditlog "User logged in." end end diff --git a/app/models/log.rb b/app/models/log.rb index 2e70374..892a17c 100644 --- a/app/models/log.rb +++ b/app/models/log.rb @@ -1,6 +1,9 @@ # require 'pry' class Log < ApplicationRecord + belongs_to :user + belongs_to :package + belongs_to :screenshot validates :message, presence: true validates :section, presence: true @@ -13,7 +16,7 @@ class Log < ApplicationRecord order('id DESC') } - def self.log(message, ip_address='0.0.0.0', section='frontend', level='info') + def self.log(message, ip: nil, user: nil, section: 'frontend', level: 'info', screenshot: nil, package: nil) log = Log.new ip_address = request.remote_ip if defined? request @@ -22,8 +25,10 @@ class Log < ApplicationRecord log.message = message log.section = section log.level = level - log.ip_address = ip_address - log.token = token + log.ip_address = ip.to_s + log.screenshot = screenshot + log.package = package + log.user = user # current_user logger.error "Could not log to database - validation errors: #{log.errors.to_a}" unless log.valid? log.save # log diff --git a/app/models/user.rb b/app/models/user.rb index a2475c5..65a0b08 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -1,5 +1,5 @@ class User < ApplicationRecord - has_many :screenshots, :inverse_of=>:user + has_many :screenshots, inverse_of: :user # Include default devise modules. Others available are: # :confirmable, :lockable, :timeoutable and :omniauthable @@ -78,7 +78,14 @@ class User < ApplicationRecord password: generated_password, pseudo: true ) - Log.log "New pseudo user for anonymous upload created: #{new_user}" return new_user end + + def to_s + if self.pseudo + "Pseudo user" + else + "#{self.email} (#{self.provider})" + end + end end diff --git a/app/views/my/logs.slim b/app/views/my/logs.slim index 94419f0..56c4d0b 100644 --- a/app/views/my/logs.slim +++ b/app/views/my/logs.slim @@ -14,16 +14,34 @@ table thead tr - th Message - th Level - th Section - th IP + th Message + th Level + th Section + th Package + th Screenshot + th User + th IP tbody - @logs.each do |log| tr - td #{log.message } - td #{log.level } - td #{log.section } + td #{log.message} + td #{log.level} + td #{log.section} + td + - if log.package + = log.package.name + - else + ' - + td + - if log.screenshot + = log.screenshot.id + - else + ' - + td + - if log.user + = log.user.to_s + - else + ' - td #{log.ip_address} - else diff --git a/db/migrate/20210303170421_add_user_field_to_log.rb b/db/migrate/20210303170421_add_user_field_to_log.rb new file mode 100644 index 0000000..7a9a46d --- /dev/null +++ b/db/migrate/20210303170421_add_user_field_to_log.rb @@ -0,0 +1,5 @@ +class AddUserFieldToLog < ActiveRecord::Migration[6.1] + def change + add_column :logs, :user, :string, default: nil + end +end diff --git a/db/migrate/20210303170557_remove_token_field_from_log.rb b/db/migrate/20210303170557_remove_token_field_from_log.rb new file mode 100644 index 0000000..9a6d372 --- /dev/null +++ b/db/migrate/20210303170557_remove_token_field_from_log.rb @@ -0,0 +1,5 @@ +class RemoveTokenFieldFromLog < ActiveRecord::Migration[6.1] + def change + remove_column :logs, :token + end +end diff --git a/db/migrate/20210303203723_change_type_of_user_field_in_logs.rb b/db/migrate/20210303203723_change_type_of_user_field_in_logs.rb new file mode 100644 index 0000000..9f120bc --- /dev/null +++ b/db/migrate/20210303203723_change_type_of_user_field_in_logs.rb @@ -0,0 +1,6 @@ +class ChangeTypeOfUserFieldInLogs < ActiveRecord::Migration[6.1] + def change + remove_column :logs, :user + add_column :logs, :user_id, :integer + end +end diff --git a/db/migrate/20210303213303_add_screenshot_and_package_field_to_logs.rb b/db/migrate/20210303213303_add_screenshot_and_package_field_to_logs.rb new file mode 100644 index 0000000..12b60b5 --- /dev/null +++ b/db/migrate/20210303213303_add_screenshot_and_package_field_to_logs.rb @@ -0,0 +1,6 @@ +class AddScreenshotAndPackageFieldToLogs < ActiveRecord::Migration[6.1] + def change + add_column :logs, :package_id, :integer + add_column :logs, :screenshot_id, :integer + end +end diff --git a/db/schema.rb b/db/schema.rb index e9c0a10..6ee9265 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: 2021_02_28_191717) do +ActiveRecord::Schema.define(version: 2021_03_03_213303) do # These are extensions that must be enabled in order to support this database enable_extension "plpgsql" @@ -59,10 +59,12 @@ ActiveRecord::Schema.define(version: 2021_02_28_191717) do t.string "message" t.string "level" t.string "section" - t.string "token" t.inet "ip_address" t.datetime "created_at", null: false t.datetime "updated_at", null: false + t.integer "user_id" + t.integer "package_id" + t.integer "screenshot_id" end create_table "packages", id: :serial, force: :cascade do |t| diff --git a/doc/README.DeveloperNotes.md b/doc/README.DeveloperNotes.md index 7e0e3c7..d9de953 100644 --- a/doc/README.DeveloperNotes.md +++ b/doc/README.DeveloperNotes.md @@ -53,9 +53,9 @@ How Debian repositories are organized: https://wiki.debian.org/RepositoryFormat Logging from within the application into the database's "log" table: -Log.log "foo" +auditlog "foo" or with section (default='frontend'): -Log.log "foo", "importer" +Log.log "foo", nil, nil, "importer" ---