Skip to content

fix: predicate functions should respect action grouping (#50)#51

Merged
vincentvanbush merged 1 commit intomasterfrom
50-bug-predicate-functions-should-respect-action-grouping
Dec 22, 2025
Merged

fix: predicate functions should respect action grouping (#50)#51
vincentvanbush merged 1 commit intomasterfrom
50-bug-predicate-functions-should-respect-action-grouping

Conversation

@vincentvanbush
Copy link
Collaborator

Pull Request

Description

Alows authorization module to answer true if permission is granted for :show when action grouping contains show: [:read] and :read is given explicitly.

Type of Change

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • Documentation update
  • Performance improvement
  • Code refactoring
  • Test improvements
  • CI/CD improvements

Related Issues

Fixes #50

Changes Made

  • Add authorization_module argument to Permit.can? function to allow traversing the action grouping map, so that with an a: [:b, :c] grouping and :b: and :c: granted explicilty, permission to :a is also resolved as true.

Testing

Added test examples that ensure that with a: [:b, :c] grouping, :b and :c permissions coexisting make :a resolve to true, while :b alone doesn't, and permission to :a doesn't imply :b and :c.

Test Environment

  • Elixir version: 1.19
  • OTP version: 28

Test Cases

  • All existing tests pass
  • New tests added for new functionality at appropriate levels
  • Manual testing performed

Documentation

  • Updated README.md (if applicable)
  • Updated documentation comments (with examples for new features)
  • Updated CHANGELOG.md (if applicable)

Code Quality

  • Code follows the existing style conventions
  • Self-review of the code has been performed
  • Code has been commented, particularly in hard-to-understand areas
  • No new linting warnings introduced
  • No new Dialyzer warnings introduced

Backward Compatibility

  • This change is backward compatible
  • This change includes breaking changes (please describe below)
  • Migration guide provided for breaking changes

Breaking Changes

If the app relied on the prior behaviour, it won't be kept compatible. That is, most commonly, in Phoenix the grouping would be given as show: [:read], which means that the :show action requires the :read permission. Permit.Phoenix accounts for this, but up until now Permit itself didn't.

Performance Impact

  • No performance impact
  • Performance improvement
  • Potential performance regression (please describe)

Performance Notes

Security Considerations

  • No security impact
  • Security improvement
  • Potential security impact (please describe)

Additional Notes

Screenshots/Examples

See added tests

Checklist

  • I have read the Contributing Guidelines
  • I have performed a self-review of my code
  • I have commented my code, particularly in hard-to-understand areas
  • I have made corresponding changes to the documentation
  • My changes generate no new warnings
  • I have added tests that prove my fix is effective or that my feature works
  • New and existing unit tests pass locally with my changes
  • Any dependent changes have been merged and published

Reviewer Notes


@vincentvanbush vincentvanbush linked an issue Dec 19, 2025 that may be closed by this pull request
3 tasks
@codecov
Copy link

codecov bot commented Dec 19, 2025

Codecov Report

❌ Patch coverage is 80.00000% with 2 lines in your changes missing coverage. Please review.

Files with missing lines Patch % Lines
lib/permit.ex 80.00% 2 Missing ⚠️
Files with missing lines Coverage Δ
lib/permit.ex 87.50% <80.00%> (-12.50%) ⬇️
🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

When an action depends on other permissions (e.g., show: [:read]),
the authorization check should verify if the required permissions are granted.
"""
use ExUnit.Case, async: true
Copy link

Choose a reason for hiding this comment

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

🔥

@vincentvanbush vincentvanbush merged commit 3ff710d into master Dec 22, 2025
16 checks passed
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.

[Bug]: Predicate functions should respect action grouping

2 participants