-
Notifications
You must be signed in to change notification settings - Fork 388
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
feat!: Request
Revamp
#1938
feat!: Request
Revamp
#1938
Changes from 4 commits
af65d57
8f15e14
a76a967
51f7ec2
767978c
739617e
7d24e5e
a71618b
ba4e8e1
ee0c81b
1540144
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -12,22 +12,11 @@ | |
// See the License for the specific language governing permissions and | ||
// limitations under the License. | ||
|
||
import {GaxiosOptions} from 'gaxios'; | ||
import {Gaxios, GaxiosOptions} from 'gaxios'; | ||
|
||
import {Headers} from './authclient'; | ||
import {HeadersInit} from './authclient'; | ||
import {Crypto, createCrypto, fromArrayBufferToHex} from '../crypto/crypto'; | ||
|
||
type HttpMethod = | ||
| 'GET' | ||
| 'POST' | ||
| 'PUT' | ||
| 'PATCH' | ||
| 'HEAD' | ||
| 'DELETE' | ||
| 'CONNECT' | ||
| 'OPTIONS' | ||
| 'TRACE'; | ||
|
||
/** Interface defining the AWS authorization header map for signed requests. */ | ||
interface AwsAuthHeaderMap { | ||
amzDate?: string; | ||
|
@@ -60,15 +49,15 @@ interface GenerateAuthHeaderMapOptions { | |
// The AWS service URL query string. | ||
canonicalQuerystring: string; | ||
// The HTTP method used to call this API. | ||
method: HttpMethod; | ||
method: string; | ||
// The AWS region. | ||
region: string; | ||
// The AWS security credentials. | ||
securityCredentials: AwsSecurityCredentials; | ||
// The optional request payload if available. | ||
requestPayload?: string; | ||
// The optional additional headers needed for the requested AWS API. | ||
additionalAmzHeaders?: Headers; | ||
additionalAmzHeaders?: HeadersInit; | ||
} | ||
|
||
/** AWS Signature Version 4 signing algorithm identifier. */ | ||
|
@@ -113,7 +102,7 @@ export class AwsRequestSigner { | |
*/ | ||
async getRequestOptions(amzOptions: GaxiosOptions): Promise<GaxiosOptions> { | ||
if (!amzOptions.url) { | ||
throw new Error('"url" is required in "amzOptions"'); | ||
throw new RangeError('"url" is required in "amzOptions"'); | ||
} | ||
// Stringify JSON requests. This will be set in the request body of the | ||
// generated signed request. | ||
|
@@ -127,19 +116,26 @@ export class AwsRequestSigner { | |
const additionalAmzHeaders = amzOptions.headers; | ||
const awsSecurityCredentials = await this.getCredentials(); | ||
const uri = new URL(url); | ||
|
||
if (typeof requestPayload !== 'string' && requestPayload !== undefined) { | ||
throw new TypeError( | ||
`'requestPayload' is expected to be a string if provided. Got: ${requestPayload}` | ||
); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. do we know what requestPayload type could be? This could be an ugly error There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We want it to be a string, however we marshal objects to strings a few lines above it. This is preserving the existing functionality and makes it clearer, we used to throw later in the call stack. |
||
} | ||
|
||
const headerMap = await generateAuthenticationHeaderMap({ | ||
crypto: this.crypto, | ||
host: uri.host, | ||
canonicalUri: uri.pathname, | ||
canonicalQuerystring: uri.search.substr(1), | ||
canonicalQuerystring: uri.search.slice(1), | ||
method, | ||
region: this.region, | ||
securityCredentials: awsSecurityCredentials, | ||
requestPayload, | ||
additionalAmzHeaders, | ||
}); | ||
// Append additional optional headers, eg. X-Amz-Target, Content-Type, etc. | ||
const headers: {[key: string]: string} = Object.assign( | ||
const headers = Gaxios.mergeHeaders( | ||
// Add x-amz-date if available. | ||
headerMap.amzDate ? {'x-amz-date': headerMap.amzDate} : {}, | ||
{ | ||
|
@@ -149,7 +145,7 @@ export class AwsRequestSigner { | |
additionalAmzHeaders || {} | ||
); | ||
if (awsSecurityCredentials.token) { | ||
Object.assign(headers, { | ||
Gaxios.mergeHeaders(headers, { | ||
'x-amz-security-token': awsSecurityCredentials.token, | ||
}); | ||
} | ||
|
@@ -159,7 +155,7 @@ export class AwsRequestSigner { | |
headers, | ||
}; | ||
|
||
if (typeof requestPayload !== 'undefined') { | ||
if (requestPayload !== undefined) { | ||
awsSignedReq.body = requestPayload; | ||
} | ||
|
||
|
@@ -223,7 +219,9 @@ async function getSigningKey( | |
async function generateAuthenticationHeaderMap( | ||
options: GenerateAuthHeaderMapOptions | ||
): Promise<AwsAuthHeaderMap> { | ||
const additionalAmzHeaders = options.additionalAmzHeaders || {}; | ||
const additionalAmzHeaders = Gaxios.mergeHeaders( | ||
options.additionalAmzHeaders | ||
); | ||
const requestPayload = options.requestPayload || ''; | ||
// iam.amazonaws.com host => iam service. | ||
// sts.us-east-2.amazonaws.com => sts service. | ||
|
@@ -237,38 +235,38 @@ async function generateAuthenticationHeaderMap( | |
// Format: '%Y%m%d'. | ||
const dateStamp = now.toISOString().replace(/[-]/g, '').replace(/T.*/, ''); | ||
|
||
// Change all additional headers to be lower case. | ||
const reformattedAdditionalAmzHeaders: Headers = {}; | ||
Object.keys(additionalAmzHeaders).forEach(key => { | ||
reformattedAdditionalAmzHeaders[key.toLowerCase()] = | ||
additionalAmzHeaders[key]; | ||
}); | ||
// Add AWS token if available. | ||
if (options.securityCredentials.token) { | ||
reformattedAdditionalAmzHeaders['x-amz-security-token'] = | ||
options.securityCredentials.token; | ||
additionalAmzHeaders.set( | ||
'x-amz-security-token', | ||
options.securityCredentials.token | ||
); | ||
} | ||
// Header keys need to be sorted alphabetically. | ||
const amzHeaders = Object.assign( | ||
const amzHeaders = Gaxios.mergeHeaders( | ||
{ | ||
host: options.host, | ||
}, | ||
// Previously the date was not fixed with x-amz- and could be provided manually. | ||
// https://github.com/boto/botocore/blob/879f8440a4e9ace5d3cf145ce8b3d5e5ffb892ef/tests/unit/auth/aws4_testsuite/get-header-value-trim.req | ||
reformattedAdditionalAmzHeaders.date ? {} : {'x-amz-date': amzDate}, | ||
reformattedAdditionalAmzHeaders | ||
additionalAmzHeaders.has('date') ? {} : {'x-amz-date': amzDate}, | ||
additionalAmzHeaders | ||
); | ||
let canonicalHeaders = ''; | ||
const signedHeadersList = Object.keys(amzHeaders).sort(); | ||
|
||
// TypeScript is missing `Headers#keys` at the time of writing | ||
const signedHeadersList = [ | ||
...(amzHeaders as Headers & {keys: () => string[]}).keys(), | ||
].sort(); | ||
signedHeadersList.forEach(key => { | ||
canonicalHeaders += `${key}:${amzHeaders[key]}\n`; | ||
canonicalHeaders += `${key}:${amzHeaders.get(key)}\n`; | ||
}); | ||
const signedHeaders = signedHeadersList.join(';'); | ||
|
||
const payloadHash = await options.crypto.sha256DigestHex(requestPayload); | ||
// https://docs.aws.amazon.com/general/latest/gr/sigv4-create-canonical-request.html | ||
const canonicalRequest = | ||
`${options.method}\n` + | ||
`${options.method.toUpperCase()}\n` + | ||
`${options.canonicalUri}\n` + | ||
`${options.canonicalQuerystring}\n` + | ||
`${canonicalHeaders}\n` + | ||
|
@@ -298,7 +296,7 @@ async function generateAuthenticationHeaderMap( | |
|
||
return { | ||
// Do not return x-amz-date if date is available. | ||
amzDate: reformattedAdditionalAmzHeaders.date ? undefined : amzDate, | ||
amzDate: additionalAmzHeaders.has('date') ? undefined : amzDate, | ||
authorizationHeader, | ||
canonicalQuerystring: options.canonicalQuerystring, | ||
}; | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why is this type changing? seems more flexible? also not clear if this change has to do with the gaxios upgrade.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The
HttpMethod
was a handwritten version for what the spec provides. The spec accepts any method as a string.