feat: Add initial scope checks via decorators #7737

valya · 2023-11-16T16:18:44Z

This commit also includes a refactor of the variables controller/service and a new Licensed decorator

Github issue / Community forum post (link here to close automatically):

netroy

Can we please split out the changes to existing decorators to a separate commit (or preferably a separate PR)?

valya · 2023-11-16T16:38:56Z

Can we please split out the changes to existing decorators to a separate commit (or preferably a separate PR)?

Do you mean adding the RequireGlobalScope and Licensed decorators and the associated changes to registerController.ts? Or do you mean the changes to all the existing controllers?

netroy · 2023-11-16T16:46:05Z

Can we please split out the changes to existing decorators to a separate commit (or preferably a separate PR)?

Do you mean adding the RequireGlobalScope and Licensed decorators and the associated changes to registerController.ts? Or do you mean the changes to all the existing controllers?

Licensed decorator could be a separate feat PR. Then RequireGlobalScope decorator could be a second feat PR. after that all the changes to variables routes could be a refactor PR.

…-authorized-decorator-owner-checks

ivov

Nice work!

packages/@n8n/permissions/src/types.ts

packages/cli/src/ExternalSecrets/ExternalSecrets.controller.ee.ts

packages/cli/src/controllers/invitation.controller.ts

packages/cli/src/decorators/Licensed.ee.ts

packages/cli/src/decorators/registerController.ts

packages/cli/src/environments/variables/variables.controller.ee.ts

packages/cli/src/permissions/roles.ts

packages/cli/src/sso/saml/routes/saml.controller.ee.ts

packages/cli/src/decorators/Licensed.ee.ts

netroy · 2023-11-22T16:29:30Z

