feat: Secure OAuth callback handling (#653)

Author: arjankowski

src/commands/login.js

@@ -25,6 +25,26 @@ const GENERIC_OAUTH_CLIENT_ID = 'udz8zp4yue87uk9dzq4xk425kkwvqvh1';
 const GENERIC_OAUTH_CLIENT_SECRET = 'iZ1MbvC3ZaF25nbJli7IsKdRHAxfu3fn';
 const SUPPORTED_DEFAULT_APP_PORTS = [3000, 3001, 4000, 5000, 8080];
 const DEFAULT_ENVIRONMENT_NAME = 'oauth';
+const OAUTH_CALLBACK_TIMEOUT_MS = 5 * 60 * 1000;
+const LOOPBACK_HOST = 'localhost';
+const DEFAULT_OPEN_AUTHORIZE_IN_BROWSER = (
+	openFn,
+	apps,
+	authorizeUrl,
+	useIncognito
+) => {
+	if (useIncognito) {
+		openFn(authorizeUrl, {
+			newInstance: true,
+			app: { name: apps.browserPrivate },
+		});
+	} else {
+		openFn(authorizeUrl);
+	}
+};
+
+let oauthCallbackTimeoutMs = OAUTH_CALLBACK_TIMEOUT_MS;
+let openAuthorizeInBrowser = DEFAULT_OPEN_AUTHORIZE_IN_BROWSER;
 
 async function promptForPlatformAppCredentials(inquirerModule, clientId) {
 	if (!clientId) {
@@ -111,7 +131,7 @@ class OAuthLoginCommand extends BoxCommand {
 		let useDefaultBoxApp = false;
 		const environmentsObject = await this.getEnvironments();
 		const port = flags.port;
-		const redirectUri = `http://localhost:${port}/callback`;
+		const redirectUri = `http://${LOOPBACK_HOST}:${port}/callback`;
 		const isUnsupportedDefaultAppPort = () =>
 			useDefaultBoxApp && !SUPPORTED_DEFAULT_APP_PORTS.includes(port);
 		let environment;
@@ -157,13 +177,6 @@ class OAuthLoginCommand extends BoxCommand {
 			useDefaultBoxApp = forceDefaultBoxApp;
 		}
 
-		if (isUnsupportedDefaultAppPort()) {
-			this.info(
-				chalk`{red Unsupported port "${port}" for the Official Box CLI app flow. Supported ports: ${SUPPORTED_DEFAULT_APP_PORTS.join(', ')}}`
-			);
-			return;
-		}
-
 		if (this.flags.reauthorize) {
 			// Keep the selected existing environment config for reauthorization.
 		} else if (useDefaultBoxApp) {
@@ -205,13 +218,6 @@ class OAuthLoginCommand extends BoxCommand {
 			}
 			useDefaultBoxApp = answers.useDefaultBoxApp;
 
-			if (isUnsupportedDefaultAppPort()) {
-				this.info(
-					chalk`{red Unsupported port "${port}" for the Official Box CLI app flow. Supported ports: ${SUPPORTED_DEFAULT_APP_PORTS.join(', ')}}`
-				);
-				return;
-			}
-
 			environment = {
 				clientId: answers.clientId,
 				clientSecret: answers.clientSecret,
@@ -221,6 +227,13 @@ class OAuthLoginCommand extends BoxCommand {
 			};
 		}
 
+		if (isUnsupportedDefaultAppPort()) {
+			this.info(
+				chalk`{red Unsupported port "${port}" for the Official Box CLI app flow. Supported ports: ${SUPPORTED_DEFAULT_APP_PORTS.join(', ')}}`
+			);
+			return;
+		}
+
 		const environmentName = environment.name;
 		const sdkConfig = Object.freeze({
 			analyticsClient: {
@@ -234,14 +247,89 @@ class OAuthLoginCommand extends BoxCommand {
 		this._configureSdk(sdk, sdkConfig);
 
 		const app = express();
+		let callbackHandled = false;
+
+		// Keep run() blocked until callback flow completes.
+		// This prevents command exit before the OAuth redirect returns.
+		let resolveCallbackFlow;
+		const callbackFlowDone = new Promise((resolve) => {
+			resolveCallbackFlow = resolve;
+		});
+		let callbackTimeout;
+
+		// Timeout and callback may race, so teardown must be idempotent.
+		// This guard ensures cleanup and resolve happen only once.
+		let callbackFlowResolved = false;
+
 		let server;
+		try {
+			// Bind only to loopback to avoid exposing callback externally.
+			// Browser redirect is local, so external interfaces are unnecessary.
+			server = await new Promise((resolve, reject) => {
+				const s = app.listen(port, LOOPBACK_HOST);
+				s.once('listening', () => resolve(s));
+				s.once('error', reject);
+			});
+		} catch (error) {
+			if (error.code === 'EADDRINUSE') {
+				throw new BoxCLIError(
+					`Port ${port} is already in use. Please close the application using this port or use --port to specify a different port.`,
+					error
+				);
+			}
+			throw new BoxCLIError(
+				`Failed to start local OAuth server on port ${port}: ${error.message}`,
+				error
+			);
+		}
 
-		server = app.listen(port);
+		const shutdownServer = () => {
+			if (!server) {
+				return;
+			}
+			server.close();
+			if (typeof server.closeAllConnections === 'function') {
+				server.closeAllConnections();
+			}
+		};
+
+		// Use one finalize path so all exits apply the same cleanup.
+		// This keeps timeout and callback completion behavior consistent.
+		const finalizeCallbackFlow = () => {
+			if (callbackFlowResolved) {
+				return;
+			}
+			callbackFlowResolved = true;
+			clearTimeout(callbackTimeout);
+			shutdownServer();
+			resolveCallbackFlow();
+		};
+
+		// Bound callback wait time to avoid hanging sessions forever.
+		// If user abandons auth, the command exits predictably.
+		callbackTimeout = setTimeout(() => {
+			if (callbackHandled) {
+				return;
+			}
+			this.info(
+				chalk`{red Login timed out waiting for OAuth callback after ${oauthCallbackTimeoutMs / 1000} seconds.}`
+			);
+			finalizeCallbackFlow();
+		}, oauthCallbackTimeoutMs);
 
 		const state = nanoid(32);
 		const pkce = useDefaultBoxApp ? generatePKCE() : null;
 
 		app.get('/callback', async (request, res) => {
+			// Reject replayed callbacks after a completion was already accepted.
+			// This enforces single-use semantics for the local callback endpoint.
+			if (callbackHandled) {
+				res.status(409).send('OAuth callback already handled.');
+				return;
+			}
+
+			callbackHandled = true;
+
 			try {
 				if (request.query.state !== state) {
 					throw new BoxCLIError(
@@ -266,7 +354,6 @@ class OAuthLoginCommand extends BoxCommand {
 					});
 				});
 				const client = sdk.getPersistentClient(tokenInfo, tokenCache);
-
 				const user = await client.users.get('me');
 
 				environmentsObject.environments[environmentName] = environment;
@@ -309,9 +396,10 @@ class OAuthLoginCommand extends BoxCommand {
 					'Unknown error';
 				DEBUG.execute('Login error: %O', error);
 				this.info(chalk`{red Login failed: ${errorMessage}}`);
+				res.status(500).send('Login failed. Please check the CLI output for details.');
+
 			} finally {
-				server.close();
-				server.closeAllConnections();
+				finalizeCallbackFlow();
 			}
 		});
 
@@ -355,24 +443,22 @@ class OAuthLoginCommand extends BoxCommand {
 				},
 			]);
 			http.get(
-				`http://localhost:${port}/callback?state=${authInfo.state}&code=${authInfo.code}`
+				`http://${LOOPBACK_HOST}:${port}/callback?state=${authInfo.state}&code=${authInfo.code}`
 			);
 		} else {
-			if (flags['incognito-browser']) {
-				open(authorizeUrl, {
-					newInstance: true,
-					app: { name: apps.browserPrivate },
-				});
-			} else {
-				open(authorizeUrl);
-			}
+			openAuthorizeInBrowser(
+				open,
+				apps,
+				authorizeUrl,
+				flags['incognito-browser']
+			);
 			this.info(
 				useDefaultBoxApp
 					? chalk`{yellow If authorization fails, verify that you are using one of the supported ports for the Official Box CLI app flow and restart the login command.}`
 					: chalk`{yellow If you are redirected to the Files view, make sure your Redirect URI is configured correctly and restart the login command.}`
 			);
 		}
-		await new Promise((resolve) => setTimeout(resolve, 1000));
+		await callbackFlowDone;
 	}
 }
 
@@ -443,4 +529,16 @@ module.exports = OAuthLoginCommand;
 module.exports._test = {
 	promptForAuthMethod,
 	promptForPlatformAppCredentials,
+	setOAuthCallbackTimeoutMs(timeoutMs) {
+		oauthCallbackTimeoutMs = timeoutMs;
+	},
+	resetOAuthCallbackTimeoutMs() {
+		oauthCallbackTimeoutMs = OAUTH_CALLBACK_TIMEOUT_MS;
+	},
+	setOpenAuthorizeInBrowser(fn) {
+		openAuthorizeInBrowser = fn;
+	},
+	resetOpenAuthorizeInBrowser() {
+		openAuthorizeInBrowser = DEFAULT_OPEN_AUTHORIZE_IN_BROWSER;
+	},
 };