feature/increase_test_coverage #112

Merged

alexvanin merged 3 commits from r.loginov/frostfs-http-gw:feature/increase_test_coverage into master

2024-05-14 11:32:47 +00:00

r.loginov commented

2024-05-06 17:54:11 +00:00

Collaborator

Increasing the test coverage of the token package
Adding integration tests using bearer token

Coverage:

tokens = 87.5 % -> 100 %

- Increasing the test coverage of the token package - Adding integration tests using bearer token Coverage: - tokens = 87.5 % -> 100 %

r.loginov self-assigned this 2024-05-06 17:54:11 +00:00

r.loginov added 2 commits 2024-05-06 17:54:12 +00:00

aaa13bc20f [##] tokens: Extend test coverage

Signed-off-by: Roman Loginov <r.loginov@yadro.com>

/ DCO (pull_request) Failing after 59s Details

/ Vulncheck (pull_request) Failing after 1m57s Details

/ Builds (1.20) (pull_request) Successful in 2m25s Details

/ Builds (1.21) (pull_request) Successful in 2m20s Details

/ Lint (pull_request) Successful in 4m7s Details

/ Tests (1.20) (pull_request) Successful in 2m12s Details

/ Tests (1.21) (pull_request) Successful in 1m39s Details

bc558e1d4e [##] Add integration test with bearer token

Signed-off-by: Roman Loginov <r.loginov@yadro.com>

r.loginov force-pushed feature/increase_test_coverage from bc558e1d4e to 2cbd9f47f6

2024-05-06 18:02:27 +00:00

Compare

r.loginov commented

2024-05-06 18:08:12 +00:00

Poster

Collaborator

I also wanted to keep the aio version 1.2.7, however, in this case, when the impersonate flag is set, the bearer token frostfs-node will give an error code = 1024 message = invalid signature (I understand that at that time there was not yet this flag). If I don't use this flag, I get an error from frostfs-node of the following type: code = 2048 message = access to object operation denied: access to operation OBJECT_PUT is denied by extended ACL check: bearer token owner differs from the request sender.
I form the token as follows:

diff --git a/cmd/http-gw/integration_test.go b/cmd/http-gw/integration_test.go  
index c22551c..44a2717 100644  
--- a/cmd/http-gw/integration_test.go  
+++ b/cmd/http-gw/integration_test.go  
@@ -1,5 +1,3 @@  
-//go:build integration  
-  
 package main  
   
 import (  
@@ -9,6 +7,7 @@ import (  
    "encoding/base64"  
    "encoding/json"  
    "fmt"  
+   "git.frostfs.info/TrueCloudLab/frostfs-sdk-go/eacl"  
    "io"  
    "mime/multipart"  
    "net/http"  
@@ -49,6 +48,7 @@ func TestIntegration(t *testing.T) {  
    rootCtx := context.Background()  
    aioImage := "truecloudlab/frostfs-aio:"  
    versions := []string{  
+      "1.2.7",  
       "1.3.0",  
    }  
    key, err := keys.NewPrivateKeyFromHex("1dd37fba80fec4e6a6f13fd708d8dcb3b29def768017052f6c930fa1c5d90bbb")  
@@ -66,7 +66,7 @@ func TestIntegration(t *testing.T) {  
       CID, err := createContainer(ctx, t, clientPool, ownerID, version)  
       require.NoError(t, err, version)  
   
-      token := makeBearerToken(t, key, ownerID)  
+      token := makeBearerToken(t, key, ownerID, version)  
   
       t.Run("simple put "+version, func(t *testing.T) { simplePut(ctx, t, clientPool, CID, version) })  
       t.Run("put with bearer token in header"+version, func(t *testing.T) { putWithBearerTokenInHeader(ctx, t, clientPool, CID, token) })  
@@ -515,11 +515,15 @@ func putObject(ctx context.Context, t *testing.T, clientPool *pool.Pool, ownerID  
    return id  
 }  
   
-func makeBearerToken(t *testing.T, key *keys.PrivateKey, ownerID user.ID) string {  
+func makeBearerToken(t *testing.T, key *keys.PrivateKey, ownerID user.ID, version string) string {  
    tkn := new(bearer.Token)  
    tkn.ForUser(ownerID)  
    tkn.SetExp(10000)  
-   tkn.SetImpersonate(true)  
+   if version == "1.2.7" {  
+      tkn.SetEACLTable(*eacl.NewTable())  
+   } else {  
+      tkn.SetImpersonate(true)  
+   }  
   
    err := tkn.Sign(key.PrivateKey)  
    require.NoError(t, err)

I also wanted to keep the aio version 1.2.7, however, in this case, when the impersonate flag is set, the bearer token frostfs-node will give an error `code = 1024 message = invalid signature` (I understand that at that time there was not yet this flag). If I don't use this flag, I get an error from frostfs-node of the following type: `code = 2048 message = access to object operation denied: access to operation OBJECT_PUT is denied by extended ACL check: bearer token owner differs from the request sender`. I form the token as follows: ```diff diff --git a/cmd/http-gw/integration_test.go b/cmd/http-gw/integration_test.go index c22551c..44a2717 100644 --- a/cmd/http-gw/integration_test.go +++ b/cmd/http-gw/integration_test.go @@ -1,5 +1,3 @@ -//go:build integration - package main import ( @@ -9,6 +7,7 @@ import ( "encoding/base64" "encoding/json" "fmt" + "git.frostfs.info/TrueCloudLab/frostfs-sdk-go/eacl" "io" "mime/multipart" "net/http" @@ -49,6 +48,7 @@ func TestIntegration(t *testing.T) { rootCtx := context.Background() aioImage := "truecloudlab/frostfs-aio:" versions := []string{ + "1.2.7", "1.3.0", } key, err := keys.NewPrivateKeyFromHex("1dd37fba80fec4e6a6f13fd708d8dcb3b29def768017052f6c930fa1c5d90bbb") @@ -66,7 +66,7 @@ func TestIntegration(t *testing.T) { CID, err := createContainer(ctx, t, clientPool, ownerID, version) require.NoError(t, err, version) - token := makeBearerToken(t, key, ownerID) + token := makeBearerToken(t, key, ownerID, version) t.Run("simple put "+version, func(t *testing.T) { simplePut(ctx, t, clientPool, CID, version) }) t.Run("put with bearer token in header"+version, func(t *testing.T) { putWithBearerTokenInHeader(ctx, t, clientPool, CID, token) }) @@ -515,11 +515,15 @@ func putObject(ctx context.Context, t *testing.T, clientPool *pool.Pool, ownerID return id } -func makeBearerToken(t *testing.T, key *keys.PrivateKey, ownerID user.ID) string { +func makeBearerToken(t *testing.T, key *keys.PrivateKey, ownerID user.ID, version string) string { tkn := new(bearer.Token) tkn.ForUser(ownerID) tkn.SetExp(10000) - tkn.SetImpersonate(true) + if version == "1.2.7" { + tkn.SetEACLTable(*eacl.NewTable()) + } else { + tkn.SetImpersonate(true) + } err := tkn.Sign(key.PrivateKey) require.NoError(t, err) ```

r.loginov requested review from storage-services-committers 2024-05-06 18:08:26 +00:00

r.loginov requested review from storage-services-developers 2024-05-06 18:08:27 +00:00

dkirillov commented

2024-05-07 12:36:02 +00:00

Collaborator

You've got

code = 2048 message = access to object operation denied: access to operation OBJECT_PUT is denied by extended ACL check: bearer token owner differs from the request sender

because of server starts with random key rather than that key for which you create bearer token.
Log says this:

2024-05-07T15:33:30.072+0300	info	http-gw/app.go:331	no wallet path specified, creating ephemeral key automatically for this run

You should start server (runServer function) with this key (you can create tmp wallet file and remove it after test passed, see example creation of tmp file here )

You've got ``` code = 2048 message = access to object operation denied: access to operation OBJECT_PUT is denied by extended ACL check: bearer token owner differs from the request sender ``` because of server starts with random key rather than that key for which you create bearer token. Log says this: ``` 2024-05-07T15:33:30.072+0300 info http-gw/app.go:331 no wallet path specified, creating ephemeral key automatically for this run ``` You should start server (`runServer` function) with [this key](https://git.frostfs.info/r.loginov/frostfs-http-gw/src/commit/2cbd9f47f67770f33de8d6f86e81d9a31aff07e2/cmd/http-gw/integration_test.go#L54) (you can create tmp wallet file and remove it after test passed, see example creation of tmp file [here](https://git.frostfs.info/TrueCloudLab/frostfs-http-gw/src/commit/11965deb4188e78bdbe92b5d4b17e41620f2a86d/internal/handler/multipart_test.go#L33-L36) )

dkirillov commented

2024-05-07 12:37:14 +00:00

Collaborator

Can you also bump golang.org/x/net to v0.23.0?

Can you also bump ` golang.org/x/net` to `v0.23.0`?

r.loginov force-pushed feature/increase_test_coverage from 2cbd9f47f6 to 5b7b872dcd

2024-05-08 06:58:29 +00:00

Compare

dkirillov commented

2024-05-08 08:08:34 +00:00

Collaborator

Can you also bump golang.org/x/net to v0.23.0?

There is another problem with vulncheck

> Can you also bump golang.org/x/net to v0.23.0? There is another problem with vulncheck

dkirillov approved these changes 2024-05-08 08:09:55 +00:00

r.loginov commented

2024-05-08 09:50:48 +00:00

Poster

Collaborator

Can you also bump golang.org/x/net to v0.23.0?

There is another problem with vulncheck

Yes, the problem there is that there is a vulnerability in the net package when using version 1.21.9. At the moment, the current version 1.21.9, not 1.21.10, is downloaded during the vulncheck workflow. Perhaps this is due to the fact that version 1.21.10 was released only 6 hours ago (May 8 at 6:32 AM). Do I understand correctly that at the moment we cannot influence the download of a more up-to-date version?

ps. I checked locally with the go version 1.21.10 - everything is OK, there are no vulnerabilities.

> > Can you also bump golang.org/x/net to v0.23.0? > > There is another problem with vulncheck Yes, the problem there is that there is a vulnerability in the net package when using version 1.21.9. At the moment, the current version 1.21.9, not 1.21.10, is downloaded during the vulncheck workflow. Perhaps this is due to the fact that version 1.21.10 [was released](https://github.com/actions/go-versions/releases/tag/1.21.10-8995788853) only 6 hours ago (May 8 at 6:32 AM). Do I understand correctly that at the moment we cannot influence the download of a more up-to-date version? ps. I checked locally with the go version 1.21.10 - everything is OK, there are no vulnerabilities.

r.loginov commented

2024-05-08 14:15:14 +00:00

Poster

Collaborator

I restarted the vulncheck workflow again, this time the correct version was used, and there are no more vulnerabilities.