Skip to content

Commit 17f2318

Browse files
wxiaoguangwxiaoguang
authored
Use User.ID instead of User.Name in ActivityPub API for Person IRI (#23823)
Thanks to @trwnh Close #23802 The ActivityPub id is an HTTPS URI that should remain constant, even if the user changes their name.
1 parent 5115ffa commit 17f2318

File tree

7 files changed

+58
-30
lines changed

7 files changed

+58
-30
lines changed

‎routers/api/v1/activitypub/person.go

+11-9
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@
44
package activitypub
55

66
import (
7+
"fmt"
78
"net/http"
89
"strings"
910

@@ -18,22 +19,23 @@ import (
1819

1920
// Person function returns the Person actor for a user
2021
func Person(ctx *context.APIContext) {
21-
// swagger:operation GET /activitypub/user/{username} activitypub activitypubPerson
22+
// swagger:operation GET /activitypub/user-id/{user-id} activitypub activitypubPerson
2223
// ---
2324
// summary: Returns the Person actor for a user
2425
// produces:
2526
// - application/json
2627
// parameters:
27-
// - name: username
28+
// - name: user-id
2829
// in: path
29-
// description: username of the user
30-
// type: string
30+
// description: user ID of the user
31+
// type: integer
3132
// required: true
3233
// responses:
3334
// "200":
3435
// "$ref": "#/responses/ActivityPub"
3536

36-
link := strings.TrimSuffix(setting.AppURL, "/") + "/api/v1/activitypub/user/" + ctx.ContextUser.Name
37+
// TODO: the setting.AppURL during the test doesn't follow the definition: "It always has a '/' suffix"
38+
link := fmt.Sprintf("%s/api/v1/activitypub/user-id/%d", strings.TrimSuffix(setting.AppURL, "/"), ctx.ContextUser.ID)
3739
person := ap.PersonNew(ap.IRI(link))
3840

3941
person.Name = ap.NaturalLanguageValuesNew()
@@ -85,16 +87,16 @@ func Person(ctx *context.APIContext) {
8587

8688
// PersonInbox function handles the incoming data for a user inbox
8789
func PersonInbox(ctx *context.APIContext) {
88-
// swagger:operation POST /activitypub/user/{username}/inbox activitypub activitypubPersonInbox
90+
// swagger:operation POST /activitypub/user-id/{user-id}/inbox activitypub activitypubPersonInbox
8991
// ---
9092
// summary: Send to the inbox
9193
// produces:
9294
// - application/json
9395
// parameters:
94-
// - name: username
96+
// - name: user-id
9597
// in: path
96-
// description: username of the user
97-
// type: string
98+
// description: user ID of the user
99+
// type: integer
98100
// required: true
99101
// responses:
100102
// "204":

‎routers/api/v1/api.go

+5
Original file line numberDiff line numberDiff line change
@@ -704,10 +704,15 @@ func Routes(ctx gocontext.Context) *web.Route {
704704
if setting.Federation.Enabled {
705705
m.Get("/nodeinfo", misc.NodeInfo)
706706
m.Group("/activitypub", func() {
707+
// deprecated, remove in 1.20, use /user-id/{user-id} instead
707708
m.Group("/user/{username}", func() {
708709
m.Get("", activitypub.Person)
709710
m.Post("/inbox", activitypub.ReqHTTPSignature(), activitypub.PersonInbox)
710711
}, context_service.UserAssignmentAPI())
712+
m.Group("/user-id/{user-id}", func() {
713+
m.Get("", activitypub.Person)
714+
m.Post("/inbox", activitypub.ReqHTTPSignature(), activitypub.PersonInbox)
715+
}, context_service.UserIDAssignmentAPI())
711716
})
712717
}
713718
m.Get("/signing-key.gpg", misc.SigningKey)

‎routers/web/webfinger.go

+2-2
Original file line numberDiff line numberDiff line change
@@ -85,7 +85,7 @@ func WebfingerQuery(ctx *context.Context) {
8585

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

‎services/context/user.go

+21
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,27 @@ func UserAssignmentWeb() func(ctx *context.Context) {
2929
}
3030
}
3131

32+
// UserIDAssignmentAPI returns a middleware to handle context-user assignment for api routes
33+
func UserIDAssignmentAPI() func(ctx *context.APIContext) {
34+
return func(ctx *context.APIContext) {
35+
userID := ctx.ParamsInt64(":user-id")
36+
37+
if ctx.IsSigned && ctx.Doer.ID == userID {
38+
ctx.ContextUser = ctx.Doer
39+
} else {
40+
var err error
41+
ctx.ContextUser, err = user_model.GetUserByID(ctx, userID)
42+
if err != nil {
43+
if user_model.IsErrUserNotExist(err) {
44+
ctx.Error(http.StatusNotFound, "GetUserByID", err)
45+
} else {
46+
ctx.Error(http.StatusInternalServerError, "GetUserByID", err)
47+
}
48+
}
49+
}
50+
}
51+
}
52+
3253
// UserAssignmentAPI returns a middleware to handle context-user assignment for api routes
3354
func UserAssignmentAPI() func(ctx *context.APIContext) {
3455
return func(ctx *context.APIContext) {

‎templates/swagger/v1_json.tmpl

+8-8
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,7 @@
2323
},
2424
"basePath": "{{AppSubUrl | JSEscape | Safe}}/api/v1",
2525
"paths": {
26-
"/activitypub/user/{username}": {
26+
"/activitypub/user-id/{user-id}": {
2727
"get": {
2828
"produces": [
2929
"application/json"
@@ -35,9 +35,9 @@
3535
"operationId": "activitypubPerson",
3636
"parameters": [
3737
{
38-
"type": "string",
39-
"description": "username of the user",
40-
"name": "username",
38+
"type": "integer",
39+
"description": "user ID of the user",
40+
"name": "user-id",
4141
"in": "path",
4242
"required": true
4343
}
@@ -49,7 +49,7 @@
4949
}
5050
}
5151
},
52-
"/activitypub/user/{username}/inbox": {
52+
"/activitypub/user-id/{user-id}/inbox": {
5353
"post": {
5454
"produces": [
5555
"application/json"
@@ -61,9 +61,9 @@
6161
"operationId": "activitypubPersonInbox",
6262
"parameters": [
6363
{
64-
"type": "string",
65-
"description": "username of the user",
66-
"name": "username",
64+
"type": "integer",
65+
"description": "user ID of the user",
66+
"name": "user-id",
6767
"in": "path",
6868
"required": true
6969
}

‎tests/integration/api_activitypub_person_test.go

+10-10
Original file line numberDiff line numberDiff line change
@@ -29,8 +29,9 @@ func TestActivityPubPerson(t *testing.T) {
2929
}()
3030

3131
onGiteaRun(t, func(*testing.T, *url.URL) {
32+
userID := 2
3233
username := "user2"
33-
req := NewRequestf(t, "GET", fmt.Sprintf("/api/v1/activitypub/user/%s", username))
34+
req := NewRequestf(t, "GET", fmt.Sprintf("/api/v1/activitypub/user-id/%v", userID))
3435
resp := MakeRequest(t, req, http.StatusOK)
3536
body := resp.Body.Bytes()
3637
assert.Contains(t, string(body), "@context")
@@ -42,9 +43,9 @@ func TestActivityPubPerson(t *testing.T) {
4243
assert.Equal(t, ap.PersonType, person.Type)
4344
assert.Equal(t, username, person.PreferredUsername.String())
4445
keyID := person.GetID().String()
45-
assert.Regexp(t, fmt.Sprintf("activitypub/user/%s$", username), keyID)
46-
assert.Regexp(t, fmt.Sprintf("activitypub/user/%s/outbox$", username), person.Outbox.GetID().String())
47-
assert.Regexp(t, fmt.Sprintf("activitypub/user/%s/inbox$", username), person.Inbox.GetID().String())
46+
assert.Regexp(t, fmt.Sprintf("activitypub/user-id/%v$", userID), keyID)
47+
assert.Regexp(t, fmt.Sprintf("activitypub/user-id/%v/outbox$", userID), person.Outbox.GetID().String())
48+
assert.Regexp(t, fmt.Sprintf("activitypub/user-id/%v/inbox$", userID), person.Inbox.GetID().String())
4849

4950
pubKey := person.PublicKey
5051
assert.NotNil(t, pubKey)
@@ -66,9 +67,9 @@ func TestActivityPubMissingPerson(t *testing.T) {
6667
}()
6768

6869
onGiteaRun(t, func(*testing.T, *url.URL) {
69-
req := NewRequestf(t, "GET", "/api/v1/activitypub/user/nonexistentuser")
70+
req := NewRequestf(t, "GET", "/api/v1/activitypub/user-id/999999999")
7071
resp := MakeRequest(t, req, http.StatusNotFound)
71-
assert.Contains(t, resp.Body.String(), "user redirect does not exist")
72+
assert.Contains(t, resp.Body.String(), "user does not exist")
7273
})
7374
}
7475

@@ -85,7 +86,7 @@ func TestActivityPubPersonInbox(t *testing.T) {
8586

8687
onGiteaRun(t, func(*testing.T, *url.URL) {
8788
appURL := setting.AppURL
88-
setting.AppURL = srv.URL
89+
setting.AppURL = srv.URL + "/"
8990
defer func() {
9091
setting.Database.LogSQL = false
9192
setting.AppURL = appURL
@@ -94,11 +95,10 @@ func TestActivityPubPersonInbox(t *testing.T) {
9495
ctx := context.Background()
9596
user1, err := user_model.GetUserByName(ctx, username1)
9697
assert.NoError(t, err)
97-
user1url := fmt.Sprintf("%s/api/v1/activitypub/user/%s#main-key", srv.URL, username1)
98+
user1url := fmt.Sprintf("%s/api/v1/activitypub/user-id/1#main-key", srv.URL)
9899
c, err := activitypub.NewClient(user1, user1url)
99100
assert.NoError(t, err)
100-
username2 := "user2"
101-
user2inboxurl := fmt.Sprintf("%s/api/v1/activitypub/user/%s/inbox", srv.URL, username2)
101+
user2inboxurl := fmt.Sprintf("%s/api/v1/activitypub/user-id/2/inbox", srv.URL)
102102

103103
// Signed request succeeds
104104
resp, err := c.Post([]byte{}, user2inboxurl)

‎tests/integration/webfinger_test.go

+1-1
Original file line numberDiff line numberDiff line change
@@ -52,7 +52,7 @@ func TestWebfinger(t *testing.T) {
5252
var jrd webfingerJRD
5353
DecodeJSON(t, resp, &jrd)
5454
assert.Equal(t, "acct:user2@"+appURL.Host, jrd.Subject)
55-
assert.ElementsMatch(t, []string{user.HTMLURL(), appURL.String() + "api/v1/activitypub/user/" + url.PathEscape(user.Name)}, jrd.Aliases)
55+
assert.ElementsMatch(t, []string{user.HTMLURL(), appURL.String() + "api/v1/activitypub/user-id/" + fmt.Sprint(user.ID)}, jrd.Aliases)
5656

5757
req = NewRequest(t, "GET", fmt.Sprintf("/.well-known/webfinger?resource=acct:%s@%s", user.LowerName, "unknown.host"))
5858
MakeRequest(t, req, http.StatusBadRequest)

0 commit comments

Comments
 (0)