diff --git a/vault/identity_store_oidc.go b/vault/identity_store_oidc.go index 405685a38c..8dd8be2606 100644 --- a/vault/identity_store_oidc.go +++ b/vault/identity_store_oidc.go @@ -494,6 +494,23 @@ func (i *IdentityStore) pathOIDCCreateUpdateKey(ctx context.Context, req *logica return logical.ErrorResponse(errorMessage), nil } } + + // ensure any clients referencing this key do not already have a id_token_ttl + // greater than the key's verification_ttl + clients, err := i.clientsReferencingTargetKeyName(ctx, req, name) + if err != nil { + return nil, err + } + for _, client := range clients { + if client.IDTokenTTL > key.VerificationTTL { + errorMessage := fmt.Sprintf( + "unable to update key %q because it is currently referenced by one or more clients with an id_token_ttl greater than %d seconds", + name, + key.VerificationTTL/time.Second, + ) + return logical.ErrorResponse(errorMessage), nil + } + } } if allowedClientIDsRaw, ok := d.GetOk("allowed_client_ids"); ok { diff --git a/vault/identity_store_oidc_provider.go b/vault/identity_store_oidc_provider.go index 3b5ff19174..b6476543f0 100644 --- a/vault/identity_store_oidc_provider.go +++ b/vault/identity_store_oidc_provider.go @@ -850,6 +850,7 @@ func (i *IdentityStore) pathOIDCCreateUpdateClient(ctx context.Context, req *log return logical.ErrorResponse("the key parameter is required"), nil } + var key namedKey // enforce key existence on client creation entry, err := req.Storage.Get(ctx, namedKeyConfigPath+client.Key) if err != nil { @@ -858,6 +859,9 @@ func (i *IdentityStore) pathOIDCCreateUpdateClient(ctx context.Context, req *log if entry == nil { return logical.ErrorResponse("key %q does not exist", client.Key), nil } + if err := entry.DecodeJSON(&key); err != nil { + return nil, err + } if idTokenTTLRaw, ok := d.GetOk("id_token_ttl"); ok { client.IDTokenTTL = time.Duration(idTokenTTLRaw.(int)) * time.Second @@ -865,6 +869,10 @@ func (i *IdentityStore) pathOIDCCreateUpdateClient(ctx context.Context, req *log client.IDTokenTTL = time.Duration(d.Get("id_token_ttl").(int)) * time.Second } + if client.IDTokenTTL > key.VerificationTTL { + return logical.ErrorResponse("a client's id_token_ttl cannot be greater than the verification_ttl of the key it references"), nil + } + if accessTokenTTLRaw, ok := d.GetOk("access_token_ttl"); ok { client.AccessTokenTTL = time.Duration(accessTokenTTLRaw.(int)) * time.Second } else if req.Operation == logical.CreateOperation { diff --git a/vault/identity_store_oidc_provider_test.go b/vault/identity_store_oidc_provider_test.go index 4c5baff4c0..252ecc03ff 100644 --- a/vault/identity_store_oidc_provider_test.go +++ b/vault/identity_store_oidc_provider_test.go @@ -828,7 +828,8 @@ func TestOIDC_Path_OIDC_ProviderReadPublicKey(t *testing.T) { Operation: logical.CreateOperation, Storage: storage, Data: map[string]interface{}{ - "key": "test-key-1", + "key": "test-key-1", + "id_token_ttl": "1m", }, }) @@ -883,7 +884,8 @@ func TestOIDC_Path_OIDC_ProviderReadPublicKey(t *testing.T) { Operation: logical.CreateOperation, Storage: storage, Data: map[string]interface{}{ - "key": "test-key-2", + "key": "test-key-2", + "id_token_ttl": "1m", }, }) @@ -972,6 +974,53 @@ func TestOIDC_Path_OIDC_ProviderClient_NilKeyEntry(t *testing.T) { expectStrings(t, []string{resp.Data["error"].(string)}, expectedStrings) } +// TestOIDC_Path_OIDC_ProviderClient_InvalidTokenTTL tests the TokenTTL validation +func TestOIDC_Path_OIDC_ProviderClient_InvalidTokenTTL(t *testing.T) { + c, _, _ := TestCoreUnsealed(t) + ctx := namespace.RootContext(nil) + storage := &logical.InmemStorage{} + + // Create a test key "test-key" + c.identityStore.HandleRequest(ctx, &logical.Request{ + Path: "oidc/key/test-key", + Operation: logical.CreateOperation, + Data: map[string]interface{}{ + "verification_ttl": int64(60), + }, + Storage: storage, + }) + + // Create a test client "test-client" with an id_token_ttl longer than the + // verification_ttl -- should fail with error + resp, err := c.identityStore.HandleRequest(ctx, &logical.Request{ + Path: "oidc/client/test-client", + Operation: logical.CreateOperation, + Data: map[string]interface{}{ + "key": "test-key", + "id_token_ttl": int64(3600), + }, + Storage: storage, + }) + expectError(t, resp, err) + // validate error message + expectedStrings := map[string]interface{}{ + "a client's id_token_ttl cannot be greater than the verification_ttl of the key it references": true, + } + expectStrings(t, []string{resp.Data["error"].(string)}, expectedStrings) + + // Read "test-client" + respReadTestClient, err := c.identityStore.HandleRequest(ctx, &logical.Request{ + Path: "oidc/client/test-client", + Operation: logical.ReadOperation, + Storage: storage, + }) + // Ensure that "test-client" was not created + expectSuccess(t, respReadTestClient, err) + if respReadTestClient != nil { + t.Fatalf("Expected a nil response but instead got:\n%#v", respReadTestClient) + } +} + // TestOIDC_Path_OIDC_ProviderClient_UpdateKey tests that a client // does not allow key modification on Update operations func TestOIDC_Path_OIDC_ProviderClient_UpdateKey(t *testing.T) { @@ -1007,7 +1056,8 @@ func TestOIDC_Path_OIDC_ProviderClient_UpdateKey(t *testing.T) { Operation: logical.CreateOperation, Storage: storage, Data: map[string]interface{}{ - "key": "test-key1", + "key": "test-key1", + "id_token_ttl": "1m", }, }) expectSuccess(t, resp, err) @@ -1018,7 +1068,8 @@ func TestOIDC_Path_OIDC_ProviderClient_UpdateKey(t *testing.T) { Operation: logical.UpdateOperation, Storage: storage, Data: map[string]interface{}{ - "key": "test-key2", + "key": "test-key2", + "id_token_ttl": "1m", }, }) expectError(t, resp, err) @@ -1088,7 +1139,8 @@ func TestOIDC_Path_OIDC_ProviderClient(t *testing.T) { Operation: logical.CreateOperation, Storage: storage, Data: map[string]interface{}{ - "key": "test-key", + "key": "test-key", + "id_token_ttl": "1m", }, }) expectSuccess(t, resp, err) @@ -1104,7 +1156,7 @@ func TestOIDC_Path_OIDC_ProviderClient(t *testing.T) { "redirect_uris": []string{}, "assignments": []string{}, "key": "test-key", - "id_token_ttl": int64(86400), + "id_token_ttl": int64(60), "access_token_ttl": int64(86400), "client_id": resp.Data["client_id"], "client_secret": resp.Data["client_secret"], @@ -1207,6 +1259,7 @@ func TestOIDC_Path_OIDC_ProviderClient_Deduplication(t *testing.T) { Storage: storage, Data: map[string]interface{}{ "key": "test-key", + "id_token_ttl": "1m", "assignments": []string{"test-assignment1", "test-assignment1"}, "redirect_uris": []string{"http://example.com", "http://notduplicate.com", "http://example.com"}, }, @@ -1224,7 +1277,7 @@ func TestOIDC_Path_OIDC_ProviderClient_Deduplication(t *testing.T) { "redirect_uris": []string{"http://example.com", "http://notduplicate.com"}, "assignments": []string{"test-assignment1"}, "key": "test-key", - "id_token_ttl": int64(86400), + "id_token_ttl": int64(60), "access_token_ttl": int64(86400), "client_id": resp.Data["client_id"], "client_secret": resp.Data["client_secret"], @@ -1351,7 +1404,8 @@ func TestOIDC_Path_OIDC_ProviderClient_List(t *testing.T) { Operation: logical.CreateOperation, Storage: storage, Data: map[string]interface{}{ - "key": "test-key", + "key": "test-key", + "id_token_ttl": "1m", }, }) @@ -1360,7 +1414,8 @@ func TestOIDC_Path_OIDC_ProviderClient_List(t *testing.T) { Operation: logical.CreateOperation, Storage: storage, Data: map[string]interface{}{ - "key": "test-key", + "key": "test-key", + "id_token_ttl": "1m", }, }) @@ -1948,8 +2003,9 @@ func TestOIDC_Path_OIDC_ProviderAssignment_DeleteWithExistingClient(t *testing.T Operation: logical.CreateOperation, Storage: storage, Data: map[string]interface{}{ - "key": "test-key", - "assignments": []string{"test-assignment"}, + "key": "test-key", + "assignments": []string{"test-assignment"}, + "id_token_ttl": "1m", }, }) expectSuccess(t, resp, err) diff --git a/vault/identity_store_oidc_test.go b/vault/identity_store_oidc_test.go index 5274ada714..45a5da3ee9 100644 --- a/vault/identity_store_oidc_test.go +++ b/vault/identity_store_oidc_test.go @@ -527,6 +527,29 @@ func TestOIDC_Path_OIDCKey_InvalidTokenTTL(t *testing.T) { Storage: storage, }) expectError(t, resp, err) + + // Create a client that depends on test key + resp, err = c.identityStore.HandleRequest(ctx, &logical.Request{ + Path: "oidc/client/test-client", + Operation: logical.CreateOperation, + Data: map[string]interface{}{ + "key": "test-key", + "id_token_ttl": "4m", + }, + Storage: storage, + }) + expectSuccess(t, resp, err) + + // Update test key "test-key" -- should fail since id_token_ttl is greater than 2m + resp, err = c.identityStore.HandleRequest(ctx, &logical.Request{ + Path: "oidc/key/test-key", + Operation: logical.UpdateOperation, + Data: map[string]interface{}{ + "verification_ttl": "2m", + }, + Storage: storage, + }) + expectError(t, resp, err) } // TestOIDC_Path_OIDCKey tests the List operation for keys