Skip to content

Commit

Permalink
fix: Authentication type improvements and timeout fix (#1605)
Browse files Browse the repository at this point in the history
  • Loading branch information
vonagam authored and daffl committed Oct 7, 2019
1 parent 4e4d10a commit 19854d3
Show file tree
Hide file tree
Showing 7 changed files with 26 additions and 21 deletions.
4 changes: 2 additions & 2 deletions packages/authentication-client/src/core.ts
Original file line number Diff line number Diff line change
Expand Up @@ -149,7 +149,7 @@ export class AuthenticationClient {
return authPromise;
}

authenticate (authentication: AuthenticationRequest, params?: Params): Promise<AuthenticationResult> {
authenticate (authentication?: AuthenticationRequest, params?: Params): Promise<AuthenticationResult> {
if (!authentication) {
return this.reAuthenticate();
}
Expand All @@ -172,7 +172,7 @@ export class AuthenticationClient {
return promise;
}

logout () {
logout (): Promise<AuthenticationResult | null> {
return Promise.resolve(this.app.get('authentication'))
.then(() => this.service.remove(null)
.then((authResult: AuthenticationResult) => this.removeAccessToken()
Expand Down
7 changes: 3 additions & 4 deletions packages/authentication-client/src/index.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
import { AuthenticationClient, AuthenticationClientOptions } from './core';
import * as hooks from './hooks';
import { Application } from '@feathersjs/feathers';
import { AuthenticationResult, AuthenticationRequest } from '@feathersjs/authentication';
import { Storage, MemoryStorage, StorageWrapper } from './storage';

declare module '@feathersjs/feathers' {
Expand All @@ -10,9 +9,9 @@ declare module '@feathersjs/feathers' {
rest?: any;
primus?: any;
authentication: AuthenticationClient;
authenticate (authentication?: AuthenticationRequest): Promise<AuthenticationResult>;
reAuthenticate (force: boolean): Promise<AuthenticationResult>;
logout (): Promise<AuthenticationResult>;
authenticate: AuthenticationClient['authenticate'];
reAuthenticate: AuthenticationClient['reAuthenticate'];
logout: AuthenticationClient['logout'];
}
}

Expand Down
6 changes: 3 additions & 3 deletions packages/authentication/src/core.ts
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ export interface AuthenticationRequest {
[key: string]: any;
}

export type ConnectionEvent = 'login'|'logout'|'disconnect';
export type ConnectionEvent = 'login' | 'logout' | 'disconnect';

export interface AuthenticationStrategy {
/**
Expand Down Expand Up @@ -63,11 +63,11 @@ export interface AuthenticationStrategy {
* @param req The HTTP request
* @param res The HTTP response
*/
parse? (req: IncomingMessage, res: ServerResponse): Promise<AuthenticationRequest|null>;
parse? (req: IncomingMessage, res: ServerResponse): Promise<AuthenticationRequest | null>;
}

export interface JwtVerifyOptions extends VerifyOptions {
algorithm?: string|string[];
algorithm?: string | string[];
}

/**
Expand Down
4 changes: 2 additions & 2 deletions packages/authentication/src/hooks/authenticate.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ export interface AuthenticateHookSettings {
strategies: string[];
}

export default (originalSettings: string|AuthenticateHookSettings, ...originalStrategies: string[]) => {
export default (originalSettings: string | AuthenticateHookSettings, ...originalStrategies: string[]) => {
const settings = typeof originalSettings === 'string'
? { strategies: flatten([ originalSettings, ...originalStrategies ]) }
: originalSettings;
Expand Down Expand Up @@ -54,7 +54,7 @@ export default (originalSettings: string|AuthenticateHookSettings, ...originalSt
context.params = merge({}, params, omit(authResult, 'accessToken'), { authenticated: true });

return context;
} else if (!authentication && provider) {
} else if (provider) {
throw new NotAuthenticated('Not authenticated');
}

Expand Down
9 changes: 5 additions & 4 deletions packages/authentication/src/jwt.ts
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,7 @@ export class JWTStrategy extends AuthenticationBaseStrategy {
const timer = lt.setTimeout(() => this.app.emit('disconnect', connection), duration);

debug(`Registering connection expiration timer for ${duration}ms`);
lt.clearTimeout(this.expirationTimers.get(connection));
this.expirationTimers.set(connection, timer);

debug('Adding authentication information to connection');
Expand All @@ -56,6 +57,7 @@ export class JWTStrategy extends AuthenticationBaseStrategy {

delete connection.authentication;
lt.clearTimeout(this.expirationTimers.get(connection));
this.expirationTimers.delete(connection);
}
}

Expand Down Expand Up @@ -128,8 +130,7 @@ export class JWTStrategy extends AuthenticationBaseStrategy {
}

async parse (req: IncomingMessage) {
const result = { strategy: this.name };
const { header, schemes }: { header: any, schemes: string[] } = this.configuration;
const { header, schemes }: { header: string, schemes: string[] } = this.configuration;
const headerValue = req.headers && req.headers[header.toLowerCase()];

if (!headerValue || typeof headerValue !== 'string') {
Expand All @@ -138,7 +139,7 @@ export class JWTStrategy extends AuthenticationBaseStrategy {

debug('Found parsed header value');

const [ , scheme = null, schemeValue = null ] = headerValue.match(SPLIT_HEADER) || [];
const [ , scheme, schemeValue ] = headerValue.match(SPLIT_HEADER) || [];
const hasScheme = scheme && schemes.some(
current => new RegExp(current, 'i').test(scheme)
);
Expand All @@ -148,7 +149,7 @@ export class JWTStrategy extends AuthenticationBaseStrategy {
}

return {
...result,
strategy: this.name,
accessToken: hasScheme ? schemeValue : headerValue
};
}
Expand Down
15 changes: 9 additions & 6 deletions packages/authentication/src/service.ts
Original file line number Diff line number Diff line change
Expand Up @@ -114,7 +114,7 @@ export class AuthenticationService extends AuthenticationBase implements Partial
* @param id The JWT to remove or null
* @param params Service call parameters
*/
async remove (id: null|string, params: Params) {
async remove (id: string | null, params: Params) {
const { authentication } = params;
const { authStrategies } = this.configuration;

Expand All @@ -131,11 +131,10 @@ export class AuthenticationService extends AuthenticationBase implements Partial
/**
* Validates the service configuration.
*/
setup () {
setup (this: AuthenticationService & ServiceAddons<AuthenticationResult>) {
// The setup method checks for valid settings and registers the
// connection and event (login, logout) hooks
const { secret, service, entity, entityId } = this.configuration;
const self = this as unknown as ServiceAddons<AuthenticationResult>;

if (typeof secret !== 'string') {
throw new Error(`A 'secret' must be provided in your authentication configuration`);
Expand All @@ -155,15 +154,19 @@ export class AuthenticationService extends AuthenticationBase implements Partial
}
}

self.hooks({
this.hooks({
after: {
create: [ connection('login'), event('login') ],
remove: [ connection('logout'), event('logout') ]
}
});

if (typeof self.publish === 'function') {
self.publish(() => null);
this.app.on('disconnect', async (connection) => {
await this.handleConnection('disconnect', connection);
});

if (typeof this.publish === 'function') {
this.publish(() => null);
}
}
}
2 changes: 2 additions & 0 deletions packages/authentication/test/jwt.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -127,6 +127,8 @@ describe('authentication/jwt', () => {
const disconnection = await new Promise(resolve => app.once('disconnect', resolve));

assert.strictEqual(disconnection, connection);

assert.ok(!connection.authentication);
});

it('deletes authentication information on remove', async () => {
Expand Down

0 comments on commit 19854d3

Please sign in to comment.