Skip to content

Commit

Permalink
Use User.ID instead of User.Name in ActivityPub API for Person IRI (g…
Browse files Browse the repository at this point in the history 
…o-gitea#23823)

Thanks to @trwnh

Close go-gitea#23802

The ActivityPub id is an HTTPS URI that should remain constant, even if
the user changes their name.
  • Loading branch information
@wxiaoguang
wxiaoguang authored Apr 4, 2023
1 parent 5115ffa commit 17f2318
Show file tree
Hide file tree
Showing 7 changed files with 58 additions and 30 deletions.
20 changes: 11 additions & 9 deletions routers/api/v1/activitypub/person.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
package activitypub

import (
"fmt"
"net/http"
"strings"

Expand All @@ -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()
Expand Down Expand Up @@ -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":
Expand Down
5 changes: 5 additions & 0 deletions routers/api/v1/api.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
4 changes: 2 additions & 2 deletions routers/web/webfinger.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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))
Expand All @@ -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),
},
}

Expand Down
21 changes: 21 additions & 0 deletions services/context/user.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down
16 changes: 8 additions & 8 deletions templates/swagger/v1_json.tmpl
View file
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@
},
"basePath": "{{AppSubUrl | JSEscape | Safe}}/api/v1",
"paths": {
"/activitypub/user/{username}": {
"/activitypub/user-id/{user-id}": {
"get": {
"produces": [
"application/json"
Expand All @@ -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
}
Expand All @@ -49,7 +49,7 @@
}
}
},
"/activitypub/user/{username}/inbox": {
"/activitypub/user-id/{user-id}/inbox": {
"post": {
"produces": [
"application/json"
Expand All @@ -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
}
Expand Down
20 changes: 10 additions & 10 deletions tests/integration/api_activitypub_person_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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")
Expand All @@ -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)
Expand All @@ -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")
})
}

Expand All @@ -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
Expand All @@ -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)
Expand Down
2 changes: 1 addition & 1 deletion tests/integration/webfinger_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down

0 comments on commit 17f2318

Please sign in to comment.