Skip to content

Commit

Permalink
Improved error handling for performAuthorizationRequest
Browse files Browse the repository at this point in the history
Resolves: openid#94
  • Loading branch information
pixtron committed Dec 6, 2019
1 parent f34322d commit 1338804
Show file tree
Hide file tree
Showing 11 changed files with 86 additions and 50 deletions.
3 changes: 2 additions & 1 deletion built/authorization_request_handler.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -52,8 +52,9 @@ export declare abstract class AuthorizationRequestHandler {
setAuthorizationNotifier(notifier: AuthorizationNotifier): AuthorizationRequestHandler;
/**
* Makes an authorization request.
* Returns a `Promise<null>`, when the request was sent succesfully.
*/
abstract performAuthorizationRequest(configuration: AuthorizationServiceConfiguration, request: AuthorizationRequest): void;
abstract performAuthorizationRequest(configuration: AuthorizationServiceConfiguration, request: AuthorizationRequest): Promise<null>;
/**
* Checks if an authorization flow can be completed, and completes it.
* The handler returns a `Promise<AuthorizationRequestResponse>` if ready, or a `Promise<null>`
Expand Down
2 changes: 1 addition & 1 deletion built/authorization_request_handler.js

Large diffs are not rendered by default.

2 changes: 1 addition & 1 deletion built/node_support/node_request_handler.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,6 @@ export declare class NodeBasedHandler extends AuthorizationRequestHandler {
httpServerPort: number;
authorizationPromise: Promise<AuthorizationRequestResponse | null> | null;
constructor(httpServerPort?: number, utils?: QueryStringUtils, crypto?: Crypto);
performAuthorizationRequest(configuration: AuthorizationServiceConfiguration, request: AuthorizationRequest): void;
performAuthorizationRequest(configuration: AuthorizationServiceConfiguration, request: AuthorizationRequest): Promise<null>;
protected completeAuthorizationRequest(): Promise<AuthorizationRequestResponse | null>;
}
19 changes: 13 additions & 6 deletions built/node_support/node_request_handler.js

Large diffs are not rendered by default.

2 changes: 1 addition & 1 deletion built/redirect_based_handler.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ export declare class RedirectRequestHandler extends AuthorizationRequestHandler
storageBackend: StorageBackend;
locationLike: LocationLike;
constructor(storageBackend?: StorageBackend, utils?: BasicQueryStringUtils, locationLike?: LocationLike, crypto?: Crypto);
performAuthorizationRequest(configuration: AuthorizationServiceConfiguration, request: AuthorizationRequest): void;
performAuthorizationRequest(configuration: AuthorizationServiceConfiguration, request: AuthorizationRequest): Promise<null>;
/**
* Attempts to introspect the contents of storage backend and completes the
* request.
Expand Down
35 changes: 19 additions & 16 deletions built/redirect_based_handler.js

Large diffs are not rendered by default.

6 changes: 5 additions & 1 deletion src/app/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -113,7 +113,11 @@ export class App {
});

if (this.configuration) {
this.authorizationHandler.performAuthorizationRequest(this.configuration, request);
this.authorizationHandler.performAuthorizationRequest(this.configuration, request)
.catch(error => {
log('Something bad happened', error);
this.showMessage(`Something bad happened ${error}`)
});
} else {
this.showMessage(
'Fetch Authorization Service configuration, before you make the authorization request.');
Expand Down
3 changes: 2 additions & 1 deletion src/authorization_request_handler.ts
Original file line number Diff line number Diff line change
Expand Up @@ -143,10 +143,11 @@ export abstract class AuthorizationRequestHandler {

/**
* Makes an authorization request.
* Returns a `Promise<null>`, when the request was sent succesfully.
*/
abstract performAuthorizationRequest(
configuration: AuthorizationServiceConfiguration,
request: AuthorizationRequest): void;
request: AuthorizationRequest): Promise<null>;

