Fix package permission(Read and Write) is not as expected

[littleplus]

I create an access token with package(Read and Write) permission from web:

Client Log:

DEBU[0000] endpoints for <Domain>/<Repo>/<Module>:20240508-8654cc5-linux-amd64: [{false https://<Domain>v2 false false true 0xc000492680}]
DEBU[0001] continuing on error (errcode.Error) unauthorized: reqPackageAccess
no such manifest: <Domain>/<Repo>/<Module>:20240508-8654cc5-linux-amd64

Server Log:

2024/05/08 02:05:58 ...eb/routing/logger.go:102:func1() [I] router: completed GET /v2/<Repo>/<Module>/manifests/20240508-8654cc5-linux-amd64 for <Client-IP>:0, 401 Unauthorized in 1.1ms @ packages/api.go:43(packages.ContainerRoutes.reqPackageAccess)

However, I can't pull the image with the same token, and I find the code, where write token is not permitted to do read package:

gitea/routers/api/packages/api.go

Line 48 in d9b37d0

if accessMode == perm.AccessModeRead {

[lunny]

Good catch. Can you add some tests for the PR?

[littleplus]

@lunny Where the test file should I create in?

Is it /routers/api/packages/api_test.go ?

[wxiaoguang]

Ideally the logic could be:

if accessMode == read {
    scopeMatched = hasScope(Read) || hasScope(Write)
} else if accessMode == write {
    scopeMatched = hasWrite
}

It is clearer than if accessMode == perm.AccessModeWrite || !scopeMatched

[littleplus]

@wxiaoguang Of course, I have the same idea before, and I also notice that the duplicated code will bring more unmaintainability while trying to edit the logic of func hasScope, and maybe create funcs called hasAllScopes and hasOneOfScopes should be the more reusable way.

[wxiaoguang]

Ping the original author @jolheiser

[lunny]

@lunny Where the test file should I create in?

Is it /routers/api/packages/api_test.go ?

For unit test, it's OK.

[wxiaoguang]

However, I can't pull the image with the same token, and I find the code, where write token is not permitted to do read package:

By reading the code:

	accessTokenScopeReadPackageBits  accessTokenScopeBitmap = 1 << iota
	accessTokenScopeWritePackageBits accessTokenScopeBitmap = 1<<iota | accessTokenScopeReadPackageBits

So package:write should already allow "read". And this test succeeds:

func TestTemp(t *testing.T) {
	matched, err := AccessTokenScopeWritePackage.HasScope(AccessTokenScopeReadPackage)
	assert.NoError(t, err)
	assert.True(t, matched)
}

So the existing logic looks right. Could you explain why it doesn't work in your case?

[littleplus]

Yes, the code seem right, and I lost my first key and can not reproduce with new key. Looks like it's my mistake, I will close it first before I find the problem.