Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[F] Add spam detection & public access mitigations #3722

Merged
merged 12 commits into from
Feb 23, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 9 additions & 0 deletions api/.rubocop.yml
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ Metrics/BlockLength:

Metrics/MethodLength:
Exclude:
- "app/controllers/concerns/validation.rb"
- "app/operations/**/*.rb"
Max: 15

Expand Down Expand Up @@ -56,10 +57,15 @@ Layout/EmptyLinesAroundAttributeAccessor:
Layout/SpaceAroundMethodCallOperator:
Enabled: true

Layout/EmptyLineAfterGuardClause:
Enabled: false

Layout/EmptyLinesAroundClassBody:
Enabled: false

Layout/LineLength:
Exclude:
- "app/services/setting_sections/*.rb"
Max: 140

Layout/BeginEndAlignment: # (new in 0.91)
Expand Down Expand Up @@ -164,6 +170,9 @@ Style/BisectedAttrAccessor:
Style/CaseLikeIf:
Enabled: true

Style/CharacterLiteral:
Enabled: false

Style/ExplicitBlockArgument:
Enabled: false

Expand Down
4 changes: 2 additions & 2 deletions api/Gemfile
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ gem "crass", "~> 1.0.5"
gem "csl-styles", "~> 1.0"
gem "cssbeautify"
gem "css_parser", "~> 1.0"
gem "dalli", "~> 3.2.3"
gem "dalli", "2.7.11"
gem "data_uri", "~> 0.1.0"
gem "dotenv-rails", "~> 2.0"
gem "draper", "~> 3.1"
Expand Down Expand Up @@ -115,7 +115,7 @@ gem "signet", "~> 0.10"
gem "sinatra", "~>2.2"
gem "statesman", "~> 3.4"
gem "statesman-events", "~> 0.0.1"
gem "store_model", "~> 0.7.0"
gem "store_model", "~> 2.2.0"
gem "strip_attributes", "~> 1.13.0"
gem "terrapin", "~> 0.6.0"
gem "tus-server", "~> 2.0"
Expand Down
8 changes: 4 additions & 4 deletions api/Gemfile.lock
Original file line number Diff line number Diff line change
Expand Up @@ -152,7 +152,7 @@ GEM
css_parser (1.14.0)
addressable
cssbeautify (0.2.0)
dalli (3.2.6)
dalli (2.7.11)
data_uri (0.1.0)
database_cleaner-active_record (2.1.0)
activerecord (>= 5.a)
Expand Down Expand Up @@ -732,7 +732,7 @@ GEM
statesman (3.5.0)
statesman-events (0.0.1)
statesman (>= 1.3)
store_model (0.7.0)
store_model (2.2.0)
activerecord (>= 5.2)
strip_attributes (1.13.0)
activemodel (>= 3.0, < 8.0)
Expand Down Expand Up @@ -832,7 +832,7 @@ DEPENDENCIES
csl-styles (~> 1.0)
css_parser (~> 1.0)
cssbeautify
dalli (~> 3.2.3)
dalli (= 2.7.11)
data_uri (~> 0.1.0)
database_cleaner-active_record (~> 2.1.0)
database_cleaner-redis (~> 2.0)
Expand Down Expand Up @@ -942,7 +942,7 @@ DEPENDENCIES
spring-watcher-listen (~> 2.1.0)
statesman (~> 3.4)
statesman-events (~> 0.0.1)
store_model (~> 0.7.0)
store_model (~> 2.2.0)
strip_attributes (~> 1.13.0)
terrapin (~> 0.6.0)
test-prof (~> 1.0)
Expand Down
24 changes: 19 additions & 5 deletions api/app/authorizers/annotation_authorizer.rb
Original file line number Diff line number Diff line change
@@ -1,9 +1,8 @@
class AnnotationAuthorizer < ApplicationAuthorizer
# frozen_string_literal: true

# There are cases where all users can CRUD annotations.
def self.default(_able, _user, _options = {})
true
end
# @see Annotation
class AnnotationAuthorizer < ApplicationAuthorizer
requires_trusted_or_established_user!

# rubocop:disable Metrics/CyclomaticComplexity, Metrics/PerceivedComplexity
def creatable_by?(user, _options = {})
Expand Down Expand Up @@ -46,6 +45,15 @@ def readable_by?(user, _options = {})

private

def trusted_or_established_user?(user)
user&.created?(resource) || super
end

# Only public annotations need reputation to create.
def requires_reputation_to_create?
annotation_is_public?
end

def user_can_notate_text?(user)
resource&.text&.notatable_by? user
end
Expand Down Expand Up @@ -74,4 +82,10 @@ def user_is_not_in_reading_group?(user)
resource.reading_group.users.exclude? user
end

class << self
# There are cases where all users can CRUD annotations.
def default(_able, _user, _options = {})
true
end
end
end
55 changes: 55 additions & 0 deletions api/app/authorizers/application_authorizer.rb
Original file line number Diff line number Diff line change
@@ -1,4 +1,8 @@
# frozen_string_literal: true

# Other authorizers should subclass this one
#
# @abstract
class ApplicationAuthorizer < Authority::Authorizer
include ActiveSupport::Configurable
include SerializableAuthorization
Expand Down Expand Up @@ -32,6 +36,13 @@ def known_user?(user)
user.role.present?
end

# @param [User] user
def trusted_or_established_user?(user)
return false if user.blank?

user.established? || user.trusted?
end

# @param [User] user
# @param [#projects] resource
def resource_belongs_to_updatable_project?(user, resource)
Expand Down Expand Up @@ -196,5 +207,49 @@ def has_role?(user, role, on: :any)
user.has_cached_role? role, actual_on
end

# Authorizers that should override their create check
# in order to ensure that a user has been trusted or
# established in the instance in order to create the
# associated resource.
#
# @return [void]
def requires_trusted_or_established_user!
include RequiresTrustedOrEstablishedUser
end
end

# @api private
module RequiresTrustedOrEstablishedUser
extend ActiveSupport::Concern

included do
prepend RequiresTrustedOrEstablishedUser::WrapperMethods
end

# Override this in authorizers where only certain models
# require a reputation in order to create them.
#
# For instance, public reading groups.
# @abstract
def requires_reputation_to_create?
true
end

# Boolean complement of {#requires_reputation_to_create?},
# defined for legibility.
def requires_no_reputation_to_create?
!requires_reputation_to_create?
end

# This module gets prepended during inclusion
#
# @api private
module WrapperMethods
def creatable_by?(user, options = {})
return false unless requires_no_reputation_to_create? || trusted_or_established_user?(user)

super
end
end
end
end
17 changes: 13 additions & 4 deletions api/app/authorizers/comment_authorizer.rb
Original file line number Diff line number Diff line change
@@ -1,11 +1,11 @@
# frozen_string_literal: true

# @see Comment
class CommentAuthorizer < ApplicationAuthorizer
requires_trusted_or_established_user!

expose_abilities [:read_deleted]

def self.default(_able, _user, _options = {})
true
end

def creatable_by?(user, options = {})
return comment_is_on_readable_annotation_for?(user) if comment_is_on_annotation?

Expand Down Expand Up @@ -44,4 +44,13 @@ def comment_is_on_annotation?
resource.on_annotation?
end

def trusted_or_established_user?(user)
user&.created?(resource) || super
end

class << self
def default(_able, _user, _options = {})
true
end
end
end
10 changes: 10 additions & 0 deletions api/app/authorizers/reading_group_authorizer.rb
Original file line number Diff line number Diff line change
@@ -1,4 +1,9 @@
# frozen_string_literal: true

# @see ReadingGroup
class ReadingGroupAuthorizer < ApplicationAuthorizer
requires_trusted_or_established_user!

def creatable_by?(user, _options = {})
return false unless known_user?(user)
return false if reading_groups_disabled?
Expand All @@ -24,6 +29,11 @@ def readable_by?(user, _options = {})

private

# Only public reading groups need reputation to create.
def requires_reputation_to_create?
resource.public?
end

def moderator?(user)
user.has_role?(:moderator, resource)
end
Expand Down
9 changes: 5 additions & 4 deletions api/app/controllers/concerns/authentication.rb
Original file line number Diff line number Diff line change
Expand Up @@ -14,16 +14,17 @@ module Authentication

CURRENT_USER_PRELOADS = %w(roles favorites).freeze

# @param [User, nil]
attr_reader :current_user

private

def load_current_user
@current_user = User.preload(CURRENT_USER_PRELOADS).find(decoded_auth_token[:user_id])
rescue JWT::DecodeError
nil
end

def current_user
@current_user
else
RequestStore[:current_user] = @current_user
end

# This method gets the current user based on the user_id included
Expand Down
Loading
Loading