diff --git a/services/convert/notification.go b/services/convert/notification.go index 7ddd8ea5d14..fee1de40749 100644 --- a/services/convert/notification.go +++ b/services/convert/notification.go @@ -24,19 +24,22 @@ func ToNotificationThread(ctx context.Context, n *activities_model.Notification) } // since user only get notifications when he has access to use minimal access mode - if n.Repository != nil { - perm, err := access_model.GetIndividualUserRepoPermission(ctx, n.Repository, n.User) - if err != nil { - log.Error("GetIndividualUserRepoPermission failed: %v", err) - return result - } - if perm.HasAnyUnitAccessOrPublicAccess() { // if user has been revoked access to repo, do not show repo info - result.Repository = ToRepo(ctx, n.Repository, perm) - // This permission is not correct and we should not be reporting it - for repository := result.Repository; repository != nil; repository = repository.Parent { - repository.Permissions = nil - } - } + if n.Repository == nil { + return result + } + perm, err := access_model.GetIndividualUserRepoPermission(ctx, n.Repository, n.User) + if err != nil { + log.Error("GetIndividualUserRepoPermission failed: %v", err) + return result + } + // if the user has been revoked access to the repo, do not leak repo or subject info + if !perm.HasAnyUnitAccessOrPublicAccess() { + return result + } + result.Repository = ToRepo(ctx, n.Repository, perm) + // This permission is not correct and we should not be reporting it + for repository := result.Repository; repository != nil; repository = repository.Parent { + repository.Permissions = nil } // handle Subject diff --git a/services/convert/notification_test.go b/services/convert/notification_test.go index 5718258672e..277ee83ec23 100644 --- a/services/convert/notification_test.go +++ b/services/convert/notification_test.go @@ -39,6 +39,36 @@ func TestToNotificationThreadOmitsRepoWhenAccessRevoked(t *testing.T) { assert.Nil(t, thread.Repository) } +func TestToNotificationThreadOmitsSubjectWhenAccessRevoked(t *testing.T) { + assert.NoError(t, unittest.PrepareTestDatabase()) + + ctx := t.Context() + // repo 2 is private; user 4 has no access to it + repo := unittest.AssertExistsAndLoadBean(t, &repo_model.Repository{ID: 2}) + assert.NoError(t, repo.LoadOwner(ctx)) + issue := unittest.AssertExistsAndLoadBean(t, &issues_model.Issue{ID: 4, RepoID: repo.ID}) + user := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 4}) + + n := &activities_model.Notification{ + ID: 12345, + UserID: user.ID, + RepoID: repo.ID, + Status: activities_model.NotificationStatusUnread, + Source: activities_model.NotificationSourceIssue, + IssueID: issue.ID, + UpdatedUnix: timeutil.TimeStampNow(), + Issue: issue, + Repository: repo, + User: user, + } + + thread := ToNotificationThread(ctx, n) + + // must not leak private issue metadata once access is revoked + assert.Nil(t, thread.Repository) + assert.Nil(t, thread.Subject) +} + func TestToNotificationThread(t *testing.T) { require.NoError(t, unittest.PrepareTestDatabase())