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

[CCI] aws-sdk instead of third-party aws4 for signing request? #464

Closed
timursaurus opened this issue Mar 28, 2023 · 4 comments
Closed

[CCI] aws-sdk instead of third-party aws4 for signing request? #464

timursaurus opened this issue Mar 28, 2023 · 4 comments
Labels
CCI College Contributor Initiative 🧗 enhancement New feature or request

Comments

@timursaurus
Copy link
Collaborator

timursaurus commented Mar 28, 2023

Should I swap the third-party library aws4 for official amazon aws-sdk packages for signing requests? @aws-sdk/signature-v4 and @aws-crypto/sha256-js in the client TypeScript rewrite

The current implementation in the JS client

  function buildSignedRequestObject(request = {}) {
    request.service = opts.service;
    request.region = opts.region;
    request.headers = request.headers || {};
    request.headers['host'] = request.hostname;
    const signed = aws4.sign(request, credentialsState.credentials);
    signed.headers['x-amz-content-sha256'] = crypto
      .createHash('sha256')
      .update(request.body || '', 'utf8')
      .digest('hex');
    return signed;
  }

In the client rewrite, I propose to use official aws-sdk packages. There wouldn't be any problems, right?
Example using the SDK (more clean, IMO):

import { SignatureV4 } from '@aws-sdk/signature-v4'
import { Sha256 } from '@aws-crypto/sha256-js'

const sigv4 = new SignatureV4({
    service: init.service,
    region: init.region
    credentials: {
      ...init.credentials
    },
    sha256: Sha256,
})

const signed = await sigv4.sign({
   ...
   hostname: url.hostname
   ...
})

And since the JS client already uses aws-sdk packages for v3, it would make more sense to ditch third-party aws4?

@timursaurus timursaurus changed the title aws-sdk instead of third-party aws4 for signing request? [CCI] aws-sdk instead of third-party aws4 for signing request? Mar 28, 2023
@dblock
Copy link
Member

dblock commented Mar 28, 2023

+1 on switching to the official SDK

@dblock dblock added the 🧗 enhancement New feature or request label Mar 28, 2023
@nhtruong
Copy link
Collaborator

The rewrite is an opportunity to make things right. So go at it :)

A note on writing the API methods: Instead of writing those methods manually, we should generate them from API Spec. The API spec is currently being backfilled so we should wait on that.

Also I think all concerns regarding the rewrite should be put in this ticket instead of opening a new issue every time something comes up.

@timursaurus
Copy link
Collaborator Author

oh! that's interesting, got it

@harshavamsi harshavamsi added the CCI College Contributor Initiative label Mar 29, 2023
@nhtruong
Copy link
Collaborator

Closing this issue because it should be a comment in #269

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CCI College Contributor Initiative 🧗 enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

4 participants