Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

IAM: Ability to add a clientId to an existing OpenIdConnectProvider #32421

Open
1 of 2 tasks
shaundclements opened this issue Dec 9, 2024 · 8 comments
Open
1 of 2 tasks
Labels
@aws-cdk/aws-iam Related to AWS Identity and Access Management effort/medium Medium work item – several days of effort feature-request A feature should be added or improved. p2

Comments

@shaundclements
Copy link

shaundclements commented Dec 9, 2024

Describe the feature

When using the OpenIdConnectProvider it would be great to be able to add ClientIds to an existing provisioned one.
The only supported methods are:
applyRemovalPolicy(policy)
toString()
static fromOpenIdConnectProviderArn(scope, id, openIdConnectProviderArn)

ref: https://docs.aws.amazon.com/cdk/api/v2/docs/aws-cdk-lib.aws_iam.OpenIdConnectProvider.html

Use Case

When you use the OpenIdConnectProvider.fromOpenIdConnectProviderArn to reference an existing one, it would be really useful to be able to add additional clientIds to support more than one client using the same OIDCProvider.
We are limited in that you can only create one OIDC Providers with the same location/url per account so we end up sharing the same OIDC Provider.
Each environment would then want to add their own ClientId to the created OIDCProvider

Proposed Solution

const oidcProvider = aws_iam.OpenIdConnectProvider.fromOpenIdConnectProviderArn(
      this,
      'OidcProvider',
      props.oidcProviderArn,
    );

oidcProvider.addClientId("GUID");

Other Information

It would be good to have the clientIds as a SET so that when a clientId is added (and already there), its not duplicated in a list or breaks the deployment because already there.

Acknowledgements

  • I may be able to implement this feature request
  • This feature might incur a breaking change

CDK version used

2.133.0

Environment details (OS name and version, etc.)

macOS Sonoma. Version: 14.5

@shaundclements shaundclements added feature-request A feature should be added or improved. needs-triage This issue or PR still needs to be triaged. labels Dec 9, 2024
@github-actions github-actions bot added the @aws-cdk/aws-iam Related to AWS Identity and Access Management label Dec 9, 2024
@pahud
Copy link
Contributor

pahud commented Dec 9, 2024

oidcProvider.addClientId("GUID");

I am not sure if adding an addClientId() for the interface is the way to go but I agree fromOpenIdConnectProviderArn() should allow to specify more options.

@pahud pahud added p2 effort/medium Medium work item – several days of effort and removed needs-triage This issue or PR still needs to be triaged. labels Dec 9, 2024
@shaundclements
Copy link
Author

addClientId was my attempt to be consistent with the creation property (clientIds) when first creating an OpenIdConnectProvider, ref:
image

@nmussy
Copy link
Contributor

nmussy commented Dec 10, 2024

I am not sure if adding an addClientId() for the interface is the way to go

Agreed, a setter would allow to add, remove or update the list of client IDs with just a single method.

It also looks like the custom resource already handles ClientIDList updates, so this should be a fairly simple change if you want to give it a shot @shaundclements:

// if client ID list has changed, determine "diff" because the API is add/remove
const oldClients: string[] = (event.OldResourceProperties.ClientIDList || []).sort();
const diff = arrayDiff(oldClients, clients);
external.log(`client ID diff: ${JSON.stringify(diff)}`);
for (const addClient of diff.adds) {
external.log(`adding client id "${addClient}" to provider ${providerArn}`);
await external.addClientIDToOpenIDConnectProvider({
OpenIDConnectProviderArn: providerArn,
ClientID: addClient,
});
}
for (const deleteClient of diff.deletes) {
external.log(`removing client id "${deleteClient}" from provider ${providerArn}`);
await external.removeClientIDFromOpenIDConnectProvider({
OpenIDConnectProviderArn: providerArn,
ClientID: deleteClient,
});
}

@shaundclements
Copy link
Author

if we went with a setter method, we would also need a read property to get the existing list of ClientIds (so that we can add one more to it)

@nmussy
Copy link
Contributor

nmussy commented Dec 10, 2024

What I had in mind would overwrite the existing values, either constructed or imported:

const provider = new OpenIdConnectProvider(stack, 'MyProvider', {
  url: 'https://my-url',
});
provider.clientIds = ['client1'];

// ...
const importedProvider = OpenIdConnectProvider.fromOpenIdConnectProviderArn(stack, 'provider', provider.openIdConnectProviderArn);
importedProvider.clientIds = ['client1', 'client2'];

This would allow both adding to the list of clients but also to remove previous values

@shaundclements
Copy link
Author

This would unfortunately not work for separate environment pipelines which share the same OIDCProvider and just add their clientId to the existing provisioned one. They wont necesarily have knowledge of the other clientIds, so I was after an 'add' method rather than replace.

@nmussy
Copy link
Contributor

nmussy commented Dec 10, 2024

Sending a list of client IDs to add to the custom resource would clash with its existing ClientIDList attribute, I think it would get a bit confusing DX wise.

For your specific use-case, I would say a custom resource of your own is probably the way to go, see the CDK docs and the existing OidcProvider handler

@shaundclements
Copy link
Author

I think extending method support for OpenIdConnectProvider.fromOpenIdConnectProviderArn is more preferrable to workaround suggestion

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
@aws-cdk/aws-iam Related to AWS Identity and Access Management effort/medium Medium work item – several days of effort feature-request A feature should be added or improved. p2
Projects
None yet
Development

No branches or pull requests

3 participants