From af3aee5132cb84f7d2a814293bb0f763516dca2f Mon Sep 17 00:00:00 2001 From: Sung Hon Wu Date: Thu, 9 May 2019 15:10:02 -0700 Subject: [PATCH 1/4] When configuring JWT roles, consider bound claims when considering if there is at least one bound constraint --- path_role_test.go | 96 +++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 96 insertions(+) diff --git a/path_role_test.go b/path_role_test.go index 15657214..57f04a57 100644 --- a/path_role_test.go +++ b/path_role_test.go @@ -140,6 +140,102 @@ func TestPath_Create(t *testing.T) { if !strings.HasPrefix(resp.Error().Error(), "must have at least one bound constraint") { t.Fatalf("unexpected err: %v", resp) } + + // Test has bound subject + data = map[string]interface{}{ + "role_type": "jwt", + "user_claim": "user", + "policies": "test", + "bound_subject": "testsub", + } + + req = &logical.Request{ + Operation: logical.CreateOperation, + Path: "role/test2", + Storage: storage, + Data: data, + } + + resp, err = b.HandleRequest(context.Background(), req) + if err != nil { + t.Fatal(err) + } + if resp != nil && resp.IsError() { + t.Fatalf("did not expect error") + } + + // Test has audience + data = map[string]interface{}{ + "role_type": "jwt", + "user_claim": "user", + "policies": "test", + "bound_audiences": "vault", + } + + req = &logical.Request{ + Operation: logical.CreateOperation, + Path: "role/test2", + Storage: storage, + Data: data, + } + + resp, err = b.HandleRequest(context.Background(), req) + if err != nil { + t.Fatal(err) + } + if resp != nil && resp.IsError() { + t.Fatalf("did not expect error") + } + + // Test has cidr + data = map[string]interface{}{ + "role_type": "jwt", + "user_claim": "user", + "policies": "test", + "bound_cidrs": "127.0.0.1/8", + } + + req = &logical.Request{ + Operation: logical.CreateOperation, + Path: "role/test2", + Storage: storage, + Data: data, + } + + resp, err = b.HandleRequest(context.Background(), req) + if err != nil { + t.Fatal(err) + } + if resp != nil && resp.IsError() { + t.Fatalf("did not expect error") + } + + // Test has bound claims + data = map[string]interface{}{ + "role_type": "jwt", + "user_claim": "user", + "policies": "test", + "bound_claims": map[string]interface{}{ + "foo": 10, + "bar": "baz", + }, + } + + req = &logical.Request{ + Operation: logical.CreateOperation, + Path: "role/test2", + Storage: storage, + Data: data, + } + + resp, err = b.HandleRequest(context.Background(), req) + if err != nil { + t.Fatal(err) + } + if resp != nil && resp.IsError() { + t.Fatalf("did not expect error") + } + } func TestPath_OIDCCreate(t *testing.T) { From 7493ee6c713f4a6a6c5d6c46c38981d3b997485e Mon Sep 17 00:00:00 2001 From: Sung Hon Wu Date: Thu, 9 May 2019 15:11:26 -0700 Subject: [PATCH 2/4] Revert "When configuring JWT roles, consider bound claims when considering if there is at least one bound constraint" This reverts commit af3aee5132cb84f7d2a814293bb0f763516dca2f. --- path_role_test.go | 96 ----------------------------------------------- 1 file changed, 96 deletions(-) diff --git a/path_role_test.go b/path_role_test.go index 57f04a57..15657214 100644 --- a/path_role_test.go +++ b/path_role_test.go @@ -140,102 +140,6 @@ func TestPath_Create(t *testing.T) { if !strings.HasPrefix(resp.Error().Error(), "must have at least one bound constraint") { t.Fatalf("unexpected err: %v", resp) } - - // Test has bound subject - data = map[string]interface{}{ - "role_type": "jwt", - "user_claim": "user", - "policies": "test", - "bound_subject": "testsub", - } - - req = &logical.Request{ - Operation: logical.CreateOperation, - Path: "role/test2", - Storage: storage, - Data: data, - } - - resp, err = b.HandleRequest(context.Background(), req) - if err != nil { - t.Fatal(err) - } - if resp != nil && resp.IsError() { - t.Fatalf("did not expect error") - } - - // Test has audience - data = map[string]interface{}{ - "role_type": "jwt", - "user_claim": "user", - "policies": "test", - "bound_audiences": "vault", - } - - req = &logical.Request{ - Operation: logical.CreateOperation, - Path: "role/test2", - Storage: storage, - Data: data, - } - - resp, err = b.HandleRequest(context.Background(), req) - if err != nil { - t.Fatal(err) - } - if resp != nil && resp.IsError() { - t.Fatalf("did not expect error") - } - - // Test has cidr - data = map[string]interface{}{ - "role_type": "jwt", - "user_claim": "user", - "policies": "test", - "bound_cidrs": "127.0.0.1/8", - } - - req = &logical.Request{ - Operation: logical.CreateOperation, - Path: "role/test2", - Storage: storage, - Data: data, - } - - resp, err = b.HandleRequest(context.Background(), req) - if err != nil { - t.Fatal(err) - } - if resp != nil && resp.IsError() { - t.Fatalf("did not expect error") - } - - // Test has bound claims - data = map[string]interface{}{ - "role_type": "jwt", - "user_claim": "user", - "policies": "test", - "bound_claims": map[string]interface{}{ - "foo": 10, - "bar": "baz", - }, - } - - req = &logical.Request{ - Operation: logical.CreateOperation, - Path: "role/test2", - Storage: storage, - Data: data, - } - - resp, err = b.HandleRequest(context.Background(), req) - if err != nil { - t.Fatal(err) - } - if resp != nil && resp.IsError() { - t.Fatalf("did not expect error") - } - } func TestPath_OIDCCreate(t *testing.T) { From 4e3170eecb8cebb5728fc5469ccf90f66e35a703 Mon Sep 17 00:00:00 2001 From: Sung Hon Wu Date: Thu, 9 May 2019 15:18:19 -0700 Subject: [PATCH 3/4] When configuring JWT roles, consider bound claims when considering if there is at least one bound constraint --- path_role.go | 3 +- path_role_test.go | 95 +++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 97 insertions(+), 1 deletion(-) diff --git a/path_role.go b/path_role.go index 59a0bdc8..d9b12e51 100644 --- a/path_role.go +++ b/path_role.go @@ -401,7 +401,8 @@ func (b *jwtAuthBackend) pathRoleCreateUpdate(ctx context.Context, req *logical. if roleType != "oidc" { if len(role.BoundAudiences) == 0 && len(role.BoundCIDRs) == 0 && - role.BoundSubject == "" { + role.BoundSubject == "" && + len(role.BoundClaims) == 0 { return logical.ErrorResponse("must have at least one bound constraint when creating/updating a role"), nil } } diff --git a/path_role_test.go b/path_role_test.go index 15657214..b2728327 100644 --- a/path_role_test.go +++ b/path_role_test.go @@ -140,6 +140,101 @@ func TestPath_Create(t *testing.T) { if !strings.HasPrefix(resp.Error().Error(), "must have at least one bound constraint") { t.Fatalf("unexpected err: %v", resp) } + + // Test has bound subject + data = map[string]interface{}{ + "role_type": "jwt", + "user_claim": "user", + "policies": "test", + "bound_subject": "testsub", + } + + req = &logical.Request{ + Operation: logical.CreateOperation, + Path: "role/test2", + Storage: storage, + Data: data, + } + + resp, err = b.HandleRequest(context.Background(), req) + if err != nil { + t.Fatal(err) + } + if resp != nil && resp.IsError() { + t.Fatalf("did not expect error") + } + + // Test has audience + data = map[string]interface{}{ + "role_type": "jwt", + "user_claim": "user", + "policies": "test", + "bound_audiences": "vault", + } + + req = &logical.Request{ + Operation: logical.CreateOperation, + Path: "role/test2", + Storage: storage, + Data: data, + } + + resp, err = b.HandleRequest(context.Background(), req) + if err != nil { + t.Fatal(err) + } + if resp != nil && resp.IsError() { + t.Fatalf("did not expect error") + } + + // Test has cidr + data = map[string]interface{}{ + "role_type": "jwt", + "user_claim": "user", + "policies": "test", + "bound_cidrs": "127.0.0.1/8", + } + + req = &logical.Request{ + Operation: logical.CreateOperation, + Path: "role/test2", + Storage: storage, + Data: data, + } + + resp, err = b.HandleRequest(context.Background(), req) + if err != nil { + t.Fatal(err) + } + if resp != nil && resp.IsError() { + t.Fatalf("did not expect error") + } + + // Test has bound claims + data = map[string]interface{}{ + "role_type": "jwt", + "user_claim": "user", + "policies": "test", + "bound_claims": map[string]interface{}{ + "foo": 10, + "bar": "baz", + }, + } + + req = &logical.Request{ + Operation: logical.CreateOperation, + Path: "role/test2", + Storage: storage, + Data: data, + } + + resp, err = b.HandleRequest(context.Background(), req) + if err != nil { + t.Fatal(err) + } + if resp != nil && resp.IsError() { + t.Fatalf("did not expect error") + } } func TestPath_OIDCCreate(t *testing.T) { From 43a650fc5a34939198776917ae791d8aa32ba876 Mon Sep 17 00:00:00 2001 From: Sung Hon Wu Date: Tue, 14 May 2019 14:38:00 -0700 Subject: [PATCH 4/4] Address test comment feedback --- path_role_test.go | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/path_role_test.go b/path_role_test.go index b2728327..105a1ba9 100644 --- a/path_role_test.go +++ b/path_role_test.go @@ -125,7 +125,7 @@ func TestPath_Create(t *testing.T) { req = &logical.Request{ Operation: logical.CreateOperation, - Path: "role/test2", + Path: "role/test3", Storage: storage, Data: data, } @@ -151,7 +151,7 @@ func TestPath_Create(t *testing.T) { req = &logical.Request{ Operation: logical.CreateOperation, - Path: "role/test2", + Path: "role/test4", Storage: storage, Data: data, } @@ -174,7 +174,7 @@ func TestPath_Create(t *testing.T) { req = &logical.Request{ Operation: logical.CreateOperation, - Path: "role/test2", + Path: "role/test5", Storage: storage, Data: data, } @@ -197,7 +197,7 @@ func TestPath_Create(t *testing.T) { req = &logical.Request{ Operation: logical.CreateOperation, - Path: "role/test2", + Path: "role/test6", Storage: storage, Data: data, } @@ -223,7 +223,7 @@ func TestPath_Create(t *testing.T) { req = &logical.Request{ Operation: logical.CreateOperation, - Path: "role/test2", + Path: "role/test7", Storage: storage, Data: data, }