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

feat(app-shell): enforce 'view' permission check for main application route #2459

Merged
merged 2 commits into from
Jan 27, 2022

Conversation

emmenko
Copy link
Member

@emmenko emmenko commented Jan 27, 2022

Main branch: #2430

With the new Custom Application permissions, users can assign permissions to Teams to grant or not to grant access to the application.

If a user is a member of a Team without permissions assigned to this Custom Application, the application should be not accessible to that user.

Usually it's up to the developer to enforce the check on the route level. However, people might forget to do so, resulting in the Custom Application (route) to be available to users even though they shouldn't have permissions to do so.

To enforce that as a default, this check is now performed within the AppShell.

NOTE: if a user does not have permissions, the API requests will fail anyway. This is more of a UX improvement to simply prevent the user to access the application in the first place.

@vercel
Copy link

vercel bot commented Jan 27, 2022

This pull request is being automatically deployed with Vercel (learn more).
To see the status of your deployment, click below or on the icon next to each commit.

🔍 Inspect: https://vercel.com/commercetools/merchant-center-application-kit/5VgQkRvbQ291dzaQ3dgQzLG6jcEs
✅ Preview: https://merchant-center-application-kit-git-nm-rou-34ad24-commercetools.vercel.app

@changeset-bot
Copy link

changeset-bot bot commented Jan 27, 2022

🦋 Changeset detected

Latest commit: 16cc09b

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 4 packages
Name Type
@commercetools-frontend/application-shell Minor
@commercetools-frontend/cypress Minor
merchant-center-application-template-starter Patch
playground Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

const ApplicationRoute = <AdditionalEnvironmentProperties extends {}>(
props: Props<AdditionalEnvironmentProperties>
) => {
if (props.disableRoutePermissionCheck) {
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

FYI: We would use this flag internally to bail out of this behavior, as permissions work a bit differently in our default applications.

@emmenko emmenko requested review from tdeekens and a team January 27, 2022 14:22
@vercel vercel bot temporarily deployed to Preview January 27, 2022 15:22 Inactive
@vercel vercel bot temporarily deployed to Preview January 27, 2022 16:36 Inactive
@vercel vercel bot temporarily deployed to Preview January 27, 2022 17:36 Inactive
@vercel vercel bot temporarily deployed to Preview January 27, 2022 17:44 Inactive
@emmenko emmenko merged commit f70e307 into nm-org-level-v21 Jan 27, 2022
@emmenko emmenko deleted the nm-route-permission-check branch January 27, 2022 18:00
@ghost ghost mentioned this pull request Jan 27, 2022
emmenko added a commit that referenced this pull request Feb 2, 2022
* refactor: app config required fields, menu links structure, drop deprecated menu.json support

* refactor(cli): options of build and compile-html commands

* refactor(test-utils): remove deprecated options, change some default flags

* feat(test-utils): add helper mapResourceAccessToAppliedPermissions

* feat(app-config): expose helper functions for entryPointUriPath

* refactor: migrate starter template and adjust create-mc-app patching logic

* chore: drop node v12

* docs: update changesets

* fix: ts error

* fix(mc-scripts): typos

* fix: better defaults in test-utils

* fix: node engine

* test: fix assertions

* test: fix assertions

* test: fix more tests setup

* test: fix assertions

* refactor(playground): remove unnecessary test

* fix: prettier

* refactor: expose entryPoint helpers from app-shell

* fix: expose helpers from new ssr entry point

* refactor(schema): validate entryPointUriPath using regex

* fix: fake url placeholder

* chore: leftover

* fix: placeholder url

* test: always use the new loginByOidc command

* chore: min node 14"

* refactor(cypress): improve logs

* fix(cypress): navigate to channels page using link

* chore: remove unnecessary types dependency

* chore: missing env vars

* refactor(create-mc-app): improve validations

* fix(starter): provide initial project key

* fix(ci): missing env vars

* fix(ci): remove whitespace

* fix(create-mc-app): missing dep

* fix(create-mc-app): allow to pass initial project key via CLI option

* fix(create-mc-app): allow to skip prompts with default values using --yes option

* fix(cypress): try to have a more stable percy snapshot

* refactor: disable percy snapshot for starter welcome page

* chore: prerelease mode

* docs: update refs

* ci(changesets): version packages (rc) (#2454)

Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>

* refactor: use version range for Babel packages

* chore: ignore .cache folder from tests

* docs(assets): update readme

* chore: upgrade to eslint v8

* docs: changeset

* refactor: apply suggestions from code review

Co-authored-by: Tobias Deekens <tobias.deekens@commercetools.com>

* chore: update eslint deps to latest

* fix: eslint types resolution

* fix: keep callback function for waitForElementToBeRemoved

* chore: fix eslint deps

* ci(changesets): version packages (rc) (#2456)

Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>

* chore: include .nvmrc in starter template

* fix(mc-scripts): include webpack fallback polyfills for url and querystring

* chore(mc-scripts): upgrade react-dev-utils to v12

* ci(changesets): version packages (rc) (#2457)

Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>

* docs(application-config): add links to doc for schema properties

* refactor(constants): the applicationId is required

* refactor(mc-scripts): remove webpack workaround and migrate deprecated dev server function call

* docs: update readme/package.json links to docs specific pages

* fix: compilation errors

* test: fixes

* feat(app-shell): enforce 'view' permission check for main application route (#2459)

* feat(app-shell): enforce 'view' permission check for main application route

* test: disable permission check in most of the internal tests

* ci(changesets): version packages (rc) (#2458)

Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>

* fix(mc-scripts): webpackdevserver deprecation warnings

* fix: unset initialProjectKey for account app

* ci(changesets): version packages (rc) (#2460)

Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>

* chore: upgrade gatsby/docs dependencies

* refactor: clean up props

* refactor(babel-preset): update dependencies and comments

* refactor(app-shell): make onRegisterErrorListeners optional

* fix: eslint config setup

* ci(changesets): version packages (rc) (#2462)

Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>

* refactor: improve test-utils defaults

* ci(changesets): version packages (rc) (#2463)

Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>

* fix: eslint config depends on babel preset, define more preconfigured babel preset entry points by environment

* refactor(mc-scripts): adjust webpack dev server error message for /api/graphql route

* ci(changesets): version packages (rc) (#2464)

Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>

* chore: update peer deps min versions

* chore: update lockfile

* Upgrade to Stylelint v14 (#2465)

* chore: upgrade stylelint to v14

* docs: update readme of jest-stylelint-runner

* refactor: remove stylelint support for css-in-js

* fix: lint only css files

* fix(stylelint): ignore some rules

* ci(changesets): version packages (rc) (#2466)

Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>

* refactor(webpack): update css-loader to v6, support both .mod.css and .module.css (#2467)

* refactor(webpack): update css-loader to v6, support both .mod.css and .module.css

* fix(webpack): use correct option

* ci(changesets): version packages (rc) (#2468)

* chore: exit prerelease mode

Co-authored-by: CT Release Bot <24736072+ct-release-bot@users.noreply.github.com>
Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
Co-authored-by: Tobias Deekens <tobias.deekens@commercetools.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants