Remove unused AP C2S endpoints #749

Merged

floatingghost merged 5 commits from who-wants-to-yeet-c2s-i-want-to-yeet-c2s into develop 2024-04-24 16:59:58 +00:00

Conversation 4 Commits 5 Files Changed 4 +8 -462

floatingghost commented 2024-04-19 10:26:18 +00:00

Owner

Copy Link

basically removes the POST endpoints for AP C2S - keeps the GET ones because they're sometimes used in S2S and you need it for the spec

basically removes the `POST` endpoints for AP C2S - keeps the `GET` ones because they're sometimes used in S2S and you need it for the spec

floatingghost added 3 commits 2024-04-19 10:26:19 +00:00

ddb8a5ef73 yeet AP C2S support ...

literally nothing uses C2S AP, and it's another route into core
systems which requires analysis and maintenance. A second API
is just extra surface for potentially bad things so let's take
it out back and obliterate it

2c7e5b2287 changelog entry

1ed975636b Keep READ endpoints, purge WRITE

Oneric commented 2024-04-19 16:14:09 +00:00

Member

Copy Link

Afaik C2S-exclusive API consists of

• parts from core spec:
  • POST .../outbox
  • GET users/:nick/inbox sorry, seems this can per spec be accessed even without C2S, provided it is filtered by perms (in either case GET /inbox, the shared inbox SHOULD be publicly readable regardless of C2S iiuc — but it appears this SHOULD isn’t implemented in the first place?)
• *oma extensions for use with C2S¹
  • GET /api/ap/whoami/ to find out one’s own AP id (and from the own actor representation read out additional endpoints)
  • POST /api/ap/upload_media for uploading media in C2S; since its repsonse doesn’t contain a MastoAPI media_id it’s useless for anything but C2S writes

Notably, GET .../outbox which was purged by the first and restored in the third commit is part of S2S.

The third commit also restored GET /users/:nick/inbox which afaik is C2S-exclusive, and didn’t restore the read-only (albeit non-standard) whoami. It’s useless without C2S, but technically at odds with the commit message.

With /api/ap/upload_media removed, we should laos stop filling out this field in our AP actor representation. Now, i would have said we probably should keep it in our litepub-0.1.jsonld context to not break processing of stored older representations, but turns out it was never added here anyway ^^`

A complete removal should probably also drop the :activitypub_client pipeline, but if you want to first test the waters here, leaving it for now probably makes sense to make a later revert easier in the unlikely case anyone actually relies on this incomplete C2S implementation (and steps up to take care of the necessary maintenance)

---

[1]: eventhough Pleroma added extensions for C2S, its C2S implementation never implemented all parts required by spec

Afaik C2S-exclusive API consists of - parts from core spec: - `POST .../outbox` - ~~`GET users/:nick/inbox`~~ *sorry, seems this can per spec be accessed even without C2S, provided it is filtered by perms* *(in either case `GET /inbox`, the shared inbox [SHOULD be publicly readable](https://www.w3.org/TR/activitypub/#sharedInbox) regardless of C2S iiuc — but it appears this SHOULD isn’t implemented in the first place?)* - \*oma extensions for use with C2S¹ - `GET /api/ap/whoami/` to find out one’s own AP id *(and from the own actor representation read out additional endpoints)* - `POST /api/ap/upload_media` for uploading media in C2S; since its repsonse doesn’t contain a MastoAPI `media_id` it’s useless for anything but C2S writes Notably, `GET .../outbox` which was purged by the first and restored in the third commit is part of S2S. The third commit ~~also restored `GET /users/:nick/inbox` which afaik is C2S-exclusive, and~~ didn’t restore the read-only (albeit non-standard) `whoami`. It’s useless without C2S, but technically at odds with the commit message. With `/api/ap/upload_media` removed, we should laos stop filling out this field in our AP actor representation. Now, i would have said we probably should keep it in our `litepub-0.1.jsonld` context to not break processing of stored older representations, but turns out it was never added here anyway ^^` A complete removal should probably also drop the `:activitypub_client` pipeline, but if you want to first test the waters here, leaving it for now probably makes sense to make a later revert easier in the unlikely case anyone actually relies on this incomplete C2S implementation *(and steps up to take care of the necessary maintenance)* --- <small> <strong>[1]</strong>: eventhough Pleroma added extensions for C2S, its C2S implementation never implemented all parts required by spec </small>

Oneric commented 2024-04-19 16:45:44 +00:00

Member

Copy Link

Correction: GET /user/inbox is mentioned in spec regardless of C2S, but must be filtered to account for who’s sending the request.

In practice we just return HTTP 403 with either {"error": "Invalid credentials"} when accessed with no or well, invalid credentials and "can't read inbox of user1 as user2" (note the lack of an enclosing JSON object) when trying to read someone else’s inbox.
Meaning, in practice it’s currently kinda useless for anything but checking the own home feed.

If we go ahead with a complete C2S removal, it might be safe to (later) just stub it out to always reply with HTTP 403.

Correction: `GET /user/inbox` is mentioned in spec regardless of C2S, but must be filtered to account for who’s sending the request. In practice we just return HTTP 403 with either `{"error": "Invalid credentials"}` when accessed with no or well, invalid credentials and `"can't read inbox of user1 as user2"` *(note the lack of an enclosing JSON object)* when trying to read someone else’s inbox. Meaning, in practice it’s currently kinda useless for anything but checking the own home feed. If we go ahead with a complete C2S removal, it might be safe to (later) just stub it out to always reply with HTTP 403.

floatingghost added 1 commit 2024-04-23 13:36:05 +00:00

3e199242b0 remove upload_media from AP representation

floatingghost added 1 commit 2024-04-23 13:37:12 +00:00

92168fa5a1 Merge remote-tracking branch 'origin/develop' into who-wants-to-yeet-c2s-i-want-to-yeet-c2s

floatingghost commented 2024-04-23 13:38:15 +00:00

Author

Owner

Copy Link

gonna run on IHBA to see if it breaks anything, but i think this is mostly finalized for an initial removal of c2s write endpoints to make sure nobody actually uses it

gonna run on IHBA to see if it breaks anything, but i think this is mostly finalized for an initial removal of c2s write endpoints to make sure nobody actually uses it

floatingghost commented 2024-04-24 16:59:53 +00:00

Author

Owner

Copy Link

does not break anything, no 404s noted in logs, we good

planned for next release to remove any extra bits and pieces and remove the vestigial home-timeline-reading if this change doesn't upset anyone

does not break anything, no 404s noted in logs, we good planned for next release to remove any extra bits and pieces and remove the vestigial home-timeline-reading if this change doesn't upset anyone

floatingghost merged commit 1e48a37545 into develop 2024-04-24 16:59:58 +00:00

floatingghost deleted branch who-wants-to-yeet-c2s-i-want-to-yeet-c2s 2024-04-24 16:59:59 +00:00

floatingghost referenced this issue from a commit 2024-04-24 17:00:00 +00:00

Merge pull request 'Remove unused AP C2S endpoints' (#749) from who-wants-to-yeet-c2s-i-want-to-yeet-c2s into develop

Sign in to join this conversation.

Reviewers

No reviewers

Labels

Clear labels   

approved, awaiting change

  

bug

Something is not working

  

configuration

This requires config changes

  

documentation

This is about human-readable docs

  

duplicate

This issue or pull request already exists

  

enhancement

New feature

  

extremely low priority

  

feature request

Something new?

  

Fix it yourself

Support has been attempted

  

help wanted

Need some help

  

invalid

Something is wrong

  

mastodon_api

  

needs docs

  

needs tests

  

not a bug

  

planned

  

pleroma_api

  

privacy

  

question

More information is needed

  

static_fe

  

triage

  

wontfix

This won't be fixed

No Label

approved, awaiting change

bug

configuration

documentation

duplicate

enhancement

extremely low priority

feature request

Fix it yourself

help wanted

invalid

mastodon_api

needs docs

needs tests

not a bug

planned

pleroma_api

privacy

question

static_fe

triage

wontfix

Milestone

Clear milestone

No items

No Milestone

Projects

Clear projects

No project

Assignees

Clear assignees

No Assignees

2 Participants

Notifications

Subscribe

Due Date

The due date is invalid or out of range. Please use the format 'yyyy-mm-dd'.

No due date set.

Dependencies

No dependencies set.

Reference: AkkomaGang/akkoma#749

No description provided.