federation: include required actor fields in minimal user fallback
ActivityPub spec demands each actor has at least an inbox and outbox.
Furthermore, the current representation wouldn’t even be accepted by
ourselves, since our processing requires objects to be flagged with a
sensible type else we don't know what to do with it.
Including the nickname is just a peemptive measure.
There were no reports of this causing problems in real-world deployments
and at least for federation with other Akkoma instances we should have
never run into this, since we _always_ expose the full representation of
the instance actor and atm also always use the latter for fetching
remote content (which prevents us from fetching followers-only content).
Nonetheless, serving something which violates spec and we wouldn’t even
accept ourselves seems obviously bad, so fix it and add tests to prevent
this from reoccuring.
Fixes bug introduced in 8f322456a0
This commit is contained in:
parent
e8b8941e40
commit
f754feb67d
5 changed files with 60 additions and 4 deletions
|
|
@ -43,6 +43,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/).
|
|||
- fixed blocked servers being able to access local objects when authorized fetch isn’t enabled
|
||||
even when the remote server identifies itselfs
|
||||
- fixed handling of inlined "featured" collections
|
||||
- fixed user endpoint serving invalid ActivityPub for minimal, authfetch-fallback responses
|
||||
|
||||
### Changed
|
||||
- Internal and relay actors are now again represented with type "Application"
|
||||
|
|
|
|||
|
|
@ -62,7 +62,7 @@ defp relay_active?(conn, _) do
|
|||
Render the user's AP data
|
||||
WARNING: we cannot actually check if the request has a fragment! so let's play defensively
|
||||
- IF we have a valid signature, serve full user
|
||||
- IF we do not, and authorized_fetch_mode is enabled, serve the key only
|
||||
- IF we do not, and authorized_fetch_mode is enabled, serve only the key and bare minimum info
|
||||
- OTHERWISE, serve the full actor (since we don't need to worry about the signature)
|
||||
"""
|
||||
def user(%{assigns: %{valid_signature: true}} = conn, params) do
|
||||
|
|
@ -94,7 +94,7 @@ def render_key_only_user(conn, %{"nickname" => nickname}) do
|
|||
conn
|
||||
|> put_resp_content_type("application/activity+json")
|
||||
|> put_view(UserView)
|
||||
|> render("keys.json", %{user: user})
|
||||
|> render("stripped_user.json", %{user: user})
|
||||
else
|
||||
nil -> {:error, :not_found}
|
||||
%{local: false} -> {:error, :not_found}
|
||||
|
|
|
|||
|
|
@ -113,7 +113,10 @@ def render("user.json", %{user: user}) do
|
|||
|> Map.merge(Utils.make_json_ld_header())
|
||||
end
|
||||
|
||||
def render("keys.json", %{user: user}) do
|
||||
# For unauthenticated requests when authfetch is enabled.
|
||||
# Still serve the key and the bare minimum of required fields
|
||||
# to avoid being stuck in an infinite "cannot verify" loop with remotes.
|
||||
def render("stripped_user.json", %{user: user}) do
|
||||
{:ok, public_key} = User.SigningKey.public_key_pem(user)
|
||||
|
||||
%{
|
||||
|
|
@ -122,7 +125,14 @@ def render("keys.json", %{user: user}) do
|
|||
"id" => User.SigningKey.key_id_of_local_user(user),
|
||||
"owner" => user.ap_id,
|
||||
"publicKeyPem" => public_key
|
||||
}
|
||||
},
|
||||
# REQUIRED fields per AP spec
|
||||
"inbox" => "#{user.ap_id}/inbox",
|
||||
"outbox" => "#{user.ap_id}/outbox",
|
||||
# allow type-based processing
|
||||
"type" => user.actor_type,
|
||||
# since Mastodon requires a WebFinger address for all users, this seems like a good idea
|
||||
"preferredUsername" => user.nickname
|
||||
}
|
||||
|> Map.merge(Utils.make_json_ld_header())
|
||||
end
|
||||
|
|
|
|||
|
|
@ -133,6 +133,28 @@ test "it returns a json representation of the user with accept application/ld+js
|
|||
assert json_response(conn, 200) == UserView.render("user.json", %{user: user})
|
||||
end
|
||||
|
||||
test "it returns a minimal json representation of the user when autfetch is enabled but no signature",
|
||||
%{
|
||||
conn: conn
|
||||
} do
|
||||
clear_config([:activitypub, :authorized_fetch_mode], true)
|
||||
|
||||
user = insert(:user) |> with_signing_key()
|
||||
|
||||
conn =
|
||||
conn
|
||||
|> assign(:valid_signature, false)
|
||||
|> put_req_header(
|
||||
"accept",
|
||||
"application/ld+json; profile=\"https://www.w3.org/ns/activitystreams\""
|
||||
)
|
||||
|> get("/users/#{user.nickname}")
|
||||
|
||||
user = User.get_cached_by_id(user.id)
|
||||
|
||||
assert json_response(conn, 200) == UserView.render("stripped_user.json", %{user: user})
|
||||
end
|
||||
|
||||
test "it returns 404 for remote users", %{
|
||||
conn: conn
|
||||
} do
|
||||
|
|
|
|||
|
|
@ -7,9 +7,32 @@ defmodule Pleroma.Web.ActivityPub.UserViewTest do
|
|||
import Pleroma.Factory
|
||||
|
||||
alias Pleroma.User
|
||||
alias Pleroma.Web.ActivityPub.ObjectValidators.UserValidator
|
||||
alias Pleroma.Web.ActivityPub.UserView
|
||||
alias Pleroma.Web.CommonAPI
|
||||
|
||||
test "Renders a user such that we accept it ourselves" do
|
||||
user =
|
||||
insert(:user)
|
||||
|> with_signing_key()
|
||||
|
||||
representation = UserView.render("user.json", %{user: user})
|
||||
validation_res = UserValidator.validate(representation, [])
|
||||
|
||||
assert match?({:ok, _user, _meta}, validation_res)
|
||||
end
|
||||
|
||||
test "Renders a minimal user such that we accept it ourselves" do
|
||||
user =
|
||||
insert(:user)
|
||||
|> with_signing_key()
|
||||
|
||||
representation = UserView.render("stripped_user.json", %{user: user})
|
||||
validation_res = UserValidator.validate(representation, [])
|
||||
|
||||
assert match?({:ok, _user, _meta}, validation_res)
|
||||
end
|
||||
|
||||
test "Renders a user, including the public key" do
|
||||
user =
|
||||
insert(:user)
|
||||
|
|
|
|||
Loading…
Reference in a new issue