Use User.ID instead of User.Name in ActivityPub API for Person IRI (#23823) (#23905)

Backport #23823 by @wxiaoguang

Thanks to @trwnh

Close #23802

The ActivityPub id is an HTTPS URI that should remain constant, even if
the user changes their name.

Co-authored-by: wxiaoguang <wxiaoguang@gmail.com>
This commit is contained in:
Giteabot 2023-04-03 23:41:57 -04:00 committed by GitHub
parent d752f0d7d0
commit 9836b7db7b
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
7 changed files with 58 additions and 30 deletions

View file

@ -4,6 +4,7 @@
package activitypub package activitypub
import ( import (
"fmt"
"net/http" "net/http"
"strings" "strings"
@ -18,22 +19,23 @@ import (
// Person function returns the Person actor for a user // Person function returns the Person actor for a user
func Person(ctx *context.APIContext) { func Person(ctx *context.APIContext) {
// swagger:operation GET /activitypub/user/{username} activitypub activitypubPerson // swagger:operation GET /activitypub/user-id/{user-id} activitypub activitypubPerson
// --- // ---
// summary: Returns the Person actor for a user // summary: Returns the Person actor for a user
// produces: // produces:
// - application/json // - application/json
// parameters: // parameters:
// - name: username // - name: user-id
// in: path // in: path
// description: username of the user // description: user ID of the user
// type: string // type: integer
// required: true // required: true
// responses: // responses:
// "200": // "200":
// "$ref": "#/responses/ActivityPub" // "$ref": "#/responses/ActivityPub"
link := strings.TrimSuffix(setting.AppURL, "/") + "/api/v1/activitypub/user/" + ctx.ContextUser.Name // TODO: the setting.AppURL during the test doesn't follow the definition: "It always has a '/' suffix"
link := fmt.Sprintf("%s/api/v1/activitypub/user-id/%d", strings.TrimSuffix(setting.AppURL, "/"), ctx.ContextUser.ID)
person := ap.PersonNew(ap.IRI(link)) person := ap.PersonNew(ap.IRI(link))
person.Name = ap.NaturalLanguageValuesNew() person.Name = ap.NaturalLanguageValuesNew()
@ -85,16 +87,16 @@ func Person(ctx *context.APIContext) {
// PersonInbox function handles the incoming data for a user inbox // PersonInbox function handles the incoming data for a user inbox
func PersonInbox(ctx *context.APIContext) { func PersonInbox(ctx *context.APIContext) {
// swagger:operation POST /activitypub/user/{username}/inbox activitypub activitypubPersonInbox // swagger:operation POST /activitypub/user-id/{user-id}/inbox activitypub activitypubPersonInbox
// --- // ---
// summary: Send to the inbox // summary: Send to the inbox
// produces: // produces:
// - application/json // - application/json
// parameters: // parameters:
// - name: username // - name: user-id
// in: path // in: path
// description: username of the user // description: user ID of the user
// type: string // type: integer
// required: true // required: true
// responses: // responses:
// "204": // "204":

View file

@ -704,10 +704,15 @@ func Routes(ctx gocontext.Context) *web.Route {
if setting.Federation.Enabled { if setting.Federation.Enabled {
m.Get("/nodeinfo", misc.NodeInfo) m.Get("/nodeinfo", misc.NodeInfo)
m.Group("/activitypub", func() { m.Group("/activitypub", func() {
// deprecated, remove in 1.20, use /user-id/{user-id} instead
m.Group("/user/{username}", func() { m.Group("/user/{username}", func() {
m.Get("", activitypub.Person) m.Get("", activitypub.Person)
m.Post("/inbox", activitypub.ReqHTTPSignature(), activitypub.PersonInbox) m.Post("/inbox", activitypub.ReqHTTPSignature(), activitypub.PersonInbox)
}, context_service.UserAssignmentAPI()) }, context_service.UserAssignmentAPI())
m.Group("/user-id/{user-id}", func() {
m.Get("", activitypub.Person)
m.Post("/inbox", activitypub.ReqHTTPSignature(), activitypub.PersonInbox)
}, context_service.UserIDAssignmentAPI())
}) })
} }
m.Get("/signing-key.gpg", misc.SigningKey) m.Get("/signing-key.gpg", misc.SigningKey)

View file

@ -85,7 +85,7 @@ func WebfingerQuery(ctx *context.Context) {
aliases := []string{ aliases := []string{
u.HTMLURL(), u.HTMLURL(),
appURL.String() + "api/v1/activitypub/user/" + url.PathEscape(u.Name), appURL.String() + "api/v1/activitypub/user-id/" + fmt.Sprint(u.ID),
} }
if !u.KeepEmailPrivate { if !u.KeepEmailPrivate {
aliases = append(aliases, fmt.Sprintf("mailto:%s", u.Email)) aliases = append(aliases, fmt.Sprintf("mailto:%s", u.Email))
@ -104,7 +104,7 @@ func WebfingerQuery(ctx *context.Context) {
{ {
Rel: "self", Rel: "self",
Type: "application/activity+json", Type: "application/activity+json",
Href: appURL.String() + "api/v1/activitypub/user/" + url.PathEscape(u.Name), Href: appURL.String() + "api/v1/activitypub/user-id/" + fmt.Sprint(u.ID),
}, },
} }

View file

@ -29,6 +29,27 @@ func UserAssignmentWeb() func(ctx *context.Context) {
} }
} }
// UserIDAssignmentAPI returns a middleware to handle context-user assignment for api routes
func UserIDAssignmentAPI() func(ctx *context.APIContext) {
return func(ctx *context.APIContext) {
userID := ctx.ParamsInt64(":user-id")
if ctx.IsSigned && ctx.Doer.ID == userID {
ctx.ContextUser = ctx.Doer
} else {
var err error
ctx.ContextUser, err = user_model.GetUserByID(ctx, userID)
if err != nil {
if user_model.IsErrUserNotExist(err) {
ctx.Error(http.StatusNotFound, "GetUserByID", err)
} else {
ctx.Error(http.StatusInternalServerError, "GetUserByID", err)
}
}
}
}
}
// UserAssignmentAPI returns a middleware to handle context-user assignment for api routes // UserAssignmentAPI returns a middleware to handle context-user assignment for api routes
func UserAssignmentAPI() func(ctx *context.APIContext) { func UserAssignmentAPI() func(ctx *context.APIContext) {
return func(ctx *context.APIContext) { return func(ctx *context.APIContext) {

View file

@ -23,7 +23,7 @@
}, },
"basePath": "{{AppSubUrl | JSEscape | Safe}}/api/v1", "basePath": "{{AppSubUrl | JSEscape | Safe}}/api/v1",
"paths": { "paths": {
"/activitypub/user/{username}": { "/activitypub/user-id/{user-id}": {
"get": { "get": {
"produces": [ "produces": [
"application/json" "application/json"
@ -35,9 +35,9 @@
"operationId": "activitypubPerson", "operationId": "activitypubPerson",
"parameters": [ "parameters": [
{ {
"type": "string", "type": "integer",
"description": "username of the user", "description": "user ID of the user",
"name": "username", "name": "user-id",
"in": "path", "in": "path",
"required": true "required": true
} }
@ -49,7 +49,7 @@
} }
} }
}, },
"/activitypub/user/{username}/inbox": { "/activitypub/user-id/{user-id}/inbox": {
"post": { "post": {
"produces": [ "produces": [
"application/json" "application/json"
@ -61,9 +61,9 @@
"operationId": "activitypubPersonInbox", "operationId": "activitypubPersonInbox",
"parameters": [ "parameters": [
{ {
"type": "string", "type": "integer",
"description": "username of the user", "description": "user ID of the user",
"name": "username", "name": "user-id",
"in": "path", "in": "path",
"required": true "required": true
} }

View file

@ -29,8 +29,9 @@ func TestActivityPubPerson(t *testing.T) {
}() }()
onGiteaRun(t, func(*testing.T, *url.URL) { onGiteaRun(t, func(*testing.T, *url.URL) {
userID := 2
username := "user2" username := "user2"
req := NewRequestf(t, "GET", fmt.Sprintf("/api/v1/activitypub/user/%s", username)) req := NewRequestf(t, "GET", fmt.Sprintf("/api/v1/activitypub/user-id/%v", userID))
resp := MakeRequest(t, req, http.StatusOK) resp := MakeRequest(t, req, http.StatusOK)
body := resp.Body.Bytes() body := resp.Body.Bytes()
assert.Contains(t, string(body), "@context") assert.Contains(t, string(body), "@context")
@ -42,9 +43,9 @@ func TestActivityPubPerson(t *testing.T) {
assert.Equal(t, ap.PersonType, person.Type) assert.Equal(t, ap.PersonType, person.Type)
assert.Equal(t, username, person.PreferredUsername.String()) assert.Equal(t, username, person.PreferredUsername.String())
keyID := person.GetID().String() keyID := person.GetID().String()
assert.Regexp(t, fmt.Sprintf("activitypub/user/%s$", username), keyID) assert.Regexp(t, fmt.Sprintf("activitypub/user-id/%v$", userID), keyID)
assert.Regexp(t, fmt.Sprintf("activitypub/user/%s/outbox$", username), person.Outbox.GetID().String()) assert.Regexp(t, fmt.Sprintf("activitypub/user-id/%v/outbox$", userID), person.Outbox.GetID().String())
assert.Regexp(t, fmt.Sprintf("activitypub/user/%s/inbox$", username), person.Inbox.GetID().String()) assert.Regexp(t, fmt.Sprintf("activitypub/user-id/%v/inbox$", userID), person.Inbox.GetID().String())
pubKey := person.PublicKey pubKey := person.PublicKey
assert.NotNil(t, pubKey) assert.NotNil(t, pubKey)
@ -66,9 +67,9 @@ func TestActivityPubMissingPerson(t *testing.T) {
}() }()
onGiteaRun(t, func(*testing.T, *url.URL) { onGiteaRun(t, func(*testing.T, *url.URL) {
req := NewRequestf(t, "GET", "/api/v1/activitypub/user/nonexistentuser") req := NewRequestf(t, "GET", "/api/v1/activitypub/user-id/999999999")
resp := MakeRequest(t, req, http.StatusNotFound) resp := MakeRequest(t, req, http.StatusNotFound)
assert.Contains(t, resp.Body.String(), "user redirect does not exist") assert.Contains(t, resp.Body.String(), "user does not exist")
}) })
} }
@ -85,7 +86,7 @@ func TestActivityPubPersonInbox(t *testing.T) {
onGiteaRun(t, func(*testing.T, *url.URL) { onGiteaRun(t, func(*testing.T, *url.URL) {
appURL := setting.AppURL appURL := setting.AppURL
setting.AppURL = srv.URL setting.AppURL = srv.URL + "/"
defer func() { defer func() {
setting.Database.LogSQL = false setting.Database.LogSQL = false
setting.AppURL = appURL setting.AppURL = appURL
@ -94,11 +95,10 @@ func TestActivityPubPersonInbox(t *testing.T) {
ctx := context.Background() ctx := context.Background()
user1, err := user_model.GetUserByName(ctx, username1) user1, err := user_model.GetUserByName(ctx, username1)
assert.NoError(t, err) assert.NoError(t, err)
user1url := fmt.Sprintf("%s/api/v1/activitypub/user/%s#main-key", srv.URL, username1) user1url := fmt.Sprintf("%s/api/v1/activitypub/user-id/1#main-key", srv.URL)
c, err := activitypub.NewClient(user1, user1url) c, err := activitypub.NewClient(user1, user1url)
assert.NoError(t, err) assert.NoError(t, err)
username2 := "user2" user2inboxurl := fmt.Sprintf("%s/api/v1/activitypub/user-id/2/inbox", srv.URL)
user2inboxurl := fmt.Sprintf("%s/api/v1/activitypub/user/%s/inbox", srv.URL, username2)
// Signed request succeeds // Signed request succeeds
resp, err := c.Post([]byte{}, user2inboxurl) resp, err := c.Post([]byte{}, user2inboxurl)

View file

@ -52,7 +52,7 @@ func TestWebfinger(t *testing.T) {
var jrd webfingerJRD var jrd webfingerJRD
DecodeJSON(t, resp, &jrd) DecodeJSON(t, resp, &jrd)
assert.Equal(t, "acct:user2@"+appURL.Host, jrd.Subject) assert.Equal(t, "acct:user2@"+appURL.Host, jrd.Subject)
assert.ElementsMatch(t, []string{user.HTMLURL(), appURL.String() + "api/v1/activitypub/user/" + url.PathEscape(user.Name)}, jrd.Aliases) assert.ElementsMatch(t, []string{user.HTMLURL(), appURL.String() + "api/v1/activitypub/user-id/" + fmt.Sprint(user.ID)}, jrd.Aliases)
req = NewRequest(t, "GET", fmt.Sprintf("/.well-known/webfinger?resource=acct:%s@%s", user.LowerName, "unknown.host")) req = NewRequest(t, "GET", fmt.Sprintf("/.well-known/webfinger?resource=acct:%s@%s", user.LowerName, "unknown.host"))
MakeRequest(t, req, http.StatusBadRequest) MakeRequest(t, req, http.StatusBadRequest)