Skip to content
Open
Show file tree
Hide file tree
Changes from 11 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
4 changes: 0 additions & 4 deletions app/controllers/api/base_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -94,8 +94,4 @@ def render_bad_request(error = "bad request")
error = error.message if error.is_a?(Exception)
render json: { error: error.to_s }, status: :bad_request
end

def owner?
@api_key.owner.owns_gem?(@rubygem)
end
end
6 changes: 5 additions & 1 deletion app/controllers/api/v1/deletions_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ def validate_gem_and_version
if [email protected]?
render plain: response_with_mfa_warning(t(:this_rubygem_could_not_be_found)),
status: :not_found
elsif !@rubygem.owned_by?(@api_key.user)
elsif !can_delete_gem?(@api_key.user)
render_forbidden response_with_mfa_warning("You do not have permission to delete this gem.")
else
begin
Expand All @@ -45,4 +45,8 @@ def validate_gem_and_version
end
end
end

def can_delete_gem?(user)
GemPermissions.new(@rubygem, user).can_push?
end
end
55 changes: 55 additions & 0 deletions app/models/gem_permissions.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,55 @@
class GemPermissions
def initialize(rubygem, user)
@rubygem = rubygem
@user = user
end

def can_push?
return false if user.blank?

if rubygem.owned_by_organization?
org_member_in_role?(:maintainer) || user_in_role?(:maintainer)
else
user_in_role?(:maintainer)
end
end

def can_manage_owners?
role_granted?(:owner)
end

def can_perform_gem_admin?
role_granted?(:admin)
end

private

attr_reader :rubygem, :user

def role_granted?(minimum_role)
return false if @user.blank?

if rubygem.owned_by_organization?
org_member_in_role?(minimum_role)
else
user_in_role?(minimum_role)
end
end

def org_member_in_role?(minimum_role)
case minimum_role
when :maintainer
rubygem.organization.user_is_member?(user)
when :admin
rubygem.organization.administered_by?(user)
when :owner
rubygem.organization.owned_by?(user)
else
false
end
end

def user_in_role?(minimum_role)
rubygem.ownerships.user_with_minimum_role(user, minimum_role).exists?
end
Comment on lines +39 to +54
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it makes sense to leave these here for now, but I foresee a future where org- (and team-) based permissions are checked in a separate class.

end
2 changes: 1 addition & 1 deletion app/models/oidc/trusted_publisher/github_action.rb
Original file line number Diff line number Diff line change
Expand Up @@ -147,7 +147,7 @@ def repository = "#{repository_owner}/#{repository_name}"

def workflow_slug = ".github/workflows/#{workflow_filename}"

def owns_gem?(rubygem) = rubygem_trusted_publishers.exists?(rubygem: rubygem)
def can_push?(rubygem) = rubygem_trusted_publishers.exists?(rubygem: rubygem)

private

Expand Down
8 changes: 8 additions & 0 deletions app/models/organization.rb
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,14 @@ def user_is_member?(user)
memberships.exists?(user: user)
end

def owned_by?(user)
memberships.where(user: user).with_minimum_role(:owner).exists?
end

def administered_by?(user)
memberships.where(user: user).with_minimum_role(:admin).exists?
end

def self.find_by_handle(handle)
find_by("lower(handle) = lower(?)", handle)
end
Expand Down
4 changes: 2 additions & 2 deletions app/models/pusher.rb
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ def process
end

def authorize
(rubygem.pushable? && (api_key.user? || find_pending_trusted_publisher)) || owner.owns_gem?(rubygem) || notify_unauthorized
(rubygem.pushable? && (api_key.user? || find_pending_trusted_publisher)) || owner.can_push?(rubygem) || notify_unauthorized
end

def verify_gem_scope
Expand Down Expand Up @@ -260,7 +260,7 @@ def republish_notification(version)
notify("It appears that #{version.full_name} did not finish pushing.\n" \
"Please contact [email protected] for assistance if you pushed this gem more than a minute ago.", 409)
else
different_owner = "pushed by a previous owner of this gem " unless owner.owns_gem?(version.rubygem)
different_owner = "pushed by a previous owner of this gem " unless owner.can_push?(version.rubygem)
notify("A yanked version #{different_owner}already exists (#{version.full_name}).\n" \
"Repushing of gem versions is not allowed. Please use a new version and retry", 409)
end
Expand Down
12 changes: 0 additions & 12 deletions app/models/rubygem.rb
Original file line number Diff line number Diff line change
Expand Up @@ -185,18 +185,6 @@ def indexed_versions?
versions.indexed.any?
end

def owned_by?(user)
return false unless user
ownerships.exists?(user_id: user.id) || (owned_by_organization? && user_authorized_for_organization?(user))
end

def owned_by_with_role?(user, minimum_required_role)
return false if user.blank?
ownerships.user_with_minimum_role(user, minimum_required_role).exists?
rescue KeyError
false
end

def unconfirmed_ownerships
ownerships_including_unconfirmed.unconfirmed
end
Expand Down
4 changes: 2 additions & 2 deletions app/models/user.rb
Original file line number Diff line number Diff line change
Expand Up @@ -275,8 +275,8 @@ def blocked?
blocked_email.present?
end

