Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(editor): Replace isInstanceOwner checks with scopes where applicable #7858

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
20 commits
Select commit Hold shift + click to select a range
39d377d
fix(editor): Check scope for source control
cstuncsik Nov 29, 2023
3e965ea
fix: source control unit test
cstuncsik Nov 29, 2023
57f52a5
fix: Check for scope permission for log streaming
cstuncsik Nov 29, 2023
c56f48b
fix: Add scope check for log streaming
cstuncsik Nov 29, 2023
69ed5fd
fix: fix typo
alexgrozav Nov 29, 2023
e323161
fix: Use scope permission in external secrets store
cstuncsik Nov 29, 2023
0cc82df
Merge branch 'pay-1068-replace-isinstanceowner-checks-with-scopes-whe…
cstuncsik Nov 29, 2023
7dda838
fix: Use scope permission in node helpers mixin
cstuncsik Nov 29, 2023
e96d603
feat: replace roles where applicable
alexgrozav Nov 29, 2023
15822b7
Merge branch 'pay-1068-replace-isinstanceowner-checks-with-scopes-whe…
alexgrozav Nov 29, 2023
d708722
fix: Add and use `instanceOwner` permission
cstuncsik Nov 29, 2023
68d4522
Merge branch 'pay-1068-replace-isinstanceowner-checks-with-scopes-whe…
cstuncsik Nov 29, 2023
dc8d064
Merge remote-tracking branch 'origin/master' into pay-1068-replace-is…
cstuncsik Nov 29, 2023
af3b337
Merge remote-tracking branch 'origin/master' into pay-1068-replace-is…
cstuncsik Nov 29, 2023
a8b0deb
fix: Lint fix
cstuncsik Nov 29, 2023
1c6d4f5
Merge remote-tracking branch 'origin/master' into pay-1068-replace-is…
cstuncsik Nov 29, 2023
bd7cf4e
fix: Addressing code review requests from Alex
cstuncsik Nov 29, 2023
b4ba942
fix: Fix unit test
cstuncsik Nov 29, 2023
c10d844
Merge remote-tracking branch 'origin/master' into pay-1068-replace-is…
cstuncsik Nov 30, 2023
6c19092
Merge remote-tracking branch 'origin/master' into pay-1068-replace-is…
cstuncsik Dec 4, 2023
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 8 additions & 8 deletions packages/editor-ui/src/api/workflow-webhooks.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,25 +7,25 @@ const CONTACT_EMAIL_SUBMISSION_ENDPOINT = '/accounts/onboarding';

export async function fetchNextOnboardingPrompt(
instanceId: string,
currentUer: IUser,
currentUser: IUser,
): Promise<IOnboardingCallPrompt> {
return get(N8N_API_BASE_URL, ONBOARDING_PROMPTS_ENDPOINT, {
instance_id: instanceId,
user_id: `${instanceId}#${currentUer.id}`,
is_owner: currentUer.isOwner,
survey_results: currentUer.personalizationAnswers,
user_id: `${instanceId}#${currentUser.id}`,
is_owner: currentUser.isOwner,
survey_results: currentUser.personalizationAnswers,
});
}

