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

Cognito: support for partial UpdateUserPoolClient calls #5221

Open
2 tasks
dennisameling opened this issue May 14, 2024 · 1 comment
Open
2 tasks

Cognito: support for partial UpdateUserPoolClient calls #5221

dennisameling opened this issue May 14, 2024 · 1 comment
Labels
feature-request A feature should be added or improved. service-api This issue is due to a problem in a service API, not the SDK implementation.

Comments

@dennisameling
Copy link

Describe the feature

The UpdateUserPoolClient should support partial updates instead of having to provide every single parameter in the request.

Use Case

Currently, the process to update a user pool client through the SDK is very cumbersome. For example, if we only want to update the callbackURLs on an existing user pool client, we have to pass every single property with the existing value to the UpdateUserPoolClientRequest. This is because Cognito doesn't support partial updates:

If you don't provide a value for an attribute, Amazon Cognito sets it to its default value.

import software.amazon.awssdk.services.cognitoidentityprovider.CognitoIdentityProviderClient
import software.amazon.awssdk.services.cognitoidentityprovider.model.DescribeUserPoolClientRequest
import software.amazon.awssdk.services.cognitoidentityprovider.model.UpdateUserPoolClientRequest

class CognitoClient(
    private val cognitoConfig: CognitoConfig,
    private val cognitoClient: CognitoIdentityProviderClient,
) {
    // redacted other methods

    fun addCallbackUrlToAppClient(clientId: String, callbackUrl: String) {
        val describeUserPoolClientRequest = DescribeUserPoolClientRequest.builder()
            .clientId(clientId)
            .userPoolId(cognitoConfig.userPoolId)
            .build()
        
        val existingUserPoolClient = cognitoClient
            .describeUserPoolClient(describeUserPoolClientRequest)
            .userPoolClient()

        val existingCallbackUrls = existingUserPoolClient.callbackURLs()

        if (existingCallbackUrls.contains(callbackUrl)) {
            logger.info("Callback URL $callbackUrl already exists for client $clientId, skipping.")
            return
        }

        // We need to copy the list because it's immutable.
        val updatedCallbackUrls = existingCallbackUrls.toMutableList()
        updatedCallbackUrls.add(callbackUrl)

        /**
         * From the Cognito docs: "WARNING - If you don't provide a value for an attribute,
         * Amazon Cognito sets it to its default value."
         *
         * So here we are, setting all the attributes to their already existing values.
         */
        val updateRequest = UpdateUserPoolClientRequest.builder()
            .accessTokenValidity(existingUserPoolClient.accessTokenValidity())
            .allowedOAuthFlows(existingUserPoolClient.allowedOAuthFlows())
            .allowedOAuthFlowsUserPoolClient(existingUserPoolClient.allowedOAuthFlowsUserPoolClient())
            .allowedOAuthScopes(existingUserPoolClient.allowedOAuthScopes())
            .analyticsConfiguration(existingUserPoolClient.analyticsConfiguration())
            .authSessionValidity(existingUserPoolClient.authSessionValidity())
            // This is the only field we're _actually_ changing
            .callbackURLs(updatedCallbackUrls)
            .clientId(existingUserPoolClient.clientId())
            .clientName(existingUserPoolClient.clientName())
            .defaultRedirectURI(existingUserPoolClient.defaultRedirectURI())
            .enablePropagateAdditionalUserContextData(existingUserPoolClient.enablePropagateAdditionalUserContextData())
            .enableTokenRevocation(existingUserPoolClient.enableTokenRevocation())
            .explicitAuthFlows(existingUserPoolClient.explicitAuthFlows())
            .idTokenValidity(existingUserPoolClient.idTokenValidity())
            .logoutURLs(existingUserPoolClient.logoutURLs())
            .preventUserExistenceErrors(existingUserPoolClient.preventUserExistenceErrors())
            .readAttributes(existingUserPoolClient.readAttributes())
            .refreshTokenValidity(existingUserPoolClient.refreshTokenValidity())
            .supportedIdentityProviders(existingUserPoolClient.supportedIdentityProviders())
            .tokenValidityUnits(existingUserPoolClient.tokenValidityUnits())
            .userPoolId(existingUserPoolClient.userPoolId())
            .writeAttributes(existingUserPoolClient.writeAttributes())
            .build()

        cognitoClient.updateUserPoolClient(updateRequest)

        logger.info("Successfully added callback URL $callbackUrl to Cognito App Client $clientId")
    }
}

