hackclub / hackclub/auth

Unify Pundit user type across frontend and backend contexts

Open
#191 0 comments 0 reactions 0 assignees View on GitHub
Dominant language
Ruby
Stars
80
Forks
66
Avg merge
2d 59m
Merged PRs (30d)
6

Description

## Problem

We have two Pundit user types: `Identity` (frontend, via `IdentityAuthorizable`) and `Backend::User` (backend, via `Backend::ApplicationController`). Policies were written assuming they'd only ever see one type — and that was true when policies only ran inside controller actions, where the context is unambiguous.

Activity partials broke that assumption. Partials like `_collaborator_invited.html.erb` call `policy(activity.trackable).manage_collaborators?` to decide whether to show sensitive data (email addresses). These partials render in both frontend and backend views, so the policy can receive either user type. `ProgramPolicy#admin?` calls `user.backend_user`, which works on an `Identity` but raises `NoMethodError` on a `Backend::User` (because it *is* the backend user).

We fixed this in #188 with a `resolve_backend_user` helper that checks `user.is_a?(Backend::User)`. It works, but it's the kind of thing that'll bite again the next time someone writes `user.backend_user` in a policy method without thinking about cross-context rendering.

## Current workaround

```ruby
# in ProgramPolicy
def resolve_backend_user
user.is_a?(Backend::User) ? user : user.backend_user
end
```

Activity partials also guard with `activity.trackable && ... rescue false` for nil trackables (deleted programs) and contexts where Pundit isn't available at all (frontend `AuditLogsController`).

## Ideal end state

Every Pundit-evaluated context uses `Identity` as the user type. Backend controllers would set `pundit_user` to the `Identity` behind the `Backend::User`, and policies would check admin privileges through `identity.backend_user` uniformly. This is essentially making `IdentityAuthorizable`'s approach the default.

## What this would involve

- Backend controllers set `pundit_user` to `current_identity` instead of `current_user` (the `Backend::User`)
- Backend-specific policy checks (like `admin?`) continue to work because `Identity#backend_user` resolves the admin role
- Remove `resolve_backend_user` type-checking from `ProgramPolicy` once the user type is consistent
- Audit all policies for `user.backend_user` calls to make sure they work with the unified approach
- `AuditLogsController` (frontend) should probably include `Pundit::Authorization` with `pundit_user = current_identity` so `policy()` is available in its rendered partials

## Scope

This doesn't need to happen all at once. The workaround is solid and the comment explains why it exists. But it's worth doing before more policies end up rendering across both contexts.

Contributor guide

Open the contributing guide

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.