Skip to content
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

Fix RBAC permissions without subject ignoring conditions #10291

Merged

Conversation

Convly
Copy link
Member

@Convly Convly commented May 11, 2021

What does it do?

Fix a bug that causes the RBAC permissions that don't have any subject (for example plugins::upload.read) to ignore the attributed conditions.

Why is it needed?

Breaking RBAC (media library & other plugins)

How to test it?

  • Open the getstarted app
  • As a super admin upload an image to the media library
  • As a super admin, give the Plugins/Upload/Read media library permission to the author role (if not already set + the is-creator condition should be checked)
  • As an author, go to the media library, you shouldn't be able to see the uploaded image

Related issue(s)/PR(s)

fix #10221

@codecov
Copy link

codecov bot commented May 11, 2021

Codecov Report

Merging #10291 (8b605e4) into master (43b947b) will increase coverage by 0.01%.
The diff coverage is 100.00%.

❗ Current head 8b605e4 differs from pull request most recent head bf04393. Consider uploading reports for the commit bf04393 to get more accurate results
Impacted file tree graph

@@            Coverage Diff             @@
##           master   #10291      +/-   ##
==========================================
+ Coverage   57.82%   57.84%   +0.01%     
==========================================
  Files         184      184              
  Lines        6393     6395       +2     
  Branches     1395     1396       +1     
==========================================
+ Hits         3697     3699       +2     
  Misses       2233     2233              
  Partials      463      463              
Flag Coverage Δ
front ∅ <ø> (∅)
unit 57.84% <100.00%> (+0.01%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
...ackages/strapi-admin/services/permission/engine.js 96.38% <100.00%> (+0.08%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 43b947b...bf04393. Read the comment docs.

derrickmehaffy
derrickmehaffy previously approved these changes May 11, 2021
Copy link
Member

@derrickmehaffy derrickmehaffy left a comment

Choose a reason for hiding this comment

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

LGTM, works perfect in testing.

@Convly Convly force-pushed the fix/permission-without-subject-ignoring-conditions branch from 08b9bef to bfe580b Compare May 11, 2021 15:10
@Convly Convly force-pushed the fix/permission-without-subject-ignoring-conditions branch from bfe580b to 7fe9797 Compare May 12, 2021 08:48
alexandrebodin
alexandrebodin previously approved these changes May 12, 2021
Copy link
Member

@alexandrebodin alexandrebodin left a comment

Choose a reason for hiding this comment

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

LGTM

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
issue: bug Issue reporting a bug source: core:strapi Source is core/strapi package
Projects
None yet
Development

Successfully merging this pull request may close these issues.

RBAC - Cannot hide media assets in Upload plugin using RBAC conditionals
3 participants