Skip to content

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

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Apr 4, 2023
Merged
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
20 changes: 11 additions & 9 deletions routers/api/v1/activitypub/person.go
Original file line number Diff line number Diff line change
@@ -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":
5 changes: 5 additions & 0 deletions routers/api/v1/api.go
Original file line number Diff line number Diff line change
@@ -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)
4 changes: 2 additions & 2 deletions routers/web/webfinger.go
Original file line number Diff line number Diff line change
@@ -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),
},
}

21 changes: 21 additions & 0 deletions services/context/user.go
Original file line number Diff line number Diff line change
@@ -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) {
16 changes: 8 additions & 8 deletions templates/swagger/v1_json.tmpl
Original file line number Diff line number Diff line change
@@ -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
}
20 changes: 10 additions & 10 deletions tests/integration/api_activitypub_person_test.go
Original file line number Diff line number Diff line change
@@ -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)
2 changes: 1 addition & 1 deletion tests/integration/webfinger_test.go
Original file line number Diff line number Diff line change
@@ -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)