packages/cli/src/ExternalSecrets/ExternalSecrets.controller.ee.ts

 @RestController('/external-secrets')
 export class ExternalSecretsController {
 	constructor(private readonly secretsService: ExternalSecretsService) {}

 	@Get('/providers')
+	@RequireGlobalScope('externalSecretsStore:list')


should we consider moving auth scopes to existing decorators?
I'm wondering is there is an advantage of having a separate decorator.

We could do it as part of the routing decorators but I personally think separate function decorators look neater.

packages/cli/src/decorators/Licensed.ts

packages/cli/src/decorators/Scopes.ts

packages/cli/src/decorators/registerController.ts

netroy

Once this is ready to merge, can we please extract the decorator changes into a separate PR? and possibly the variables controller refactor into another PR?
I'd really prefer if we could keep distinct changes as distinct commits in master 🙏🏽

…-authorized-decorator-owner-checks

github-actions · 2023-11-27T14:54:16Z

⚠️ Some Cypress E2E specs are failing, please fix them before merging

cypress · 2023-11-27T14:54:21Z

Passing run #3067 ↗︎

0	288	5	0	0

Details:

🌳 🖥️ browsers:node18.12.0-chrome107 🤖 valya 🗃️ e2e/*
Project: n8n	Commit: `30346f45f2`
Status: Passed	Duration: 05:52 💡
Started: Nov 28, 2023 11:00 AM	Ended: Nov 28, 2023 11:06 AM

Review all test suite changes for PR #7737 ↗︎

github-actions · 2023-11-27T15:20:37Z

⚠️ Some Cypress E2E specs are failing, please fix them before merging

netroy

Will we update the remaining controllers in another PR, or is that not planned for now?

packages/cli/src/decorators/Scopes.ts

netroy · 2023-11-27T15:43:04Z

packages/cli/src/decorators/Scopes.ts

+import type { ScopeMetadata } from './types';
+import { CONTROLLER_REQUIRED_SCOPES } from './constants';
+
+export const RequireGlobalScope = (scope: Scope | Scope[], scopeOptions?: ScopeOptions) => {


There seems to be quite an overlap between Authorized and RequireGlobalScope. If we update all the remaining controllers that still have @Authorized on them, we could combine the two decorators, as well as the middlewares that use the metadata from these decorators.
We don't need to do that now, but we should do it soon, so that we don't end up maintaining a 3rd authorization mechanism, while we are still trying to get rid of middlewares/auth.ts

…-authorized-decorator-owner-checks

github-actions · 2023-11-28T10:01:14Z

⚠️ Some Cypress E2E specs are failing, please fix them before merging

github-actions · 2023-11-28T10:47:09Z

✅ All Cypress E2E specs passed

…-authorized-decorator-owner-checks

github-actions · 2023-11-28T11:06:26Z

✅ All Cypress E2E specs passed

* master: fix(editor): Add telemetry to workflow history (#7811) fix(editor): Fix mouse position in workflow previews (#7853) fix(editor): Fix icon for unknown node type (#7842) ci: Fix editor tests when coverage is enabled (no-changelog) (#7827) fix(editor): Suppress dev server websocket messages in workflow view (#7808) feat(core): Update LLM applications building support (no-changelog) (#7710) fix(editor): Fix push connection on WorkerList and CommunityNodes pages (no-changelog) (#7851) fix(editor): Remove ability for users to select admin role in the UI (no-changelog) (#7850) feat(core): Set up endpoint for all existing roles with license flag (#7834) fix(editor): Fix credential icon for old node type version (#7843) feat: Add initial scope checks via decorators (#7737) refactor(editor): Delete some barrel files and reduce circular dependencies (no-changelog) (#7838) fix(editor): Allow owners and admins to share workflows and credentials they don't own (#7833) refactor(core): Reorganize error hierarchy in `cli` package (no-changelog) (#7839) fix(Google Calendar Trigger Node): Fix issue preventing birthday and holiday calendars from working (#7832) fix(Google Sheets Node): Read operation execute for each item (#7800) fix(core): Node version in the user added node to workflow canvas event (no-changelog) (#7814)

# [1.19.0](https://github.com/n8n-io/n8n/compare/n8n@1.18.0...n8n@1.19.0) (2023-11-29) ### Bug Fixes * **core:** Ensure member and admin cannot be promoted to owner ([#7830](#7830)) ([9b87a59](9b87a59)), closes [/linear.app/n8n/issue/PAY-985/add-user-role-modification-endpoint#comment-62355f6](https://github.com//linear.app/n8n/issue/PAY-985/add-user-role-modification-endpoint/issues/comment-62355f6) * **core:** Prevent error messages due to statistics about data loading ([#7824](#7824)) ([847f6ac](847f6ac)) * **core:** Tighten checks for multi-main setup usage ([#7788](#7788)) ([fdb2c18](fdb2c18)) * **core:** Use AbortController to notify nodes to abort execution ([#6141](#6141)) ([d2c18c5](d2c18c5)) * **editor:** Add telemetry to workflow history ([#7811](#7811)) ([d497041](d497041)) * **editor:** Allow owners and admins to share workflows and credentials they don't own ([#7833](#7833)) ([3ab3ec9](3ab3ec9)) * **editor:** Disable context menu actions in read-only mode ([#7789](#7789)) ([902beff](902beff)) * **editor:** Fix cloud plan data loading on instance ([#7841](#7841)) ([8b99384](8b99384)) * **editor:** Fix credential icon for old node type version ([#7843](#7843)) ([4074107](4074107)) * **editor:** Fix icon for unknown node type ([#7842](#7842)) ([28ac5a7](28ac5a7)) * **editor:** Fix mouse position in workflow previews ([#7853](#7853)) ([c063398](c063398)) * **editor:** Show nice error when environment is not set up ([#7778](#7778)) ([5835e05](5835e05)) * **editor:** Suppress dev server websocket messages in workflow view ([#7808](#7808)) ([685ffd7](685ffd7)) * **Google Sheets Node:** Read operation execute for each item ([#7800](#7800)) ([d548872](d548872)) * **HTTP Request Node:** Enable expressions for binary input data fields ([#7782](#7782)) ([6208af0](6208af0)) * **Microsoft SQL Node:** Prevent double escaping table name ([#7801](#7801)) ([73ec753](73ec753)) ### Features * Add AI tool building capabilities ([#7336](#7336)) ([87def60](87def60)) * Add initial scope checks via decorators ([#7737](#7737)) ([a37f1cb](a37f1cb)) * Ado 1296 spike credential setup in templates ([#7786](#7786)) ([aae45b0](aae45b0)) * **core:** Add Support for custom CORS origins for webhooks ([#7455](#7455)) ([99a9ea4](99a9ea4)) * **core:** Allow user role modification ([#7797](#7797)) ([7a86d36](7a86d36)) * **core:** Set up endpoint for all existing roles with license flag ([#7834](#7834)) ([2356fb0](2356fb0)) * **editor:** Add node name and version to NDV node settings ([#7731](#7731)) ([da85198](da85198)) * **editor:** Add routing middleware, permission checks, RBAC store, RBAC component ([#7702](#7702)) ([67a8891](67a8891)) * **editor:** Replace middleware for Role checks with Scope checks ([#7847](#7847)) ([72852a6](72852a6)) * **editor:** Show avatars for users currently working on the same workflow ([#7763](#7763)) ([77bc8ec](77bc8ec)) * **Notion Node:** Option to simplify output in getChildBlocks operation ([#7791](#7791)) ([d667bca](d667bca)) * **Slack Node:** Add support for getting the profile of a user ([#7829](#7829)) ([90bb6ba](90bb6ba)) Co-authored-by: ivov <ivov@users.noreply.github.com>

janober · 2023-12-01T14:46:37Z

Got released with n8n@1.19.4

netroy reviewed Nov 16, 2023

View reviewed changes

n8n-assistant bot added core Enhancement outside /nodes-base and /editor-ui n8n team Authored by the n8n team labels Nov 16, 2023

valya added 3 commits November 20, 2023 13:05

refactor: Refactor variables controller into a RestController

c81b074

feat: Add Licensed decorator

440edf9

feat: Add RequireGlobalScope decorator

9a01807

valya force-pushed the pay-983-replace-all-authorized-decorator-owner-checks branch from 3557203 to 9a01807 Compare November 20, 2023 13:43

valya added 5 commits November 20, 2023 14:33

Merge remote-tracking branch 'origin/master' into pay-983-replace-all…

6f58da9

…-authorized-decorator-owner-checks

refactor: Apply scope decorator to invitation controller

65ad786

fix imports

ac92cb3

Merge remote-tracking branch 'origin/master' into pay-983-replace-all…

8d31942

…-authorized-decorator-owner-checks

add credentials share role

5241a87

ivov reviewed Nov 22, 2023

View reviewed changes

netroy reviewed Nov 22, 2023

View reviewed changes

packages/cli/src/decorators/Licensed.ee.ts Outdated Show resolved Hide resolved

netroy reviewed Nov 22, 2023

View reviewed changes

valya added 2 commits November 23, 2023 12:02

review changes

a4fdf22

refactor: Don't use full path for RequireGlobalScope decorator

336cc7a

valya requested a review from ivov November 27, 2023 10:22

ivov previously approved these changes Nov 27, 2023

View reviewed changes

packages/cli/src/decorators/Licensed.ts Outdated Show resolved Hide resolved

packages/cli/src/decorators/Scopes.ts Outdated Show resolved Hide resolved

packages/cli/src/decorators/registerController.ts Show resolved Hide resolved

small review change

7915404

valya dismissed ivov’s stale review via 7915404 November 27, 2023 10:46

another small review change

09257cc

netroy reviewed Nov 27, 2023

View reviewed changes

Merge remote-tracking branch 'origin/master' into pay-983-replace-all…

588df15

…-authorized-decorator-owner-checks

ivov previously approved these changes Nov 27, 2023

View reviewed changes

fix linting errors

b3f9156

valya dismissed ivov’s stale review via b3f9156 November 27, 2023 14:40

ivov previously approved these changes Nov 27, 2023

View reviewed changes

fix permissions error

272f891

valya dismissed ivov’s stale review via 272f891 November 27, 2023 15:30

netroy reviewed Nov 27, 2023

View reviewed changes

valya added 2 commits November 28, 2023 09:37

Merge remote-tracking branch 'origin/master' into pay-983-replace-all…

9e93182

…-authorized-decorator-owner-checks

remove unused parameter from RequireGlobalScope

7d491b2

valya force-pushed the pay-983-replace-all-authorized-decorator-owner-checks branch from 2966685 to 7d491b2 Compare November 28, 2023 09:43

ivov previously approved these changes Nov 28, 2023

View reviewed changes

Merge remote-tracking branch 'origin/master' into pay-983-replace-all…

30346f4

…-authorized-decorator-owner-checks

valya dismissed ivov’s stale review via 30346f4 November 28, 2023 10:49

ivov approved these changes Nov 28, 2023

View reviewed changes

valya merged commit a37f1cb into master Nov 28, 2023
19 checks passed

valya deleted the pay-983-replace-all-authorized-decorator-owner-checks branch November 28, 2023 11:41

rishikeshjoshi pushed a commit to rishikeshjoshi/n8n that referenced this pull request Nov 28, 2023

feat: Add initial scope checks via decorators (n8n-io#7737)

05dccc4

github-actions bot mentioned this pull request Nov 29, 2023

🚀 Release 1.19.0 #7863

Merged

janober added the Released label Dec 1, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: Add initial scope checks via decorators #7737

feat: Add initial scope checks via decorators #7737

valya commented Nov 16, 2023

netroy left a comment

valya commented Nov 16, 2023

netroy commented Nov 16, 2023

ivov left a comment

netroy Nov 22, 2023

valya Nov 23, 2023

netroy left a comment

github-actions bot commented Nov 27, 2023

cypress bot commented Nov 27, 2023 •

edited

Loading

github-actions bot commented Nov 27, 2023

netroy left a comment •

edited

Loading

netroy Nov 27, 2023

github-actions bot commented Nov 28, 2023

github-actions bot commented Nov 28, 2023

github-actions bot commented Nov 28, 2023

janober commented Dec 1, 2023

feat: Add initial scope checks via decorators #7737

feat: Add initial scope checks via decorators #7737

Conversation

valya commented Nov 16, 2023

netroy left a comment

Choose a reason for hiding this comment

valya commented Nov 16, 2023

netroy commented Nov 16, 2023

ivov left a comment

Choose a reason for hiding this comment

netroy Nov 22, 2023

Choose a reason for hiding this comment

valya Nov 23, 2023

Choose a reason for hiding this comment

netroy left a comment

Choose a reason for hiding this comment

github-actions bot commented Nov 27, 2023

cypress bot commented Nov 27, 2023 • edited Loading

Passing run #3067 ↗︎

Review all test suite changes for PR #7737 ↗︎

github-actions bot commented Nov 27, 2023

netroy left a comment • edited Loading

Choose a reason for hiding this comment

netroy Nov 27, 2023

Choose a reason for hiding this comment

github-actions bot commented Nov 28, 2023

github-actions bot commented Nov 28, 2023

github-actions bot commented Nov 28, 2023

janober commented Dec 1, 2023

cypress bot commented Nov 27, 2023 •

edited

Loading

netroy left a comment •

edited

Loading