identity: enforce key param and key existence on role creation

changelog/12208.txt

@@ -0,0 +1,3 @@
+```release-note:bug
+identity: handle creation of role without a key parameter
+```

vault/identity_store_oidc.go

@@ -927,6 +927,9 @@ func (i *IdentityStore) pathOIDCCreateUpdateRole(ctx context.Context, req *logic
 
 	name := d.Get("name").(string)
 
+	i.oidcLock.Lock()
+	defer i.oidcLock.Unlock()
+
 	var role role
 	if req.Operation == logical.UpdateOperation {
 		entry, err := req.Storage.Get(ctx, roleConfigPath+name)
@@ -946,6 +949,10 @@ func (i *IdentityStore) pathOIDCCreateUpdateRole(ctx context.Context, req *logic
 		role.Key = d.Get("key").(string)
 	}
 
+	if role.Key == "" {
+		return logical.ErrorResponse("the key parameter is required"), nil
+	}
+
 	if template, ok := d.GetOk("template"); ok {
 		role.Template = template.(string)
 	} else if req.Operation == logical.CreateOperation {
@@ -995,12 +1002,12 @@ func (i *IdentityStore) pathOIDCCreateUpdateRole(ctx context.Context, req *logic
 	if err != nil {
 		return nil, err
 	}
-	if entry != nil {
-		if err := entry.DecodeJSON(&key); err != nil {
-			return nil, err
-		}
+	if entry == nil {
+		return logical.ErrorResponse("cannot find key %q", role.Key), nil
+	}
+	if err := entry.DecodeJSON(&key); err != nil {
+		return nil, err
 	}
-
 	if role.TokenTTL > key.VerificationTTL {
 		return logical.ErrorResponse("a role's token ttl cannot be longer than the verification_ttl of the key it references"), nil
 	}

vault/identity_store_oidc_test.go

@@ -18,6 +18,51 @@ import (
 	"gopkg.in/square/go-jose.v2/jwt"
 )
 
+// TestOIDC_Path_OIDC_RoleNoKeyParameter tests that a role cannot be created
+// without a key parameter
+func TestOIDC_Path_OIDC_RoleNoKeyParameter(t *testing.T) {
+	c, _, _ := TestCoreUnsealed(t)
+	ctx := namespace.RootContext(nil)
+	storage := &logical.InmemStorage{}
+
+	// Create a test role "test-role1" without a key param -- should fail
+	resp, err := c.identityStore.HandleRequest(ctx, &logical.Request{
+		Path:      "oidc/role/test-role1",
+		Operation: logical.CreateOperation,
+		Storage:   storage,
+	})
+	expectError(t, resp, err)
+	// validate error message
+	expectedStrings := map[string]interface{}{
+		"the key parameter is required": true,
+	}
+	expectStrings(t, []string{resp.Data["error"].(string)}, expectedStrings)
+}
+
+// TestOIDC_Path_OIDC_RoleNilKeyEntry tests taht a role cannot be created when
+// a key parameter is provided but the key does not exist
+func TestOIDC_Path_OIDC_RoleNilKeyEntry(t *testing.T) {
+	c, _, _ := TestCoreUnsealed(t)
+	ctx := namespace.RootContext(nil)
+	storage := &logical.InmemStorage{}
+
+	// Create a test role "test-role1" with a non-existent key -- should fail
+	resp, err := c.identityStore.HandleRequest(ctx, &logical.Request{
+		Path:      "oidc/role/test-role1",
+		Operation: logical.CreateOperation,
+		Data: map[string]interface{}{
+			"key": "test-key",
+		},
+		Storage: storage,
+	})
+	expectError(t, resp, err)
+	// validate error message
+	expectedStrings := map[string]interface{}{
+		"cannot find key \"test-key\"": true,
+	}
+	expectStrings(t, []string{resp.Data["error"].(string)}, expectedStrings)
+}
+
 // TestOIDC_Path_OIDCRoleRole tests CRUD operations for roles
 func TestOIDC_Path_OIDCRoleRole(t *testing.T) {
 	c, _, _ := TestCoreUnsealed(t)
@@ -1196,6 +1241,7 @@ func expectError(t *testing.T, resp *logical.Response, err error) {
 // expectString fails unless every string in actualStrings is also included in expectedStrings and
 // the length of actualStrings and expectedStrings are the same
 func expectStrings(t *testing.T, actualStrings []string, expectedStrings map[string]interface{}) {
+	t.Helper()
 	if len(actualStrings) != len(expectedStrings) {
 		t.Fatalf("expectStrings mismatch:\nactual strings:\n%#v\nexpected strings:\n%#v\n", actualStrings, expectedStrings)
 	}