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

Connection.request params expected RequestOptions without querystring #951

Closed
villasv opened this issue Aug 21, 2019 · 7 comments · Fixed by #957
Closed

Connection.request params expected RequestOptions without querystring #951

villasv opened this issue Aug 21, 2019 · 7 comments · Fixed by #957

Comments

@villasv
Copy link
Contributor

villasv commented Aug 21, 2019

🐛 Bug Report

The typing for Connection.request (defined here) states that params is of type RequestOptions.

At runtime, I observe the following object for an indices.exists call:

{ method: 'HEAD',
  path: '/_alias/idxname',
  body: null,
  querystring: '',
  headers:
   { 'User-Agent':
      'elasticsearch-js/7.3.0 (linux 4.19.57-microsoft-standard-x64; Node.js v10.14.1)' },

But RequestOptions has no attributes body and querystring. I expected to find those in TransportRequestParams instead. But TransportRequestParams also has headers missing, so it can't be it either (defined here).

To Reproduce

I ran into this when trying to sign requests with AWS credentials.

import { Connection as UnsignedConnection } from '@elastic/elasticsearch';
import * as AWS from 'aws-sdk';
import RequestSigner from 'aws-sdk/lib/signers/v4';
import { ClientRequest, RequestOptions, IncomingMessage } from 'http';

class AwsElasticsearchError extends Error {}


class AwsSignedConnection extends UnsignedConnection {
  public request(
    params: RequestOptions,
    callback: (err: Error | null, response: IncomingMessage | null) => void,
  ): ClientRequest {
    const signedParams = this.signParams(params);
    return super.request(signedParams, callback);
  }

  private signParams(params: any): RequestOptions {
    const region = AWS.config.region || process.env.AWS_DEFAULT_REGION;
    if (!region) {
      throw new AwsElasticsearchError('missing region configuration');
    }

    const endpoint = new AWS.Endpoint(this.url.href);
    const request = new AWS.HttpRequest(endpoint, region);

    request.method = params.method;
    request.path = params.querystring
      ? `${params.path}/?${params.querystring}`
      : params.path;
    request.body = params.body;

    request.headers = params.headers;
    request.headers.Host = endpoint.host;

    const signer = new RequestSigner(request, 'es');
    signer.addAuthorization(AWS.config.credentials, new Date());
    return request;
  }
}

export { AwsSignedConnection, UnsignedConnection, AwsElasticsearchError };

...

Expected behavior

No type errors.

Your Environment

  • node version: 10.14.1
  • @elastic/elasticsearch version: =7.3.0
  • os: Windows
@delvedor
Copy link
Member

delvedor commented Sep 4, 2019

Hello, sorry for the delay! You are right, we should add body and querystring here:

interface RequestOptions extends http.ClientRequestArgs {
asStream?: boolean;
}

This is happening because the client is using the Node.js http.request API, which uses .end to send the body, while the querystring lives in search.

Would you mind open a pull request to fix this?

@jawadmjn
Copy link

jawadmjn commented Apr 6, 2020

Hi @villasv I stuck on the same issue, I have to send a Signed Request to Aws elasticSearch.
For example in a simple request:

var endpoint = new AWS.Endpoint(domain);
var request = new AWS.HttpRequest(endpoint, region);

request.method = 'PUT';
request.path += index + '/' + type + '/' + id;
request.body = JSON.stringify(document);
request.headers['host'] = domain;
request.headers['Content-Type'] = 'application/json';
request.headers['Content-Length'] = Buffer.byteLength(request.body);

var credentials = new AWS.EnvironmentCredentials('AWS');
var signer = new AWS.Signers.V4(request, 'es');
signer.addAuthorization(credentials, new Date());


I don't think that Documentation are explaining it well.

Can you please help me in this thing ?

@villasv
Copy link
Contributor Author

villasv commented Apr 6, 2020

Hi @jawadmjn, here's the code I've been using:

import { Connection as UnsignedConnection } from '@elastic/elasticsearch';
import * as AWS from 'aws-sdk';
import RequestSigner from 'aws-sdk/lib/signers/v4';
import { ClientRequest, RequestOptions, IncomingMessage } from 'http';

class AwsElasticsearchError extends Error {}


class AwsSignedConnection extends UnsignedConnection {
  public request(
    params: RequestOptions,
    callback: (err: Error | null, response: IncomingMessage | null) => void,
  ): ClientRequest {
    const signedParams = this.signParams(params);
    return super.request(signedParams, callback);
  }

  private signParams(params: any): RequestOptions {
    const region = AWS.config.region || process.env.AWS_DEFAULT_REGION;
    if (!region) {
      throw new AwsElasticsearchError('missing region configuration');
    }

    const endpoint = new AWS.Endpoint(this.url.href);
    const request = new AWS.HttpRequest(endpoint, region);

    request.method = params.method;
    request.path = params.querystring
      ? `${params.path}/?${params.querystring}`
      : params.path;
    request.body = params.body;

    request.headers = params.headers;
    request.headers.Host = endpoint.host;

    const signer = new RequestSigner(request, 'es');
    signer.addAuthorization(AWS.config.credentials, new Date());
    return request;
  }
}

export { AwsSignedConnection, UnsignedConnection, AwsElasticsearchError };

@jawadmjn
Copy link

jawadmjn commented Apr 7, 2020

@villasv thanks man I have this, and how are you doing this part, part of injecting this connection in the client,
what I have is

`

    let awsSignedConnection = new AwsSignedConnection(this.aws, this.esHost);
    
    let requestParams = {
        path: `/${this.index}/_search/`,
        method: "POST",
    };

    const client = new Client({
        node: this.esHost,
        Connection: awsSignedConnection.request(requestParams)
    });

    return client.search(params);

`

@jawadmjn
Copy link

jawadmjn commented Apr 7, 2020

TS2345: Argument of type '{ node: any; Connection: ClientRequest; }' is not assignable to parameter of type 'ClientOptions'.   Types of property 'Connection' are incompatible.     Type 'ClientRequest' is missing the following properties from type 'typeof Connection': prototype, statuses, roles

@villasv
Copy link
Contributor Author

villasv commented Apr 7, 2020

That's not you're supposed to use connection classes. Here's an example:

  const esClient = new Client({
    Connection: AWS.config.credentials ? AwsSignedConnection : UnsignedConnection,
    node: 'http://localhost:9200', // esHost
  });

Then you can just use client.search(...) without worrying about signatures.

@jawadmjn
Copy link

jawadmjn commented Apr 8, 2020

@villasv You are a rock star, It worked man. Thanks a lot 👍 :)

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

Successfully merging a pull request may close this issue.

3 participants