Skip to content

Commit

Permalink
fix(authentication): don't crash if an error occurs when initializing…
Browse files Browse the repository at this point in the history
… the authentication client (#862)
  • Loading branch information
Guillaume Gautreau authored Oct 26, 2023
1 parent decc179 commit 265b1b4
Show file tree
Hide file tree
Showing 2 changed files with 132 additions and 43 deletions.
51 changes: 34 additions & 17 deletions packages/forestadmin-client/src/auth/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,21 +12,25 @@ export default class AuthService implements ForestAdminAuthServiceInterface {

constructor(private options: ForestAdminClientOptionsWithDefaults) {}

/**
* Initialize the authentication client upfront. This speeds up the first
* authentication request.
*/
public async init(): Promise<void> {
if (this.client) return;

// We can't use 'Issuer.discover' because the oidc config is behind an auth-wall.
const url = '/oidc/.well-known/openid-configuration';
const config = await ServerUtils.query<IssuerMetadata>(this.options, 'get', url);
const issuer = new Issuer(config);
const registration = { token_endpoint_auth_method: 'none' as ClientAuthMethod };

this.client = await (issuer.Client as ClientExt).register(registration, {
initialAccessToken: this.options.envSecret,
});
try {
await this.createClient();
} catch (e) {
// Sometimes the authentication client can't be initialized because of a
// server or network error. We don't want the application to crash.
this.options.logger(
'Warn',
// eslint-disable-next-line max-len
`Error while registering the authentication client. Authentication might not work: ${e.message}`,
);
}
}

async getUserInfo(renderingId: number, accessToken: string): Promise<UserInfo> {
public async getUserInfo(renderingId: number, accessToken: string): Promise<UserInfo> {
const url = `/liana/v2/renderings/${renderingId}/authorization`;
const headers = { 'forest-token': accessToken };

Expand Down Expand Up @@ -56,14 +60,12 @@ export default class AuthService implements ForestAdminAuthServiceInterface {
scope: string;
state: string;
}): Promise<string> {
if (!this.client) throw new Error('AuthService not initialized');
await this.createClient();

const url = this.client.authorizationUrl({
return this.client.authorizationUrl({
scope,
state,
});

return url;
}

public async generateTokens({
Expand All @@ -73,7 +75,7 @@ export default class AuthService implements ForestAdminAuthServiceInterface {
query: ParsedUrlQuery;
state: string;
}): Promise<Tokens> {
if (!this.client) throw new Error('AuthService not initialized');
await this.createClient();

try {
const tokens = await this.client.callback(undefined, query, { state });
Expand All @@ -86,6 +88,21 @@ export default class AuthService implements ForestAdminAuthServiceInterface {
}
}

protected async createClient() {
if (this.client) return;

// We can't use async 'Issuer.discover' because the oidc config is behind an auth-wall.
const url = '/oidc/.well-known/openid-configuration';
const config = await ServerUtils.query<IssuerMetadata>(this.options, 'get', url);
const issuer = new Issuer(config);

const registration = { token_endpoint_auth_method: 'none' as ClientAuthMethod };

this.client = await (issuer.Client as ClientExt).register(registration, {
initialAccessToken: this.options.envSecret,
});
}

private handleError(e: Error) {
if (e instanceof errors.OPError) {
throw new AuthenticationError(e);
Expand Down
124 changes: 98 additions & 26 deletions packages/forestadmin-client/test/auth/index.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,21 @@ describe('AuthService', () => {
{ initialAccessToken: options.envSecret },
);
});

it('should only log the error and not throw if creating the client throws', async () => {
serverQueryMock.mockImplementationOnce(() => {
throw new Error('myError');
});

const options = factories.forestAdminClientOptions.build();
const service = new AuthService(options);

await expect(service.init()).resolves.toBe(undefined);
expect(options.logger).toHaveBeenCalledWith(
'Warn',
'Error while registering the authentication client. Authentication might not work: myError',
);
});
});

describe('getUserInfo', () => {
Expand Down Expand Up @@ -133,19 +148,6 @@ describe('AuthService', () => {
state: 'myState',
});
});

test('should throw if the client is not initialized', async () => {
const options = factories.forestAdminClientOptions.build();
const unitializedService = new AuthService(options);

const fn = () =>
unitializedService.generateAuthorizationUrl({
scope: 'myScope',
state: 'myState',
});

await expect(fn).rejects.toThrow('AuthService not initialized');
});
});

describe('generateTokens', () => {
Expand All @@ -164,19 +166,6 @@ describe('AuthService', () => {
});
});

test('should throw if the client is not initialized', async () => {
const options = factories.forestAdminClientOptions.build();
const unitializedService = new AuthService(options);

const fn = () =>
unitializedService.generateTokens({
query: { code: 'myCode' },
state: 'myState',
});

await expect(fn).rejects.toThrow('AuthService not initialized');
});

test('should translate an OPError to an AuthenticationError', async () => {
const opError = new openidClient.errors.OPError({
error: 'myError',
Expand Down Expand Up @@ -208,4 +197,87 @@ describe('AuthService', () => {
});
});
});

describe('if initialization failed', () => {
let fakeClient: {
authorizationUrl: jest.Mock;
callback: jest.Mock;
};
let service: AuthService;

beforeEach(async () => {
fakeClient = {
authorizationUrl: jest.fn(),
callback: jest.fn(),
};
const fakeIssuer = {
Client: { register: jest.fn().mockReturnValue(fakeClient) },
};

issuerMock.mockReturnValue(fakeIssuer);
const options = factories.forestAdminClientOptions.build();
service = new AuthService(options);
});

describe('generateAuthorizationUrl', () => {
it('should try to reinit the client if initialization failed', async () => {
fakeClient.authorizationUrl.mockResolvedValue('myAuthorizationUrl');

await service.generateAuthorizationUrl({
scope: 'myScope',
state: 'myState',
});

expect(issuerMock).toHaveBeenCalledTimes(1);
expect(fakeClient.authorizationUrl).toHaveBeenCalledTimes(1);
});

it('should throw an error if reinitialization failed', async () => {
fakeClient.authorizationUrl.mockResolvedValue('myAuthorizationUrl');
const error = new Error('myError');
issuerMock.mockImplementationOnce(() => {
throw error;
});

const fn = () =>
service.generateAuthorizationUrl({
scope: 'myScope',
state: 'myState',
});

await expect(fn).rejects.toEqual(error);
});
});

describe('generateTokens', () => {
it('should try to reinit the client if initialization failed', async () => {
fakeClient.callback.mockResolvedValue({
access_token: 'myAccessToken',
});

await service.generateTokens({
query: { code: 'myCode' },
state: 'myState',
});

expect(issuerMock).toHaveBeenCalledTimes(1);
expect(fakeClient.callback).toHaveBeenCalledTimes(1);
});

it('should throw an error if reinitialization failed', async () => {
fakeClient.authorizationUrl.mockResolvedValue('myAuthorizationUrl');
const error = new Error('myError');
issuerMock.mockImplementationOnce(() => {
throw error;
});

await expect(
service.generateTokens({
query: { code: 'myCode' },
state: 'myState',
}),
).rejects.toEqual(error);
});
});
});
});

0 comments on commit 265b1b4

Please sign in to comment.