From 04c7d3c83a61f9c02bf36ee280a20ad57b6834af Mon Sep 17 00:00:00 2001 From: Dan Schultz Date: Tue, 7 Nov 2023 15:58:24 -0500 Subject: [PATCH] Move token management to dedicated class We had organically coupled token management with the initial authentication flow, but they don't actually belong together. This separates token management (e.g. utilization of refresh tokens) from the SSH authentication system. It also refactors the sftp session handler to use the token manager rather than the authentication session. While doing these refactors we took out a redundant environment variable. Issue #289 Duplicated FusionAuth env vars --- .env.example | 1 - src/classes/AuthTokenManager.ts | 99 +++++++++++++++++++++++ src/classes/AuthenticationSession.ts | 116 +++++---------------------- src/classes/SftpSessionHandler.ts | 12 +-- src/classes/SshConnectionHandler.ts | 22 ++--- src/classes/SshSessionHandler.ts | 10 +-- src/server.ts | 3 - 7 files changed, 140 insertions(+), 123 deletions(-) create mode 100644 src/classes/AuthTokenManager.ts diff --git a/.env.example b/.env.example index d387a783..739d99db 100644 --- a/.env.example +++ b/.env.example @@ -40,6 +40,5 @@ PERMANENT_API_BASE_PATH=${LOCAL_TEMPORARY_AUTH_TOKEN} # See https://fusionauth.io/docs/v1/tech/apis/api-keys FUSION_AUTH_HOST=${FUSION_AUTH_HOST} FUSION_AUTH_KEY=${FUSION_AUTH_KEY} -FUSION_AUTH_SFTP_APP_ID=${FUSION_AUTH_SFTP_APP_ID} FUSION_AUTH_SFTP_CLIENT_ID=${FUSION_AUTH_SFTP_CLIENT_ID} FUSION_AUTH_SFTP_CLIENT_SECRET=${FUSION_AUTH_SFTP_CLIENT_SECRET} diff --git a/src/classes/AuthTokenManager.ts b/src/classes/AuthTokenManager.ts new file mode 100644 index 00000000..1ed16fee --- /dev/null +++ b/src/classes/AuthTokenManager.ts @@ -0,0 +1,99 @@ +import { logger } from '../logger'; +import { AuthTokenRefreshError } from '../errors/AuthTokenRefreshError'; +import { + getFusionAuthClient, + isPartialClientResponse, +} from '../fusionAuth'; + +export class AuthTokenManager { + public readonly username: string; + + private readonly fusionAuthClient; + + private refreshToken = ''; + + private authToken = ''; + + private authTokenExpiresAt = new Date(); + + private fusionAuthClientId = ''; + + private fusionAuthClientSecret = ''; + + public constructor( + username: string, + refreshToken: string, + fusionAuthClientId: string, + fusionAuthClientSecret: string, + ) { + this.username = username; + this.refreshToken = refreshToken; + this.fusionAuthClientId = fusionAuthClientId; + this.fusionAuthClientSecret = fusionAuthClientSecret; + this.fusionAuthClient = getFusionAuthClient(); + } + + public async getAuthToken() { + if (this.tokenWouldExpireSoon()) { + await this.resetAuthTokenUsingRefreshToken(); + } + return this.authToken; + } + + private async resetAuthTokenUsingRefreshToken(): Promise { + let clientResponse; + try { + /** + * Fusion auth sdk wrongly mandates last two params (scope, user_code) + * hence the need to pass two empty strings here. + * See: https://github.com/FusionAuth/fusionauth-typescript-client/issues/42 + */ + clientResponse = await this.fusionAuthClient.exchangeRefreshTokenForAccessToken( + this.refreshToken, + this.fusionAuthClientId, + this.fusionAuthClientSecret, + '', + '', + ); + } catch (error: unknown) { + let message: string; + if (isPartialClientResponse(error)) { + message = error.exception.error_description ?? error.exception.message; + } else { + message = error instanceof Error ? error.message : JSON.stringify(error); + } + logger.verbose(`Error obtaining refresh token: ${message}`); + throw new AuthTokenRefreshError(`Error obtaining refresh token: ${message}`); + } + + if (!clientResponse.response.access_token) { + logger.warn('No access token in response:', clientResponse.response); + throw new AuthTokenRefreshError('Response does not contain access_token'); + } + + if (!clientResponse.response.expires_in) { + logger.warn('Response lacks token TTL (expires_in):', clientResponse.response); + throw new AuthTokenRefreshError('Response lacks token TTL (expires_in)'); + } + + /** + * The exchange refresh token for access token endpoint does not return a timestamp, + * it returns expires_in in seconds. + * So we need to create the timestamp to be consistent with what is first + * returned upon initial authentication + */ + this.authToken = clientResponse.response.access_token; + this.authTokenExpiresAt = new Date( + Date.now() + (clientResponse.response.expires_in * 1000), + ); + logger.debug('New access token obtained:', clientResponse.response); + } + + private tokenWouldExpireSoon(expirationThresholdInSeconds = 300): boolean { + const currentTime = new Date(); + const remainingTokenLife = ( + (this.authTokenExpiresAt.getTime() - currentTime.getTime()) / 1000 + ); + return remainingTokenLife <= expirationThresholdInSeconds; + } +} diff --git a/src/classes/AuthenticationSession.ts b/src/classes/AuthenticationSession.ts index aae6e35f..1a7378c2 100644 --- a/src/classes/AuthenticationSession.ts +++ b/src/classes/AuthenticationSession.ts @@ -3,7 +3,6 @@ import { getFusionAuthClient, isPartialClientResponse, } from '../fusionAuth'; -import { AuthTokenRefreshError } from '../errors/AuthTokenRefreshError'; import type { KeyboardAuthContext } from 'ssh2'; import type { TwoFactorMethod } from '@fusionauth/typescript-client'; @@ -13,108 +12,36 @@ enum FusionAuthStatusCode { SuccessNeedsTwoFactorAuth = 242, } -export class AuthenticationSession { - private authToken = ''; - - public refreshToken = ''; - - public readonly authContext; +type SuccessHandler = (refreshToken: string) => void; - private authTokenExpiresAt = new Date(); +export class AuthenticationSession { + private readonly authContext; private readonly fusionAuthClient; + private readonly successHandler: SuccessHandler; + private twoFactorId = ''; private twoFactorMethods: TwoFactorMethod[] = []; - private fusionAuthAppId = ''; - private fusionAuthClientId = ''; - private fusionAuthClientSecret = ''; - public constructor( authContext: KeyboardAuthContext, - fusionAuthAppId: string, fusionAuthClientId: string, - fusionAuthClientSecret: string, + successHandler: SuccessHandler, ) { this.authContext = authContext; - this.fusionAuthAppId = fusionAuthAppId; this.fusionAuthClientId = fusionAuthClientId; - this.fusionAuthClientSecret = fusionAuthClientSecret; this.fusionAuthClient = getFusionAuthClient(); + this.successHandler = successHandler; } public invokeAuthenticationFlow(): void { this.promptForPassword(); } - public async getAuthToken() { - if (this.tokenWouldExpireSoon()) { - await this.getAuthTokenUsingRefreshToken(); - } - return this.authToken; - } - - private async getAuthTokenUsingRefreshToken(): Promise { - let clientResponse; - try { - /** - * Fusion auth sdk wrongly mandates last two params (scope, user_code) - * hence the need to pass two empty strings here. - * See: https://github.com/FusionAuth/fusionauth-typescript-client/issues/42 - */ - clientResponse = await this.fusionAuthClient.exchangeRefreshTokenForAccessToken( - this.refreshToken, - this.fusionAuthClientId, - this.fusionAuthClientSecret, - '', - '', - ); - } catch (error: unknown) { - let message: string; - if (isPartialClientResponse(error)) { - message = error.exception.message; - } else { - message = error instanceof Error ? error.message : JSON.stringify(error); - } - logger.verbose(`Error obtaining refresh token: ${message}`); - throw new AuthTokenRefreshError(`Error obtaining refresh token: ${message}`); - } - - if (!clientResponse.response.access_token) { - logger.warn('No access token in response:', clientResponse.response); - throw new AuthTokenRefreshError('Response does not contain access_token'); - } - - if (!clientResponse.response.expires_in) { - logger.warn('Response lacks token TTL (expires_in):', clientResponse.response); - throw new AuthTokenRefreshError('Response lacks token TTL (expires_in)'); - } - - /** - * The exchange refresh token for access token endpoint does not return a timestamp, - * it returns expires_in in seconds. - * So we need to create the timestamp to be consistent with what is first - * returned upon initial authentication - */ - this.authToken = clientResponse.response.access_token; - this.authTokenExpiresAt = new Date( - Date.now() + (clientResponse.response.expires_in * 1000), - ); - logger.debug('New access token obtained:', clientResponse.response); - } - - private tokenWouldExpireSoon(expirationThresholdInSeconds = 300): boolean { - const currentTime = new Date(); - const remainingTokenLife = ( - (this.authTokenExpiresAt.getTime() - currentTime.getTime()) / 1000 - ); - return remainingTokenLife <= expirationThresholdInSeconds; - } - private promptForPassword(): void { this.authContext.prompt( { @@ -129,29 +56,20 @@ export class AuthenticationSession { private processPasswordResponse([password]: string[]): void { this.fusionAuthClient.login({ - applicationId: this.fusionAuthAppId, + applicationId: this.fusionAuthClientId, loginId: this.authContext.username, password, }).then((clientResponse) => { switch (clientResponse.statusCode) { case FusionAuthStatusCode.Success: { - if (clientResponse.response.token !== undefined) { - logger.verbose('Successful password authentication attempt.', { - username: this.authContext.username, - }); - this.authToken = clientResponse.response.token; - if (clientResponse.response.refreshToken) { - this.refreshToken = clientResponse.response.refreshToken; - this.authTokenExpiresAt = new Date( - clientResponse.response.tokenExpirationInstant ?? 0, - ); - } else { - logger.warn('No refresh token in response :', clientResponse.response); - this.authContext.reject(); - } + logger.verbose('Successful password authentication attempt.', { + username: this.authContext.username, + }); + if (clientResponse.response.refreshToken) { + this.successHandler(clientResponse.response.refreshToken); this.authContext.accept(); } else { - logger.warn('No auth token in response', clientResponse.response); + logger.warn('No refresh token in response :', clientResponse.response); this.authContext.reject(); } return; @@ -159,7 +77,9 @@ export class AuthenticationSession { case FusionAuthStatusCode.SuccessButUnregisteredInApp: { const userId: string = clientResponse.response.user?.id ?? ''; this.registerUserInApp(userId) - .then(() => { this.processPasswordResponse([password]); }) + .then(() => { + this.processPasswordResponse([password]); + }) .catch((error) => { logger.warn('Error during registration and authentication:', error); this.authContext.reject(); @@ -203,7 +123,7 @@ export class AuthenticationSession { try { const clientResponse = await this.fusionAuthClient.register(userId, { registration: { - applicationId: this.fusionAuthAppId, + applicationId: this.fusionAuthClientId, }, }); diff --git a/src/classes/SftpSessionHandler.ts b/src/classes/SftpSessionHandler.ts index 8051344b..4a47e58c 100644 --- a/src/classes/SftpSessionHandler.ts +++ b/src/classes/SftpSessionHandler.ts @@ -8,7 +8,7 @@ import { generateFileEntry } from '../utils'; import { MissingTemporaryFileError } from '../errors'; import { PermanentFileSystem } from './PermanentFileSystem'; import { TemporaryFileManager } from './TemporaryFileManager'; -import type { AuthenticationSession } from './AuthenticationSession'; +import type { AuthTokenManager } from './AuthTokenManager'; import type { PermanentFileSystemManager } from './PermanentFileSystemManager'; import type { TemporaryFile } from './TemporaryFileManager'; import type { @@ -61,15 +61,15 @@ export class SftpSessionHandler { private readonly permanentFileSystemManager: PermanentFileSystemManager; - private readonly authenticationSession: AuthenticationSession; + private readonly authTokenManager: AuthTokenManager; public constructor( sftpConnection: SFTPWrapper, - authenticationSession: AuthenticationSession, + authTokenManager: AuthTokenManager, permanentFileSystemManager: PermanentFileSystemManager, ) { this.sftpConnection = sftpConnection; - this.authenticationSession = authenticationSession; + this.authTokenManager = authTokenManager; this.permanentFileSystemManager = permanentFileSystemManager; } @@ -1364,8 +1364,8 @@ export class SftpSessionHandler { private async getCurrentPermanentFileSystem(): Promise { return this.permanentFileSystemManager .getCurrentPermanentFileSystemForUser( - this.authenticationSession.authContext.username, - await this.authenticationSession.getAuthToken(), + this.authTokenManager.username, + await this.authTokenManager.getAuthToken(), ); } } diff --git a/src/classes/SshConnectionHandler.ts b/src/classes/SshConnectionHandler.ts index 4a703220..e84ee7ca 100644 --- a/src/classes/SshConnectionHandler.ts +++ b/src/classes/SshConnectionHandler.ts @@ -1,6 +1,7 @@ import { logger } from '../logger'; import { AuthenticationSession } from './AuthenticationSession'; import { SshSessionHandler } from './SshSessionHandler'; +import { AuthTokenManager } from './AuthTokenManager'; import type { AuthContext, Session, @@ -10,9 +11,7 @@ import type { PermanentFileSystemManager } from './PermanentFileSystemManager'; export class SshConnectionHandler { private readonly permanentFileSystemManager: PermanentFileSystemManager; - private authSession?: AuthenticationSession; - - private fusionAuthSftpAppId = ''; + private authTokenManager?: AuthTokenManager; private fusionAuthSftpClientId = ''; @@ -20,12 +19,10 @@ export class SshConnectionHandler { public constructor( permanentFileSystemManager: PermanentFileSystemManager, - fusionAuthSftpAppId: string, fusionAuthSftpClientId: string, fusionAuthSftpClientSecret: string, ) { this.permanentFileSystemManager = permanentFileSystemManager; - this.fusionAuthSftpAppId = fusionAuthSftpAppId; this.fusionAuthSftpClientId = fusionAuthSftpClientId; this.fusionAuthSftpClientSecret = fusionAuthSftpClientSecret; } @@ -43,12 +40,17 @@ export class SshConnectionHandler { case 'keyboard-interactive': { const authenticationSession = new AuthenticationSession( authContext, - this.fusionAuthSftpAppId, this.fusionAuthSftpClientId, - this.fusionAuthSftpClientSecret, + (refreshToken) => { + this.authTokenManager = new AuthTokenManager( + authContext.username, + refreshToken, + this.fusionAuthSftpClientId, + this.fusionAuthSftpClientSecret, + ); + }, ); authenticationSession.invokeAuthenticationFlow(); - this.authSession = authenticationSession; return; } case 'none': @@ -131,14 +133,14 @@ export class SshConnectionHandler { ): void { logger.verbose('SSH request for a new session'); const session = accept(); - if (this.authSession === undefined) { + if (this.authTokenManager === undefined) { logger.verbose('Closing SSH session immediately (no authentication context)'); session.close(); return; } const sessionHandler = new SshSessionHandler( session, - this.authSession, + this.authTokenManager, this.permanentFileSystemManager, ); session.on('sftp', sessionHandler.onSftp.bind(sessionHandler)); diff --git a/src/classes/SshSessionHandler.ts b/src/classes/SshSessionHandler.ts index de494745..7f08b3a8 100644 --- a/src/classes/SshSessionHandler.ts +++ b/src/classes/SshSessionHandler.ts @@ -1,6 +1,6 @@ import { logger } from '../logger'; import { SftpSessionHandler } from './SftpSessionHandler'; -import type { AuthenticationSession } from './AuthenticationSession'; +import { AuthTokenManager } from './AuthTokenManager'; import type { PermanentFileSystemManager } from './PermanentFileSystemManager'; import type { Session, @@ -10,17 +10,17 @@ import type { export class SshSessionHandler { private readonly permanentFileSystemManager: PermanentFileSystemManager; - private readonly authenticationSession: AuthenticationSession; + private readonly authTokenManager: AuthTokenManager; private readonly session: Session; public constructor( session: Session, - authenticationSession: AuthenticationSession, + authTokenManager: AuthTokenManager, permanentFileSystemManager: PermanentFileSystemManager, ) { this.session = session; - this.authenticationSession = authenticationSession; + this.authTokenManager = authTokenManager; this.permanentFileSystemManager = permanentFileSystemManager; } @@ -35,7 +35,7 @@ export class SshSessionHandler { const sftpConnection = accept(); const sftpSessionHandler = new SftpSessionHandler( sftpConnection, - this.authenticationSession, + this.authTokenManager, this.permanentFileSystemManager, ); sftpConnection.on('OPEN', sftpSessionHandler.openHandler.bind(sftpSessionHandler)); diff --git a/src/server.ts b/src/server.ts index 7fb56e3b..11a2380d 100644 --- a/src/server.ts +++ b/src/server.ts @@ -13,7 +13,6 @@ import type { const { SSH_HOST_KEY_PATH, - FUSION_AUTH_SFTP_APP_ID, FUSION_AUTH_SFTP_CLIENT_ID, FUSION_AUTH_SFTP_CLIENT_SECRET, } = requireEnv( @@ -21,7 +20,6 @@ const { 'FUSION_AUTH_KEY', 'PERMANENT_API_BASE_PATH', 'SSH_HOST_KEY_PATH', - 'FUSION_AUTH_SFTP_APP_ID', 'FUSION_AUTH_SFTP_CLIENT_ID', 'FUSION_AUTH_SFTP_CLIENT_SECRET', ); @@ -41,7 +39,6 @@ const connectionListener = (client: Connection): void => { logger.verbose('New connection'); const connectionHandler = new SshConnectionHandler( permanentFileSystemManager, - FUSION_AUTH_SFTP_APP_ID, FUSION_AUTH_SFTP_CLIENT_ID, FUSION_AUTH_SFTP_CLIENT_SECRET, );