Proposed Solution

This probably will need updates on the Cognito service side. However, I do think it would be nice if the SDK could provide a convenience method in the meantime. I guess something like:

import software.amazon.awssdk.services.cognitoidentityprovider.CognitoIdentityProviderClient
import software.amazon.awssdk.services.cognitoidentityprovider.model.DescribeUserPoolClientRequest
import software.amazon.awssdk.services.cognitoidentityprovider.model.UpdateUserPoolClientRequest

class CognitoClient(
    private val cognitoConfig: CognitoConfig,
    private val cognitoClient: CognitoIdentityProviderClient,
) {
    // redacted other methods

    fun addCallbackUrlToAppClient(clientId: String, callbackUrl: String) {
        val describeUserPoolClientRequest = DescribeUserPoolClientRequest.builder()
            .clientId(clientId)
            .userPoolId(cognitoConfig.userPoolId)
            .build()
        
        val existingUserPoolClient = cognitoClient
            .describeUserPoolClient(describeUserPoolClientRequest)
            .userPoolClient()

        val existingCallbackUrls = existingUserPoolClient.callbackURLs()

        if (existingCallbackUrls.contains(callbackUrl)) {
            logger.info("Callback URL $callbackUrl already exists for client $clientId, skipping.")
            return
        }

        // We need to copy the list because it's immutable.
        val updatedCallbackUrls = existingCallbackUrls.toMutableList()
        updatedCallbackUrls.add(callbackUrl)

        val updateRequest = UpdateUserPoolClientRequest.builder()
            .callbackURLs(updatedCallbackUrls)
            .clientId(existingUserPoolClient.clientId())
            .build()

        cognitoClient.updateUserPoolClient(updateRequest, true)

        logger.info("Successfully added callback URL $callbackUrl to Cognito App Client $clientId")
    }
}

Note that updateUserPoolClient now takes a second parameter. This parameter could be named something like keepExistingParameters and default to false, so it's not a breaking change. If true, it could get the existing user pool (either by doing a describeUserPoolClient under the hood or require an existing UserPoolClientType to be passed).

Other Information

Related Cognito docs: https://docs.aws.amazon.com/cognito-user-identity-pools/latest/APIReference/API_UpdateUserPoolClient.html

Acknowledgements

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

AWS Java SDK version used

2.25.50

JDK version used

17.0.10

Operating System and version

macOS 14.4.1

@dennisameling dennisameling added feature-request A feature should be added or improved. needs-triage This issue or PR still needs to be triaged. labels May 14, 2024
@debora-ito
Copy link
Member

Hi @dennisameling thank you for reaching out.

Note that updateUserPoolClient now takes a second parameter. This parameter could be named something like keepExistingParameters and default to false, so it's not a breaking change. If true, it could get the existing user pool (either by doing a describeUserPoolClient under the hood or require an existing UserPoolClientType to be passed).

I understand the feature you're proposing, but we don't do a lot of this type of client-side customization. We try to keep a standard experience across all language SDKs, and client-side changes require manual work by each SDK team. Adding a control attribute like keepExistingParameters on the service side is the path forward.

I'll raise your feedback to the Cognito team, and will update here when I hear back.

@debora-ito debora-ito added service-api This issue is due to a problem in a service API, not the SDK implementation. and removed needs-triage This issue or PR still needs to be triaged. labels May 15, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature-request A feature should be added or improved. service-api This issue is due to a problem in a service API, not the SDK implementation.
Projects
None yet
Development

No branches or pull requests

2 participants