/**
* Checks if an authorization flow can be completed, and completes it.
Expand Down
5 changes: 4 additions & 1 deletion src/node_app/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -86,7 +86,10 @@ export class App {
}, new NodeCrypto());

log('Making authorization request ', configuration, request);
this.authorizationHandler.performAuthorizationRequest(configuration, request);
this.authorizationHandler.performAuthorizationRequest(configuration, request)
.catch(error => {
log('Something bad happened ', error);
});
}

makeRefreshTokenRequest(
Expand Down
22 changes: 17 additions & 5 deletions src/node_support/node_request_handler.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
import * as EventEmitter from 'events';
import * as Http from 'http';
import * as Url from 'url';
import {AppAuthError} from '../errors';
import {AuthorizationRequest} from '../authorization_request';
import {AuthorizationRequestHandler, AuthorizationRequestResponse} from '../authorization_request_handler';
import {AuthorizationError, AuthorizationResponse} from '../authorization_response';
Expand All @@ -29,6 +30,7 @@ import {NodeCrypto} from './crypto_utils';
import opener = require('opener');

class ServerEventsEmitter extends EventEmitter {
static ON_START = 'start';
static ON_UNABLE_TO_START = 'unable_to_start';
static ON_AUTHORIZATION_RESPONSE = 'authorization_response';
}
Expand Down Expand Up @@ -91,10 +93,8 @@ export class NodeBasedHandler extends AuthorizationRequestHandler {
response.end('Close your browser to continue');
};

this.authorizationPromise = new Promise<AuthorizationRequestResponse>((resolve, reject) => {
emitter.once(ServerEventsEmitter.ON_UNABLE_TO_START, () => {
reject(`Unable to create HTTP server at port ${this.httpServerPort}`);
});
this.authorizationPromise = new Promise<AuthorizationRequestResponse|null>((resolve) => {
emitter.once(ServerEventsEmitter.ON_UNABLE_TO_START, () => resolve(null));
emitter.once(ServerEventsEmitter.ON_AUTHORIZATION_RESPONSE, (result: any) => {
server.close();
// resolve pending promise
Expand All @@ -112,11 +112,23 @@ export class NodeBasedHandler extends AuthorizationRequestHandler {
const url = this.buildRequestUrl(configuration, request);
log('Making a request to ', request, url);
opener(url);
emitter.emit(ServerEventsEmitter.ON_START);
})
.catch((error) => {
log('Something bad happened ', error);
emitter.emit(ServerEventsEmitter.ON_UNABLE_TO_START);
emitter.emit(ServerEventsEmitter.ON_UNABLE_TO_START, error);
});

return new Promise<null>((resolve, reject) => {
emitter.once(ServerEventsEmitter.ON_UNABLE_TO_START, (error) => {
reject(new AppAuthError(
`Unable to create HTTP server at port ${this.httpServerPort}`,
{origError: error}
));
});

emitter.once(ServerEventsEmitter.ON_START, () => resolve(null));
});
}

protected completeAuthorizationRequest(): Promise<AuthorizationRequestResponse|null> {
Expand Down
37 changes: 21 additions & 16 deletions src/redirect_based_handler.ts
Original file line number Diff line number Diff line change
Expand Up @@ -57,24 +57,29 @@ export class RedirectRequestHandler extends AuthorizationRequestHandler {
performAuthorizationRequest(
configuration: AuthorizationServiceConfiguration,
request: AuthorizationRequest) {
const handle = this.crypto.generateRandom(10);
return new Promise<null>((resolve, reject) => {
const handle = this.crypto.generateRandom(10);

// before you make request, persist all request related data in local storage.
const persisted = Promise.all([
this.storageBackend.setItem(AUTHORIZATION_REQUEST_HANDLE_KEY, handle),
// Calling toJson() adds in the code & challenge when possible
request.toJson().then(
result =>
this.storageBackend.setItem(authorizationRequestKey(handle), JSON.stringify(result))),
this.storageBackend.setItem(
authorizationServiceConfigurationKey(handle), JSON.stringify(configuration.toJson())),
]);
// before you make request, persist all request related data in local storage.
const persisted = Promise.all([
this.storageBackend.setItem(AUTHORIZATION_REQUEST_HANDLE_KEY, handle),
// Calling toJson() adds in the code & challenge when possible
request.toJson().then(
result => this.storageBackend.setItem(
authorizationRequestKey(handle), JSON.stringify(result))),
this.storageBackend.setItem(
authorizationServiceConfigurationKey(handle), JSON.stringify(configuration.toJson())),
]);

persisted.then(() => {
// make the redirect request
let url = this.buildRequestUrl(configuration, request);
log('Making a request to ', request, url);
this.locationLike.assign(url);
persisted
.then(() => {
// make the redirect request
let url = this.buildRequestUrl(configuration, request);
log('Making a request to ', request, url);
this.locationLike.assign(url);
resolve(null);
})
.catch(error => reject(error));
});
}

Expand Down

0 comments on commit 1338804

Please sign in to comment.