Make restricted users can access public repositories (#35693)

Fix #35690

Change the "restricted user" behavior introduced by #6274. Now
restricted user can also access public repositories when sign-in is not
required.

For required sign-in, the behavior isn't changed.

models/organization/org.go

@@ -429,6 +429,10 @@ func HasOrgOrUserVisible(ctx context.Context, orgOrUser, user *user_model.User)
 		return true
 	}
 
+	if !setting.Service.RequireSignInViewStrict && orgOrUser.Visibility == structs.VisibleTypePublic {
+		return true
+	}
+
 	if (orgOrUser.Visibility == structs.VisibleTypePrivate || user.IsRestricted) && !OrgFromUser(orgOrUser).hasMemberWithUserID(ctx, user.ID) {
 		return false
 	}

models/organization/org_test.go

@@ -13,7 +13,9 @@ import (
 	repo_model "code.gitea.io/gitea/models/repo"
 	"code.gitea.io/gitea/models/unittest"
 	user_model "code.gitea.io/gitea/models/user"
+	"code.gitea.io/gitea/modules/setting"
 	"code.gitea.io/gitea/modules/structs"
+	"code.gitea.io/gitea/modules/test"
 
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
@@ -382,6 +384,12 @@ func TestHasOrgVisibleTypePublic(t *testing.T) {
 	assert.True(t, test1) // owner of org
 	assert.True(t, test2) // user not a part of org
 	assert.True(t, test3) // logged out user
+
+	restrictedUser := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 29, IsRestricted: true})
+	require.True(t, restrictedUser.IsRestricted)
+	assert.True(t, organization.HasOrgOrUserVisible(t.Context(), org.AsUser(), restrictedUser))
+	defer test.MockVariableValue(&setting.Service.RequireSignInViewStrict, true)()
+	assert.False(t, organization.HasOrgOrUserVisible(t.Context(), org.AsUser(), restrictedUser))
 }
 
 func TestHasOrgVisibleTypeLimited(t *testing.T) {

models/perm/access/access.go

@@ -13,6 +13,8 @@ import (
 	"code.gitea.io/gitea/models/perm"
 	repo_model "code.gitea.io/gitea/models/repo"
 	user_model "code.gitea.io/gitea/models/user"
+	"code.gitea.io/gitea/modules/setting"
+	"code.gitea.io/gitea/modules/structs"
 
 	"xorm.io/builder"
 )
@@ -41,7 +43,12 @@ func accessLevel(ctx context.Context, user *user_model.User, repo *repo_model.Re
 		restricted = user.IsRestricted
 	}
 
-	if !restricted && !repo.IsPrivate {
+	if err := repo.LoadOwner(ctx); err != nil {
+		return mode, err
+	}
+
+	repoIsFullyPublic := !setting.Service.RequireSignInViewStrict && repo.Owner.Visibility == structs.VisibleTypePublic && !repo.IsPrivate
+	if (restricted && repoIsFullyPublic) || (!restricted && !repo.IsPrivate) {
 		mode = perm.AccessModeRead
 	}
 

models/perm/access/access_test.go

@@ -12,6 +12,7 @@ import (
 	repo_model "code.gitea.io/gitea/models/repo"
 	"code.gitea.io/gitea/models/unittest"
 	user_model "code.gitea.io/gitea/models/user"
+	"code.gitea.io/gitea/modules/setting"
 
 	"github.com/stretchr/testify/assert"
 )
@@ -51,7 +52,14 @@ func TestAccessLevel(t *testing.T) {
 	assert.NoError(t, err)
 	assert.Equal(t, perm_model.AccessModeNone, level)
 
-	// restricted user has no access to a public repo
+	// restricted user has default access to a public repo if no sign-in is required
+	setting.Service.RequireSignInViewStrict = false
+	level, err = access_model.AccessLevel(t.Context(), user29, repo1)
+	assert.NoError(t, err)
+	assert.Equal(t, perm_model.AccessModeRead, level)
+
+	// restricted user has no access to a public repo if sign-in is required
+	setting.Service.RequireSignInViewStrict = true
 	level, err = access_model.AccessLevel(t.Context(), user29, repo1)
 	assert.NoError(t, err)
 	assert.Equal(t, perm_model.AccessModeNone, level)

models/repo/repo_list.go

@@ -642,6 +642,17 @@ func SearchRepositoryIDsByCondition(ctx context.Context, cond builder.Cond) ([]i
 		Find(&repoIDs)
 }
 
+func userAllPublicRepoCond(cond builder.Cond, orgVisibilityLimit []structs.VisibleType) builder.Cond {
+	return cond.Or(builder.And(
+		builder.Eq{"`repository`.is_private": false},
+		// Aren't in a private organisation or limited organisation if we're not logged in
+		builder.NotIn("`repository`.owner_id", builder.Select("id").From("`user`").Where(
+			builder.And(
+				builder.Eq{"type": user_model.UserTypeOrganization},
+				builder.In("visibility", orgVisibilityLimit)),
+		))))
+}
+
 // AccessibleRepositoryCondition takes a user a returns a condition for checking if a repository is accessible
 func AccessibleRepositoryCondition(user *user_model.User, unitType unit.Type) builder.Cond {
 	cond := builder.NewCond()
@@ -651,15 +662,8 @@ func AccessibleRepositoryCondition(user *user_model.User, unitType unit.Type) bu
 		if user == nil || user.ID <= 0 {
 			orgVisibilityLimit = append(orgVisibilityLimit, structs.VisibleTypeLimited)
 		}
-		// 1. Be able to see all non-private repositories that either:
+		// 1. Be able to see all non-private repositories
-		cond = cond.Or(builder.And(
+		cond = userAllPublicRepoCond(cond, orgVisibilityLimit)
-			builder.Eq{"`repository`.is_private": false},
-			// 2. Aren't in an private organisation or limited organisation if we're not logged in
-			builder.NotIn("`repository`.owner_id", builder.Select("id").From("`user`").Where(
-				builder.And(
-					builder.Eq{"type": user_model.UserTypeOrganization},
-					builder.In("visibility", orgVisibilityLimit)),
-			))))
 	}
 
 	if user != nil {
@@ -683,6 +687,9 @@ func AccessibleRepositoryCondition(user *user_model.User, unitType unit.Type) bu
 		if !user.IsRestricted {
 			// 5. Be able to see all public repos in private organizations that we are an org_user of
 			cond = cond.Or(userOrgPublicRepoCond(user.ID))
+		} else if !setting.Service.RequireSignInViewStrict {
+			orgVisibilityLimit := []structs.VisibleType{structs.VisibleTypePrivate, structs.VisibleTypeLimited}
+			cond = userAllPublicRepoCond(cond, orgVisibilityLimit)
 		}
 	}
 

models/repo/repo_list_test.go

@@ -10,9 +10,14 @@ import (
 	"code.gitea.io/gitea/models/db"
 	repo_model "code.gitea.io/gitea/models/repo"
 	"code.gitea.io/gitea/models/unittest"
+	user_model "code.gitea.io/gitea/models/user"
 	"code.gitea.io/gitea/modules/optional"
+	"code.gitea.io/gitea/modules/setting"
+	"code.gitea.io/gitea/modules/structs"
+	"code.gitea.io/gitea/modules/test"
 
 	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
 )
 
 func getTestCases() []struct {
@@ -182,7 +187,16 @@ func getTestCases() []struct {
 
 func TestSearchRepository(t *testing.T) {
 	assert.NoError(t, unittest.PrepareTestDatabase())
+	t.Run("SearchRepositoryPublic", testSearchRepositoryPublic)
+	t.Run("SearchRepositoryPublicRestricted", testSearchRepositoryRestricted)
+	t.Run("SearchRepositoryPrivate", testSearchRepositoryPrivate)
+	t.Run("SearchRepositoryNonExistingOwner", testSearchRepositoryNonExistingOwner)
+	t.Run("SearchRepositoryWithInDescription", testSearchRepositoryWithInDescription)
+	t.Run("SearchRepositoryNotInDescription", testSearchRepositoryNotInDescription)
+	t.Run("SearchRepositoryCases", testSearchRepositoryCases)
+}
 
+func testSearchRepositoryPublic(t *testing.T) {
 	// test search public repository on explore page
 	repos, count, err := repo_model.SearchRepositoryByName(t.Context(), repo_model.SearchRepoOptions{
 		ListOptions: db.ListOptions{
@@ -211,9 +225,54 @@ func TestSearchRepository(t *testing.T) {
 	assert.NoError(t, err)
 	assert.Equal(t, int64(2), count)
 	assert.Len(t, repos, 2)
+}
 
+func testSearchRepositoryRestricted(t *testing.T) {
+	user2 := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 2})
+	restrictedUser := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 29, IsRestricted: true})
+
+	performSearch := func(t *testing.T, user *user_model.User) (publicRepoIDs []int64) {
+		repos, count, err := repo_model.SearchRepositoryByName(t.Context(), repo_model.SearchRepoOptions{
+			ListOptions: db.ListOptions{Page: 1, PageSize: 10000},
+			Actor:       user,
+		})
+		require.NoError(t, err)
+		assert.Len(t, repos, int(count))
+		for _, repo := range repos {
+			require.NoError(t, repo.LoadOwner(t.Context()))
+			if repo.Owner.Visibility == structs.VisibleTypePublic && !repo.IsPrivate {
+				publicRepoIDs = append(publicRepoIDs, repo.ID)
+			}
+		}
+		return publicRepoIDs
+	}
+
+	normalPublicRepoIDs := performSearch(t, user2)
+	require.Greater(t, len(normalPublicRepoIDs), 10) // quite a lot
+
+	t.Run("RestrictedUser-NoSignInRequirement", func(t *testing.T) {
+		// restricted user can also see public repositories if no "required sign-in"
+		repoIDs := performSearch(t, restrictedUser)
+		assert.ElementsMatch(t, normalPublicRepoIDs, repoIDs)
+	})
+
+	defer test.MockVariableValue(&setting.Service.RequireSignInViewStrict, true)()
+
+	t.Run("NormalUser-RequiredSignIn", func(t *testing.T) {
+		// normal user can still see all public repos, not affected by "required sign-in"
+		repoIDs := performSearch(t, user2)
+		assert.ElementsMatch(t, normalPublicRepoIDs, repoIDs)
+	})
+	t.Run("RestrictedUser-RequiredSignIn", func(t *testing.T) {
+		// restricted user can see only their own repo
+		repoIDs := performSearch(t, restrictedUser)
+		assert.Equal(t, []int64{4}, repoIDs)
+	})
+}
+
+func testSearchRepositoryPrivate(t *testing.T) {
 	// test search private repository on explore page
-	repos, count, err = repo_model.SearchRepositoryByName(t.Context(), repo_model.SearchRepoOptions{
+	repos, count, err := repo_model.SearchRepositoryByName(t.Context(), repo_model.SearchRepoOptions{
 		ListOptions: db.ListOptions{
 			Page:     1,
 			PageSize: 10,
@@ -242,16 +301,18 @@ func TestSearchRepository(t *testing.T) {
 	assert.NoError(t, err)
 	assert.Equal(t, int64(3), count)
 	assert.Len(t, repos, 3)
+}
 
-	// Test non existing owner
+func testSearchRepositoryNonExistingOwner(t *testing.T) {
-	repos, count, err = repo_model.SearchRepositoryByName(t.Context(), repo_model.SearchRepoOptions{OwnerID: unittest.NonexistentID})
+	repos, count, err := repo_model.SearchRepositoryByName(t.Context(), repo_model.SearchRepoOptions{OwnerID: unittest.NonexistentID})
 
 	assert.NoError(t, err)
 	assert.Empty(t, repos)
 	assert.Equal(t, int64(0), count)
+}
 
-	// Test search within description
+func testSearchRepositoryWithInDescription(t *testing.T) {
-	repos, count, err = repo_model.SearchRepository(t.Context(), repo_model.SearchRepoOptions{
+	repos, count, err := repo_model.SearchRepository(t.Context(), repo_model.SearchRepoOptions{
 		ListOptions: db.ListOptions{
 			Page:     1,
 			PageSize: 10,
@@ -266,9 +327,10 @@ func TestSearchRepository(t *testing.T) {
 		assert.Equal(t, "test_repo_14", repos[0].Name)
 	}
 	assert.Equal(t, int64(1), count)
+}
 
-	// Test NOT search within description
+func testSearchRepositoryNotInDescription(t *testing.T) {
-	repos, count, err = repo_model.SearchRepository(t.Context(), repo_model.SearchRepoOptions{
+	repos, count, err := repo_model.SearchRepository(t.Context(), repo_model.SearchRepoOptions{
 		ListOptions: db.ListOptions{
 			Page:     1,
 			PageSize: 10,
@@ -281,7 +343,9 @@ func TestSearchRepository(t *testing.T) {
 	assert.NoError(t, err)
 	assert.Empty(t, repos)
 	assert.Equal(t, int64(0), count)
+}
 
+func testSearchRepositoryCases(t *testing.T) {
 	testCases := getTestCases()
 
 	for _, testCase := range testCases {