mirror of
https://github.com/n8n-io/n8n.git
synced 2026-09-01 15:47:41 +08:00
feat: Disallow usage of unsupported protocols in oauth fields (#25170)
This commit is contained in:
@@ -50,6 +50,8 @@ import { OwnershipService } from '@/services/ownership.service';
|
||||
import { ProjectService } from '@/services/project.service.ee';
|
||||
import { RoleService } from '@/services/role.service';
|
||||
|
||||
import { validateOAuthUrl } from '@/oauth/validate-oauth-url';
|
||||
|
||||
import { CredentialsFinderService } from './credentials-finder.service';
|
||||
|
||||
export type CredentialsGetSharedOptions =
|
||||
@@ -471,6 +473,11 @@ export class CredentialsService {
|
||||
// @ts-ignore
|
||||
updateData.data.oauthTokenData = decryptedData.oauthTokenData;
|
||||
}
|
||||
|
||||
this.checkCredentialData(
|
||||
updateData.type,
|
||||
updateData.data as unknown as ICredentialDataDecryptedObject,
|
||||
);
|
||||
return updateData;
|
||||
}
|
||||
|
||||
@@ -887,7 +894,35 @@ export class CredentialsService {
|
||||
}
|
||||
}
|
||||
|
||||
// TODO: add further validation if needed
|
||||
this.validateOAuthCredentialUrls(type, data);
|
||||
}
|
||||
|
||||
/**
|
||||
* Validates that OAuth credential URL fields (authUrl, accessTokenUrl, etc.) use http/https only.
|
||||
* No-op if the credential type is not OAuth1 or OAuth2 (including extended types).
|
||||
*/
|
||||
private validateOAuthCredentialUrls(type: string, data: ICredentialDataDecryptedObject) {
|
||||
const parentTypes = this.credentialTypes.getParentTypes(type) ?? [];
|
||||
const isOAuth2 = type === 'oAuth2Api' || parentTypes.includes('oAuth2Api');
|
||||
const isOAuth1 = type === 'oAuth1Api' || parentTypes.includes('oAuth1Api');
|
||||
if (isOAuth2) {
|
||||
const oauthUrlFields = ['authUrl', 'accessTokenUrl', 'serverUrl'] as const;
|
||||
for (const field of oauthUrlFields) {
|
||||
const value = data[field];
|
||||
if (typeof value === 'string' && value.trim() !== '') {
|
||||
validateOAuthUrl(value);
|
||||
}
|
||||
}
|
||||
}
|
||||
if (isOAuth1) {
|
||||
const oauthUrlFields = ['authUrl', 'requestTokenUrl', 'accessTokenUrl'] as const;
|
||||
for (const field of oauthUrlFields) {
|
||||
const value = data[field];
|
||||
if (typeof value === 'string' && value.trim() !== '') {
|
||||
validateOAuthUrl(value);
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
|
||||
@@ -1185,7 +1185,8 @@ describe('OauthService', () => {
|
||||
jest.spyOn(service, 'getOAuthCredentials').mockResolvedValue(oauthCredentials);
|
||||
jest.spyOn(service, 'encryptAndSaveData').mockResolvedValue(undefined);
|
||||
|
||||
const authUri = await service.generateAOauth2AuthUri(credential, {
|
||||
const generateAOuth2AuthUriBound = service.generateAOauth2AuthUri.bind(service);
|
||||
const authUri = await generateAOuth2AuthUriBound(credential, {
|
||||
cid: credential.id,
|
||||
origin: 'static-credential',
|
||||
userId: 'user-id',
|
||||
@@ -1203,6 +1204,31 @@ describe('OauthService', () => {
|
||||
state: expect.any(String), // base64State
|
||||
}),
|
||||
]);
|
||||
|
||||
// Reject javascript: and data: protocols in OAuth2 URLs (XSS)
|
||||
jest.spyOn(service, 'getOAuthCredentials').mockResolvedValue({
|
||||
...oauthCredentials,
|
||||
authUrl: "javascript:alert('Hacked')//",
|
||||
});
|
||||
const promiseJs = generateAOuth2AuthUriBound(credential, {
|
||||
cid: credential.id,
|
||||
origin: 'static-credential',
|
||||
userId: 'user-id',
|
||||
});
|
||||
await expect(promiseJs).rejects.toThrow(BadRequestError);
|
||||
await expect(promiseJs).rejects.toThrow(/OAuth url must use HTTP or HTTPS protocol/);
|
||||
|
||||
jest.spyOn(service, 'getOAuthCredentials').mockResolvedValue({
|
||||
...oauthCredentials,
|
||||
accessTokenUrl: 'data:text/html,<script>alert(1)</script>',
|
||||
});
|
||||
const promiseData = generateAOuth2AuthUriBound(credential, {
|
||||
cid: credential.id,
|
||||
origin: 'static-credential',
|
||||
userId: 'user-id',
|
||||
});
|
||||
await expect(promiseData).rejects.toThrow(BadRequestError);
|
||||
await expect(promiseData).rejects.toThrow(/OAuth url must use HTTP or HTTPS protocol/);
|
||||
});
|
||||
|
||||
it('should generate auth URI with PKCE flow', async () => {
|
||||
@@ -1584,6 +1610,28 @@ describe('OauthService', () => {
|
||||
expect(externalHooks.run).toHaveBeenCalledWith('oauth1.authenticate', expect.any(Array));
|
||||
});
|
||||
|
||||
it('should reject javascript: protocol in OAuth1 URL (XSS)', async () => {
|
||||
const credential = mock<CredentialsEntity>({ id: '1', type: 'twitterOAuth1Api' });
|
||||
const oauthCredentials: OAuth1CredentialData = {
|
||||
consumerKey: 'consumer_key',
|
||||
consumerSecret: 'consumer_secret',
|
||||
requestTokenUrl: 'https://example.domain/oauth/request_token',
|
||||
authUrl: "javascript:alert('Hacked')//",
|
||||
accessTokenUrl: 'https://example.domain/oauth/access_token',
|
||||
signatureMethod: 'HMAC-SHA1',
|
||||
};
|
||||
|
||||
jest.spyOn(service, 'getOAuthCredentials').mockResolvedValue(oauthCredentials);
|
||||
|
||||
const promise = service.generateAOauth1AuthUri(credential, {
|
||||
cid: credential.id,
|
||||
origin: 'static-credential',
|
||||
userId: 'user-id',
|
||||
});
|
||||
await expect(promise).rejects.toThrow(BadRequestError);
|
||||
await expect(promise).rejects.toThrow(/OAuth url must use HTTP or HTTPS protocol/);
|
||||
});
|
||||
|
||||
it('should generate auth URI with different signature methods', async () => {
|
||||
const axios = require('axios');
|
||||
const credential = mock<CredentialsEntity>({ id: '1', type: 'twitterOAuth1Api' });
|
||||
|
||||
@@ -19,6 +19,7 @@ import { AuthError } from '@/errors/response-errors/auth.error';
|
||||
import { BadRequestError } from '@/errors/response-errors/bad-request.error';
|
||||
import { NotFoundError } from '@/errors/response-errors/not-found.error';
|
||||
import type { OAuthRequest } from '@/requests';
|
||||
import { validateOAuthUrl } from '@/oauth/validate-oauth-url';
|
||||
import { UrlService } from '@/services/url.service';
|
||||
import * as WorkflowExecuteAdditionalData from '@/workflow-execute-additional-data';
|
||||
import {
|
||||
@@ -75,6 +76,15 @@ export class OauthService {
|
||||
private readonly dynamicCredentialsProxy: DynamicCredentialsProxy,
|
||||
) {}
|
||||
|
||||
private validateOAuthUrlOrThrow(url: string): void {
|
||||
try {
|
||||
validateOAuthUrl(url);
|
||||
} catch (e) {
|
||||
this.logger.error('Invalid OAuth URL', { url, error: e });
|
||||
throw e;
|
||||
}
|
||||
}
|
||||
|
||||
getBaseUrl(oauthVersion: OauthVersion) {
|
||||
const restUrl = `${this.urlService.getInstanceBaseUrl()}/${this.globalConfig.endpoints.rest}`;
|
||||
return `${restUrl}/oauth${oauthVersion}-credential`;
|
||||
@@ -387,6 +397,9 @@ export class OauthService {
|
||||
}
|
||||
}
|
||||
|
||||
this.validateOAuthUrlOrThrow(oauthCredentials.authUrl ?? '');
|
||||
this.validateOAuthUrlOrThrow(oauthCredentials.accessTokenUrl ?? '');
|
||||
|
||||
// Generate a CSRF prevention token and send it as an OAuth2 state string
|
||||
const [csrfSecret, state] = this.createCsrfState(csrfData);
|
||||
|
||||
@@ -432,6 +445,10 @@ export class OauthService {
|
||||
const oauthCredentials: OAuth1CredentialData =
|
||||
await this.getOAuthCredentials<OAuth1CredentialData>(credential);
|
||||
|
||||
this.validateOAuthUrlOrThrow(oauthCredentials.authUrl ?? '');
|
||||
this.validateOAuthUrlOrThrow(oauthCredentials.requestTokenUrl ?? '');
|
||||
this.validateOAuthUrlOrThrow(oauthCredentials.accessTokenUrl ?? '');
|
||||
|
||||
const [csrfSecret, state] = this.createCsrfState(csrfData);
|
||||
|
||||
const signatureMethod = oauthCredentials.signatureMethod;
|
||||
|
||||
@@ -0,0 +1,28 @@
|
||||
import { BadRequestError } from '@/errors/response-errors/bad-request.error';
|
||||
|
||||
const ALLOWED_OAUTH_URL_PROTOCOLS = ['http:', 'https:'];
|
||||
|
||||
/**
|
||||
* Validates that a URL is valid and uses an allowed protocol (http or https).
|
||||
* Used for OAuth authorization, token, and server URLs to prevent javascript: and other non-http(s) schemes.
|
||||
*
|
||||
* @param url - The URL string to validate
|
||||
* @throws BadRequestError if the URL is invalid or uses a disallowed protocol
|
||||
*/
|
||||
export function validateOAuthUrl(url: string): void {
|
||||
const trimmed = url?.trim();
|
||||
if (!trimmed) return;
|
||||
|
||||
let parsed: URL;
|
||||
try {
|
||||
parsed = new URL(trimmed);
|
||||
} catch {
|
||||
throw new BadRequestError('OAuth url is not a valid URL.');
|
||||
}
|
||||
|
||||
if (!ALLOWED_OAUTH_URL_PROTOCOLS.includes(parsed.protocol)) {
|
||||
throw new BadRequestError(
|
||||
`OAuth url must use HTTP or HTTPS protocol. Invalid protocol: ${parsed.protocol}`,
|
||||
);
|
||||
}
|
||||
}
|
||||
@@ -884,6 +884,8 @@
|
||||
"credentialEdit.credentialEdit.showError.deleteCredential.title": "Problem deleting credential",
|
||||
"credentialEdit.credentialEdit.showError.generateAuthorizationUrl.message": "There was a problem generating the authorization URL",
|
||||
"credentialEdit.credentialEdit.showError.generateAuthorizationUrl.title": "OAuth Authorization Error",
|
||||
"credentialEdit.credentialEdit.showError.invalidOAuthUrl.message": "Authorization URL must use HTTP or HTTPS protocol.",
|
||||
"credentialEdit.credentialEdit.showError.invalidOAuthUrl.title": "Invalid OAuth URL",
|
||||
"credentialEdit.credentialEdit.showError.loadCredential.title": "Problem loading credential",
|
||||
"credentialEdit.credentialEdit.showError.updateCredential.title": "Problem updating credential",
|
||||
"credentialEdit.credentialEdit.showMessage.title": "Credential deleted",
|
||||
|
||||
+27
@@ -1057,6 +1057,33 @@ async function oAuthCredentialAuthorize() {
|
||||
return;
|
||||
}
|
||||
|
||||
if (url === undefined || url === '') {
|
||||
toast.showError(
|
||||
new Error(i18n.baseText('credentialEdit.credentialEdit.showError.invalidOAuthUrl.message')),
|
||||
i18n.baseText('credentialEdit.credentialEdit.showError.invalidOAuthUrl.title'),
|
||||
);
|
||||
return;
|
||||
}
|
||||
|
||||
// Prevent javascript:, data:, vbscript: and other non-http(s) protocols (XSS)
|
||||
const allowedOAuthUrlProtocols = ['http:', 'https:'];
|
||||
try {
|
||||
const parsedUrl = new URL(url);
|
||||
if (!allowedOAuthUrlProtocols.includes(parsedUrl.protocol)) {
|
||||
toast.showError(
|
||||
new Error(i18n.baseText('credentialEdit.credentialEdit.showError.invalidOAuthUrl.message')),
|
||||
i18n.baseText('credentialEdit.credentialEdit.showError.invalidOAuthUrl.title'),
|
||||
);
|
||||
return;
|
||||
}
|
||||
} catch {
|
||||
toast.showError(
|
||||
new Error(i18n.baseText('credentialEdit.credentialEdit.showError.invalidOAuthUrl.message')),
|
||||
i18n.baseText('credentialEdit.credentialEdit.showError.invalidOAuthUrl.title'),
|
||||
);
|
||||
return;
|
||||
}
|
||||
|
||||
const params =
|
||||
'scrollbars=no,resizable=yes,status=no,titlebar=noe,location=no,toolbar=no,menubar=no,width=500,height=700';
|
||||
const oauthPopup = window.open(url, 'OAuth Authorization', params);
|
||||
|
||||
Reference in New Issue
Block a user