Improve permission check of packages

models/fixtures/org_user.yml

@@ -75,3 +75,9 @@
   uid: 31
   org_id: 19
   is_public: true
+
+-
+  id: 14
+  uid: 5
+  org_id: 23
+  is_public: false

models/fixtures/team.yml

@@ -172,4 +172,15 @@
   num_repos: 0
   num_members: 0
   includes_all_repositories: false
+  can_create_org_repo: true
+
+-
+  id: 17
+  org_id: 23
+  lower_name: team14writeauth
+  name: team14WriteAuth
+  authorize: 2 # write
+  num_repos: 0
+  num_members: 1
+  includes_all_repositories: false
   can_create_org_repo: true

models/fixtures/team_unit.yml

@@ -268,3 +268,9 @@
   team_id: 9
   type: 1 # code
   access_mode: 1
+
+-
+  id: 46
+  team_id: 17
+  type: 9 # package
+  access_mode: 0

models/fixtures/team_user.yml

@@ -99,3 +99,9 @@
   org_id: 3
   team_id: 14
   uid: 2
+
+-
+  id: 18
+  org_id: 23
+  team_id: 17
+  uid: 5

modules/context/package.go

@@ -92,33 +92,25 @@ func determineAccessMode(ctx *Context) (perm.AccessMode, error) {
 		return perm.AccessModeNone, nil
 	}
 
+	// TODO: ActionUser permission check
 	accessMode := perm.AccessModeNone
 	if ctx.Package.Owner.IsOrganization() {
 		org := organization.OrgFromUser(ctx.Package.Owner)
 
-		// 1. Get user max authorize level for the org (may be none, if user is not member of the org)
-		if ctx.Doer != nil {
-			var err error
-			accessMode, err = org.GetOrgUserMaxAuthorizeLevel(ctx.Doer.ID)
+		if ctx.Doer != nil && !ctx.Doer.IsGhost() {
+			// 1. If user is logined, check all team packages permissions
+			teams, err := organization.GetUserOrgTeams(ctx, org.ID, ctx.Doer.ID)
 			if err != nil {
 				return accessMode, err
 			}
-			// If access mode is less than write check every team for more permissions
-			if accessMode < perm.AccessModeWrite {
-				teams, err := organization.GetUserOrgTeams(ctx, org.ID, ctx.Doer.ID)
-				if err != nil {
-					return accessMode, err
-				}
-				for _, t := range teams {
-					perm := t.UnitAccessMode(ctx, unit.TypePackages)
-					if accessMode < perm {
-						accessMode = perm
-					}
+			for _, t := range teams {
+				perm := t.UnitAccessMode(ctx, unit.TypePackages)
+				if accessMode < perm {
+					accessMode = perm
 				}
 			}
-		}
-		// 2. If authorize level is none, check if org is visible to user
-		if accessMode == perm.AccessModeNone && organization.HasOrgOrUserVisible(ctx, ctx.Package.Owner, ctx.Doer) {
+		} else if organization.HasOrgOrUserVisible(ctx, ctx.Package.Owner, ctx.Doer) {
+			// 2. If user is non-login, check if org is visible to non-login user
 			accessMode = perm.AccessModeRead
 		}
 	} else {

tests/integration/api_packages_test.go

@@ -157,6 +157,7 @@ func TestPackageAccess(t *testing.T) {
 	admin := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 1})
 	user := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 5})
 	inactive := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 9})
+	privatedOrg := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 23})
 
 	uploadPackage := func(doer, owner *user_model.User, expectedStatus int) {
 		url := fmt.Sprintf("/api/packages/%s/generic/test-package/1.0/file.bin", owner.Name)
@@ -170,6 +171,15 @@ func TestPackageAccess(t *testing.T) {
 	uploadPackage(inactive, user, http.StatusUnauthorized)
 	uploadPackage(admin, inactive, http.StatusCreated)
 	uploadPackage(admin, user, http.StatusCreated)
+
+	// team.authorize is write, but team_unit.access_mode is none
+	// so the user can not upload packages or get package list
+	uploadPackage(user, privatedOrg, http.StatusUnauthorized)
+
+	session := loginUser(t, user.Name)
+	tokenReadPackage := getTokenForLoggedInUser(t, session, auth_model.AccessTokenScopeReadPackage)
+	req := NewRequest(t, "GET", fmt.Sprintf("/api/v1/packages/%s?token=%s", privatedOrg.Name, tokenReadPackage))
+	MakeRequest(t, req, http.StatusForbidden)
 }
 
 func TestPackageQuota(t *testing.T) {