fix(oidc_client): omit client_secret when client_type == "public"

mmskv commented 1 year ago

Public OpenID Clients don't have a client_secret so accessing it results in a

panic: interface conversion: interface {} is nil, not string

Description

This change explicitly ignores vault_identity_oidc_client_creds's client_secret and sets it to an empty string for public clients

Checklist

[x] Added CHANGELOG entry (only for user-facing changes)
[x] Acceptance tests where run against all supported Vault Versions

Output from acceptance testing:

$ make testacc TESTARGS='-run=TestAccXXX'

...

Steps to reproduce

Create a public oidc client and attemt to query it's creds

resource "vault_identity_oidc_client" "test" {
  name             = "test"
  client_type      = "public"
  redirect_uris    = ["http://0.0.0.0:80/"]
  assignments      = ["allow_all"]
  key              = vault_identity_oidc_key.test.id
  id_token_ttl     = 600
  access_token_ttl = 1200
}

data "vault_identity_oidc_client_creds" "test" {
  name = vault_identity_oidc_client.test.name
}

You will get this error

Stack trace from the terraform-provider-vault_v3.21.0_x5 plugin:

panic: interface conversion: interface {} is nil, not string

goroutine 64 [running]:
github.com/hashicorp/terraform-provider-vault/vault.readOIDCClientCredsResource(0x0?, {0x148a680?, 0xc0008f61c0?})
    github.com/hashicorp/terraform-provider-vault/vault/data_identity_oidc_client_creds.go:64 +0x41b
github.com/hashicorp/terraform-provider-vault/internal/provider.ReadWrapper.func1(0x0?, {0x148a680, 0xc0008f61c0})
    github.com/hashicorp/terraform-provider-vault/internal/provider/provider.go:241 +0x5a
github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema.(*Resource).read(0x19ce8e8?, {0x19ce8e8?, 0xc000e67950?}, 0xd?, {0x148a680?, 0xc0008f61c0?})
    github.com/hashicorp/terraform-plugin-sdk/v2@v2.29.0/helper/schema/resource.go:783 +0x178
github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema.(*Resource).ReadDataApply(0xc00082c540, {0x19ce8e8, 0xc000e67950}, 0xc000326e00, {0x148a680, 0xc0008f61c0})
    github.com/hashicorp/terraform-plugin-sdk/v2@v2.29.0/helper/schema/resource.go:1015 +0x150
github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema.(*GRPCProviderServer).ReadDataSource(0xc000a24a50, {0x19ce8e8?, 0xc000e67830?}, 0xc000ddb560)
    github.com/hashicorp/terraform-plugin-sdk/v2@v2.29.0/helper/schema/grpc_provider.go:1237 +0x38f
github.com/hashicorp/terraform-plugin-go/tfprotov5/tf5server.(*server).ReadDataSource(0xc00078c320, {0x19ce8e8?, 0xc000e66ea0?}, 0xc000e50870)
    github.com/hashicorp/terraform-plugin-go@v0.19.0/tfprotov5/tf5server/server.go:699 +0x403
github.com/hashicorp/terraform-plugin-go/tfprotov5/internal/tfplugin5._Provider_ReadDataSource_Handler({0x156c320?, 0xc00078c320}, {0x19ce8e8, 0xc000e66ea0}, 0xc000134c40, 0x0)
    github.com/hashicorp/terraform-plugin-go@v0.19.0/tfprotov5/internal/tfplugin5/tfplugin5_grpc.pb.go:503 +0x170
google.golang.org/grpc.(*Server).processUnaryRPC(0xc0007a0b40, {0x19d4260, 0xc000a2aea0}, 0xc000e698c0, 0xc000a28390, 0x23cf3a8, 0x0)
    google.golang.org/grpc@v1.58.2/server.go:1376 +0xdd2
google.golang.org/grpc.(*Server).handleStream(0xc0007a0b40, {0x19d4260, 0xc000a2aea0}, 0xc000e698c0, 0x0)
    google.golang.org/grpc@v1.58.2/server.go:1753 +0xa36
google.golang.org/grpc.(*Server).serveStreams.func1.1()
    google.golang.org/grpc@v1.58.2/server.go:998 +0x98
created by google.golang.org/grpc.(*Server).serveStreams.func1
    google.golang.org/grpc@v1.58.2/server.go:996 +0x18c

Community Note

Please vote on this pull request by adding a 👍 reaction to the original pull request comment to help the community and maintainers prioritize this request
Please do not leave "+1" comments, they generate extra noise for pull request followers and do not help prioritize the request

hashicorp-cla commented 1 year ago

All committers have signed the CLA.

fairclothjm commented 1 year ago

@mmskv Thanks for making the updates! The acceptance tests are failing with:

=== NAME  TestDataSourceIdentityOIDCClientCreds
    testing_new.go:85: Error running post-test destroy, there may be dangling resources: exit status 1

        Error: error deleting IdentityOidcKey key: Error making API request.

        URL: DELETE http://localhost:8200/v1/identity/oidc/key/key
        Code: 400. Errors:

        * unable to delete key "key" because it is currently referenced by these clients: test-public-client-6603691130748134188

--- FAIL: TestDataSourceIdentityOIDCClientCreds (2.07s)

You should be able to run this locally with

make testacc TESTARGS='-run TestDataSourceIdentityOIDCClientCreds -v'

mmskv commented 1 year ago

@fairclothjm That's odd. The tests ran fine on my machine. Can you trigger the test CI again, I think I've fixed that.

fairclothjm commented 1 year ago

@mmskv You might be able to repro with the -race flag of go test. The test was running in parallel and CI tends to reveal race conditions more frequently. I think the issue was that the vault_identity_oidc_key resource didn't have a unique name and the other test func was colliding.

I will trigger CI with your updates though

mmskv commented 1 year ago

Thank you for your time!

hashicorp / terraform-provider-vault