feat(rest): add a SigV4 auth manager over a delegate session - #3092
Draft
plusplusjiajia wants to merge 7 commits into
Draft
feat(rest): add a SigV4 auth manager over a delegate session#3092plusplusjiajia wants to merge 7 commits into
plusplusjiajia wants to merge 7 commits into
Conversation
The crate traces the request it signs, and its redaction list covers `authorization` but not the `Original-` copy we make, so a delegate's bearer token could reach trace logs. Also reject non-UTF-8 headers rather than leave them unsigned, and hash a present-but-empty body as Java does.
Java holds one `Aws4Signer` across sessions and resolves credentials from an `AwsCredentialsProvider` per request, so the signer itself carries no credential state. Follow that: drop `AwsCredentials`, take the AWS crate's `Credentials` as a `sign` argument, and leave the provider to the auth session. Also sign `HttpRequest` rather than the concrete request type, name the amz headers, split `convert_headers`/`update_request_headers` after their Java counterparts, collapse the one-file `sigv4` module, and put the AWS dependencies behind a `sigv4` feature.
Follows Java's `RESTSigV4AuthManager`: the manager holds one credential-free signer for every session, while the session holds the credentials provider and resolves it per request. The delegate authenticates first, so its bearer token is relocated and signed over. Static credentials come from the same properties Java reads. Anything else — a role, a refreshing provider — is passed to `SigV4AuthManager::new`, which takes any `SharedCredentialsProvider`.
Java builds a fresh `AwsProperties` in every session method, so a property-built manager has to follow the merged properties in `init_session` as well, not only in `catalog_session`. The manager had no tests at all: nine mutations of it survived, including inverting the rebuild condition and swapping the delegate method. Cover it with a recording delegate, and assert the region and service through the credential scope of a real signature rather than private fields.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Which issue does this PR close?
Split out of #2660, stacked on #3082 — the first five commits are that PR and drop out once it merges.
What changes are included in this PR?
SigV4AuthManagerwires the signer intoAuthManager/AuthSession, following Java'sRESTSigV4AuthManager:Aws4Signer.create()does;resolveCredentials()insidesign, leaving caching to the provider;Original-Authorizationand signed over;AwsPropertiesin each.Properties use Java's names:
rest.signing-region,rest.signing-name(defaultexecute-api),rest.access-key-id,rest.secret-access-key, `rest.session-toke