Skip to content

Commit

Permalink
Merge branch 'master' into ADO-77-RLC-loadOptionsDependsOn
Browse files Browse the repository at this point in the history
* master:
  fix(editor): Remove pagination from binary data output (#6093)
  fix(editor): Show error in RLC if credentials are not set (#6108)
  fix(editor): Loading state for executions tab (#6100)
  ci: Additionally checkout PR head for e2e tests (no-changelog) (#6105)
  fix(core): Skip auth for controllers/routes that don't use the `Authorized` decorator, or use `Authorized('none')` (#6106)
  fix(editor): Disable changing of email and pw when SAML login enabled (#6104)
  fix(HTTP Request Node): Always lowercase headers
  fix(Compression Node): Fix issue with decompression failing with uppercase extensions (#6098)
  fix(Mattermost Node): Fix base url trailing slash error (#6097)
  feat(Item Lists Node): Split out items work on objects as well as arrays
  • Loading branch information
MiloradFilipovic committed Apr 27, 2023
2 parents c7380b1 + c6e665a commit c56d47e
Show file tree
Hide file tree
Showing 18 changed files with 610 additions and 101 deletions.
5 changes: 3 additions & 2 deletions .github/workflows/ci-pull-requests.yml
Original file line number Diff line number Diff line change
Expand Up @@ -112,12 +112,13 @@ jobs:
name: E2E [Electron/Node 16]
uses: ./.github/workflows/e2e-reusable.yml
with:
branch: ${{ github.event.pull_request.head.ref }}
branch: ${{ github.event.pull_request.base.ref }}
user: ${{ github.event.inputs.user || 'PR User' }}
spec: ${{ github.event.inputs.spec || 'e2e/0-smoke.cy.ts' }}
run-env: base:16.18.1
run-env: browsers:node16.18.0-chrome107-ff106-edge
record: false
parallel: false
pr_number: ${{ github.event.number }}
containers: '[1]'
secrets:
CYPRESS_RECORD_KEY: ${{ secrets.CYPRESS_RECORD_KEY }}
Expand Down
17 changes: 16 additions & 1 deletion .github/workflows/e2e-reusable.yml
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@ on:
description: 'GitHub branch to test.'
required: false
type: string
default: 'master'
user:
description: 'User who kicked this off.'
required: false
Expand Down Expand Up @@ -43,6 +42,10 @@ on:
required: false
default: '[1, 2, 3, 4, 5, 6, 7, 8]'
type: string
pr_number:
description: 'PR number to run tests for.'
required: false
type: number
secrets:
CYPRESS_RECORD_KEY:
description: 'Cypress record key.'
Expand Down Expand Up @@ -79,6 +82,12 @@ jobs:
repository: n8n-io/n8n
ref: ${{ inputs.branch }}

- name: Checkout PR
if: ${{ inputs.pr_number }}
run: |
git fetch origin pull/${{ inputs.pr_number }}/head
git checkout FETCH_HEAD
- name: Setup pnpm
uses: pnpm/action-setup@v2.2.4
with:
Expand Down Expand Up @@ -123,6 +132,12 @@ jobs:
repository: n8n-io/n8n
ref: ${{ inputs.branch }}

- name: Checkout PR
if: ${{ inputs.pr_number }}
run: |
git fetch origin pull/${{ inputs.pr_number }}/head
git checkout FETCH_HEAD
- name: Setup pnpm
uses: pnpm/action-setup@v2.2.4

Expand Down
6 changes: 6 additions & 0 deletions packages/cli/src/decorators/registerController.ts
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,11 @@ export const createAuthMiddleware =
res.status(403).json({ status: 'error', message: 'Unauthorized' });
};

const authFreeRoutes: string[] = [];

export const canSkipAuth = (method: string, path: string): boolean =>
authFreeRoutes.includes(`${method.toLowerCase()} ${path}`);

export const registerController = (app: Application, config: Config, controller: object) => {
const controllerClass = controller.constructor;
const controllerBasePath = Reflect.getMetadata(CONTROLLER_BASE_PATH, controllerClass) as
Expand Down Expand Up @@ -69,6 +74,7 @@ export const registerController = (app: Application, config: Config, controller:
(controller as Controller)[handlerName](req, res),
),
);
if (!authRole || authRole === 'none') authFreeRoutes.push(`${method} ${prefix}${path}`);
});

app.use(prefix, router);
Expand Down
7 changes: 2 additions & 5 deletions packages/cli/src/middlewares/auth.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ import { AUTH_COOKIE_NAME, EDITOR_UI_DIST_DIR } from '@/constants';
import { issueCookie, resolveJwtContent } from '@/auth/jwt';
import { isUserManagementEnabled } from '@/UserManagement/UserManagementHelper';
import type { UserRepository } from '@db/repositories';
import { canSkipAuth } from '@/decorators/registerController';

const jwtFromRequest = (req: Request) => {
// eslint-disable-next-line @typescript-eslint/no-unsafe-member-access
Expand Down Expand Up @@ -90,14 +91,10 @@ export const setupAuthMiddlewares = (
// skip authentication for preflight requests
req.method === 'OPTIONS' ||
staticAssets.includes(req.url.slice(1)) ||
canSkipAuth(req.method, req.path) ||
isAuthExcluded(req.url, ignoredEndpoints) ||
req.url.startsWith(`/${restEndpoint}/settings`) ||
req.url.startsWith(`/${restEndpoint}/login`) ||
req.url.startsWith(`/${restEndpoint}/resolve-signup-token`) ||
isPostUsersId(req, restEndpoint) ||
req.url.startsWith(`/${restEndpoint}/forgot-password`) ||
req.url.startsWith(`/${restEndpoint}/resolve-password-token`) ||
req.url.startsWith(`/${restEndpoint}/change-password`) ||
req.url.startsWith(`/${restEndpoint}/oauth2-credential/callback`) ||
req.url.startsWith(`/${restEndpoint}/oauth1-credential/callback`)
) {
Expand Down
Original file line number Diff line number Diff line change
@@ -1,8 +1,8 @@
<template>
<div :class="$style.container" v-if="!loading">
<div :class="$style.container">
<executions-sidebar
:executions="executions"
:loading="loading"
:loading="loading && !executions.length"
:loadingMore="loadingMore"
:temporaryExecution="temporaryExecution"
@reloadExecutions="setExecutions"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,11 +26,11 @@
@scroll="loadMore(20)"
>
<div v-if="loading" class="mr-m">
<n8n-loading :class="$style.loader" variant="p" :rows="1" />
<n8n-loading :class="$style.loader" variant="p" :rows="1" />
<n8n-loading :class="$style.loader" variant="p" :rows="1" />
<n8n-loading variant="p" :rows="1" />
<n8n-loading variant="p" :rows="1" />
<n8n-loading variant="p" :rows="1" />
</div>
<div v-if="executions.length === 0" :class="$style.noResultsContainer">
<div v-if="!loading && executions.length === 0" :class="$style.noResultsContainer">
<n8n-text color="text-base" size="medium" align="center">
{{ $locale.baseText('executionsLandingPage.noResults') }}
</n8n-text>
Expand All @@ -54,7 +54,7 @@
@retryExecution="onRetryExecution"
/>
<div v-if="loadingMore" class="mr-m">
<n8n-loading :class="$style.loader" variant="p" :rows="1" />
<n8n-loading variant="p" :rows="1" />
</div>
</div>
<div :class="$style.infoAccordion">
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,10 +22,13 @@
<n8n-text color="text-dark" align="center" tag="div">
{{ $locale.baseText('resourceLocator.mode.list.error.title') }}
</n8n-text>
<n8n-text size="small" color="text-base" v-if="hasCredential">
<n8n-text size="small" color="text-base" v-if="hasCredential || credentialsNotSet">
{{ $locale.baseText('resourceLocator.mode.list.error.description.part1') }}
<a @click="openCredential">{{
$locale.baseText('resourceLocator.mode.list.error.description.part2')
<a v-if="credentialsNotSet" @click="createNewCredential">{{
$locale.baseText('resourceLocator.mode.list.error.description.part2.noCredentials')
}}</a>
<a v-else-if="hasCredential" @click="openCredential">{{
$locale.baseText('resourceLocator.mode.list.error.description.part2.hasCredentials')
}}</a>
</n8n-text>
</div>
Expand Down Expand Up @@ -157,7 +160,12 @@ import { debounceHelper } from '@/mixins/debounce';
import stringify from 'fast-json-stable-stringify';
import { workflowHelpers } from '@/mixins/workflowHelpers';
import { nodeHelpers } from '@/mixins/nodeHelpers';
import { getAppNameFromNodeName, isResourceLocatorValue, hasOnlyListMode } from '@/utils';
import {
getAppNameFromNodeName,
isResourceLocatorValue,
hasOnlyListMode,
getMainAuthField,
} from '@/utils';
import { mapStores } from 'pinia';
import { useUIStore } from '@/stores/ui';
import { useWorkflowsStore } from '@/stores/workflows';
Expand Down Expand Up @@ -286,6 +294,17 @@ export default mixins(debounceHelper, workflowHelpers, nodeHelpers).extend({
}
return !!(node && node.credentials && Object.keys(node.credentials).length === 1);
},
credentialsNotSet(): boolean {
const nodeType = this.nodeTypesStore.getNodeType(this.node?.type);
if (nodeType) {
const usesCredentials =
nodeType.credentials !== undefined && nodeType.credentials.length > 0;
if (usesCredentials && !this.node?.credentials) {
return true;
}
}
return false;
},
inputPlaceholder(): string {
if (this.currentMode.placeholder) {
return this.currentMode.placeholder;
Expand Down Expand Up @@ -518,6 +537,18 @@ export default mixins(debounceHelper, workflowHelpers, nodeHelpers).extend({
const id = node.credentials[credentialKey].id;
this.uiStore.openExistingCredential(id);
},
createNewCredential(): void {
const nodeType = this.nodeTypesStore.getNodeType(this.node?.type);
if (!nodeType) {
return;
}
const mainAuthType = getMainAuthField(nodeType);
const showAuthSelector =
mainAuthType !== null &&
Array.isArray(mainAuthType.options) &&
mainAuthType.options?.length > 0;
this.uiStore.openNewCredential('', showAuthSelector);
},
findModeByName(name: string): INodePropertyMode | null {
if (this.parameter.modes) {
return this.parameter.modes.find((mode: INodePropertyMode) => mode.name === name) || null;
Expand Down
8 changes: 7 additions & 1 deletion packages/editor-ui/src/components/RunData.vue
Original file line number Diff line number Diff line change
Expand Up @@ -429,7 +429,13 @@
</div>
<div
:class="$style.pagination"
v-if="hasNodeRun && !hasRunError && dataCount > pageSize && !isSchemaView"
v-if="
hasNodeRun &&
!hasRunError &&
binaryData.length === 0 &&
dataCount > pageSize &&
!isSchemaView
"
v-show="!editMode.enabled"
>
<el-pagination
Expand Down
3 changes: 2 additions & 1 deletion packages/editor-ui/src/plugins/i18n/locales/en.json
Original file line number Diff line number Diff line change
Expand Up @@ -1038,7 +1038,8 @@
"resourceLocator.mode.list.disabled.title": "Change to Fixed mode to choose From List",
"resourceLocator.mode.list.error.title": "Could not load list",
"resourceLocator.mode.list.error.description.part1": "Please",
"resourceLocator.mode.list.error.description.part2": "check your credential",
"resourceLocator.mode.list.error.description.part2.hasCredentials": "check your credential",
"resourceLocator.mode.list.error.description.part2.noCredentials": "add your credential",
"resourceLocator.mode.list.noResults": "No results",
"resourceLocator.mode.list.openUrl": "Open URL",
"resourceLocator.mode.list.placeholder": "Choose...",
Expand Down
9 changes: 7 additions & 2 deletions packages/editor-ui/src/views/SettingsPersonalView.vue
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@
/>
</div>
</div>
<div v-if="!signInWithLdap">
<div v-if="!signInWithLdap && !signInWithSaml">
<div :class="$style.sectionHeader">
<n8n-heading size="large">{{ $locale.baseText('settings.personal.security') }}</n8n-heading>
</div>
Expand Down Expand Up @@ -114,7 +114,7 @@ export default mixins(showMessage).extend({
validationRules: [{ name: 'VALID_EMAIL' }],
autocomplete: 'email',
capitalize: true,
disabled: this.isLDAPFeatureEnabled && this.signInWithLdap,
disabled: (this.isLDAPFeatureEnabled && this.signInWithLdap) || this.signInWithSaml,
},
},
];
Expand All @@ -130,6 +130,11 @@ export default mixins(showMessage).extend({
isLDAPFeatureEnabled(): boolean {
return this.settingsStore.settings.enterprise.ldap === true;
},
signInWithSaml(): boolean {
return (
this.settingsStore.isSamlLoginEnabled && this.settingsStore.isDefaultAuthenticationSaml
);
},
},
methods: {
onInput() {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ export class MattermostApi implements ICredentialType {

test: ICredentialTestRequest = {
request: {
baseURL: '={{$credentials.baseUrl}}/api/v4',
baseURL: '={{$credentials.baseUrl.replace(/\\/$/, "")}}/api/v4',
url: '/users',
skipSslCertificateValidation: '={{$credentials?.allowUnauthorizedCerts}}',
},
Expand Down
6 changes: 3 additions & 3 deletions packages/nodes-base/nodes/Compression/Compression.node.ts
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@ export class Compression implements INodeType {
group: ['transform'],
subtitle: '={{$parameter["operation"]}}',
version: 1,
description: 'Compress and uncompress files',
description: 'Compress and decompress files',
defaults: {
name: 'Compression',
color: '#408000',
Expand Down Expand Up @@ -197,7 +197,7 @@ export class Compression implements INodeType {
const binaryData = this.helpers.assertBinaryData(i, binaryPropertyName);
const binaryDataBuffer = await this.helpers.getBinaryDataBuffer(i, binaryPropertyName);

if (binaryData.fileExtension === 'zip') {
if (binaryData.fileExtension?.toLowerCase() === 'zip') {
const files = await unzip(binaryDataBuffer);

for (const key of Object.keys(files)) {
Expand All @@ -213,7 +213,7 @@ export class Compression implements INodeType {

binaryObject[`${outputPrefix}${zipIndex++}`] = data;
}
} else if (binaryData.fileExtension === 'gz') {
} else if (binaryData.fileExtension?.toLowerCase() === 'gz') {
const file = await gunzip(binaryDataBuffer);

const fileName = binaryData.fileName?.split('.')[0];
Expand Down
21 changes: 15 additions & 6 deletions packages/nodes-base/nodes/HttpRequest/V3/HttpRequestV3.node.ts
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ import {
replaceNullValues,
sanitizeUiMessage,
} from '../GenericFunctions';
import { keysToLowercase } from '../../../utils/utilities';

function toText<T>(data: T) {
if (typeof data === 'object' && data !== null) {
Expand Down Expand Up @@ -1033,17 +1034,21 @@ export class HttpRequestV3 implements INodeType {
const body = this.getNodeParameter('body', itemIndex, '') as string;

const sendHeaders = this.getNodeParameter('sendHeaders', itemIndex, false) as boolean;

const headerParameters = this.getNodeParameter(
'headerParameters.parameters',
itemIndex,
[],
) as [{ name: string; value: string }];

const specifyHeaders = this.getNodeParameter(
'specifyHeaders',
itemIndex,
'keypair',
) as string;

const jsonHeadersParameter = this.getNodeParameter('jsonHeaders', itemIndex, '') as string;

const {
redirect,
batching,
Expand Down Expand Up @@ -1221,8 +1226,8 @@ export class HttpRequestV3 implements INodeType {
requestOptions.body = uploadData;
requestOptions.headers = {
...requestOptions.headers,
'Content-Length': contentLength,
'Content-Type': itemBinaryData.mimeType ?? 'application/octet-stream',
'content-length': contentLength,
'content-type': itemBinaryData.mimeType ?? 'application/octet-stream',
};
} else if (bodyContentType === 'raw') {
requestOptions.body = body;
Expand Down Expand Up @@ -1253,8 +1258,9 @@ export class HttpRequestV3 implements INodeType {

// Get parameters defined in the UI
if (sendHeaders && headerParameters) {
let additionalHeaders: IDataObject = {};
if (specifyHeaders === 'keypair') {
requestOptions.headers = headerParameters.reduce(parametersToKeyValue, {});
additionalHeaders = headerParameters.reduce(parametersToKeyValue, {});
} else if (specifyHeaders === 'json') {
// body is specified using JSON
try {
Expand All @@ -1269,8 +1275,12 @@ export class HttpRequestV3 implements INodeType {
);
}

requestOptions.headers = jsonParse(jsonHeadersParameter);
additionalHeaders = jsonParse(jsonHeadersParameter);
}
requestOptions.headers = {
...requestOptions.headers,
...keysToLowercase(additionalHeaders),
};
}

if (autoDetectResponseFormat || responseFormat === 'file') {
Expand All @@ -1290,7 +1300,7 @@ export class HttpRequestV3 implements INodeType {
requestOptions.headers = {};
}
const rawContentType = this.getNodeParameter('rawContentType', itemIndex) as string;
requestOptions.headers['Content-Type'] = rawContentType;
requestOptions.headers['content-type'] = rawContentType;
}

const authDataKeys: IAuthDataSanitizeKeys = {};
Expand Down Expand Up @@ -1338,7 +1348,6 @@ export class HttpRequestV3 implements INodeType {
try {
this.sendMessageToUI(sanitizeUiMessage(requestOptions, authDataKeys));
} catch (e) {}

if (authentication === 'genericCredentialType' || authentication === 'none') {
if (oAuth1Api) {
const requestOAuth1 = this.helpers.requestOAuth1.call(this, 'oAuth1Api', requestOptions);
Expand Down

0 comments on commit c56d47e

Please sign in to comment.