me/glitchier-soc
1
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity

Remove IP tracking columns from users table (#16409)

Browse source
This commit is contained in:
Eugen Rochko 2022-01-16 13:23:50 +01:00 committed by GitHub
parent b52fdb4c6f
commit 8e84ebf0cb
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
19 changed files with 141 additions and 75 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

2
app/controllers/api/v1/admin/accounts_controller.rb
View file

@ -94,7 +94,7 @@ class Api::V1::Admin::AccountsController < Api::BaseController
private
def set_accounts
@accounts = filtered_accounts.order(id: :desc).includes(user: [:invite_request, :invite]).to_a_paginated_by_id(limit_param(LIMIT), params_slice(:max_id, :since_id, :min_id))
@accounts = filtered_accounts.order(id: :desc).includes(user: [:invite_request, :invite, :ips]).to_a_paginated_by_id(limit_param(LIMIT), params_slice(:max_id, :since_id, :min_id))
end
def set_account

2
app/controllers/auth/sessions_controller.rb
View file

@ -147,7 +147,7 @@ class Auth::SessionsController < Devise::SessionsController
clear_attempt_from_session
user.update_sign_in!(request, new_sign_in: true)
user.update_sign_in!(new_sign_in: true)
sign_in(user)
flash.delete(:notice)

6
app/controllers/concerns/user_tracking_concern.rb
View file

@ -3,7 +3,7 @@
module UserTrackingConcern
extend ActiveSupport::Concern
UPDATE_SIGN_IN_HOURS = 24
UPDATE_SIGN_IN_FREQUENCY = 24.hours.freeze
included do
before_action :update_user_sign_in
@ -12,10 +12,10 @@ module UserTrackingConcern
private
def update_user_sign_in
current_user.update_sign_in!(request) if user_needs_sign_in_update?
current_user.update_sign_in! if user_needs_sign_in_update?
end
def user_needs_sign_in_update?
user_signed_in? && (current_user.current_sign_in_at.nil? || current_user.current_sign_in_at < UPDATE_SIGN_IN_HOURS.hours.ago)
user_signed_in? && (current_user.current_sign_in_at.nil? || current_user.current_sign_in_at < UPDATE_SIGN_IN_FREQUENCY.ago)
end
end

10
app/helpers/admin/dashboard_helper.rb
View file

@ -2,17 +2,17 @@
module Admin::DashboardHelper
def relevant_account_ip(account, ip_query)
default_ip = [account.user_current_sign_in_ip || account.user_sign_up_ip]
ips = account.user.ips.to_a
matched_ip = begin
ip_query_addr = IPAddr.new(ip_query)
account.user.recent_ips.find { |(_, ip)| ip_query_addr.include?(ip) } || default_ip
ips.find { |ip| ip_query_addr.include?(ip.ip) } || ips.first
rescue IPAddr::Error
default_ip
end.last
ips.first
end
if matched_ip
link_to matched_ip, admin_accounts_path(ip: matched_ip)
link_to matched_ip.ip, admin_accounts_path(ip: matched_ip.ip)
else
'-'
end

1
app/models/account.rb
View file

@ -123,7 +123,6 @@ class Account < ApplicationRecord
delegate :email,
:unconfirmed_email,
:current_sign_in_ip,
:current_sign_in_at,
:created_at,
:sign_up_ip,

2
app/models/account_filter.rb
View file

@ -21,7 +21,7 @@ class AccountFilter
end
def results
scope = Account.includes(:account_stat, user: [:session_activations, :invite_request]).without_instance_actor.reorder(nil)
scope = Account.includes(:account_stat, user: [:ips, :invite_request]).without_instance_actor.reorder(nil)
params.each do |key, value|
scope.merge!(scope_for(key, value.to_s.strip)) if value.present?

58
app/models/user.rb
View file

@ -14,8 +14,6 @@
# sign_in_count :integer default(0), not null
# current_sign_in_at :datetime
# last_sign_in_at :datetime
# current_sign_in_ip :inet
# last_sign_in_ip :inet
# admin :boolean default(FALSE), not null
# confirmation_token :string
# confirmed_at :datetime
@ -81,6 +79,7 @@ class User < ApplicationRecord
has_many :invites, inverse_of: :user
has_many :markers, inverse_of: :user, dependent: :destroy
has_many :webauthn_credentials, dependent: :destroy
has_many :ips, class_name: 'UserIp', inverse_of: :user
has_one :invite_request, class_name: 'UserInviteRequest', inverse_of: :user, dependent: :destroy
accepts_nested_attributes_for :invite_request, reject_if: ->(attributes) { attributes['text'].blank? && !Setting.require_invite_text }
@ -107,7 +106,7 @@ class User < ApplicationRecord
scope :inactive, -> { where(arel_table[:current_sign_in_at].lt(ACTIVE_DURATION.ago)) }
scope :active, -> { confirmed.where(arel_table[:current_sign_in_at].gteq(ACTIVE_DURATION.ago)).joins(:account).where(accounts: { suspended_at: nil }) }
scope :matches_email, ->(value) { where(arel_table[:email].matches("#{value}%")) }
scope :matches_ip, ->(value) { where('current_sign_in_ip <<= ?', value).or(where('users.sign_up_ip <<= ?', value)).or(where('users.last_sign_in_ip <<= ?', value)).or(where(id: SessionActivation.select(:user_id).where('ip <<= ?', value))) }
scope :matches_ip, ->(value) { left_joins(:ips).where('user_ips.ip <<= ?', value) }
scope :emailable, -> { confirmed.enabled.joins(:account).merge(Account.searchable) }
before_validation :sanitize_languages
@ -174,15 +173,11 @@ class User < ApplicationRecord
prepare_new_user! if new_user && approved?
end
def update_sign_in!(request, new_sign_in: false)
def update_sign_in!(new_sign_in: false)
old_current, new_current = current_sign_in_at, Time.now.utc
self.last_sign_in_at = old_current || new_current
self.current_sign_in_at = new_current
old_current, new_current = current_sign_in_ip, request.remote_ip
self.last_sign_in_ip = old_current || new_current
self.current_sign_in_ip = new_current
if new_sign_in
self.sign_in_count ||= 0
self.sign_in_count += 1
@ -201,7 +196,7 @@ class User < ApplicationRecord
end
def suspicious_sign_in?(ip)
!otp_required_for_login? && !skip_sign_in_token? && current_sign_in_at.present? && !recent_ip?(ip)
!otp_required_for_login? && !skip_sign_in_token? && current_sign_in_at.present? && !ips.where(ip: ip).exists?
end
def functional?
@ -277,31 +272,28 @@ class User < ApplicationRecord
@shows_application ||= settings.show_application
end
# rubocop:disable Naming/MethodParameterName
def token_for_app(a)
return nil if a.nil? || a.owner != self
Doorkeeper::AccessToken.find_or_create_by(application_id: a.id, resource_owner_id: id) do |t|
t.scopes = a.scopes
t.expires_in = Doorkeeper.configuration.access_token_expires_in
def token_for_app(app)
return nil if app.nil? || app.owner != self
Doorkeeper::AccessToken.find_or_create_by(application_id: app.id, resource_owner_id: id) do |t|
t.scopes = app.scopes
t.expires_in = Doorkeeper.configuration.access_token_expires_in
t.use_refresh_token = Doorkeeper.configuration.refresh_token_enabled?
end
end
# rubocop:enable Naming/MethodParameterName
def activate_session(request)
session_activations.activate(session_id: SecureRandom.hex,
user_agent: request.user_agent,
ip: request.remote_ip).session_id
session_activations.activate(
session_id: SecureRandom.hex,
user_agent: request.user_agent,
ip: request.remote_ip
).session_id
end
def clear_other_sessions(id)
session_activations.exclusive(id)
end
def session_active?(id)
session_activations.active? id
end
def web_push_subscription(session)
session.web_push_subscription.nil? ? nil : session.web_push_subscription
end
@ -364,22 +356,6 @@ class User < ApplicationRecord
setting_display_media == 'hide_all'
end
def recent_ips
@recent_ips ||= begin
arr = []
session_activations.each do |session_activation|
arr << [session_activation.updated_at, session_activation.ip]
end
arr << [current_sign_in_at, current_sign_in_ip] if current_sign_in_ip.present?
arr << [last_sign_in_at, last_sign_in_ip] if last_sign_in_ip.present?
arr << [created_at, sign_up_ip] if sign_up_ip.present?
arr.sort_by { |pair| pair.first || Time.now.utc }.uniq(&:last).reverse!
end
end
def sign_in_token_expired?
sign_in_token_sent_at.nil? || sign_in_token_sent_at < 5.minutes.ago
end
@ -410,10 +386,6 @@ class User < ApplicationRecord
private
def recent_ip?(ip)
recent_ips.any? { |(_, recent_ip)| recent_ip == ip }
end
def send_pending_devise_notifications
pending_devise_notifications.each do |notification, args, kwargs|
render_and_send_devise_message(notification, *args, **kwargs)

19
app/models/user_ip.rb Normal file
View file

@ -0,0 +1,19 @@
# frozen_string_literal: true
# == Schema Information
#
# Table name: user_ips
#
# user_id :bigint(8) primary key
# ip :inet
# used_at :datetime
#
class UserIp < ApplicationRecord
self.primary_key = :user_id
belongs_to :user, foreign_key: :user_id
def readonly?
true
end
end

13
app/serializers/rest/admin/account_serializer.rb
View file

@ -9,6 +9,7 @@ class REST::Admin::AccountSerializer < ActiveModel::Serializer
attribute :created_by_application_id, if: :created_by_application?
attribute :invited_by_account_id, if: :invited?
has_many :ips, serializer: REST::Admin::IpSerializer
has_one :account, serializer: REST::AccountSerializer
def id
@ -19,10 +20,6 @@ class REST::Admin::AccountSerializer < ActiveModel::Serializer
object.user_email
end
def ip
object.user_current_sign_in_ip.to_s.presence
end
def role
object.user_role
end
@ -74,4 +71,12 @@ class REST::Admin::AccountSerializer < ActiveModel::Serializer
def created_by_application?
object.user&.created_by_application_id&.present?
end
def ips
object.user&.ips
end
def ip
ips&.first
end
end

5
app/serializers/rest/admin/ip_serializer.rb Normal file
View file

@ -0,0 +1,5 @@
# frozen_string_literal: true
class REST::Admin::IpSerializer < ActiveModel::Serializer
attributes :ip, :used_at
end

10
app/views/admin/accounts/show.html.haml
View file

@ -156,12 +156,14 @@
%time.formatted{ datetime: @account.created_at.iso8601, title: l(@account.created_at) }= l @account.created_at
%td
- @account.user.recent_ips.each_with_index do |(_, ip), i|
- recent_ips = @account.user.ips.order(used_at: :desc).to_a
- recent_ips.each_with_index do |recent_ip, i|
%tr
- if i.zero?
%th{ rowspan: @account.user.recent_ips.size }= t('admin.accounts.most_recent_ip')
%td= ip
%td= table_link_to 'search', t('admin.accounts.search_same_ip'), admin_accounts_path(ip: ip)
%th{ rowspan: recent_ips.size }= t('admin.accounts.most_recent_ip')
%td= recent_ip.ip
%td= table_link_to 'search', t('admin.accounts.search_same_ip'), admin_accounts_path(ip: recent_ip.ip)
%tr
%th= t('admin.accounts.most_recent_activity')

2
app/views/admin_mailer/new_pending_account.text.erb
View file

@ -3,7 +3,7 @@
<%= raw t('admin_mailer.new_pending_account.body') %>
<%= @account.user_email %> (@<%= @account.username %>)
<%= @account.user_current_sign_in_ip %>
<%= @account.user_sign_up_ip %>
<% if @account.user&.invite_request&.text.present? %>
<%= quote_wrap(@account.user&.invite_request&.text) %>

2
app/workers/scheduler/ip_cleanup_scheduler.rb
View file

@ -16,7 +16,7 @@ class Scheduler::IpCleanupScheduler
def clean_ip_columns!
SessionActivation.where('updated_at < ?', IP_RETENTION_PERIOD.ago).in_batches.destroy_all
User.where('current_sign_in_at < ?', IP_RETENTION_PERIOD.ago).in_batches.update_all(last_sign_in_ip: nil, current_sign_in_ip: nil, sign_up_ip: nil)
User.where('current_sign_in_at < ?', IP_RETENTION_PERIOD.ago).in_batches.update_all(sign_up_ip: nil)
LoginActivity.where('created_at < ?', IP_RETENTION_PERIOD.ago).in_batches.destroy_all
end

15
config/initializers/devise.rb
View file

@ -1,11 +1,8 @@
require 'devise/strategies/authenticatable'
Warden::Manager.after_set_user except: :fetch do |user, warden|
if user.session_active?(warden.cookies.signed['_session_id'] || warden.raw_session['auth_id'])
session_id = warden.cookies.signed['_session_id'] || warden.raw_session['auth_id']
else
session_id = user.activate_session(warden.request)
end
session_id = warden.cookies.signed['_session_id'] || warden.raw_session['auth_id']
session_id = user.activate_session(warden.request) unless user.session_activations.active?(session_id)
warden.cookies.signed['_session_id'] = {
value: session_id,
@ -17,9 +14,13 @@ Warden::Manager.after_set_user except: :fetch do |user, warden|
end
Warden::Manager.after_fetch do |user, warden|
if user.session_active?(warden.cookies.signed['_session_id'] || warden.raw_session['auth_id'])
session_id = warden.cookies.signed['_session_id'] || warden.raw_session['auth_id']
if session_id && (session = user.session_activations.find_by(session_id: session_id))
session.update(ip: warden.request.remote_ip) if session.ip != warden.request.remote_ip
warden.cookies.signed['_session_id'] = {
value: warden.cookies.signed['_session_id'] || warden.raw_session['auth_id'],
value: session_id,
expires: 1.year.from_now,
httponly: true,
secure: (Rails.env.production? || ENV['LOCAL_HTTPS'] == 'true'),

5
db/migrate/20210616214526_create_user_ips.rb Normal file
View file

@ -0,0 +1,5 @@
class CreateUserIps < ActiveRecord::Migration[6.1]
def change
create_view :user_ips
end
end

12
db/post_migrate/20210616214135_remove_current_sign_in_ip_from_users.rb Normal file
View file

@ -0,0 +1,12 @@
# frozen_string_literal: true
class RemoveCurrentSignInIpFromUsers < ActiveRecord::Migration[5.2]
disable_ddl_transaction!
def change
safety_assured do
remove_column :users, :current_sign_in_ip, :inet
remove_column :users, :last_sign_in_ip, :inet
end
end
end

24
db/schema.rb
View file

@ -923,8 +923,6 @@ ActiveRecord::Schema.define(version: 2021_12_13_040746) do
t.integer "sign_in_count", default: 0, null: false
t.datetime "current_sign_in_at"
t.datetime "last_sign_in_at"
t.inet "current_sign_in_ip"
t.inet "last_sign_in_ip"
t.boolean "admin", default: false, null: false
t.string "confirmation_token"
t.datetime "confirmed_at"
@ -1120,6 +1118,28 @@ ActiveRecord::Schema.define(version: 2021_12_13_040746) do
SQL
add_index "instances", ["domain"], name: "index_instances_on_domain", unique: true
create_view "user_ips", sql_definition: <<-SQL
SELECT t0.user_id,
t0.ip,
max(t0.used_at) AS used_at
FROM ( SELECT users.id AS user_id,
users.sign_up_ip AS ip,
users.created_at AS used_at
FROM users
WHERE (users.sign_up_ip IS NOT NULL)
UNION ALL
SELECT session_activations.user_id,
session_activations.ip,
session_activations.updated_at
FROM session_activations
UNION ALL
SELECT login_activities.user_id,
login_activities.ip,
login_activities.created_at
FROM login_activities
WHERE (login_activities.success = true)) t0
GROUP BY t0.user_id, t0.ip;
SQL
create_view "account_summaries", materialized: true, sql_definition: <<-SQL
SELECT accounts.id AS account_id,
mode() WITHIN GROUP (ORDER BY t0.language) AS language,

26
db/views/user_ips_v01.sql Normal file
View file

@ -0,0 +1,26 @@
SELECT
user_id,
ip,
max(used_at) AS used_at
FROM (
SELECT
id AS user_id,
sign_up_ip AS ip,
created_at AS used_at
FROM users
WHERE sign_up_ip IS NOT NULL
UNION ALL
SELECT
user_id,
ip,
updated_at
FROM session_activations
UNION ALL
SELECT
user_id,
ip,
created_at
FROM login_activities
WHERE success = 't'
) AS t0
GROUP BY user_id, ip

2
spec/controllers/auth/sessions_controller_spec.rb
View file

@ -400,7 +400,7 @@ RSpec.describe Auth::SessionsController, type: :controller do
end
context 'when 2FA is disabled and IP is unfamiliar' do
let!(:user) { Fabricate(:user, email: 'x@y.com', password: 'abcdefgh', current_sign_in_at: 3.weeks.ago, current_sign_in_ip: '0.0.0.0') }
let!(:user) { Fabricate(:user, email: 'x@y.com', password: 'abcdefgh', current_sign_in_at: 3.weeks.ago) }
before do
request.remote_ip = '10.10.10.10'