Merge branch 'main' into various-sec-fixes

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

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())