forked from mirrors/forgejo
fix(activitypub): only return public activities on request (#12382)
The endpoint returning individual activities was missing access control checks, since IDs are sequential, this is not ideal. Fixes #12333 Reviewed-on: https://codeberg.org/forgejo/forgejo/pulls/12382 Reviewed-by: Gusted <gusted@noreply.codeberg.org>
This commit is contained in:
parent
508bb7f2ae
commit
169ea1d991
10 changed files with 153 additions and 15 deletions
|
|
@ -812,3 +812,36 @@ func FixActionCreatedUnixString(ctx context.Context) (int64, error) {
|
|||
}
|
||||
return 0, nil
|
||||
}
|
||||
|
||||
func (a *Action) IsActionPrivate(ctx context.Context) (bool, error) {
|
||||
if a.IsPrivate {
|
||||
return true, nil
|
||||
}
|
||||
|
||||
a.loadRepo(ctx)
|
||||
if a.Repo == nil {
|
||||
return true, repo_model.ErrRepoNotExist{}
|
||||
}
|
||||
|
||||
repo := a.Repo
|
||||
err := repo.LoadOwner(ctx)
|
||||
if err != nil {
|
||||
return true, err
|
||||
}
|
||||
|
||||
if repo.IsPrivate || repo.Owner.KeepActivityPrivate || repo.Owner.Visibility != structs.VisibleTypePublic {
|
||||
return true, nil
|
||||
}
|
||||
|
||||
a.LoadActUser(ctx)
|
||||
if a.ActUser == nil {
|
||||
return true, user_model.ErrUserNotExist{}
|
||||
}
|
||||
|
||||
user := a.ActUser
|
||||
if user.KeepActivityPrivate || user.Visibility != structs.VisibleTypePublic {
|
||||
return true, nil
|
||||
}
|
||||
|
||||
return false, nil
|
||||
}
|
||||
|
|
|
|||
|
|
@ -371,3 +371,28 @@ func TestGetIssueInfos(t *testing.T) {
|
|||
assert.Equal(t, test.field3, issueInfos[2])
|
||||
}
|
||||
}
|
||||
|
||||
func TestIsPrivate(t *testing.T) {
|
||||
defer unittest.OverrideFixtures("models/activities/fixtures/TestIsPrivate")()
|
||||
require.NoError(t, unittest.PrepareTestDatabase())
|
||||
|
||||
tt := []struct {
|
||||
activityID int64
|
||||
private bool
|
||||
}{
|
||||
{1, true}, // private repo
|
||||
{3, false}, // public activities, public repo
|
||||
{11, true}, // private activities
|
||||
}
|
||||
|
||||
for _, test := range tt {
|
||||
ctx := t.Context()
|
||||
action, err := activities_model.GetActivityByID(ctx, test.activityID)
|
||||
require.NoError(t, err)
|
||||
|
||||
private, err := action.IsActionPrivate(ctx)
|
||||
require.NoError(t, err)
|
||||
|
||||
assert.Equal(t, test.private, private, "action ID: %d", test.activityID)
|
||||
}
|
||||
}
|
||||
|
|
|
|||
14
models/activities/error.go
Normal file
14
models/activities/error.go
Normal file
|
|
@ -0,0 +1,14 @@
|
|||
// Copyright 2026 The Forgejo Authors. All rights reserved.
|
||||
// SPDX-License-Identifier: GPL-3.0-or-later
|
||||
|
||||
package activities
|
||||
|
||||
import "fmt"
|
||||
|
||||
type ErrActivityPrivate struct {
|
||||
id int64
|
||||
}
|
||||
|
||||
func (err ErrActivityPrivate) Error() string {
|
||||
return fmt.Sprintf("Activity with id %d is private", err.id)
|
||||
}
|
||||
7
models/activities/fixtures/TestIsPrivate/action.yml
Normal file
7
models/activities/fixtures/TestIsPrivate/action.yml
Normal file
|
|
@ -0,0 +1,7 @@
|
|||
- id: 11
|
||||
user_id: 44
|
||||
op_type: 1 # create repo
|
||||
act_user_id: 44 # private user activities
|
||||
repo_id: 60 # public
|
||||
is_private: false
|
||||
created_unix: 1680454039
|
||||
36
models/activities/fixtures/TestIsPrivate/user.yml
Normal file
36
models/activities/fixtures/TestIsPrivate/user.yml
Normal file
|
|
@ -0,0 +1,36 @@
|
|||
- id: 44
|
||||
lower_name: user44
|
||||
name: user44
|
||||
full_name: user44
|
||||
email: user44@example.com
|
||||
keep_email_private: false
|
||||
email_notifications_preference: enabled
|
||||
passwd: ZogKvWdyEx:password
|
||||
passwd_hash_algo: dummy
|
||||
must_change_password: false
|
||||
login_source: 0
|
||||
login_name: user44
|
||||
type: 0
|
||||
salt: ZogKvWdyEx
|
||||
max_repo_creation: -1
|
||||
is_active: true
|
||||
is_admin: false
|
||||
is_restricted: false
|
||||
allow_git_hook: false
|
||||
allow_import_local: false
|
||||
allow_create_organization: true
|
||||
prohibit_login: false
|
||||
avatar: ""
|
||||
avatar_email: user44@example.com
|
||||
use_custom_avatar: true
|
||||
num_followers: 0
|
||||
num_following: 0
|
||||
num_stars: 0
|
||||
num_repos: 0
|
||||
num_teams: 0
|
||||
num_members: 0
|
||||
visibility: 0
|
||||
repo_admin_change_team_access: false
|
||||
theme: ""
|
||||
keep_activity_private: true
|
||||
created_unix: 1672578380
|
||||
|
|
@ -81,4 +81,4 @@
|
|||
act_user_id: 40
|
||||
repo_id: 60 # public
|
||||
is_private: false
|
||||
created_unix: 1577404800 # end of heatmap
|
||||
created_unix: 1577404800 # end of heatmap
|
||||
|
|
|
|||
|
|
@ -148,6 +148,17 @@ func getActivity(ctx *context.APIContext, id int64) (*forgefed.ForgeUserActivity
|
|||
return nil, err
|
||||
}
|
||||
|
||||
private, err := action.IsActionPrivate(ctx)
|
||||
if err != nil {
|
||||
ctx.Error(http.StatusInternalServerError, "action.IsActionPrivate", err.Error())
|
||||
return nil, err
|
||||
}
|
||||
|
||||
if private {
|
||||
ctx.NotFound()
|
||||
return nil, activities.ErrActivityPrivate{}
|
||||
}
|
||||
|
||||
actions := activities.ActionList{action}
|
||||
if err := actions.LoadAttributes(ctx); err != nil {
|
||||
ctx.Error(http.StatusInternalServerError, "action.LoadAttributes", err.Error())
|
||||
|
|
|
|||
|
|
@ -13,6 +13,7 @@ import (
|
|||
|
||||
activities_model "forgejo.org/models/activities"
|
||||
issues_model "forgejo.org/models/issues"
|
||||
"forgejo.org/models/repo"
|
||||
fm "forgejo.org/modules/forgefed"
|
||||
"forgejo.org/modules/json"
|
||||
"forgejo.org/modules/markup"
|
||||
|
|
@ -20,6 +21,10 @@ import (
|
|||
)
|
||||
|
||||
func ActionToForgeUserActivity(ctx context.Context, action *activities_model.Action) (fm.ForgeUserActivity, error) {
|
||||
if action.Repo == nil {
|
||||
return fm.ForgeUserActivity{}, repo.ErrRepoNotExist{}
|
||||
}
|
||||
|
||||
render := func(format string, args ...any) string {
|
||||
return fmt.Sprintf(`<a href="%s" rel="nofollow">%s</a> %s`, action.ActUser.HTMLURL(), action.GetActDisplayName(ctx), fmt.Sprintf(format, args...))
|
||||
}
|
||||
|
|
|
|||
|
|
@ -9,8 +9,8 @@ import (
|
|||
activities_model "forgejo.org/models/activities"
|
||||
"forgejo.org/models/forgefed"
|
||||
"forgejo.org/models/user"
|
||||
"forgejo.org/modules/log"
|
||||
"forgejo.org/modules/setting"
|
||||
"forgejo.org/modules/structs"
|
||||
"forgejo.org/services/convert"
|
||||
|
||||
ap "github.com/go-ap/activitypub"
|
||||
|
|
@ -64,17 +64,16 @@ func NotifyActivityPubFollowers(ctx context.Context, actions []activities_model.
|
|||
return nil
|
||||
}
|
||||
for _, act := range actions {
|
||||
if act.Repo != nil {
|
||||
if act.Repo.IsPrivate {
|
||||
continue
|
||||
}
|
||||
if act.Repo.Owner.KeepActivityPrivate || act.Repo.Owner.Visibility != structs.VisibleTypePublic {
|
||||
continue
|
||||
}
|
||||
}
|
||||
if act.ActUser.KeepActivityPrivate || act.ActUser.Visibility != structs.VisibleTypePublic {
|
||||
private, err := act.IsActionPrivate(ctx)
|
||||
if err != nil {
|
||||
log.Error("Failed to check if action is private: %s", err.Error())
|
||||
continue
|
||||
}
|
||||
|
||||
if private {
|
||||
continue
|
||||
}
|
||||
|
||||
if err := SendUserActivity(ctx, act.ActUser, &act); err != nil {
|
||||
return err
|
||||
}
|
||||
|
|
|
|||
|
|
@ -23,6 +23,7 @@ import (
|
|||
"forgejo.org/services/federation"
|
||||
"forgejo.org/tests"
|
||||
|
||||
ap "github.com/go-ap/activitypub"
|
||||
"github.com/stretchr/testify/assert"
|
||||
"github.com/stretchr/testify/require"
|
||||
)
|
||||
|
|
@ -159,15 +160,22 @@ func TestActivityPubPersonInboxNoteToDistant(t *testing.T) {
|
|||
require.NoError(t, err)
|
||||
assert.Equal(t, http.StatusOK, resp.StatusCode)
|
||||
|
||||
// distant request activity & activity note
|
||||
localUser2ActivityNote := fmt.Sprintf("%v/activities/1", localUser2URL)
|
||||
localUser2Activity := fmt.Sprintf("%v/activities/1/activity", localUser2URL)
|
||||
// ID of create activity and note delivered to distant
|
||||
activity, err := ap.UnmarshalJSON([]byte(mock.LastPost))
|
||||
require.NoError(t, err)
|
||||
|
||||
resp, err = c.Get(localUser2ActivityNote)
|
||||
createNote := activity.(*ap.Create)
|
||||
localUser2ActivityNote, err := createNote.Object.GetID().URL()
|
||||
require.NoError(t, err)
|
||||
|
||||
localUser2Activity, err := createNote.GetID().URL()
|
||||
require.NoError(t, err)
|
||||
|
||||
resp, err = c.Get(localUser2ActivityNote.String())
|
||||
require.NoError(t, err)
|
||||
assert.Equal(t, http.StatusOK, resp.StatusCode)
|
||||
|
||||
resp, err = c.Get(localUser2Activity)
|
||||
resp, err = c.Get(localUser2Activity.String())
|
||||
require.NoError(t, err)
|
||||
assert.Equal(t, http.StatusOK, resp.StatusCode)
|
||||
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue