Skip to content

refactor: remove request from *auth files #1023

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

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion FETCH_MIGRATION.md
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@ Code will be on the `master` branch.
- [x] Generate api with `npm run generate`
- [x] Match src/gen/api.ts to new generated layout (it changes slightly)
- [ ] Fix errors in /src folder (due to new api)
- [ ] migrate src/auth.ts, the dependent implementations (ex: azure_auth, gcp_auth etc) and tests to fetch api from request
- [x] migrate src/auth.ts, the dependent implementations (ex: azure_auth, gcp_auth etc) and tests to fetch api from request
- [ ] migrate src/log.ts and its tests to fetch api from request
- major remaining work is fixing up async signatures and return piping
- [ ] migrate src/watch.ts and its tests to fetch api from request
Expand Down
6 changes: 1 addition & 5 deletions src/azure_auth.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
import * as proc from 'child_process';
import https = require('https');
import * as jsonpath from 'jsonpath-plus';
import request = require('request');

import { Authenticator } from './auth';
import { User } from './config_types';
Expand All @@ -27,10 +26,7 @@ export class AzureAuth implements Authenticator {
return user.authProvider.name === 'azure';
}

public async applyAuthentication(
user: User,
opts: request.Options | https.RequestOptions,
): Promise<void> {
public async applyAuthentication(user: User, opts: https.RequestOptions): Promise<void> {
const token = this.getToken(user);
if (token) {
opts.headers!.Authorization = `Bearer ${token}`;
Expand Down
9 changes: 3 additions & 6 deletions src/exec_auth.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import execa = require('execa');
import { OutgoingHttpHeaders } from 'http';
import https = require('https');
import request = require('request');

import { Authenticator } from './auth';
import { User } from './config_types';
Expand Down Expand Up @@ -36,10 +36,7 @@ export class ExecAuth implements Authenticator {
);
}

public async applyAuthentication(
user: User,
opts: request.Options | https.RequestOptions,
): Promise<void> {
public async applyAuthentication(user: User, opts: https.RequestOptions): Promise<void> {
const credential = this.getCredential(user);
if (!credential) {
return;
Expand All @@ -53,7 +50,7 @@ export class ExecAuth implements Authenticator {
const token = this.getToken(credential);
if (token) {
if (!opts.headers) {
opts.headers = [];
opts.headers = {} as OutgoingHttpHeaders;
}
opts.headers!.Authorization = `Bearer ${token}`;
}
Expand Down
33 changes: 17 additions & 16 deletions src/exec_auth_test.ts
Original file line number Diff line number Diff line change
@@ -1,12 +1,12 @@
import { expect, use } from 'chai';
import chaiAsPromised = require('chai-as-promised');
import chaiAsPromised from 'chai-as-promised';
use(chaiAsPromised);

import * as shell from 'shelljs';

import execa = require('execa');
import request = require('request');
import https = require('https');
import execa from 'execa';
import https from 'https';
import { OutgoingHttpHeaders } from 'http';

import { ExecAuth } from './exec_auth';
import { User } from './config_types';
Expand Down Expand Up @@ -70,8 +70,8 @@ describe('ExecAuth', () => {
stdout: JSON.stringify({ status: { token: 'foo' } }),
} as execa.ExecaSyncReturnValue;
};
const opts = {} as request.Options;
opts.headers = [];
const opts = {} as https.RequestOptions;
opts.headers = {} as OutgoingHttpHeaders;
auth.applyAuthentication(
{
name: 'user',
Expand Down Expand Up @@ -115,8 +115,9 @@ describe('ExecAuth', () => {
},
},
};
const opts = {} as request.Options;
opts.headers = [];
const opts = {} as https.RequestOptions;
opts.headers = {} as OutgoingHttpHeaders;
opts.headers = {} as OutgoingHttpHeaders;

auth.applyAuthentication(user, opts);
expect(opts.headers.Authorization).to.be.undefined;
Expand Down Expand Up @@ -158,8 +159,8 @@ describe('ExecAuth', () => {
},
};

const opts = {} as request.Options;
opts.headers = [];
const opts = {} as https.RequestOptions;
opts.headers = {} as OutgoingHttpHeaders;

await auth.applyAuthentication(user, opts);
expect(opts.headers.Authorization).to.equal(`Bearer ${tokenValue}`);
Expand All @@ -181,8 +182,8 @@ describe('ExecAuth', () => {

it('should return null on no exec info', async () => {
const auth = new ExecAuth();
const opts = {} as request.Options;
opts.headers = [];
const opts = {} as https.RequestOptions;
opts.headers = {} as OutgoingHttpHeaders;

await auth.applyAuthentication({} as User, opts);
expect(opts.headers.Authorization).to.be.undefined;
Expand Down Expand Up @@ -216,8 +217,8 @@ describe('ExecAuth', () => {
},
},
};
const opts = {} as request.Options;
opts.headers = [];
const opts = {} as https.RequestOptions;
opts.headers = {} as OutgoingHttpHeaders;

const promise = auth.applyAuthentication(user, opts);
return expect(promise).to.eventually.be.rejected;
Expand All @@ -242,8 +243,8 @@ describe('ExecAuth', () => {
} as execa.ExecaSyncReturnValue;
};
process.env.BLABBLE = 'flubble';
const opts = {} as request.Options;
opts.headers = [];
const opts = {} as https.RequestOptions;
opts.headers = {} as OutgoingHttpHeaders;

await auth.applyAuthentication(
{
Expand Down
2 changes: 1 addition & 1 deletion src/exec_test.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import { expect } from 'chai';
import WebSocket = require('isomorphic-ws');
import WebSocket from 'isomorphic-ws';
import { ReadableStreamBuffer, WritableStreamBuffer } from 'stream-buffers';
import { anyFunction, anything, capture, instance, mock, verify, when } from 'ts-mockito';

Expand Down
6 changes: 1 addition & 5 deletions src/file_auth.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
import fs = require('fs');
import https = require('https');
import request = require('request');

import { Authenticator } from './auth';
import { User } from './config_types';
Expand All @@ -13,10 +12,7 @@ export class FileAuth implements Authenticator {
return user.authProvider && user.authProvider.config && user.authProvider.config.tokenFile;
}

public async applyAuthentication(
user: User,
opts: request.Options | https.RequestOptions,
): Promise<void> {
public async applyAuthentication(user: User, opts: https.RequestOptions): Promise<void> {
if (this.token == null) {
this.refreshToken(user.authProvider.config.tokenFile);
}
Expand Down
18 changes: 9 additions & 9 deletions src/file_auth_test.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import { expect } from 'chai';
import mockfs = require('mock-fs');

import request = require('request');
import { OutgoingHttpHeaders } from 'http';
import https from 'https';
import mockfs from 'mock-fs';

import { User } from './config_types';
import { FileAuth } from './file_auth';
Expand All @@ -24,8 +24,8 @@ describe('FileAuth', () => {
},
} as User;

const opts = {} as request.Options;
opts.headers = [];
const opts = {} as https.RequestOptions;
opts.headers = {} as OutgoingHttpHeaders;

await auth.applyAuthentication(user, opts);
expect(opts.headers.Authorization).to.equal(`Bearer ${token}`);
Expand All @@ -49,8 +49,8 @@ describe('FileAuth', () => {
},
} as User;

const opts = {} as request.Options;
opts.headers = [];
const opts = {} as https.RequestOptions;
opts.headers = {} as OutgoingHttpHeaders;

await auth.applyAuthentication(user, opts);
expect(opts.headers.Authorization).to.equal(`Bearer ${token}`);
Expand All @@ -73,8 +73,8 @@ describe('FileAuth', () => {
},
} as User;

const opts = {} as request.Options;
opts.headers = [];
const opts = {} as https.RequestOptions;
opts.headers = {} as OutgoingHttpHeaders;

await auth.applyAuthentication(user, opts);
expect(opts.headers.Authorization).to.equal(`Bearer ${token}`);
Expand Down
6 changes: 1 addition & 5 deletions src/gcp_auth.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
import * as proc from 'child_process';
import https = require('https');
import * as jsonpath from 'jsonpath-plus';
import request = require('request');

import { Authenticator } from './auth';
import { User } from './config_types';
Expand All @@ -26,10 +25,7 @@ export class GoogleCloudPlatformAuth implements Authenticator {
return user.authProvider.name === 'gcp';
}

public async applyAuthentication(
user: User,
opts: request.Options | https.RequestOptions,
): Promise<void> {
public async applyAuthentication(user: User, opts: https.RequestOptions): Promise<void> {
const token = this.getToken(user);
if (token) {
opts.headers!.Authorization = `Bearer ${token}`;
Expand Down
5 changes: 2 additions & 3 deletions src/oidc_auth.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
import https = require('https');
import https from 'https';
import { Client, Issuer } from 'openid-client';
import request = require('request');
import { base64url } from 'rfc4648';
import { TextDecoder } from 'util';

Expand Down Expand Up @@ -56,7 +55,7 @@ export class OpenIDConnectAuth implements Authenticator {
*/
public async applyAuthentication(
user: User,
opts: request.Options | https.RequestOptions,
opts: https.RequestOptions,
overrideClient?: any,
): Promise<void> {
const token = await this.getToken(user, overrideClient);
Expand Down
39 changes: 20 additions & 19 deletions src/oidc_auth_test.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import { expect } from 'chai';
import * as request from 'request';
import { OutgoingHttpHeaders } from 'http';
import https from 'https';
import { base64url } from 'rfc4648';
import { TextEncoder } from 'util';

Expand Down Expand Up @@ -74,8 +75,8 @@ describe('OIDCAuth', () => {
},
} as User;

const opts = {} as request.Options;
opts.headers = [];
const opts = {} as https.RequestOptions;
opts.headers = {} as OutgoingHttpHeaders;
await auth.applyAuthentication(user, opts);
expect(opts.headers.Authorization).to.be.undefined;
});
Expand All @@ -95,8 +96,8 @@ describe('OIDCAuth', () => {
},
} as User;

const opts = {} as request.Options;
opts.headers = [];
const opts = {} as https.RequestOptions;
opts.headers = {} as OutgoingHttpHeaders;
await auth.applyAuthentication(user, opts);
expect(opts.headers.Authorization).to.be.undefined;
});
Expand All @@ -114,8 +115,8 @@ describe('OIDCAuth', () => {
},
} as User;

const opts = {} as request.Options;
opts.headers = [];
const opts = {} as https.RequestOptions;
opts.headers = {} as OutgoingHttpHeaders;
(auth as any).currentTokenExpiration = Date.now() / 1000 + 1000;
await auth.applyAuthentication(user, opts, {});
expect(opts.headers.Authorization).to.equal('Bearer fakeToken');
Expand All @@ -136,8 +137,8 @@ describe('OIDCAuth', () => {
},
} as User;

const opts = {} as request.Options;
opts.headers = [];
const opts = {} as https.RequestOptions;
opts.headers = {} as OutgoingHttpHeaders;
await auth.applyAuthentication(user, opts);
expect(opts.headers.Authorization).to.be.undefined;
});
Expand All @@ -157,8 +158,8 @@ describe('OIDCAuth', () => {
},
} as User;

const opts = {} as request.Options;
opts.headers = [];
const opts = {} as https.RequestOptions;
opts.headers = {} as OutgoingHttpHeaders;
await auth.applyAuthentication(user, opts);
expect(opts.headers.Authorization).to.equal(`Bearer ${token}`);
});
Expand All @@ -178,8 +179,8 @@ describe('OIDCAuth', () => {
},
} as User;

const opts = {} as request.Options;
opts.headers = [];
const opts = {} as https.RequestOptions;
opts.headers = {} as OutgoingHttpHeaders;
await auth.applyAuthentication(user, opts, {});
expect(opts.headers.Authorization).to.be.undefined;
});
Expand All @@ -198,8 +199,8 @@ describe('OIDCAuth', () => {
},
} as User;

const opts = {} as request.Options;
opts.headers = [];
const opts = {} as https.RequestOptions;
opts.headers = {} as OutgoingHttpHeaders;
(auth as any).currentTokenExpiration = Date.now() / 1000 + 1000;
await auth.applyAuthentication(user, opts, {});
expect(opts.headers.Authorization).to.equal('Bearer fakeToken');
Expand All @@ -219,8 +220,8 @@ describe('OIDCAuth', () => {
},
} as User;

const opts = {} as request.Options;
opts.headers = [];
const opts = {} as https.RequestOptions;
opts.headers = {} as OutgoingHttpHeaders;
(auth as any).currentTokenExpiration = Date.now() / 1000 - 5000;
const newExpiration = Date.now() / 1000 + 120;
await auth.applyAuthentication(user, opts, {
Expand Down Expand Up @@ -252,8 +253,8 @@ describe('OIDCAuth', () => {
},
} as User;

const opts = {} as request.Options;
opts.headers = [];
const opts = {} as https.RequestOptions;
opts.headers = {} as OutgoingHttpHeaders;
const newExpiration = Date.now() / 1000 + 120;
(auth as any).currentTokenExpiration = 0;
await auth.applyAuthentication(user, opts, {
Expand Down