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

Support for specifying an http.Agent #402

Merged
merged 14 commits into from
Dec 7, 2018
Merged

Support for specifying an http.Agent #402

merged 14 commits into from
Dec 7, 2018

Conversation

hiranya911
Copy link
Contributor

@hiranya911 hiranya911 commented Nov 29, 2018

go/firebase-admin-node-proxy

Tested with mitmproxy and tunnel2:

    const tunnel = require('tunnel2');
    const proxyAgent = tunnel.httpsOverHttp({
      proxy: {
        host: 'localhost',
        port: 8080,
      }
    });
    const options = getOptions();
    options.httpAgent = proxyAgent;
    const app = admin.initializeApp(options, 'proxyApp');
    return admin.auth(app).createUser({}).then((user) => {
      return admin.auth(app).deleteUser(user.uid);
    }).catch((err) => {
      console.log(err);
    });

Sample request captured at mitmproxy:

2018-11-29 16:19:51 POST https://identitytoolkit.googleapis.com/v1/projects/my-project-id/accounts
                         ← 200 OK application/json 114b 497msDetail
X-Client-Version:  Node/Admin/6.3.0                                                                                                                                                         
Authorization:     Bearer xxxxxx
Content-Type:      application/json;charset=utf-8                                                                                                                                           
Content-Length:    2                                                                                                                                                                        
Host:              identitytoolkit.googleapis.com:443                                                                                                                                       
Connection:        close                                                                                                                                                                    
JSON                                                                                                                                                                                  [m:auto]
{}

return mockAppWithAgent.delete();
});

it('should use the HTTP agent set in request', () => {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is it any different than 'should use the HTTP agent set in request' above. They look pretty similar.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The first one was for the HttpClient class. This one's for AuthorizedHttpClient, which is a subclass of the former. The child class has the ability to use the agent configured at app-level, but also supports overriding it at request-level. These tests verify this.

@danpe
Copy link

danpe commented Dec 5, 2018

Hey @hiranya911, @Feiyang1
Is there a planned merge/release date for that issue?

@hiranya911
Copy link
Contributor Author

@danpe nothing concrete. As soon as the code review is done.

@hiranya911 hiranya911 merged commit 42b5806 into master Dec 7, 2018
@hiranya911 hiranya911 deleted the hkj-http-agent branch December 7, 2018 19:01
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 this pull request may close these issues.

4 participants