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

Separate API endpoints more strictly/use different varification method #1077

Open
pfefferle opened this issue Dec 13, 2024 · 4 comments
Open
Labels
[Pri] TBD [Type] Bug Something isn't working

Comments

@pfefferle
Copy link
Member

pfefferle commented Dec 13, 2024

The current codebase does not strictly separate the endpoints we need "locally", like for example the endpoints needed by the blocks, from the ActivityPub APIs.

[Update] An alternate solution #1078

This could cause some issues in the future, or at lest bring some more complexity, when it comes to signature verification for example. Request to the ActivityPub endpoints should be verified, local requests might require a session or are open in general.

This is already a problem when you turn on Authorized-Fetch!!!

Copy link

This issue could use some more labels, to help prioritize and categorize our work. Could you please add at least a [Type], a [Feature], and a [Pri] label?

pfefferle added a commit that referenced this issue Dec 13, 2024
This is a proposal to use the `permission_callback`, instead of a general hook, to verify signatures.

The advantage is, that it is easier to enable/disable verification for specific endpoints this way.

See #1077
@akirk
Copy link
Member

akirk commented Dec 13, 2024

In #1078 you show that technically it's not necessary, more of a question of adding the right permisson_callbacks. I don't disagree with separating it out but it might be just more efficient for now to be very dilligent about the callbacks.

@pfefferle pfefferle added [Pri] High [Type] Bug Something isn't working labels Dec 13, 2024
@pfefferle pfefferle changed the title Separate API endpoints more strictly Separate API endpoints more strictly/use different varification method Dec 13, 2024
@obenland
Copy link
Member

A couple of options come to mind for me. We could group Editor related endpoints in a subdirectory of /includes/rest and give them their own namespace. We could also have all endpoint classes extend the same class that has the signature verification as one of its methods, so they all inherit it.

@pfefferle
Copy link
Member Author

I think we can discuss other ways to optimize the structure, but #1078 should fix the issue for now.

pfefferle added a commit that referenced this issue Dec 17, 2024
* Use a more explicit signature verification

This is a proposal to use the `permission_callback`, instead of a general hook, to verify signatures.

The advantage is, that it is easier to enable/disable verification for specific endpoints this way.

See #1077

* phpcs fix

* fix test

* ignore this for now

* changelog

* keep the old error and change the function name to be more desciptive

props @jeherve

* add some phpdoc

props @jeherve

* verify actor endpoints

* no need to mention `activitypub` here

* rename functions

* Update includes/rest/class-server.php

Co-authored-by: Konstantin Obenland <[email protected]>

* Update includes/rest/class-server.php

Co-authored-by: Konstantin Obenland <[email protected]>

* messed up search/replace

* one last change

* add some checks to prevent PHP warnings

* add integration test

* fix phpcs

---------

Co-authored-by: Konstantin Obenland <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Pri] TBD [Type] Bug Something isn't working
Projects
None yet
Development

No branches or pull requests

3 participants