Skip to content

Commit

Permalink
Avoid associating a nobody user with a project or package
Browse files Browse the repository at this point in the history
Strange way to define a custom validation, it raises an exception
instead of adding errors as usual because the error doesn't reach the
view due to the Relationship::AddRole@add_role handling.

Co-authored-by: David Kang <dkang@suse.com>
  • Loading branch information
saraycp and DavidKang committed Jun 17, 2019
1 parent f11509d commit 9dad7aa
Show file tree
Hide file tree
Showing 2 changed files with 17 additions and 0 deletions.
9 changes: 9 additions & 0 deletions src/api/app/models/relationship.rb
Expand Up @@ -36,6 +36,8 @@ class Relationship < ApplicationRecord
message: 'User and group can not exist at the same time'
}, if: proc { |relationship| relationship.group.present? }

validate :allowed_user

# don't use "is not null" - it won't be in index
scope :projects, -> { where.not(project_id: nil) }
scope :packages, -> { where.not(package_id: nil) }
Expand Down Expand Up @@ -126,6 +128,13 @@ def check_global_role
errors.add(:base,
"global role #{role.title} is not allowed.")
end

# NOTE: Adding a normal validation, the error doesn't reach the view due to
# Relationship::AddRole#add_role handling.
# We could also check other banned users, not only nobody.
def allowed_user
raise NotFoundError, "Couldn't find user #{user.login}" if user && user.is_nobody?
end
end

# == Schema Information
Expand Down
8 changes: 8 additions & 0 deletions src/api/spec/models/relationship_spec.rb
Expand Up @@ -43,6 +43,14 @@
skip('This is imposible to happen with the actual validations and how the object is created')
end

context 'with banned user' do
let(:nobody) { create(:user_nobody) }

subject { Relationship.add_user(project, nobody, role, true, true) }

it { expect { subject }.to raise_error(NotFoundError, "Couldn't find user #{nobody.login}") }
end

context 'with valid data' do
before do
subject
Expand Down

0 comments on commit 9dad7aa

Please sign in to comment.