fix(core): Make OAuth1/OAuth2 callback not require auth (#10263)

n8n-io · Jul 31, 2024 · a8e2774 · a8e2774
1 parent 2a09a03
commit a8e2774
Show file tree

Hide file tree

Showing 4 changed files with 11 additions and 22 deletions.
diff --git a/packages/cli/src/auth/auth.service.ts b/packages/cli/src/auth/auth.service.ts
@@ -42,10 +42,6 @@ const skipBrowserIdCheckEndpoints = [
 
 	// We need to exclude binary-data downloading endpoint because we can't send custom headers on `<embed>` tags
 	`/${restEndpoint}/binary-data/`,
-
-	// oAuth callback urls aren't called by the frontend. therefore we can't send custom header on these requests
-	`/${restEndpoint}/oauth1-credential/callback`,
-	`/${restEndpoint}/oauth2-credential/callback`,
 ];
 
 @Service()

diff --git a/packages/cli/src/controllers/oauth/oAuth1Credential.controller.ts b/packages/cli/src/controllers/oauth/oAuth1Credential.controller.ts
@@ -99,9 +99,8 @@ export class OAuth1CredentialController extends AbstractOAuthController {
 	}
 
 	/** Verify and store app code. Generate access tokens and store for respective credential */
-	@Get('/callback', { usesTemplates: true })
+	@Get('/callback', { usesTemplates: true, skipAuth: true })
 	async handleCallback(req: OAuthRequest.OAuth1Credential.Callback, res: Response) {
-		const userId = req.user?.id;
 		try {
 			const { oauth_verifier, oauth_token, state: encodedState } = req.query;
 
@@ -124,7 +123,7 @@ export class OAuth1CredentialController extends AbstractOAuthController {
 			const credential = await this.getCredentialWithoutUser(credentialId);
 			if (!credential) {
 				const errorMessage = 'OAuth1 callback failed because of insufficient permissions';
-				this.logger.error(errorMessage, { userId, credentialId });
+				this.logger.error(errorMessage, { credentialId });
 				return this.renderCallbackError(res, errorMessage);
 			}
 
@@ -138,7 +137,7 @@ export class OAuth1CredentialController extends AbstractOAuthController {
 
 			if (this.verifyCsrfState(decryptedDataOriginal, state)) {
 				const errorMessage = 'The OAuth1 callback state is invalid!';
-				this.logger.debug(errorMessage, { userId, credentialId });
+				this.logger.debug(errorMessage, { credentialId });
 				return this.renderCallbackError(res, errorMessage);
 			}
 
@@ -156,7 +155,7 @@ export class OAuth1CredentialController extends AbstractOAuthController {
 			try {
 				oauthToken = await axios.request(options);
 			} catch (error) {
-				this.logger.error('Unable to fetch tokens for OAuth1 callback', { userId, credentialId });
+				this.logger.error('Unable to fetch tokens for OAuth1 callback', { credentialId });
 				const errorResponse = new NotFoundError('Unable to get access tokens!');
 				return sendErrorResponse(res, errorResponse);
 			}
@@ -172,15 +171,11 @@ export class OAuth1CredentialController extends AbstractOAuthController {
 			await this.encryptAndSaveData(credential, decryptedDataOriginal);
 
 			this.logger.verbose('OAuth1 callback successful for new credential', {
-				userId,
 				credentialId,
 			});
 			return res.render('oauth-callback');
 		} catch (error) {
-			this.logger.error('OAuth1 callback failed because of insufficient user permissions', {
-				userId,
-			});
-			// Error response
+			this.logger.error('OAuth1 callback failed because of insufficient user permissions');
 			return sendErrorResponse(res, error as Error);
 		}
 	}

diff --git a/packages/cli/src/controllers/oauth/oAuth2Credential.controller.ts b/packages/cli/src/controllers/oauth/oAuth2Credential.controller.ts
@@ -80,9 +80,8 @@ export class OAuth2CredentialController extends AbstractOAuthController {
 	}
 
 	/** Verify and store app code. Generate access tokens and store for respective credential */
-	@Get('/callback', { usesTemplates: true })
+	@Get('/callback', { usesTemplates: true, skipAuth: true })
 	async handleCallback(req: OAuthRequest.OAuth2Credential.Callback, res: Response) {
-		const userId = req.user?.id;
 		try {
 			const { code, state: encodedState } = req.query;
 			if (!code || !encodedState) {
@@ -104,7 +103,7 @@ export class OAuth2CredentialController extends AbstractOAuthController {
 			const credential = await this.getCredentialWithoutUser(credentialId);
 			if (!credential) {
 				const errorMessage = 'OAuth2 callback failed because of insufficient permissions';
-				this.logger.error(errorMessage, { userId, credentialId });
+				this.logger.error(errorMessage, { credentialId });
 				return this.renderCallbackError(res, errorMessage);
 			}
 
@@ -118,7 +117,7 @@ export class OAuth2CredentialController extends AbstractOAuthController {
 
 			if (this.verifyCsrfState(decryptedDataOriginal, state)) {
 				const errorMessage = 'The OAuth2 callback state is invalid!';
-				this.logger.debug(errorMessage, { userId, credentialId });
+				this.logger.debug(errorMessage, { credentialId });
 				return this.renderCallbackError(res, errorMessage);
 			}
 
@@ -157,7 +156,7 @@ export class OAuth2CredentialController extends AbstractOAuthController {
 
 			if (oauthToken === undefined) {
 				const errorMessage = 'Unable to get OAuth2 access tokens!';
-				this.logger.error(errorMessage, { userId, credentialId });
+				this.logger.error(errorMessage, { credentialId });
 				return this.renderCallbackError(res, errorMessage);
 			}
 
@@ -174,7 +173,6 @@ export class OAuth2CredentialController extends AbstractOAuthController {
 			await this.encryptAndSaveData(credential, decryptedDataOriginal);
 
 			this.logger.verbose('OAuth2 callback successful for credential', {
-				userId,
 				credentialId,
 			});
 

diff --git a/packages/cli/src/requests.ts b/packages/cli/src/requests.ts
@@ -371,7 +371,7 @@ export declare namespace MFA {
 export declare namespace OAuthRequest {
 	namespace OAuth1Credential {
 		type Auth = AuthenticatedRequest<{}, {}, {}, { id: string }>;
-		type Callback = AuthenticatedRequest<
+		type Callback = AuthlessRequest<
 			{},
 			{},
 			{},
@@ -383,7 +383,7 @@ export declare namespace OAuthRequest {
 
 	namespace OAuth2Credential {
 		type Auth = AuthenticatedRequest<{}, {}, {}, { id: string }>;
-		type Callback = AuthenticatedRequest<{}, {}, {}, { code: string; state: string }>;
+		type Callback = AuthlessRequest<{}, {}, {}, { code: string; state: string }>;
 	}
 }