Skip to content

Commit

Permalink
refactor: remove request from *auth files
Browse files Browse the repository at this point in the history
* `azure_auth`
* `exec_auth`
* `file_auth`
* `gcp_auth`
* `oidc_auth`
  • Loading branch information
mstruebing committed Mar 24, 2023
1 parent d9626f9 commit c55ce1a
Show file tree
Hide file tree
Showing 10 changed files with 54 additions and 70 deletions.
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
31 changes: 15 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,7 @@ describe('ExecAuth', () => {
stdout: JSON.stringify({ status: { token: 'foo' } }),
} as execa.ExecaSyncReturnValue;
};
const opts = {} as request.Options;
opts.headers = [];
const opts = {} as https.RequestOptions;
auth.applyAuthentication(
{
name: 'user',
Expand Down Expand Up @@ -115,8 +114,8 @@ describe('ExecAuth', () => {
},
},
};
const opts = {} as request.Options;
opts.headers = [];
const opts = {} as https.RequestOptions;
opts.headers = {} as OutgoingHttpHeaders;

auth.applyAuthentication(user, opts);
expect(opts.headers.Authorization).to.be.undefined;
Expand Down Expand Up @@ -158,8 +157,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 +180,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 +215,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 +241,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

0 comments on commit c55ce1a

Please sign in to comment.