Skip to content

Commit 7c11a73

Browse files
KN4CK3Rlafriks
andauthored
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 <[email protected]>
1 parent 49a4464 commit 7c11a73

File tree

4 files changed

+34
-3
lines changed

4 files changed

+34
-3
lines changed

Diff for: modules/context/package.go

+6-3
Original file line numberDiff line numberDiff line change
@@ -85,12 +85,15 @@ func packageAssignment(ctx *Context, errCb func(int, string, interface{})) {
8585
}
8686

8787
func determineAccessMode(ctx *Context) (perm.AccessMode, error) {
88-
accessMode := perm.AccessModeNone
89-
9088
if setting.Service.RequireSignInView && ctx.Doer == nil {
91-
return accessMode, nil
89+
return perm.AccessModeNone, nil
9290
}
9391

92+
if ctx.Doer != nil && !ctx.Doer.IsGhost() && (!ctx.Doer.IsActive || ctx.Doer.ProhibitLogin) {
93+
return perm.AccessModeNone, nil
94+
}
95+
96+
accessMode := perm.AccessModeNone
9497
if ctx.Package.Owner.IsOrganization() {
9598
org := organization.OrgFromUser(ctx.Package.Owner)
9699

Diff for: routers/api/packages/api.go

+2
Original file line numberDiff line numberDiff line change
@@ -58,6 +58,7 @@ func Routes(ctx gocontext.Context) *web.Route {
5858
authGroup := auth.NewGroup(authMethods...)
5959
r.Use(func(ctx *context.Context) {
6060
ctx.Doer = authGroup.Verify(ctx.Req, ctx.Resp, ctx, ctx.Session)
61+
ctx.IsSigned = ctx.Doer != nil
6162
})
6263

6364
r.Group("/{username}", func() {
@@ -316,6 +317,7 @@ func ContainerRoutes(ctx gocontext.Context) *web.Route {
316317
authGroup := auth.NewGroup(authMethods...)
317318
r.Use(func(ctx *context.Context) {
318319
ctx.Doer = authGroup.Verify(ctx.Req, ctx.Resp, ctx, ctx.Session)
320+
ctx.IsSigned = ctx.Doer != nil
319321
})
320322

321323
r.Get("", container.ReqContainerAccess, container.DetermineSupport)

Diff for: tests/integration/api_packages_container_test.go

+4
Original file line numberDiff line numberDiff line change
@@ -471,6 +471,10 @@ func TestPackageContainer(t *testing.T) {
471471

472472
assert.Equal(t, fmt.Sprintf("%d", len(blobContent)), resp.Header().Get("Content-Length"))
473473
assert.Equal(t, blobDigest, resp.Header().Get("Docker-Content-Digest"))
474+
475+
req = NewRequest(t, "HEAD", fmt.Sprintf("%s/blobs/%s", url, blobDigest))
476+
addTokenAuthHeader(req, anonymousToken)
477+
MakeRequest(t, req, http.StatusOK)
474478
})
475479

476480
t.Run("GetBlob", func(t *testing.T) {

Diff for: tests/integration/api_packages_test.go

+22
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,7 @@ import (
2525

2626
func TestPackageAPI(t *testing.T) {
2727
defer tests.PrepareTestEnv(t)()
28+
2829
user := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 4})
2930
session := loginUser(t, user.Name)
3031
token := getTokenForLoggedInUser(t, session)
@@ -144,6 +145,27 @@ func TestPackageAPI(t *testing.T) {
144145
})
145146
}
146147

148+
func TestPackageAccess(t *testing.T) {
149+
defer tests.PrepareTestEnv(t)()
150+
151+
admin := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 1})
152+
user := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 5})
153+
inactive := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 9})
154+
155+
uploadPackage := func(doer, owner *user_model.User, expectedStatus int) {
156+
url := fmt.Sprintf("/api/packages/%s/generic/test-package/1.0/file.bin", owner.Name)
157+
req := NewRequestWithBody(t, "PUT", url, bytes.NewReader([]byte{1}))
158+
AddBasicAuthHeader(req, doer.Name)
159+
MakeRequest(t, req, expectedStatus)
160+
}
161+
162+
uploadPackage(user, inactive, http.StatusUnauthorized)
163+
uploadPackage(inactive, inactive, http.StatusUnauthorized)
164+
uploadPackage(inactive, user, http.StatusUnauthorized)
165+
uploadPackage(admin, inactive, http.StatusCreated)
166+
uploadPackage(admin, user, http.StatusCreated)
167+
}
168+
147169
func TestPackageCleanup(t *testing.T) {
148170
defer tests.PrepareTestEnv(t)()
149171

0 commit comments

Comments
 (0)