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

Consider removing Client Side Validation for AWS SDK Service Operations #614

Open
texastony opened this issue Sep 29, 2024 · 3 comments
Open

Comments

@texastony
Copy link
Contributor

texastony commented Sep 29, 2024

User Story

As a Smithy-Dafny user,
I do not want to enforce AWS Services' server side validations
in my Smithy-Dafny generated clients,
as that makes the clients brittle to Service changes.

Details

The Smithy Models parsed by Smithy-Dafny MUST support
some "client-side" constraints on user input,
such as @required.

But these user input validations SHOULD NOT
be applied to the responses of AWS SDK Services,
and it is questionable if the requests to AWS SDK Services
need any of these of validations as well.

(I believe) The Smithy Specification designates what
constraints/validations should be applied client side,
and which should be left to the server.

For non-local services,
Smithy-Dafny should follow this guidance.

Example

Some Dafny code that is likely unnecessary:

    var maybeReEncryptResponse := kmsClient.ReEncrypt(reEncryptRequest);
    var reEncryptResponse :- maybeReEncryptResponse
    .MapFailure(e => Types.ComAmazonawsKms(ComAmazonawsKms := e));

    :- Need(
      && reEncryptResponse.CiphertextBlob.Some?
      && KMS.IsValid_CiphertextType(reEncryptResponse.CiphertextBlob.value),
      Types.KeyManagementException(
        message := "Invalid response from AWS KMS ReEncrypt: Invalid ciphertext.")
    );

It is very hard to imagine that KMS would return an invalid ciphertext.

Alternative

If Smithy-Dafny wants to continue enforce client side validation
for AWS Service requests and responses,
Smithy-Dafny may want to consider generating additional Error
shapes for AWS Service's: InvalidRequestException & InvalidResponseException.

These can be used to clearly signal to the Dafny developers
writing logic that it was local validation that failed
as compared to server side.

@texastony
Copy link
Contributor Author

I think I am going to use the Alternative to unblock me.

I have created KeyManagementException as a catch all for
either request or response validation errors.

Then,
I can create:

  type KmsError = e: Types.Error | (e.ComAmazonawsKms? || e.KeyManagementException?) witness *

Any methods that call KMS can return KmsError as the failure type,
and the business logic does not need to un-wrap the Local Service error,
Types.Error,
to understand if it is Server Side or Client Side validation that failed the request.

@robin-aws
Copy link
Contributor

I think it depends on how you're using the Dafny service SDK. If you're using it in production code, you will want the client to follow the Smithy specification and bias for returning a response even if it's invalid according to the model. But it can also be useful to assume all responses are valid and explicitly catch cases when they are not, if you're using Dafny to test if the service always behaves according to its model. So I think this should eventually at least be a configuration option, even if the default should probably be the first mode.

Will your alternative involve having smithy-dafny emit that KeyManagementException type itself, or can you safely add that in the MPL instead?

@texastony
Copy link
Contributor Author

I just added it to the MPL,
in my branch.

Which allows me to distinguish between KMS Errors,
which maybe opaque and access denied,
or our potentially overzealous client side validation.

See aws/aws-cryptographic-material-providers-library@8f17d17

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants