Skip to content

Introduce User::PermissionsOverview and use it to render permissions to admins #11188

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

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

davidcornu
Copy link
Member

@davidcornu davidcornu commented Aug 1, 2025

Summary of the problem

  1. The introduction of sub-organizations (Allow organizations to create sub-organizations #9739) added an additional degree of complexity to our permissions system as we now need to reason about both the current event and its ancestors
    def self.role_at_least?(user, event, role)
    return false unless event.present? && role.present?
    return true if user&.admin?
    if role.to_s == "reader"
    return event.ancestor_organizer_positions.reader_access.where(user:).exists?
    end
    if role.to_s == "member"
    # Only check direct organizer positions, unless the user is a manager of an ancestor
    return event.organizer_positions.member_access.where(user:).exists? || event.ancestor_organizer_positions.manager_access.where(user:).exists?
    end
    if role.to_s == "manager"
    return event.ancestor_organizer_positions.manager_access.where(user:).exists?
    end
    false
    end
    using WITH RECURSIVE queries

    hcb/app/models/event.rb

    Lines 123 to 136 in da06bad

    def ancestor_ids
    [id] + Event.connection.execute(<<-SQL).map { |row| row["id"] }
    WITH RECURSIVE parent_events AS (
    SELECT id, parent_id
    FROM events
    WHERE id = #{id}
    UNION ALL
    SELECT e.id, e.parent_id
    FROM events e
    INNER JOIN parent_events pe ON e.id = pe.parent_id
    )
    SELECT id FROM parent_events WHERE id != #{id};
    SQL
    end
    to obtain the relevant OrganizerPosition records.
  2. We call OrganizerPosition.role_at_least? quite a bit, both directly and indirectly via helpers and Pundit policy objects. Given its current structure it isn't particularly amenable to cacheing (e.g. we run a different query for each role we're asking about and the information we retrieve doesn't tell us whether the user has greater access than that role).

Describe your changes

To improve things a little I'd like to add

  1. A User::PermissionsOverview class that has logic to retrieve the complete graph of events the user is associated with (both the event ancestors and descendants) and figure out what role they should have for each of them based on the existing OrganizerPosition.role_at_least? logic
  2. Tests for (1) that also compare the output with OrganizerPosition.role_at_least?
  3. A new admin-visible permissions section on the user page that renders this information
    CleanShot 2025-08-01 at 14 52 01@2x

For the time being I see this as a useful auditing and debugging tool, and it gives us a good starting point should we decide to cache all of a user's permissions.

@davidcornu davidcornu force-pushed the david/push-swlynprzxxqt branch from 3b1e64e to 01b2fb3 Compare August 1, 2025 18:56
@davidcornu davidcornu marked this pull request as ready for review August 1, 2025 18:57
@davidcornu davidcornu requested review from a team as code owners August 1, 2025 18:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant