Remove accessToken option from gr-auth
Gerrit.Auth.setup() is not called from anywhere. So we can just remove
it and everything related to it. This was originally added for
integrating Gerrit into Google Cloud products. See change 110335,
change 110349, change 117930.
Release-Notes: skip
Change-Id: I6bd404f483f9a3a5b9344109e5e4be293f03adbc
diff --git a/polygerrit-ui/app/services/gr-auth/gr-auth.ts b/polygerrit-ui/app/services/gr-auth/gr-auth.ts
index a6b4fdd..75aad72 100644
--- a/polygerrit-ui/app/services/gr-auth/gr-auth.ts
+++ b/polygerrit-ui/app/services/gr-auth/gr-auth.ts
@@ -7,7 +7,6 @@
import {AuthRequestInit, Finalizable} from '../../types/types';
export enum AuthType {
XSRF_TOKEN = 'xsrf_token',
- ACCESS_TOKEN = 'access_token',
}
export enum AuthStatus {
@@ -17,17 +16,6 @@
ERROR = 3,
}
-export interface Token {
- access_token?: string;
- expires_at?: string;
-}
-
-export type GetTokenCallback = () => Promise<Token | null>;
-
-export interface DefaultAuthOptions {
- credentials: RequestCredentials;
-}
-
export const authServiceToken = define<AuthService>('auth-service');
export interface AuthService extends Finalizable {
@@ -42,11 +30,6 @@
clearCache(): void;
/**
- * Enable cross-domain authentication using OAuth access token.
- */
- setup(getToken: GetTokenCallback, defaultOptions: DefaultAuthOptions): void;
-
- /**
* Perform network fetch with authentication.
*/
fetch(url: string, options?: AuthRequestInit): Promise<Response>;
diff --git a/polygerrit-ui/app/services/gr-auth/gr-auth_impl.ts b/polygerrit-ui/app/services/gr-auth/gr-auth_impl.ts
index fb53d5e..bd2734d 100644
--- a/polygerrit-ui/app/services/gr-auth/gr-auth_impl.ts
+++ b/polygerrit-ui/app/services/gr-auth/gr-auth_impl.ts
@@ -6,22 +6,9 @@
import {AuthRequestInit, Finalizable} from '../../types/types';
import {fire} from '../../utils/event-util';
import {getBaseUrl} from '../../utils/url-util';
-import {
- AuthService,
- AuthStatus,
- AuthType,
- DefaultAuthOptions,
- GetTokenCallback,
- Token,
-} from './gr-auth';
+import {AuthService, AuthStatus, AuthType} from './gr-auth';
export const MAX_AUTH_CHECK_WAIT_TIME_MS = 1000 * 30; // 30s
-const MAX_GET_TOKEN_RETRIES = 2;
-
-interface ValidToken extends Token {
- access_token: string;
- expires_at: string;
-}
interface AuthRequestInitWithHeaders extends AuthRequestInit {
// RequestInit define headers as optional property with a type
@@ -38,7 +25,6 @@
// AuthStatus to API
static TYPE = {
XSRF_TOKEN: AuthType.XSRF_TOKEN,
- ACCESS_TOKEN: AuthType.ACCESS_TOKEN,
};
static STATUS = {
@@ -56,20 +42,6 @@
private _status = AuthStatus.UNDETERMINED;
- private retriesLeft = MAX_GET_TOKEN_RETRIES;
-
- private cachedTokenPromise: Promise<Token | null> | null = null;
-
- private type?: AuthType;
-
- private defaultOptions: AuthRequestInit = {};
-
- private getToken: GetTokenCallback;
-
- constructor() {
- this.getToken = () => Promise.resolve(this.cachedTokenPromise);
- }
-
get baseUrl() {
return getBaseUrl();
}
@@ -143,37 +115,14 @@
}
/**
- * Enable cross-domain authentication using OAuth access token.
- */
- setup(getToken: GetTokenCallback, defaultOptions: DefaultAuthOptions) {
- this.retriesLeft = MAX_GET_TOKEN_RETRIES;
- if (getToken) {
- this.type = AuthType.ACCESS_TOKEN;
- this.cachedTokenPromise = null;
- this.getToken = getToken;
- }
- this.defaultOptions = {};
- if (defaultOptions) {
- this.defaultOptions.credentials = defaultOptions.credentials;
- }
- }
-
- /**
* Perform network fetch with authentication.
*/
fetch(url: string, options?: AuthRequestInit): Promise<Response> {
const optionsWithHeaders: AuthRequestInitWithHeaders = {
headers: new Headers(),
- ...this.defaultOptions,
...options,
};
- if (this.type === AuthType.ACCESS_TOKEN) {
- return this._getAccessToken().then(accessToken =>
- this._fetchWithAccessToken(url, optionsWithHeaders, accessToken)
- );
- } else {
- return this._fetchWithXsrfToken(url, optionsWithHeaders);
- }
+ return this._fetchWithXsrfToken(url, optionsWithHeaders);
}
// private but used in test
@@ -191,23 +140,6 @@
return result;
}
- // private but used in test
- _isTokenValid(token: Token | null): token is ValidToken {
- if (!token) {
- return false;
- }
- if (!token.access_token || !token.expires_at) {
- return false;
- }
-
- const expiration = new Date(Number(token.expires_at) * 1000);
- if (Date.now() >= expiration.getTime()) {
- return false;
- }
-
- return true;
- }
-
private _fetchWithXsrfToken(
url: string,
options: AuthRequestInitWithHeaders
@@ -222,72 +154,6 @@
return this._ensureBodyLoaded(fetch(url, options));
}
- private _getAccessToken(): Promise<string | null> {
- if (!this.cachedTokenPromise) {
- this.cachedTokenPromise = this.getToken();
- }
- return this.cachedTokenPromise.then(token => {
- if (this._isTokenValid(token)) {
- this.retriesLeft = MAX_GET_TOKEN_RETRIES;
- return token.access_token;
- }
- if (this.retriesLeft > 0) {
- this.retriesLeft--;
- this.cachedTokenPromise = null;
- return this._getAccessToken();
- }
- // Fall back to anonymous access.
- return null;
- });
- }
-
- private _fetchWithAccessToken(
- url: string,
- options: AuthRequestInitWithHeaders,
- accessToken: string | null
- ): Promise<Response> {
- const params = [];
-
- if (accessToken) {
- params.push(`access_token=${accessToken}`);
- const baseUrl = this.baseUrl;
- const pathname = baseUrl
- ? url.substring(url.indexOf(baseUrl) + baseUrl.length)
- : url;
- if (!pathname.startsWith('/a/')) {
- url = url.replace(pathname, '/a' + pathname);
- }
- }
-
- const method = options.method || 'GET';
- let contentType = options.headers.get('Content-Type');
-
- // For all requests with body, ensure json content type.
- if (!contentType && options.body) {
- contentType = 'application/json';
- }
-
- if (method !== 'GET') {
- options.method = 'POST';
- params.push(`$m=${method}`);
- // If a request is not GET, and does not have a body, ensure text/plain
- // content type.
- if (!contentType) {
- contentType = 'text/plain';
- }
- }
-
- if (contentType) {
- options.headers.set('Content-Type', 'text/plain');
- params.push(`$ct=${encodeURIComponent(contentType)}`);
- }
-
- if (params.length) {
- url = url + (url.indexOf('?') === -1 ? '?' : '&') + params.join('&');
- }
- return this._ensureBodyLoaded(fetch(url, options));
- }
-
private _ensureBodyLoaded(response: Promise<Response>): Promise<Response> {
return response.then(response => {
if (!response.ok) {
diff --git a/polygerrit-ui/app/services/gr-auth/gr-auth_mock.ts b/polygerrit-ui/app/services/gr-auth/gr-auth_mock.ts
index 9cdd37e..693772c 100644
--- a/polygerrit-ui/app/services/gr-auth/gr-auth_mock.ts
+++ b/polygerrit-ui/app/services/gr-auth/gr-auth_mock.ts
@@ -5,12 +5,7 @@
*/
import {AuthRequestInit} from '../../types/types';
import {fire} from '../../utils/event-util';
-import {
- AuthService,
- AuthStatus,
- DefaultAuthOptions,
- GetTokenCallback,
-} from './gr-auth';
+import {AuthService, AuthStatus} from './gr-auth';
import {Auth} from './gr-auth_impl';
export class GrAuthMock implements AuthService {
@@ -55,8 +50,6 @@
clearCache() {}
- setup(_getToken: GetTokenCallback, _defaultOptions: DefaultAuthOptions) {}
-
fetch(_url: string, _options?: AuthRequestInit): Promise<Response> {
return Promise.resolve(new Response());
}
diff --git a/polygerrit-ui/app/services/gr-auth/gr-auth_test.ts b/polygerrit-ui/app/services/gr-auth/gr-auth_test.ts
index b9cef86..a7138fc 100644
--- a/polygerrit-ui/app/services/gr-auth/gr-auth_test.ts
+++ b/polygerrit-ui/app/services/gr-auth/gr-auth_test.ts
@@ -5,9 +5,7 @@
*/
import '../../test/common-test-setup';
import {Auth} from './gr-auth_impl';
-import {stubBaseUrl} from '../../test/test-utils';
import {SinonFakeTimers} from 'sinon';
-import {DefaultAuthOptions} from './gr-auth';
import {assert} from '@open-wc/testing';
import {AuthRequestInit} from '../../types/types';
@@ -196,134 +194,4 @@
assert.equal(options.headers.get('X-Gerrit-Auth'), 'foobar');
});
});
-
- suite('cors (access token)', () => {
- let fakeFetch: sinon.SinonStub;
-
- setup(() => {
- fakeFetch = sinon
- .stub(window, 'fetch')
- .returns(Promise.resolve({...new Response(), ok: true}));
- });
-
- let getToken: sinon.SinonStub;
-
- const makeToken = (accessToken?: string) => {
- return {
- access_token: accessToken || 'zbaz',
- expires_at: new Date(Date.now() + 10e8).getTime(),
- };
- };
-
- setup(() => {
- getToken = sinon.stub();
- getToken.returns(Promise.resolve(makeToken()));
- const defaultOptions: DefaultAuthOptions = {
- credentials: 'include',
- };
- auth.setup(getToken, defaultOptions);
- });
-
- test('base url support', async () => {
- const baseUrl = 'http://foo';
- stubBaseUrl(baseUrl);
- await auth.fetch(baseUrl + '/url', {bar: 'bar'} as AuthRequestInit);
- const [url] = fakeFetch.lastCall.args;
- assert.equal(url, 'http://foo/a/url?access_token=zbaz');
- });
-
- test('fetch not signed in', async () => {
- getToken.returns(Promise.resolve());
- await auth.fetch('/url', {bar: 'bar'} as AuthRequestInit);
- const [url, options] = fakeFetch.lastCall.args;
- assert.equal(url, '/url');
- assert.equal(options.bar, 'bar');
- assert.equal(Object.keys(options.headers).length, 0);
- });
-
- test('fetch signed in', async () => {
- await auth.fetch('/url', {bar: 'bar'} as AuthRequestInit);
- const [url, options] = fakeFetch.lastCall.args;
- assert.equal(url, '/a/url?access_token=zbaz');
- assert.equal(options.bar, 'bar');
- });
-
- test('getToken calls are cached', async () => {
- await Promise.all([auth.fetch('/url-one'), auth.fetch('/url-two')]);
- assert.equal(getToken.callCount, 1);
- });
-
- test('getToken refreshes token', async () => {
- const isTokenValidStub = sinon.stub(auth, '_isTokenValid');
- isTokenValidStub
- .onFirstCall()
- .returns(true)
- .onSecondCall()
- .returns(false)
- .onThirdCall()
- .returns(true);
- await auth.fetch('/url-one');
- getToken.returns(Promise.resolve(makeToken('bzzbb')));
- await auth.fetch('/url-two');
-
- const [[firstUrl], [secondUrl]] = fakeFetch.args;
- assert.equal(firstUrl, '/a/url-one?access_token=zbaz');
- assert.equal(secondUrl, '/a/url-two?access_token=bzzbb');
- });
-
- test('signed in token error falls back to anonymous', async () => {
- getToken.returns(Promise.resolve('rubbish'));
- await auth.fetch('/url', {bar: 'bar'} as AuthRequestInit);
- const [url, options] = fakeFetch.lastCall.args;
- assert.equal(url, '/url');
- assert.equal(options.bar, 'bar');
- });
-
- test('_isTokenValid', () => {
- assert.isFalse(auth._isTokenValid(null));
- assert.isFalse(auth._isTokenValid({}));
- assert.isFalse(auth._isTokenValid({access_token: 'foo'}));
- assert.isFalse(
- auth._isTokenValid({
- access_token: 'foo',
- expires_at: `${Date.now() / 1000 - 1}`,
- })
- );
- assert.isTrue(
- auth._isTokenValid({
- access_token: 'foo',
- expires_at: `${Date.now() / 1000 + 1}`,
- })
- );
- });
-
- test('HTTP PUT with content type', async () => {
- const originalOptions = {
- method: 'PUT',
- headers: new Headers({'Content-Type': 'mail/pigeon'}),
- };
- await auth.fetch('/url', originalOptions);
- assert.isTrue(getToken.called);
- const [url, options] = fakeFetch.lastCall.args;
- assert.include(url, '$ct=mail%2Fpigeon');
- assert.include(url, '$m=PUT');
- assert.include(url, 'access_token=zbaz');
- assert.equal(options.method, 'POST');
- assert.equal(options.headers.get('Content-Type'), 'text/plain');
- });
-
- test('HTTP PUT without content type', async () => {
- const originalOptions = {
- method: 'PUT',
- };
- await auth.fetch('/url', originalOptions);
- assert.isTrue(getToken.called);
- const [url, options] = fakeFetch.lastCall.args;
- assert.include(url, '$ct=text%2Fplain');
- assert.include(url, '$m=PUT');
- assert.include(url, 'access_token=zbaz');
- assert.equal(options.method, 'POST');
- assert.equal(options.headers.get('Content-Type'), 'text/plain');
- });
- });
});
