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.

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 8b13f5492c..21797bd1a0 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 fe3f31b13d..a8ba40740e 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)