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

When receiving an "Update" activity check permissions #893

Merged
merged 3 commits into from
Jul 25, 2024

Conversation

BentiGorlich
Copy link
Member

Right now we do not check any permissions when receiving an "Update" (=edit) activity.
That means at the moment any user can send an "Update" activity to any post and mbin would just apply that update, regardless of whether the user should be able to do so as long as the HTTP headers are valid.

This PR implements a permission like this: a user should be able to modify a post if:

  • they are from the same instance as the author (global mods and admins, and of course the author themselves)
  • or they are from the same instance as the magazine (magazine mods and admins)
  • or they are a moderator in the magazine the user posted to

@BentiGorlich BentiGorlich added bug Something isn't working activitypub ActivityPub related issues backend Backend related issues and pull requests security Issues and pull requests that address security concerns labels Jul 10, 2024
@BentiGorlich BentiGorlich self-assigned this Jul 10, 2024
@BentiGorlich
Copy link
Member Author

This is a draft right now, because I did not live-test this yet, only wrote the code. That looks good to me right now, but I still gotta test it

@BentiGorlich BentiGorlich added this to the v1.7.0 milestone Jul 16, 2024
@BentiGorlich BentiGorlich marked this pull request as ready for review July 25, 2024 16:01
@BentiGorlich
Copy link
Member Author

Everything works as expected

@BentiGorlich BentiGorlich enabled auto-merge (squash) July 25, 2024 20:51
@BentiGorlich BentiGorlich merged commit 1fbbef2 into main Jul 25, 2024
7 checks passed
@BentiGorlich BentiGorlich deleted the fix/permission-check-on-update branch July 25, 2024 20:52
BentiGorlich added a commit that referenced this pull request Aug 7, 2024
I noticed that editing an entry was not really working sometimes, the reason being (after testing locally) that it tries to parse a url from the users and magazines apId. This is not working, as the ap id looks like this: `normaluser@mbin.instance.tld`

That is not a url, so yeah that cannot work. Since this has been used to check whether a user is from the same instance as the content they try to edit, this check is failing. So right now only remote moderators can edit their content

Fixes Regression from #893
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
activitypub ActivityPub related issues backend Backend related issues and pull requests bug Something isn't working security Issues and pull requests that address security concerns
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant