Fix org permission API visibility checks for hidden members and private orgs (#36798)

routers/api/v1/org/org.go

@@ -135,7 +135,7 @@ func GetUserOrgsPermissions(ctx *context.APIContext) {
 
 	op := api.OrganizationPermissions{}
 
-	if !organization.HasOrgOrUserVisible(ctx, o, ctx.ContextUser) {
+	if !organization.HasOrgOrUserVisible(ctx, o, ctx.Doer) {
 		ctx.APIErrorNotFound("HasOrgOrUserVisible", nil)
 		return
 	}

tests/integration/api_user_org_perm_test.go

@@ -15,23 +15,34 @@ import (
 	"github.com/stretchr/testify/assert"
 )
 
+func TestPermissionsAPI(t *testing.T) {
+	defer tests.PrepareTestEnv(t)()
+
+	t.Run("TokenNeeded", testTokenNeeded)
+	t.Run("WithOwnerUser", testWithOwnerUser)
+	t.Run("CanWriteUser", testCanWriteUser)
+	t.Run("AdminUser", testAdminUser)
+	t.Run("AdminCanNotCreateRepo", testAdminCanNotCreateRepo)
+	t.Run("CanReadUser", testCanReadUser)
+	t.Run("UnknownUser", testUnknownUser)
+	t.Run("UnknownOrganization", testUnknownOrganization)
+	t.Run("HiddenMemberPermissionsForbidden", testHiddenMemberPermissionsForbidden)
+	t.Run("PrivateOrgPermissionsNotFound", testPrivateOrgPermissionsNotFound)
+}
+
 type apiUserOrgPermTestCase struct {
 	LoginUser                       string
 	User                            string
 	Organization                    string
 	ExpectedOrganizationPermissions api.OrganizationPermissions
 }
 
-func TestTokenNeeded(t *testing.T) {
-	defer tests.PrepareTestEnv(t)()
-
+func testTokenNeeded(t *testing.T) {
 	req := NewRequest(t, "GET", "/api/v1/users/user1/orgs/org6/permissions")
 	MakeRequest(t, req, http.StatusUnauthorized)
 }
 
 func sampleTest(t *testing.T, auoptc apiUserOrgPermTestCase) {
-	defer tests.PrepareTestEnv(t)()
-
 	session := loginUser(t, auoptc.LoginUser)
 	token := getTokenForLoggedInUser(t, session, auth_model.AccessTokenScopeReadOrganization, auth_model.AccessTokenScopeReadUser)
 
@@ -48,7 +59,7 @@ func sampleTest(t *testing.T, auoptc apiUserOrgPermTestCase) {
 	assert.Equal(t, auoptc.ExpectedOrganizationPermissions.CanCreateRepository, apiOP.CanCreateRepository)
 }
 
-func TestWithOwnerUser(t *testing.T) {
+func testWithOwnerUser(t *testing.T) {
 	sampleTest(t, apiUserOrgPermTestCase{
 		LoginUser:    "user2",
 		User:         "user2",
@@ -63,7 +74,7 @@ func TestWithOwnerUser(t *testing.T) {
 	})
 }
 
-func TestCanWriteUser(t *testing.T) {
+func testCanWriteUser(t *testing.T) {
 	sampleTest(t, apiUserOrgPermTestCase{
 		LoginUser:    "user4",
 		User:         "user4",
@@ -78,7 +89,7 @@ func TestCanWriteUser(t *testing.T) {
 	})
 }
 
-func TestAdminUser(t *testing.T) {
+func testAdminUser(t *testing.T) {
 	sampleTest(t, apiUserOrgPermTestCase{
 		LoginUser:    "user1",
 		User:         "user28",
@@ -93,7 +104,7 @@ func TestAdminUser(t *testing.T) {
 	})
 }
 
-func TestAdminCanNotCreateRepo(t *testing.T) {
+func testAdminCanNotCreateRepo(t *testing.T) {
 	sampleTest(t, apiUserOrgPermTestCase{
 		LoginUser:    "user1",
 		User:         "user28",
@@ -108,7 +119,7 @@ func TestAdminCanNotCreateRepo(t *testing.T) {
 	})
 }
 
-func TestCanReadUser(t *testing.T) {
+func testCanReadUser(t *testing.T) {
 	sampleTest(t, apiUserOrgPermTestCase{
 		LoginUser:    "user1",
 		User:         "user24",
@@ -123,9 +134,7 @@ func TestCanReadUser(t *testing.T) {
 	})
 }
 
-func TestUnknowUser(t *testing.T) {
-	defer tests.PrepareTestEnv(t)()
-
+func testUnknownUser(t *testing.T) {
 	session := loginUser(t, "user1")
 	token := getTokenForLoggedInUser(t, session, auth_model.AccessTokenScopeReadUser, auth_model.AccessTokenScopeReadOrganization)
 
@@ -138,9 +147,7 @@ func TestUnknowUser(t *testing.T) {
 	assert.Equal(t, "user redirect does not exist [name: unknow]", apiError.Message)
 }
 
-func TestUnknowOrganization(t *testing.T) {
-	defer tests.PrepareTestEnv(t)()
-
+func testUnknownOrganization(t *testing.T) {
 	session := loginUser(t, "user1")
 	token := getTokenForLoggedInUser(t, session, auth_model.AccessTokenScopeReadUser, auth_model.AccessTokenScopeReadOrganization)
 
@@ -151,3 +158,38 @@ func TestUnknowOrganization(t *testing.T) {
 	DecodeJSON(t, resp, &apiError)
 	assert.Equal(t, "GetUserByName", apiError.Message)
 }
+
+func testHiddenMemberPermissionsForbidden(t *testing.T) {
+	session := loginUser(t, "user8")
+	token := getTokenForLoggedInUser(t, session, auth_model.AccessTokenScopeReadUser, auth_model.AccessTokenScopeReadOrganization)
+
+	req := NewRequest(t, "GET", "/api/v1/users/user5/orgs/privated_org/permissions").
+		AddTokenAuth(token)
+	MakeRequest(t, req, http.StatusNotFound)
+
+	adminSession := loginUser(t, "user1")
+	adminToken := getTokenForLoggedInUser(t, adminSession, auth_model.AccessTokenScopeReadUser, auth_model.AccessTokenScopeReadOrganization)
+
+	adminReq := NewRequest(t, "GET", "/api/v1/users/user5/orgs/privated_org/permissions").
+		AddTokenAuth(adminToken)
+	resp := MakeRequest(t, adminReq, http.StatusOK)
+
+	var apiOP api.OrganizationPermissions
+	DecodeJSON(t, resp, &apiOP)
+	assert.Equal(t, api.OrganizationPermissions{
+		IsOwner:             false,
+		IsAdmin:             false,
+		CanWrite:            true,
+		CanRead:             true,
+		CanCreateRepository: true,
+	}, apiOP)
+}
+
+func testPrivateOrgPermissionsNotFound(t *testing.T) {
+	session := loginUser(t, "user8")
+	token := getTokenForLoggedInUser(t, session, auth_model.AccessTokenScopeReadUser, auth_model.AccessTokenScopeReadOrganization)
+
+	req := NewRequest(t, "GET", "/api/v1/users/user5/orgs/privated_org/permissions").
+		AddTokenAuth(token)
+	MakeRequest(t, req, http.StatusNotFound)
+}