From 9836b7db7baf896c3d141270ce8ba52944ab146c Mon Sep 17 00:00:00 2001 From: Giteabot Date: Mon, 3 Apr 2023 23:41:57 -0400 Subject: [PATCH] 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 --- routers/api/v1/activitypub/person.go | 20 ++++++++++-------- routers/api/v1/api.go | 5 +++++ routers/web/webfinger.go | 4 ++-- services/context/user.go | 21 +++++++++++++++++++ templates/swagger/v1_json.tmpl | 16 +++++++------- .../api_activitypub_person_test.go | 20 +++++++++--------- tests/integration/webfinger_test.go | 2 +- 7 files changed, 58 insertions(+), 30 deletions(-) diff --git a/routers/api/v1/activitypub/person.go b/routers/api/v1/activitypub/person.go index 492930b849..bc6b82b179 100644 --- a/routers/api/v1/activitypub/person.go +++ b/routers/api/v1/activitypub/person.go @@ -4,6 +4,7 @@ package activitypub import ( + "fmt" "net/http" "strings" @@ -18,22 +19,23 @@ import ( // Person function returns the Person actor for a user 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 // produces: // - application/json // parameters: - // - name: username + // - name: user-id // in: path - // description: username of the user - // type: string + // description: user ID of the user + // type: integer // required: true // responses: // "200": // "$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.Name = ap.NaturalLanguageValuesNew() @@ -85,16 +87,16 @@ func Person(ctx *context.APIContext) { // PersonInbox function handles the incoming data for a user inbox 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 // produces: // - application/json // parameters: - // - name: username + // - name: user-id // in: path - // description: username of the user - // type: string + // description: user ID of the user + // type: integer // required: true // responses: // "204": diff --git a/routers/api/v1/api.go b/routers/api/v1/api.go index 1d2f8b18e0..1b189f28a7 100644 --- a/routers/api/v1/api.go +++ b/routers/api/v1/api.go @@ -704,10 +704,15 @@ func Routes(ctx gocontext.Context) *web.Route { if setting.Federation.Enabled { m.Get("/nodeinfo", misc.NodeInfo) m.Group("/activitypub", func() { + // deprecated, remove in 1.20, use /user-id/{user-id} instead m.Group("/user/{username}", func() { m.Get("", activitypub.Person) m.Post("/inbox", activitypub.ReqHTTPSignature(), activitypub.PersonInbox) }, 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) diff --git a/routers/web/webfinger.go b/routers/web/webfinger.go index a8e816d7b7..1442e09a31 100644 --- a/routers/web/webfinger.go +++ b/routers/web/webfinger.go @@ -85,7 +85,7 @@ func WebfingerQuery(ctx *context.Context) { aliases := []string{ 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 { aliases = append(aliases, fmt.Sprintf("mailto:%s", u.Email)) @@ -104,7 +104,7 @@ func WebfingerQuery(ctx *context.Context) { { Rel: "self", 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), }, } diff --git a/services/context/user.go b/services/context/user.go index 9dc84c3ac1..c713667bca 100644 --- a/services/context/user.go +++ b/services/context/user.go @@ -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 func UserAssignmentAPI() func(ctx *context.APIContext) { return func(ctx *context.APIContext) { diff --git a/templates/swagger/v1_json.tmpl b/templates/swagger/v1_json.tmpl index 9bab4ced80..c9bba3ae51 100644 --- a/templates/swagger/v1_json.tmpl +++ b/templates/swagger/v1_json.tmpl @@ -23,7 +23,7 @@ }, "basePath": "{{AppSubUrl | JSEscape | Safe}}/api/v1", "paths": { - "/activitypub/user/{username}": { + "/activitypub/user-id/{user-id}": { "get": { "produces": [ "application/json" @@ -35,9 +35,9 @@ "operationId": "activitypubPerson", "parameters": [ { - "type": "string", - "description": "username of the user", - "name": "username", + "type": "integer", + "description": "user ID of the user", + "name": "user-id", "in": "path", "required": true } @@ -49,7 +49,7 @@ } } }, - "/activitypub/user/{username}/inbox": { + "/activitypub/user-id/{user-id}/inbox": { "post": { "produces": [ "application/json" @@ -61,9 +61,9 @@ "operationId": "activitypubPersonInbox", "parameters": [ { - "type": "string", - "description": "username of the user", - "name": "username", + "type": "integer", + "description": "user ID of the user", + "name": "user-id", "in": "path", "required": true } diff --git a/tests/integration/api_activitypub_person_test.go b/tests/integration/api_activitypub_person_test.go index dadc417aa4..301cfba172 100644 --- a/tests/integration/api_activitypub_person_test.go +++ b/tests/integration/api_activitypub_person_test.go @@ -29,8 +29,9 @@ func TestActivityPubPerson(t *testing.T) { }() onGiteaRun(t, func(*testing.T, *url.URL) { + userID := 2 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) body := resp.Body.Bytes() 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, username, person.PreferredUsername.String()) keyID := person.GetID().String() - assert.Regexp(t, fmt.Sprintf("activitypub/user/%s$", username), keyID) - assert.Regexp(t, fmt.Sprintf("activitypub/user/%s/outbox$", username), 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$", userID), keyID) + assert.Regexp(t, fmt.Sprintf("activitypub/user-id/%v/outbox$", userID), person.Outbox.GetID().String()) + assert.Regexp(t, fmt.Sprintf("activitypub/user-id/%v/inbox$", userID), person.Inbox.GetID().String()) pubKey := person.PublicKey assert.NotNil(t, pubKey) @@ -66,9 +67,9 @@ func TestActivityPubMissingPerson(t *testing.T) { }() 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) - 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) { appURL := setting.AppURL - setting.AppURL = srv.URL + setting.AppURL = srv.URL + "/" defer func() { setting.Database.LogSQL = false setting.AppURL = appURL @@ -94,11 +95,10 @@ func TestActivityPubPersonInbox(t *testing.T) { ctx := context.Background() user1, err := user_model.GetUserByName(ctx, username1) 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) assert.NoError(t, err) - username2 := "user2" - user2inboxurl := fmt.Sprintf("%s/api/v1/activitypub/user/%s/inbox", srv.URL, username2) + user2inboxurl := fmt.Sprintf("%s/api/v1/activitypub/user-id/2/inbox", srv.URL) // Signed request succeeds resp, err := c.Post([]byte{}, user2inboxurl) diff --git a/tests/integration/webfinger_test.go b/tests/integration/webfinger_test.go index 226c25615f..f67abdbc2a 100644 --- a/tests/integration/webfinger_test.go +++ b/tests/integration/webfinger_test.go @@ -52,7 +52,7 @@ func TestWebfinger(t *testing.T) { var jrd webfingerJRD DecodeJSON(t, resp, &jrd) 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")) MakeRequest(t, req, http.StatusBadRequest)