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: Make requests with no identity work with "*" target #3278

Conversation

shahzadlone
Copy link
Member

Relevant issue(s)

Resolves #3276

Description

  • Fix the bug where a request without an identity still wouldn't be able to access a document even if there was a "*" relationship

Tasks

  • I made sure the code is well commented, particularly hard-to-understand areas.
  • I made sure the repository-held documentation is changed accordingly.
  • I made sure the pull request title adheres to the conventional commit style (the subset used in the project can be found in tools/configs/chglog/config.yml).
  • I made sure to discuss its limitations such as threats to validity, vulnerability to mistake and misuse, robustness to invalidation of assumptions, resource requirements, ...

How has this been tested?

  • Added tests

Specify the platform(s) on which this was tested:

  • WSL2 (Manjaro)

@shahzadlone shahzadlone added bug Something isn't working area/acp Related to the acp (access control) system labels Nov 27, 2024
@shahzadlone shahzadlone added this to the DefraDB v0.15 milestone Nov 27, 2024
@shahzadlone shahzadlone requested a review from a team November 27, 2024 22:13
@shahzadlone shahzadlone self-assigned this Nov 27, 2024
Copy link

codecov bot commented Nov 27, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 78.07%. Comparing base (89f9f41) to head (6a7d5b9).
Report is 1 commits behind head on develop.

Additional details and impacted files

Impacted file tree graph

@@             Coverage Diff             @@
##           develop    #3278      +/-   ##
===========================================
+ Coverage    77.97%   78.07%   +0.11%     
===========================================
  Files          382      382              
  Lines        35383    35390       +7     
===========================================
+ Hits         27587    27630      +43     
+ Misses        6149     6123      -26     
+ Partials      1647     1637      -10     
Flag Coverage Δ
all-tests 78.07% <100.00%> (+0.11%) ⬆️

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

Files with missing lines Coverage Δ
internal/db/permission/check.go 86.05% <100.00%> (+2.71%) ⬆️

... and 18 files with indirect coverage changes


Continue to review full report in Codecov by Sentry.

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

@shahzadlone shahzadlone force-pushed the lone/test/add-tests-for-non-identity-actors-after-any-actor-request branch 5 times, most recently from 1ceda7a to ef4df0a Compare November 27, 2024 23:17
Copy link
Collaborator

@fredcarle fredcarle left a comment

Choose a reason for hiding this comment

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

LGTM. Just one minor suggestion to clarify comment in the integration test.

@shahzadlone shahzadlone force-pushed the lone/test/add-tests-for-non-identity-actors-after-any-actor-request branch 2 times, most recently from aff3af2 to 6c01501 Compare December 5, 2024 22:07
@shahzadlone shahzadlone force-pushed the lone/test/add-tests-for-non-identity-actors-after-any-actor-request branch from 6c01501 to 6a7d5b9 Compare December 6, 2024 04:55
@shahzadlone shahzadlone merged commit b4a3eba into sourcenetwork:develop Dec 6, 2024
42 of 43 checks passed
@shahzadlone shahzadlone deleted the lone/test/add-tests-for-non-identity-actors-after-any-actor-request branch December 6, 2024 08:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/acp Related to the acp (access control) system bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

No identity after a "*" actor relationship still can't' access the document
3 participants