Setting abilities based on user.current_team introduces bugs for multiple browser windows opened on different teams

[pascallaliberte]

Scenario to reproduce

• Setup a super_admin? predicate method on Team, checking an ENV SUPER_ADMIN_TEAM_ID, e.g. team id 1
• In ability.rb, add a block if user.current_team&.super_admin? with a special can :manage, ExampleResource definition just for super_admin teams
• Create non-super_admin team for the user
• Create a non-super_admin-only resource
• Make sure both super_admin-only resource and the other resource are on teams/show, with the super_admin-only resource wrapped in an <% if can? :read, ExampleResource.new(team: @team) %>
• Open two browser windows (could be different browsers, it doesn't check for the user's session for current_team), and in each browsers, visit a different team dashboard/show page.

Expected result

Reloading each page should show only the resources boxes allowed

Actual result

Reloading each page shows the wrong resource boxes, as if the <% if can? :read... %> blocks don't work properly

Possible source of the bug

I think the source of the bug is that on each request, Ability.new(user) is called before current_team is set to the team of the page's context. The can? helper will make its determination based on the current_team stored in the latest page request, and not the current page request.

https://github.com/bullet-train-co/bullet_train-core/blob/397cb8111b4af97afe979be0dba982c3250ee9a1/bullet_train/app/models/concerns/current_attributes/base.rb#L12-L24

[pascallaliberte]

There's probably a way to not rely on current_team in ability.rb at all that I should know about, or a way that relies on current_team evaluation at run-time, like smarter role definitions or something.

[jagthedrummer]

I think we ultimately need to move away from setting/capturing current_team in any way. Instead we'll need to include a teams/:id segment in any routes that need a team but for which we can't determine one based on the object ownership graph.