def owns_gem?(rubygem)
rubygem.owned_by?(self)
def can_push?(rubygem)
GemPermissions.new(rubygem, self).can_push?
end

def acknowledge_policies!
Expand Down
12 changes: 0 additions & 12 deletions app/policies/application_policy.rb
Original file line number Diff line number Diff line change
Expand Up @@ -35,18 +35,6 @@ def current_user?(record_user)
user && user == record_user
end

def rubygem_owned_by?(user)
rubygem.owned_by?(user) ||
organization_member_with_role?(user, :maintainer) ||
deny(t(:forbidden))
end

def rubygem_owned_by_with_role?(user, minimum_required_role:, minimum_required_org_role: :owner)
organization_member_with_role?(user, minimum_required_org_role) ||
rubygem.owned_by_with_role?(user, minimum_required_role) ||
deny(t(:forbidden))
end

def organization_member_with_role?(user, minimum_role)
return false unless respond_to?(:organization) && organization.present?
organization.memberships.where(user: user).with_minimum_role(minimum_role).exists?
Expand Down
2 changes: 1 addition & 1 deletion app/policies/events/rubygem_event_policy.rb
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ class Scope < ApplicationPolicy::Scope
delegate :rubygem, to: :record

def show?
rubygem.owned_by?(user)
GemPermissions.new(rubygem, user).can_push?
end

def create?
Expand Down
12 changes: 9 additions & 3 deletions app/policies/oidc/rubygem_trusted_publisher_policy.rb
Original file line number Diff line number Diff line change
Expand Up @@ -5,14 +5,20 @@ class Scope < ApplicationPolicy::Scope
delegate :rubygem, to: :record

def show?
rubygem_owned_by_with_role?(user, minimum_required_role: :owner, minimum_required_org_role: :admin)
permissions.can_manage_owners? || deny
end

def create?
rubygem_owned_by_with_role?(user, minimum_required_role: :owner, minimum_required_org_role: :admin)
permissions.can_manage_owners? || deny
end

def destroy?
rubygem_owned_by_with_role?(user, minimum_required_role: :owner, minimum_required_org_role: :admin)
permissions.can_manage_owners? || deny
end

private

def permissions
GemPermissions.new(rubygem, user)
end
end
26 changes: 18 additions & 8 deletions app/policies/rubygem_policy.rb
Original file line number Diff line number Diff line change
Expand Up @@ -13,34 +13,44 @@ def create?
end

def configure_oidc?
rubygem_owned_by_with_role?(user, minimum_required_role: :owner, minimum_required_org_role: :admin)
gem_permissions.can_perform_gem_admin? || deny
end

def configure_trusted_publishers?
rubygem_owned_by_with_role?(user, minimum_required_role: :owner, minimum_required_org_role: :admin)
gem_permissions.can_perform_gem_admin? || deny
end

def show_events?
rubygem_owned_by?(user)
gem_permissions.can_push? || deny
end

def show_mfa_status?
gem_permissions.can_push? || deny
end

def show_unconfirmed_ownerships?
rubygem_owned_by_with_role?(user, minimum_required_role: :owner, minimum_required_org_role: :admin)
gem_permissions.can_perform_gem_admin? || deny
end

def add_owner?
rubygem_owned_by_with_role?(user, minimum_required_role: :owner)
gem_permissions.can_manage_owners? || deny
end

def update_owner?
rubygem_owned_by_with_role?(user, minimum_required_role: :owner)
gem_permissions.can_manage_owners? || deny
end

def remove_owner?
rubygem_owned_by_with_role?(user, minimum_required_role: :owner)
gem_permissions.can_manage_owners? || deny
end

def transfer_gem?
rubygem_owned_by_with_role?(user, minimum_required_role: :owner)
gem_permissions.can_manage_owners? || deny
end

private

def gem_permissions
GemPermissions.new(record, user)
end
end
2 changes: 1 addition & 1 deletion app/views/rubygems/_release_info.html.erb
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@
<%= links_to_owners(rubygem) %>
</div>

<% if rubygem.owned_by?(current_user) %>
<% if policy(rubygem).show_mfa_status? %>
<% if rubygem.owners.without_mfa.present? %>
<% if current_user.mfa_disabled? %>
<span class="gem__users__mfa-text mfa-warn">
Expand Down
4 changes: 2 additions & 2 deletions test/integration/push_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -189,7 +189,7 @@ class PushTest < ActionDispatch::IntegrationTest

rubygem = Rubygem.find_by!(name: "sandworm")

assert rubygem.owned_by?(@user)
assert @user.can_push?(rubygem)
assert rubygem.oidc_rubygem_trusted_publishers.exists?(trusted_publisher: pending_trusted_publisher.trusted_publisher)
end

Expand All @@ -213,7 +213,7 @@ class PushTest < ActionDispatch::IntegrationTest

rubygem = Rubygem.find_by!(name: "sandworm")

assert rubygem.owned_by?(@user)
assert @user.can_push?(rubygem)
assert rubygem.oidc_rubygem_trusted_publishers.exists?(trusted_publisher: pending_trusted_publisher.trusted_publisher)
end

Expand Down
Loading
Loading