From ccb839e0e30c3b6992fb4084dfd6550d0ddd4d1a Mon Sep 17 00:00:00 2001 From: Noah Treuhaft Date: Tue, 3 Jan 2017 12:27:12 -0800 Subject: [PATCH 1/2] Change DELETE action from "*" to "delete" With token authentication, requiring the "*" action for DELETE requests makes it impossible to administratively lock a repository against pushes and pulls but still allow deletion. This change adds a new "delete" action for DELETE requests to make that possible. Signed-off-by: Noah Treuhaft --- registry/handlers/app.go | 4 +--- registry/handlers/app_test.go | 6 +++--- 2 files changed, 4 insertions(+), 6 deletions(-) diff --git a/registry/handlers/app.go b/registry/handlers/app.go index 670a2794b..766dc4b92 100644 --- a/registry/handlers/app.go +++ b/registry/handlers/app.go @@ -901,12 +901,10 @@ func appendAccessRecords(records []auth.Access, method string, repo string) []au Action: "push", }) case "DELETE": - // DELETE access requires full admin rights, which is represented - // as "*". This may not be ideal. records = append(records, auth.Access{ Resource: resource, - Action: "*", + Action: "delete", }) } return records diff --git a/registry/handlers/app_test.go b/registry/handlers/app_test.go index 385fa4c6b..12c0b61c1 100644 --- a/registry/handlers/app_test.go +++ b/registry/handlers/app_test.go @@ -229,9 +229,9 @@ func TestAppendAccessRecords(t *testing.T) { Resource: expectedResource, Action: "push", } - expectedAllRecord := auth.Access{ + expectedDeleteRecord := auth.Access{ Resource: expectedResource, - Action: "*", + Action: "delete", } records := []auth.Access{} @@ -271,7 +271,7 @@ func TestAppendAccessRecords(t *testing.T) { records = []auth.Access{} result = appendAccessRecords(records, "DELETE", repo) - expectedResult = []auth.Access{expectedAllRecord} + expectedResult = []auth.Access{expectedDeleteRecord} if ok := reflect.DeepEqual(result, expectedResult); !ok { t.Fatalf("Actual access record differs from expected") } From a33af0587b7e7e0cfd94f7f60d4823a0084564ca Mon Sep 17 00:00:00 2001 From: Noah Treuhaft Date: Fri, 6 Jan 2017 16:08:32 -0800 Subject: [PATCH 2/2] Add test for auth token with "*" action Test that an auth token with the "*" action is allowed any action on its resource. Signed-off-by: Noah Treuhaft --- registry/auth/token/token_test.go | 21 +++++++++++++++++++++ 1 file changed, 21 insertions(+) diff --git a/registry/auth/token/token_test.go b/registry/auth/token/token_test.go index cbfe2a6b4..03dce6fa6 100644 --- a/registry/auth/token/token_test.go +++ b/registry/auth/token/token_test.go @@ -454,6 +454,27 @@ func TestAccessController(t *testing.T) { if userInfo.Name != "foo" { t.Fatalf("expected user name %q, got %q", "foo", userInfo.Name) } + + // 5. Supply a token with full admin rights, which is represented as "*". + token, err = makeTestToken( + issuer, service, + []*ResourceActions{{ + Type: testAccess.Type, + Name: testAccess.Name, + Actions: []string{"*"}, + }}, + rootKeys[0], 1, time.Now(), time.Now().Add(5*time.Minute), + ) + if err != nil { + t.Fatal(err) + } + + req.Header.Set("Authorization", fmt.Sprintf("Bearer %s", token.compactRaw())) + + _, err = accessController.Authorized(ctx, testAccess) + if err != nil { + t.Fatalf("accessController returned unexpected error: %s", err) + } } // This tests that newAccessController can handle PEM blocks in the certificate