export async function applyForOnboardingCall(
instanceId: string,
currentUer: IUser,
currentUser: IUser,
email: string,
): Promise<string> {
try {
const response = await post(N8N_API_BASE_URL, ONBOARDING_PROMPTS_ENDPOINT, {
instance_id: instanceId,
user_id: `${instanceId}#${currentUer.id}`,
user_id: `${instanceId}#${currentUser.id}`,
email,
});
return response;
Expand All @@ -36,13 +36,13 @@ export async function applyForOnboardingCall(

export async function submitEmailOnSignup(
instanceId: string,
currentUer: IUser,
currentUser: IUser,
email: string | undefined,
agree: boolean,
): Promise<string> {
return post(N8N_API_BASE_URL, CONTACT_EMAIL_SUBMISSION_ENDPOINT, {
instance_id: instanceId,
user_id: `${instanceId}#${currentUer.id}`,
user_id: `${instanceId}#${currentUser.id}`,
email,
agree,
});
Expand Down
13 changes: 5 additions & 8 deletions packages/editor-ui/src/components/MainHeader/WorkflowDetails.vue
Original file line number Diff line number Diff line change
Expand Up @@ -184,6 +184,7 @@ import { getWorkflowPermissions } from '@/permissions';
import { createEventBus } from 'n8n-design-system/utils';
import { nodeViewEventBus } from '@/event-bus';
import { genericHelpers } from '@/mixins/genericHelpers';
import { hasPermission } from '@/rbac/permissions';

const hasChanged = (prev: string[], curr: string[]) => {
if (prev.length !== curr.length) {
Expand Down Expand Up @@ -247,10 +248,7 @@ export default defineComponent({
currentUser(): IUser | null {
return this.usersStore.currentUser;
},
currentUserIsOwner(): boolean {
return this.usersStore.currentUser?.isOwner ?? false;
},
contextBasedTranslationKeys(): NestedRecord<string> {
contextBasedTranslationKeys() {
return this.uiStore.contextBasedTranslationKeys;
},
isWorkflowActive(): boolean {
Expand Down Expand Up @@ -298,7 +296,7 @@ export default defineComponent({
].includes(this.$route.name || '');
},
workflowPermissions(): IPermissions {
return getWorkflowPermissions(this.usersStore.currentUser, this.workflow);
return getWorkflowPermissions(this.currentUser, this.workflow);
},
workflowMenuItems(): Array<{}> {
const actions = [
Expand Down Expand Up @@ -330,16 +328,15 @@ export default defineComponent({
);
}

if (this.currentUserIsOwner) {
if (hasPermission(['rbac'], { rbac: { scope: 'sourceControl:push' } })) {
actions.push({
id: WORKFLOW_MENU_ACTIONS.PUSH,
label: this.$locale.baseText('menuActions.push'),
disabled:
!this.sourceControlStore.isEnterpriseSourceControlEnabled ||
!this.onWorkflowPage ||
this.onExecutionsTab ||
this.readOnlyEnv ||
!this.currentUserIsOwner,
this.readOnlyEnv,
});
}

Expand Down
2 changes: 1 addition & 1 deletion packages/editor-ui/src/components/MainSidebar.vue
Original file line number Diff line number Diff line change
Expand Up @@ -266,7 +266,7 @@ export default defineComponent({
position: 'bottom',
label: 'Admin Panel',
icon: 'home',
available: this.settingsStore.isCloudDeployment && this.usersStore.isInstanceOwner,
available: this.settingsStore.isCloudDeployment && hasPermission(['instanceOwner']),
},
{
id: 'settings',
Expand Down
14 changes: 7 additions & 7 deletions packages/editor-ui/src/components/MainSidebarSourceControl.vue
Original file line number Diff line number Diff line change
Expand Up @@ -3,12 +3,11 @@ import { computed, nextTick, ref } from 'vue';
import { useRouter } from 'vue-router';
import { createEventBus } from 'n8n-design-system/utils';
import { useI18n } from '@/composables/useI18n';
import { useMessage } from '@/composables/useMessage';
import { hasPermission } from '@/rbac/permissions';
import { useToast } from '@/composables/useToast';
import { useLoadingService } from '@/composables/useLoadingService';
import { useUIStore } from '@/stores/ui.store';
import { useSourceControlStore } from '@/stores/sourceControl.store';
import { useUsersStore } from '@/stores/users.store';
import { SOURCE_CONTROL_PULL_MODAL_KEY, SOURCE_CONTROL_PUSH_MODAL_KEY, VIEWS } from '@/constants';
import type { SourceControlAggregatedFile } from '../Interface';
import { sourceControlEventBus } from '@/event-bus/source-control';
Expand All @@ -24,9 +23,7 @@ const responseStatuses = {
const router = useRouter();
const loadingService = useLoadingService();
const uiStore = useUIStore();
const usersStore = useUsersStore();
const sourceControlStore = useSourceControlStore();
const message = useMessage();
const toast = useToast();
const i18n = useI18n();

Expand All @@ -36,8 +33,11 @@ const tooltipOpenDelay = ref(300);
const currentBranch = computed(() => {
return sourceControlStore.preferences.branchName;
});
const isInstanceOwner = computed(() => usersStore.isInstanceOwner);
const setupButtonTooltipPlacement = computed(() => (props.isCollapsed ? 'right' : 'top'));
const sourceControlAvailable = computed(
() =>
sourceControlStore.isEnterpriseSourceControlEnabled &&
hasPermission(['rbac'], { rbac: { scope: 'sourceControl:manage' } }),
);

async function pushWorkfolder() {
loadingService.startLoading();
Expand Down Expand Up @@ -125,7 +125,7 @@ const goToSourceControlSetup = async () => {

<template>
<div
v-if="sourceControlStore.isEnterpriseSourceControlEnabled && isInstanceOwner"
v-if="sourceControlAvailable"
:class="{
[$style.sync]: true,
[$style.collapsed]: isCollapsed,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@

<el-switch
class="mr-s"
:disabled="!isInstanceOwner"
:disabled="readonly"
:modelValue="nodeParameters.enabled"
@update:modelValue="onEnabledSwitched($event, destination.id)"
:title="
Expand Down Expand Up @@ -84,7 +84,7 @@ export default defineComponent({
required: true,
default: deepCopy(defaultMessageEventBusDestinationOptions),
},
isInstanceOwner: Boolean,
readonly: Boolean,
},
mounted() {
this.nodeParameters = Object.assign(
Expand All @@ -105,7 +105,7 @@ export default defineComponent({
value: DESTINATION_LIST_ITEM_ACTIONS.OPEN,
},
];
if (this.isInstanceOwner) {
if (!this.readonly) {
actions.push({
label: this.$locale.baseText('workflows.item.delete'),
value: DESTINATION_LIST_ITEM_ACTIONS.DELETE,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@
@click="sendTestEvent"
data-test-id="destination-test-button"
/>
<template v-if="isInstanceOwner">
<template v-if="canManageLogStreaming">
<n8n-icon-button
v-if="nodeParameters && hasOnceBeenSaved"
:title="$locale.baseText('settings.log-streaming.delete')"
Expand Down Expand Up @@ -117,7 +117,7 @@
:parameters="webhookDescription"
:hideDelete="true"
:nodeValues="nodeParameters"
:isReadOnly="!isInstanceOwner"
:isReadOnly="!canManageLogStreaming"
path=""
@valueChanged="valueChanged"
/>
Expand All @@ -127,7 +127,7 @@
:parameters="syslogDescription"
:hideDelete="true"
:nodeValues="nodeParameters"
:isReadOnly="!isInstanceOwner"
:isReadOnly="!canManageLogStreaming"
path=""
@valueChanged="valueChanged"
/>
Expand All @@ -137,7 +137,7 @@
:parameters="sentryDescription"
:hideDelete="true"
:nodeValues="nodeParameters"
:isReadOnly="!isInstanceOwner"
:isReadOnly="!canManageLogStreaming"
path=""
@valueChanged="valueChanged"
/>
Expand All @@ -156,7 +156,7 @@
:destinationId="destination.id"
@input="onInput"
@change="valueChanged"
:readonly="!isInstanceOwner"
:readonly="!canManageLogStreaming"
/>
</div>
</div>
Expand Down Expand Up @@ -194,7 +194,7 @@ import { LOG_STREAM_MODAL_KEY, MODAL_CONFIRM } from '@/constants';
import Modal from '@/components/Modal.vue';
import { useMessage } from '@/composables/useMessage';
import { useUIStore } from '@/stores/ui.store';
import { useUsersStore } from '@/stores/users.store';
import { hasPermission } from '@/rbac/permissions';
import { destinationToFakeINodeUi } from '@/components/SettingsLogStreaming/Helpers.ee';
import {
webhookModalDescription,
Expand Down Expand Up @@ -252,12 +252,11 @@ export default defineComponent({
headerLabel: this.destination.label,
testMessageSent: false,
testMessageResult: false,
isInstanceOwner: false,
LOG_STREAM_MODAL_KEY,
};
},
computed: {
...mapStores(useUIStore, useUsersStore, useLogStreamingStore, useNDVStore, useWorkflowsStore),
...mapStores(useUIStore, useLogStreamingStore, useNDVStore, useWorkflowsStore),
typeSelectOptions(): Array<{ value: string; label: BaseTextKey }> {
const options: Array<{ value: string; label: BaseTextKey }> = [];
for (const t of Object.values(MessageEventBusDestinationTypeNames)) {
Expand Down Expand Up @@ -306,9 +305,11 @@ export default defineComponent({
}
return items;
},
canManageLogStreaming(): boolean {
return hasPermission(['rbac'], { rbac: { scope: 'logStreaming:manage' } });
},
},
mounted() {
this.isInstanceOwner = this.usersStore.currentUser?.globalRole?.name === 'owner';
this.setupNode(
Object.assign(deepCopy(defaultMessageEventBusDestinationOptions), this.destination),
);
Expand Down
9 changes: 6 additions & 3 deletions packages/editor-ui/src/components/WorkflowSettings.vue
Original file line number Diff line number Diff line change
Expand Up @@ -381,6 +381,8 @@ import { useRootStore } from '@/stores/n8nRoot.store';
import { useWorkflowsEEStore } from '@/stores/workflows.ee.store';
import { useWorkflowsStore } from '@/stores/workflows.store';
import { createEventBus } from 'n8n-design-system/utils';
import type { IPermissions } from '@/permissions';
import { getWorkflowPermissions } from '@/permissions';

export default defineComponent({
name: 'WorkflowSettings',
Expand Down Expand Up @@ -479,6 +481,9 @@ export default defineComponent({

return this.workflowsEEStore.getWorkflowOwnerName(`${this.workflowId}`, fallback);
},
workflowPermissions(): IPermissions {
return getWorkflowPermissions(this.currentUser, this.workflow);
},
},
async mounted() {
this.executionTimeout = this.rootStore.executionTimeout;
Expand Down Expand Up @@ -584,8 +589,6 @@ export default defineComponent({
};
},
async loadWorkflowCallerPolicyOptions() {
const currentUserIsOwner = this.workflow.ownedBy?.id === this.currentUser?.id;

this.workflowCallerPolicyOptions = [
{
key: 'none',
Expand All @@ -597,7 +600,7 @@ export default defineComponent({
'workflowSettings.callerPolicy.options.workflowsFromSameOwner',
{
interpolate: {
owner: currentUserIsOwner
owner: this.workflowPermissions.isOwner
? this.$locale.baseText(
'workflowSettings.callerPolicy.options.workflowsFromSameOwner.owner',
)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,13 +8,13 @@ import { SETTINGS_STORE_DEFAULT_STATE } from '@/__tests__/utils';
import MainSidebarSourceControl from '@/components/MainSidebarSourceControl.vue';
import { useSourceControlStore } from '@/stores/sourceControl.store';
import { useUIStore } from '@/stores/ui.store';
import { useUsersStore } from '@/stores/users.store';
import { useRBACStore } from '@/stores/rbac.store';
import { createComponentRenderer } from '@/__tests__/render';

let pinia: ReturnType<typeof createTestingPinia>;
let sourceControlStore: ReturnType<typeof useSourceControlStore>;
let uiStore: ReturnType<typeof useUIStore>;
let usersStore: ReturnType<typeof useUsersStore>;
let rbacStore: ReturnType<typeof useRBACStore>;

const renderComponent = createComponentRenderer(MainSidebarSourceControl);

Expand All @@ -28,8 +28,8 @@ describe('MainSidebarSourceControl', () => {
},
});

usersStore = useUsersStore(pinia);
vi.spyOn(usersStore, 'isInstanceOwner', 'get').mockReturnValue(true);
rbacStore = useRBACStore(pinia);
vi.spyOn(rbacStore, 'hasScope').mockReturnValue(true);

sourceControlStore = useSourceControlStore();
vi.spyOn(sourceControlStore, 'isEnterpriseSourceControlEnabled', 'get').mockReturnValue(true);
Expand All @@ -38,7 +38,7 @@ describe('MainSidebarSourceControl', () => {
});

it('should render nothing when not instance owner', async () => {
vi.spyOn(usersStore, 'isInstanceOwner', 'get').mockReturnValue(false);
vi.spyOn(rbacStore, 'hasScope').mockReturnValue(false);
const { container } = renderComponent({ pinia, props: { isCollapsed: false } });
expect(container).toBeEmptyDOMElement();
});
Expand Down
8 changes: 5 additions & 3 deletions packages/editor-ui/src/components/banners/V1Banner.vue
Original file line number Diff line number Diff line change
@@ -1,23 +1,25 @@
<script lang="ts" setup>
import { computed } from 'vue';
import BaseBanner from '@/components/banners/BaseBanner.vue';
import { i18n as locale } from '@/plugins/i18n';
import { useUsersStore } from '@/stores/users.store';
import { hasPermission } from '@/rbac/permissions';
import { useUIStore } from '@/stores/ui.store';

const uiStore = useUIStore();
const usersStore = useUsersStore();

async function dismissPermanently() {
await uiStore.dismissBanner('V1', 'permanent');
}

const hasOwnerPermission = computed(() => hasPermission(['instanceOwner']));
</script>

<template>
<base-banner customIcon="info-circle" theme="warning" name="V1" :class="$style.v1container">
<template #mainContent>
<span v-html="locale.baseText('banners.v1.message')"></span>
<a
v-if="usersStore.isInstanceOwner"
v-if="hasOwnerPermission"
:class="$style.link"
@click="dismissPermanently"
data-test-id="banner-confirm-v1"
Expand Down
Loading
Loading