Decrypt oracle #87
Decrypt oracle #87
Conversation
* add documentation describing the API
| if arn.startswith("arn:") and ":alias/" not in arn: | ||
| return arn | ||
|
|
||
| raise ValueError("KMS CMK ARN provided for integration tests must be a key not an alias") |
There was a problem hiding this comment.
Can we raise a custom exception type here?
There was a problem hiding this comment.
Would that add value? The problem that this exception is identifying is that a user-provided value is incorrect. That seems to me to be the definition of ValueError.
There was a problem hiding this comment.
If you expect something to catch this exception, then yes. Its cheap to provide a custom exception, and we aren't giving up anything that ValueError provides.
There was a problem hiding this comment.
The only place this error will manifest is in pytest error logs.
* remove import safeguards to allow 3.5.0 and 3.5.1 to not fail * convert from comment-style typehints to inline
| ] | ||
| }, | ||
| { | ||
| "Effect": "Deny", |
There was a problem hiding this comment.
For this statement, I think you need to split it into two, because the NotAction and the NotResource are joined by an AND. So the statement as written says "DENY if and only if it is NOT an approved action AND it is NOT an approved resource." That means you won't get an explicit DENY if it is an unapproved action for an approved resource (or vice-versa).
There was a problem hiding this comment.
I'm not 100% on the policy combination, but splitting them into two (Action * with NotResource specifics and NotAction specifics with Resource *) made everything fail, including the CloudWatch logging. Reverted back to the (NotAction specifics with NotResource specifics) for now.
decrypt_oracle/src/aws_encryption_sdk_decrypt_oracle/key_providers/counting.py
Show resolved
Hide resolved
decrypt_oracle/src/aws_encryption_sdk_decrypt_oracle/key_providers/null.py
Show resolved
Hide resolved
Description of changes:
This provides the initial iteration of the decrypt oracle.
Not addressed in this PR is how we plan to handle CD for this API. I'll tackle that separately.
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.