mirror of
https://codeberg.org/forgejo/forgejo.git
synced 2026-05-12 22:10:25 +00:00
fix: enforce package quota against package owner, not uploader (#11442)
## What is broken
Quota on packages is not enforced when pushing to an organisation.
`enforcePackagesQuota()` calls `EvaluateForUser(ctx.Doer.ID, ...)` — it checks how much space the **uploader** personally owns, not the org being pushed to. Since packages accumulate under `package.owner_id = org_id`, the uploader always shows 0 bytes used and the check always passes.
This also means site admins bypass quota entirely when pushing to orgs (they get the service-layer admin bypass on top of the 0-byte measurement).
OCI/container routes (`/v2/...`) have the same problem but worse — `enforcePackagesQuota()` was not called on them at all.
## Fix
Check quota against `ctx.Package.Owner.ID` instead of `ctx.Doer.ID`. The package owner (the org or user being pushed to) is already available via `ctx.Package.Owner`, populated by `PackageAssignment()` before this middleware runs.
For individual user namespaces nothing changes — `ctx.Package.Owner` is the user themselves.
Also wired `enforcePackagesQuota()` into the missing OCI upload routes: `InitiateUploadBlob`, `UploadBlob`, `EndUploadBlob`, `UploadManifest` — both in the named `/{image}` group and the wildcard `/*` handler.
## Tested
Kind cluster, org `maw2` with 1 GiB quota, 2.6 GiB of container images already pushed:
- pushing a generic package to `maw2` as SA user → was 201, now 413
- pushing a generic package to `maw2` as `gitea_admin` → was 201, now 413
- initiating OCI blob upload to `maw2` as SA user → was 202, now 413
- pushing to own user namespace within quota → still 201
Co-authored-by: azhluwi <lukasz.widera@convotis.ch>
Reviewed-on: https://codeberg.org/forgejo/forgejo/pulls/11442
Reviewed-by: Andreas Ahlenstorf <aahlenst@noreply.codeberg.org>
Reviewed-by: Mathieu Fenniak <mfenniak@noreply.codeberg.org>
Co-authored-by: wejdross <wejdross@noreply.codeberg.org>
Co-committed-by: wejdross <wejdross@noreply.codeberg.org>
This commit is contained in:
parent
3934e5fea3
commit
cf51d3c888
2 changed files with 190 additions and 6 deletions
|
|
@ -94,7 +94,14 @@ func reqPackageAccess(accessMode perm.AccessMode) func(ctx *context.Context) {
|
|||
|
||||
func enforcePackagesQuota() func(ctx *context.Context) {
|
||||
return func(ctx *context.Context) {
|
||||
ok, err := quota_model.EvaluateForUser(ctx, ctx.Doer.ID, quota_model.LimitSubjectSizeAssetsPackagesAll)
|
||||
// Evaluate quota against the package owner (org or user the package is pushed to),
|
||||
// not the uploader (ctx.Doer). This enables org-level quota: all members uploading
|
||||
// to an org consume from the org's quota group, not their own personal quota.
|
||||
ownerID := ctx.Doer.ID
|
||||
if ctx.Package != nil {
|
||||
ownerID = ctx.Package.Owner.ID
|
||||
}
|
||||
ok, err := quota_model.EvaluateForUser(ctx, ownerID, quota_model.LimitSubjectSizeAssetsPackagesAll)
|
||||
if err != nil {
|
||||
log.Error("quota_model.EvaluateForUser: %v", err)
|
||||
ctx.Error(http.StatusInternalServerError, "Error checking quota")
|
||||
|
|
@ -793,11 +800,11 @@ func ContainerRoutes() *web.Route {
|
|||
r.Group("/{username}", func() {
|
||||
r.Group("/{image}", func() {
|
||||
r.Group("/blobs/uploads", func() {
|
||||
r.Post("", container.InitiateUploadBlob)
|
||||
r.Post("", enforcePackagesQuota(), container.InitiateUploadBlob)
|
||||
r.Group("/{uuid}", func() {
|
||||
r.Get("", container.GetUploadBlob)
|
||||
r.Patch("", container.UploadBlob)
|
||||
r.Put("", container.EndUploadBlob)
|
||||
r.Patch("", enforcePackagesQuota(), container.UploadBlob)
|
||||
r.Put("", enforcePackagesQuota(), container.EndUploadBlob)
|
||||
r.Delete("", container.CancelUploadBlob)
|
||||
})
|
||||
}, reqPackageAccess(perm.AccessModeWrite))
|
||||
|
|
@ -807,7 +814,7 @@ func ContainerRoutes() *web.Route {
|
|||
r.Delete("", reqPackageAccess(perm.AccessModeWrite), container.DeleteBlob)
|
||||
})
|
||||
r.Group("/manifests/{reference}", func() {
|
||||
r.Put("", reqPackageAccess(perm.AccessModeWrite), container.UploadManifest)
|
||||
r.Put("", reqPackageAccess(perm.AccessModeWrite), enforcePackagesQuota(), container.UploadManifest)
|
||||
r.Head("", container.HeadManifest)
|
||||
r.Get("", container.GetManifest)
|
||||
r.Delete("", reqPackageAccess(perm.AccessModeWrite), container.DeleteManifest)
|
||||
|
|
@ -843,6 +850,10 @@ func ContainerRoutes() *web.Route {
|
|||
return
|
||||
}
|
||||
|
||||
enforcePackagesQuota()(ctx)
|
||||
if ctx.Written() {
|
||||
return
|
||||
}
|
||||
container.InitiateUploadBlob(ctx)
|
||||
return
|
||||
}
|
||||
|
|
@ -875,8 +886,16 @@ func ContainerRoutes() *web.Route {
|
|||
if isGet {
|
||||
container.GetUploadBlob(ctx)
|
||||
} else if isPatch {
|
||||
enforcePackagesQuota()(ctx)
|
||||
if ctx.Written() {
|
||||
return
|
||||
}
|
||||
container.UploadBlob(ctx)
|
||||
} else if isPut {
|
||||
enforcePackagesQuota()(ctx)
|
||||
if ctx.Written() {
|
||||
return
|
||||
}
|
||||
container.EndUploadBlob(ctx)
|
||||
} else {
|
||||
container.CancelUploadBlob(ctx)
|
||||
|
|
@ -926,6 +945,10 @@ func ContainerRoutes() *web.Route {
|
|||
return
|
||||
}
|
||||
if isPut {
|
||||
enforcePackagesQuota()(ctx)
|
||||
if ctx.Written() {
|
||||
return
|
||||
}
|
||||
container.UploadManifest(ctx)
|
||||
} else {
|
||||
container.DeleteManifest(ctx)
|
||||
|
|
|
|||
|
|
@ -5,6 +5,7 @@ package integration
|
|||
|
||||
import (
|
||||
"bytes"
|
||||
"crypto/sha256"
|
||||
"encoding/base64"
|
||||
"fmt"
|
||||
"io"
|
||||
|
|
@ -402,7 +403,7 @@ func testAPIQuotaEnforcement(t *testing.T) {
|
|||
|
||||
t.Run("#/orgs/{org}/repos", func(t *testing.T) {
|
||||
defer tests.PrintCurrentTest(t)()
|
||||
defer env.SetRuleLimit(t, "all", 0)
|
||||
defer env.SetRuleLimit(t, "all", 0)()
|
||||
|
||||
assertCreateRepo := func(t *testing.T, orgName, repoName string, expectedStatus int) func() {
|
||||
t.Helper()
|
||||
|
|
@ -1285,6 +1286,166 @@ func testAPIQuotaEnforcement(t *testing.T) {
|
|||
env.User.Session.MakeRequest(t, req, http.StatusNoContent)
|
||||
})
|
||||
})
|
||||
|
||||
// verify that package upload quota is evaluated against the package owner, not the uploader
|
||||
t.Run("#/packages/{org}/quota-enforcement-against-owner", func(t *testing.T) {
|
||||
defer tests.PrintCurrentTest(t)()
|
||||
// Ensure the user's own quota is unlimited for this block; prior tests may have left it at 0.
|
||||
defer env.SetRuleLimit(t, "all", -1)()
|
||||
|
||||
t.Run("upload to limited org is rejected", func(t *testing.T) {
|
||||
defer tests.PrintCurrentTest(t)()
|
||||
|
||||
// Verify the user has quota remaining, so rejection is due to org quota
|
||||
req := NewRequest(t, "GET", "/api/v1/user/quota/check?subject=size:assets:packages:all").AddTokenAuth(env.User.Token)
|
||||
resp := env.User.Session.MakeRequest(t, req, http.StatusOK)
|
||||
var quotaOK bool
|
||||
DecodeJSON(t, resp, "aOK)
|
||||
assert.True(t, quotaOK, "user must have quota remaining before the rejection test")
|
||||
|
||||
body := strings.NewReader("forgejo is awesome")
|
||||
req = NewRequestWithBody(t, "PUT",
|
||||
fmt.Sprintf("/api/packages/%s/generic/org-quota-test/1.0.0/file.txt", env.Orgs.Limited.Name),
|
||||
body,
|
||||
).AddTokenAuth(env.User.Token)
|
||||
env.User.Session.MakeRequest(t, req, http.StatusRequestEntityTooLarge)
|
||||
})
|
||||
|
||||
t.Run("upload to unlimited org succeeds even when user quota is zero", func(t *testing.T) {
|
||||
defer tests.PrintCurrentTest(t)()
|
||||
|
||||
defer env.SetRuleLimit(t, "all", 0)()
|
||||
|
||||
// Verify the user's quota is zero before the upload
|
||||
req := NewRequest(t, "GET", "/api/v1/user/quota/check?subject=size:assets:packages:all").AddTokenAuth(env.User.Token)
|
||||
resp := env.User.Session.MakeRequest(t, req, http.StatusOK)
|
||||
var quotaOK bool
|
||||
DecodeJSON(t, resp, "aOK)
|
||||
assert.False(t, quotaOK, "user must have no quota before the upload test")
|
||||
|
||||
body := strings.NewReader("forgejo is awesome")
|
||||
req = NewRequestWithBody(t, "PUT",
|
||||
fmt.Sprintf("/api/packages/%s/generic/org-quota-test/1.0.0/file.txt", env.Orgs.Unlimited.Name),
|
||||
body,
|
||||
).AddTokenAuth(env.User.Token)
|
||||
env.User.Session.MakeRequest(t, req, http.StatusCreated)
|
||||
|
||||
env.WithoutQuota(t, func() {
|
||||
req := NewRequestf(t, "DELETE", "/api/v1/packages/%s/generic/org-quota-test/1.0.0", env.Orgs.Unlimited.Name).
|
||||
AddTokenAuth(env.User.Token)
|
||||
env.User.Session.MakeRequest(t, req, http.StatusNoContent)
|
||||
})
|
||||
})
|
||||
})
|
||||
|
||||
t.Run("#/v2/{org}/container/quota-enforcement-against-owner", func(t *testing.T) {
|
||||
defer tests.PrintCurrentTest(t)()
|
||||
// Ensure the user's own quota is unlimited for this block; prior tests may have left it at 0.
|
||||
defer env.SetRuleLimit(t, "all", -1)()
|
||||
|
||||
type tokenResponse struct {
|
||||
Token string `json:"token"`
|
||||
}
|
||||
|
||||
getContainerToken := func(t *testing.T, username string) string {
|
||||
t.Helper()
|
||||
req := NewRequest(t, "GET", fmt.Sprintf("%sv2/token", setting.AppURL)).
|
||||
AddBasicAuth(username)
|
||||
resp := MakeRequest(t, req, http.StatusOK)
|
||||
var tr tokenResponse
|
||||
DecodeJSON(t, resp, &tr)
|
||||
return fmt.Sprintf("Bearer %s", tr.Token)
|
||||
}
|
||||
|
||||
blobContent := []byte("quota-container-blob")
|
||||
blobDigest := fmt.Sprintf("sha256:%x", sha256.Sum256(blobContent))
|
||||
configContent := `{}`
|
||||
configDigest := fmt.Sprintf("sha256:%x", sha256.Sum256([]byte(configContent)))
|
||||
manifestContent := fmt.Sprintf(`{"schemaVersion":2,"mediaType":"application/vnd.docker.distribution.manifest.v2+json","config":{"mediaType":"application/vnd.docker.container.image.v1+json","digest":%q,"size":%d},"layers":[{"mediaType":"application/vnd.docker.image.rootfs.diff.tar.gzip","digest":%q,"size":%d}]}`,
|
||||
configDigest, len(configContent), blobDigest, len(blobContent))
|
||||
|
||||
userToken := getContainerToken(t, env.User.User.Name)
|
||||
|
||||
t.Run("upload to limited org is rejected", func(t *testing.T) {
|
||||
defer tests.PrintCurrentTest(t)()
|
||||
|
||||
// Verify the user has quota remaining, so rejection is due to org quota
|
||||
req := NewRequest(t, "GET", "/api/v1/user/quota/check?subject=size:assets:packages:all").AddTokenAuth(env.User.Token)
|
||||
resp := env.User.Session.MakeRequest(t, req, http.StatusOK)
|
||||
var quotaOK bool
|
||||
DecodeJSON(t, resp, "aOK)
|
||||
assert.True(t, quotaOK, "user must have quota remaining before the rejection test")
|
||||
|
||||
image := fmt.Sprintf("%sv2/%s/quota-test-img", setting.AppURL, env.Orgs.Limited.Name)
|
||||
|
||||
req = NewRequestWithBody(t, "POST", fmt.Sprintf("%s/blobs/uploads?digest=%s", image, blobDigest), bytes.NewReader(blobContent)).
|
||||
AddTokenAuth(userToken)
|
||||
MakeRequest(t, req, http.StatusRequestEntityTooLarge)
|
||||
|
||||
req = NewRequest(t, "POST", fmt.Sprintf("%s/blobs/uploads", image)).
|
||||
AddTokenAuth(userToken)
|
||||
resp = MakeRequest(t, req, http.StatusRequestEntityTooLarge)
|
||||
|
||||
// chunked upload: initiate returns 413, so patch/put are never reached
|
||||
// this is the result we expect, otherwise it's very hard to create multi-tenant environment
|
||||
_ = resp
|
||||
})
|
||||
|
||||
t.Run("upload to unlimited org succeeds even when user quota is zero", func(t *testing.T) {
|
||||
defer tests.PrintCurrentTest(t)()
|
||||
|
||||
defer env.SetRuleLimit(t, "all", 0)()
|
||||
|
||||
// Verify the user's quota is zero before the upload
|
||||
req := NewRequest(t, "GET", "/api/v1/user/quota/check?subject=size:assets:packages:all").AddTokenAuth(env.User.Token)
|
||||
resp := env.User.Session.MakeRequest(t, req, http.StatusOK)
|
||||
var quotaOK bool
|
||||
DecodeJSON(t, resp, "aOK)
|
||||
assert.False(t, quotaOK, "user must have no quota before the upload test")
|
||||
|
||||
image := fmt.Sprintf("%sv2/%s/quota-test-img", setting.AppURL, env.Orgs.Unlimited.Name)
|
||||
|
||||
// monolithic blob upload
|
||||
req = NewRequestWithBody(t, "POST", fmt.Sprintf("%s/blobs/uploads?digest=%s", image, blobDigest), bytes.NewReader(blobContent)).
|
||||
AddTokenAuth(userToken)
|
||||
MakeRequest(t, req, http.StatusCreated)
|
||||
|
||||
req = NewRequestWithBody(t, "POST", fmt.Sprintf("%s/blobs/uploads?digest=%s", image, configDigest), strings.NewReader(configContent)).
|
||||
AddTokenAuth(userToken)
|
||||
MakeRequest(t, req, http.StatusCreated)
|
||||
|
||||
// chunked blob upload (patch + put)
|
||||
req = NewRequest(t, "POST", fmt.Sprintf("%s/blobs/uploads", image)).
|
||||
AddTokenAuth(userToken)
|
||||
resp = MakeRequest(t, req, http.StatusAccepted)
|
||||
|
||||
uploadURL := resp.Header().Get("Location")
|
||||
contentRange := fmt.Sprintf("0-%d", len(blobContent)-1)
|
||||
req = NewRequestWithBody(t, "PATCH", setting.AppURL+uploadURL[1:], bytes.NewReader(blobContent)).
|
||||
AddTokenAuth(userToken).
|
||||
SetHeader("Content-Range", contentRange)
|
||||
resp = MakeRequest(t, req, http.StatusAccepted)
|
||||
|
||||
uploadURL = resp.Header().Get("Location")
|
||||
req = NewRequest(t, "PUT", fmt.Sprintf("%s?digest=%s", setting.AppURL+uploadURL[1:], blobDigest)).
|
||||
AddTokenAuth(userToken)
|
||||
MakeRequest(t, req, http.StatusCreated)
|
||||
|
||||
// upload manifest
|
||||
req = NewRequestWithBody(t, "PUT", fmt.Sprintf("%s/manifests/v1", image), strings.NewReader(manifestContent)).
|
||||
AddTokenAuth(userToken).
|
||||
SetHeader("Content-Type", "application/vnd.docker.distribution.manifest.v2+json")
|
||||
MakeRequest(t, req, http.StatusCreated)
|
||||
|
||||
// delete manifest
|
||||
env.WithoutQuota(t, func() {
|
||||
manifestDigest := fmt.Sprintf("sha256:%x", sha256.Sum256([]byte(manifestContent)))
|
||||
req := NewRequest(t, "DELETE", fmt.Sprintf("%s/manifests/%s", image, manifestDigest)).
|
||||
AddTokenAuth(userToken)
|
||||
MakeRequest(t, req, http.StatusAccepted)
|
||||
})
|
||||
})
|
||||
})
|
||||
}
|
||||
|
||||
func TestAPIQuotaOrgQuotaQuery(t *testing.T) {
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue