From 7c11a73833f3aa9783015e5e13871d3c298d3ef6 Mon Sep 17 00:00:00 2001
From: KN4CK3R <admin@oldschoolhack.me>
Date: Mon, 24 Oct 2022 21:23:25 +0200
Subject: [PATCH] Fix package access for admins and inactive users (#21580)
I noticed an admin is not allowed to upload packages for other users
because `ctx.IsSigned` was not set.
I added a check for `user.IsActive` and `user.ProhibitLogin` too because
both was not checked. Tests enforce this now.
Co-authored-by: Lauris BH <lauris@nix.lv>
---
modules/context/package.go | 9 +++++---
routers/api/packages/api.go | 2 ++
.../api_packages_container_test.go | 4 ++++
tests/integration/api_packages_test.go | 22 +++++++++++++++++++
4 files changed, 34 insertions(+), 3 deletions(-)
diff --git a/modules/context/package.go b/modules/context/package.go
index d12bdc49138..ce0f9a511b3 100644
--- a/modules/context/package.go
+++ b/modules/context/package.go
@@ -85,12 +85,15 @@ func packageAssignment(ctx *Context, errCb func(int, string, interface{})) {
}
func determineAccessMode(ctx *Context) (perm.AccessMode, error) {
- accessMode := perm.AccessModeNone
-
if setting.Service.RequireSignInView && ctx.Doer == nil {
- return accessMode, nil
+ return perm.AccessModeNone, nil
}
+ if ctx.Doer != nil && !ctx.Doer.IsGhost() && (!ctx.Doer.IsActive || ctx.Doer.ProhibitLogin) {
+ return perm.AccessModeNone, nil
+ }
+
+ accessMode := perm.AccessModeNone
if ctx.Package.Owner.IsOrganization() {
org := organization.OrgFromUser(ctx.Package.Owner)
diff --git a/routers/api/packages/api.go b/routers/api/packages/api.go
index 69c8dd6564b..6f53bc4ae0c 100644
--- a/routers/api/packages/api.go
+++ b/routers/api/packages/api.go
@@ -58,6 +58,7 @@ func Routes(ctx gocontext.Context) *web.Route {
authGroup := auth.NewGroup(authMethods...)
r.Use(func(ctx *context.Context) {
ctx.Doer = authGroup.Verify(ctx.Req, ctx.Resp, ctx, ctx.Session)
+ ctx.IsSigned = ctx.Doer != nil
})
r.Group("/{username}", func() {
@@ -316,6 +317,7 @@ func ContainerRoutes(ctx gocontext.Context) *web.Route {
authGroup := auth.NewGroup(authMethods...)
r.Use(func(ctx *context.Context) {
ctx.Doer = authGroup.Verify(ctx.Req, ctx.Resp, ctx, ctx.Session)
+ ctx.IsSigned = ctx.Doer != nil
})
r.Get("", container.ReqContainerAccess, container.DetermineSupport)
diff --git a/tests/integration/api_packages_container_test.go b/tests/integration/api_packages_container_test.go
index b8a3884371b..ba76ee4baa7 100644
--- a/tests/integration/api_packages_container_test.go
+++ b/tests/integration/api_packages_container_test.go
@@ -471,6 +471,10 @@ func TestPackageContainer(t *testing.T) {
assert.Equal(t, fmt.Sprintf("%d", len(blobContent)), resp.Header().Get("Content-Length"))
assert.Equal(t, blobDigest, resp.Header().Get("Docker-Content-Digest"))
+
+ req = NewRequest(t, "HEAD", fmt.Sprintf("%s/blobs/%s", url, blobDigest))
+ addTokenAuthHeader(req, anonymousToken)
+ MakeRequest(t, req, http.StatusOK)
})
t.Run("GetBlob", func(t *testing.T) {
diff --git a/tests/integration/api_packages_test.go b/tests/integration/api_packages_test.go
index 86d81994d4d..25f5b3f2a12 100644
--- a/tests/integration/api_packages_test.go
+++ b/tests/integration/api_packages_test.go
@@ -25,6 +25,7 @@ import (
func TestPackageAPI(t *testing.T) {
defer tests.PrepareTestEnv(t)()
+
user := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 4})
session := loginUser(t, user.Name)
token := getTokenForLoggedInUser(t, session)
@@ -144,6 +145,27 @@ func TestPackageAPI(t *testing.T) {
})
}
+func TestPackageAccess(t *testing.T) {
+ defer tests.PrepareTestEnv(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})
+
+ uploadPackage := func(doer, owner *user_model.User, expectedStatus int) {
+ url := fmt.Sprintf("/api/packages/%s/generic/test-package/1.0/file.bin", owner.Name)
+ req := NewRequestWithBody(t, "PUT", url, bytes.NewReader([]byte{1}))
+ AddBasicAuthHeader(req, doer.Name)
+ MakeRequest(t, req, expectedStatus)
+ }
+
+ uploadPackage(user, inactive, http.StatusUnauthorized)
+ uploadPackage(inactive, inactive, http.StatusUnauthorized)
+ uploadPackage(inactive, user, http.StatusUnauthorized)
+ uploadPackage(admin, inactive, http.StatusCreated)
+ uploadPackage(admin, user, http.StatusCreated)
+}
+
func TestPackageCleanup(t *testing.T) {
defer tests.PrepareTestEnv(t)()