From b3673efab67f3faae95cc87dcaca2b2068ebe4d6 Mon Sep 17 00:00:00 2001 From: JT Date: Thu, 28 Jan 2021 21:56:06 +0800 Subject: [PATCH 1/4] Add test with strict mode fault --- package.json | 27 ++++++++-------- src/__tests__/index.test.ts | 61 ++++++++++++++++++++++++++++++++++++- 2 files changed, 73 insertions(+), 15 deletions(-) diff --git a/package.json b/package.json index 96a3916..6ccc3c5 100644 --- a/package.json +++ b/package.json @@ -29,7 +29,7 @@ "build": "microbundle build --tsconfig ./tsconfig.json --name VueBrowserAcl", "build:watch": "microbundle watch --tsconfig ./tsconfig.json --name VueBrowserAcl", "test": "jest --config jest.config.json", - "test:watch": "jest jest --config jest.config.json --watchAll", + "test:watch": "jest --config jest.config.json --watchAll", "version": "npm run build && git add -A ./dist", "postversion": "git push && git push --tags" }, @@ -37,19 +37,18 @@ "browser-acl": "^0.9.1" }, "devDependencies": { - "@babel/core": "^7.9.6", - "@babel/preset-env": "^7.9.6", - "@nuxt/types": "^0.7.6", - "@types/jest": "^25.2.3", - "@vue/test-utils": "^1.0.3", - "babel-jest": "^26.0.1", - "jest": "^26.0.1", - "microbundle": "^0.12.0", - "ts-jest": "^26.0.0", - "typescript": "^3.9.3", - "vue": "^2.6.11", - "vue-router": "^3.2.0", - "vue-template-compiler": "^2.6.11" + "@babel/core": "^7.12.10", + "@babel/preset-env": "^7.12.11", + "@types/jest": "^26.0.20", + "@vue/test-utils": "^1.1.2", + "babel-jest": "^26.6.3", + "jest": "^26.6.3", + "microbundle": "^0.13.0", + "ts-jest": "^26.4.4", + "typescript": "^4.1.3", + "vue": "^2.6.12", + "vue-router": "^3.5.1", + "vue-template-compiler": "^2.6.12" }, "prettier": { "semi": false, diff --git a/src/__tests__/index.test.ts b/src/__tests__/index.test.ts index 750f698..222f30a 100644 --- a/src/__tests__/index.test.ts +++ b/src/__tests__/index.test.ts @@ -3,6 +3,7 @@ import { User } from '../../types' import Acl from 'browser-acl' import { Verb } from '../../types' import VueAcl from '../index' +import VueRouter, { RouteConfig } from 'vue-router' let _user: User | null @@ -26,7 +27,7 @@ describe('Bad setup', () => { beforeEach(() => { jest.spyOn(console, 'error') // @ts-ignore - console.error.mockImplementation(() => {}) + console.error.mockImplementation(() => { }) }) afterEach(() => { @@ -54,6 +55,64 @@ describe('Bad setup', () => { }) }) + +const routes: RouteConfig[] = [ + { + path: '*', + component: { + render: (c) => c('div', "Normal Route") + }, + meta: { + can: 'idle' + } + }, + { + path: '/nocanroute', + component: { + render: (c) => c('div', "Shouldn't be here in strict mode") + } + }, + { + path: '/fallback', + component: { + render: (c) => c('div', "Fallback Route") + }, + meta: { + can: true + } + } +] + +describe.only('Router integration', () => { + test('Strict mode', async () => { + const localVue = createLocalVue() + localVue.use(VueRouter) + const router = new VueRouter({ routes: routes }) + router.push(""); + localVue.use(VueAcl, getUser, (acl: Acl) => { + acl.rule('idle', true) + }, { router: router, failRoute: '/fallback', strict: true, assumeGlobal: true }) + + + const wrapper = mount( + { + template: ` + + ` + }, + { localVue, router }, + ) + + expect(wrapper.html()).toContain('Normal Route') + + router.push('/nocanroute') + + await wrapper.vm.$nextTick(); + + expect(wrapper.html()).toContain('Fallback Route') + }) +}) + describe('Global rules', () => { test('True for all', () => { const localVue = createLocalVue() From 9cf19d5cac0878997e7088f495463320ebc6875e Mon Sep 17 00:00:00 2001 From: JT Date: Thu, 28 Jan 2021 22:10:22 +0800 Subject: [PATCH 2/4] Cleanup --- src/__tests__/index.test.ts | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/src/__tests__/index.test.ts b/src/__tests__/index.test.ts index 222f30a..5f7e751 100644 --- a/src/__tests__/index.test.ts +++ b/src/__tests__/index.test.ts @@ -83,17 +83,18 @@ const routes: RouteConfig[] = [ } ] -describe.only('Router integration', () => { +describe('Router integration', () => { test('Strict mode', async () => { const localVue = createLocalVue() localVue.use(VueRouter) + const router = new VueRouter({ routes: routes }) router.push(""); + localVue.use(VueAcl, getUser, (acl: Acl) => { acl.rule('idle', true) }, { router: router, failRoute: '/fallback', strict: true, assumeGlobal: true }) - - + const wrapper = mount( { template: ` From 43f73bea88e9fc4c5bd25c59215e1dac1db1db8f Mon Sep 17 00:00:00 2001 From: JT Date: Sun, 7 Feb 2021 15:00:04 +0800 Subject: [PATCH 3/4] Potential fix for strict mode --- package.json | 8 ++--- src/__tests__/index.test.ts | 63 +++++++++++++++++++++++++++++++------ src/index.ts | 15 +++++++-- 3 files changed, 71 insertions(+), 15 deletions(-) diff --git a/package.json b/package.json index 6ccc3c5..56834b8 100644 --- a/package.json +++ b/package.json @@ -37,14 +37,14 @@ "browser-acl": "^0.9.1" }, "devDependencies": { - "@babel/core": "^7.12.10", - "@babel/preset-env": "^7.12.11", + "@babel/core": "^7.12.13", + "@babel/preset-env": "^7.12.13", "@types/jest": "^26.0.20", - "@vue/test-utils": "^1.1.2", + "@vue/test-utils": "^1.1.3", "babel-jest": "^26.6.3", "jest": "^26.6.3", "microbundle": "^0.13.0", - "ts-jest": "^26.4.4", + "ts-jest": "^26.5.0", "typescript": "^4.1.3", "vue": "^2.6.12", "vue-router": "^3.5.1", diff --git a/src/__tests__/index.test.ts b/src/__tests__/index.test.ts index 5f7e751..5a37d96 100644 --- a/src/__tests__/index.test.ts +++ b/src/__tests__/index.test.ts @@ -58,12 +58,16 @@ describe('Bad setup', () => { const routes: RouteConfig[] = [ { - path: '*', + path: "*", + redirect: '/home' + }, + { + path: '/home', component: { render: (c) => c('div', "Normal Route") }, meta: { - can: 'idle' + can: 'see-home' } }, { @@ -80,19 +84,20 @@ const routes: RouteConfig[] = [ meta: { can: true } + } ] describe('Router integration', () => { - test('Strict mode', async () => { + test('Strict mode directs to fallback route with no meta', async () => { const localVue = createLocalVue() localVue.use(VueRouter) - const router = new VueRouter({ routes: routes }) + const router = new VueRouter({ routes: routes, mode: 'hash' }) + router.push(""); - localVue.use(VueAcl, getUser, (acl: Acl) => { - acl.rule('idle', true) + acl.rule('see-home', true) }, { router: router, failRoute: '/fallback', strict: true, assumeGlobal: true }) const wrapper = mount( @@ -102,15 +107,55 @@ describe('Router integration', () => { ` }, { localVue, router }, - ) + ) + + await wrapper.vm.$nextTick(); expect(wrapper.html()).toContain('Normal Route') - router.push('/nocanroute') + router.push('/nocanroute').catch((err) => { + expect(err).toBe('[Error: Redirected when going from "/home" to "/nocanroute" via a navigation guard.]') + }) await wrapper.vm.$nextTick(); - expect(wrapper.html()).toContain('Fallback Route') + const html = wrapper.html(); + + expect(html).toContain('Fallback Route') + + }), + test('Normal mode passes route with no meta', async () => { + const localVue = createLocalVue() + localVue.use(VueRouter) + + const router = new VueRouter({ routes: routes, mode: 'hash'}) + + router.push(""); + localVue.use(VueAcl, getUser, (acl: Acl) => { + acl.rule('see-home', true) + }, { router: router, failRoute: '/fallback'}) + + const wrapper = mount( + { + template: ` + + ` + }, + { localVue, router }, + ) + + await wrapper.vm.$nextTick(); + + expect(wrapper.html()).toContain('Normal Route') + + router.push('/nocanroute'); + + await wrapper.vm.$nextTick(); + + const html = wrapper.html(); + + expect(html).toContain("Shouldn't be here in strict mode") + }) }) diff --git a/src/index.ts b/src/index.ts index e9ac3c8..0310d83 100644 --- a/src/index.ts +++ b/src/index.ts @@ -147,12 +147,23 @@ const VueAcl: VueAcl = { return chain } + const strictChain = Promise.resolve(false) as PromiseChain; + strictChain.getFail = () => null; + router.beforeEach((to: Route, from: Route, next: any) => { + //prevent infinite loop if the fallback route contains no can meta + if(opt.strict && to.path === opt.failRoute) { + return next(); + } + const metas = to.matched .filter((route) => route.meta && findCan(route.meta)) .map((route) => route.meta) - - const chain = chainCans(metas, to, from) + + // enforce strict mode + const chain = (metas.length === 0 && opt.strict) + ? strictChain + : chainCans(metas, to, from) chain.then((result) => { if (result === true) { From e7de604d9561e9ad5d5ed1555716b962f41b60d3 Mon Sep 17 00:00:00 2001 From: JT Date: Sun, 7 Feb 2021 15:40:23 +0800 Subject: [PATCH 4/4] Short circuit instead --- src/index.ts | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-) diff --git a/src/index.ts b/src/index.ts index 0310d83..c5c58c8 100644 --- a/src/index.ts +++ b/src/index.ts @@ -147,9 +147,6 @@ const VueAcl: VueAcl = { return chain } - const strictChain = Promise.resolve(false) as PromiseChain; - strictChain.getFail = () => null; - router.beforeEach((to: Route, from: Route, next: any) => { //prevent infinite loop if the fallback route contains no can meta if(opt.strict && to.path === opt.failRoute) { @@ -161,9 +158,11 @@ const VueAcl: VueAcl = { .map((route) => route.meta) // enforce strict mode - const chain = (metas.length === 0 && opt.strict) - ? strictChain - : chainCans(metas, to, from) + if(metas.length === 0 && opt.strict) { + next(opt.failRoute) + } + + const chain = chainCans(metas, to, from) chain.then((result) => { if (result === true) {