fix: redirect redirect/returnLink with query

Previously we always wrap the redirect link with `{ path }`, causing query string lost when performing the redirect. Support for `returnLink` in the URL query are also removed since they are not used anymore
Kong · Nov 27, 2023 · 6bd5686 · 6bd5686
1 parent 5d3aae3
commit 6bd5686
Show file tree

Hide file tree

Showing 11 changed files with 93 additions and 58 deletions.
diff --git a/src/components/EntityForm/EntityForm.vue b/src/components/EntityForm/EntityForm.vue
@@ -345,7 +345,7 @@ export default {
       if (this.onFormCancel) {
         this.onFormCancel()
       } else if (this.redirectPath) {
-        this.$router.push({ path: this.redirectPath })
+        this.$router.push(this.redirectPath)
       } else if (this.$router.previous) {
         this.$router.push({ ...this.$router.previous })
       } else {

diff --git a/src/components/EntityForm/NativeEntityForm.vue b/src/components/EntityForm/NativeEntityForm.vue
@@ -311,7 +311,7 @@ export default {
       if (this.onFormCancel) {
         return this.onFormCancel()
       } else if (this.redirectPath) {
-        return this.$router.push({ path: this.redirectPath })
+        return this.$router.push(this.redirectPath)
       } else if (this.$router.previous) {
         return this.$router.push({ ...this.$router.previous })
       } else {

diff --git a/src/components/EntityForm/mixins/FormPage.js b/src/components/EntityForm/mixins/FormPage.js
@@ -41,7 +41,7 @@ export default {
      */
     redirectRoute () {
       if (this.redirectPath) {
-        return { path: this.redirectPath }
+        return this.redirectPath
       }
 
       return { name: this.resourceEndpoint }
@@ -92,10 +92,10 @@ export default {
             return redirectCreateRoute === '-1' ? this.$router.go(-1) : this.$router.push({ name: redirectCreateRoute, params: this.$router.params })
           }
 
-          const link = this.redirectPath || this.returnLink || this.$route.query.returnLink
+          const link = this.redirectPath || this.returnLink
 
           if (link) {
-            return this.$router.push({ path: link })
+            return this.$router.push(link)
           }
 
           const location = typeof callback === 'function'
@@ -111,7 +111,7 @@ export default {
     async updateRecord (model) {
       return apiService.updateRecord(this.resourceEndpoint, this.id, model)
         .then(res => {
-          const link = this.redirectPath || this.returnLink || this.$route.query.returnLink || this.$route.query.redirect
+          const link = this.redirectPath || this.returnLink
 
           // if parent form defines a redirectRouteNames object, we either go back to previous page if -1 is passed
           // else we go to the named route
@@ -121,7 +121,7 @@ export default {
           }
 
           if (link) {
-            return this.$router.push({ path: link })
+            return this.$router.push(link)
           }
 
           redirectOnResponseStatus(this.$router, 200, { name: this.resourceEndpoint })(res)

diff --git a/src/components/EntityForm/mixins/RedirectMixin.js b/src/components/EntityForm/mixins/RedirectMixin.js
@@ -1,24 +1,42 @@
+const extractRedirectPath = ($route, urlKey) => {
+  const query = $route.query[urlKey]
+  let url = null
+
+  if (Array.isArray(query)) {
+    url = query[0]
+    console.warn(`extractRedirectPath: "${urlKey}" in query should not be an array, using first element this time`)
+  } else if (typeof query === 'string') {
+    url = query
+  }
+
+  return url
+}
+
 export default {
   computed: {
     redirectPath () {
-      return this.$route.query.redirect
+      return extractRedirectPath(this.$route, 'redirect')
     },
     redirectRouteQuery () {
       return { redirect: this.redirectPath }
     },
     postDeletePath () {
-      return this.$route.query.postDelete
+      return extractRedirectPath(this.$route, 'postDelete')
     },
     postDeleteRouteQuery () {
       return { postDelete: this.postDeletePath }
     },
   },
   methods: {
-    createRedirectRouteQuery (redirectPath = this.$route.fullPath) {
-      return { redirect: redirectPath }
+    createRedirectRouteQuery (redirect = this.$route.fullPath) {
+      return { redirect }
     },
-    createPostDeleteRouteQuery (postDeletePath = this.postDeletePath || this.$route.fullPath) {
-      return { postDelete: postDeletePath }
+    createPostDeleteRouteQuery (postDelete) {
+      if (typeof postDelete !== 'string') {
+        postDelete = this.postDeletePath ?? this.$route.fullPath
+      }
+
+      return { postDelete }
     },
     redirect (replace = false, goBack = false) {
       const routerFn = replace ? this.$router.replace : this.$router.push

diff --git a/src/components/HeaderBackButton.vue b/src/components/HeaderBackButton.vue
@@ -1,16 +1,18 @@
 <template>
   <KButton
     data-testid="header-back-button"
-    :to="backPath"
+    @click="back"
   >
     {{ t('global.buttons.back') }}
   </KButton>
 </template>
 
 <script setup lang="ts">
-import { computed } from 'vue'
-import { useRoute } from 'vue-router'
+import { useRouter } from 'vue-router'
+import type { RouteLocationRaw } from 'vue-router'
+
 import { useI18n } from '@/composables/useI18n'
+import { useURLFromRouteQuery } from '@/composables/useRedirect'
 
 const props = defineProps({
   entity: {
@@ -20,19 +22,19 @@ const props = defineProps({
   },
 })
 
-const route = useRoute()
 const { t } = useI18n()
+const router = useRouter()
+const redirectURL = useURLFromRouteQuery('redirect')
 
-const backPath = computed(() => {
-  const { query } = route
-  if (query?.redirect) {
-    return {
-      path: query.redirect,
-    }
+const back = () => {
+  let backTo: RouteLocationRaw = {
+    name: props.entity ? `${props.entity}-list` : 'overview',
   }
 
-  return {
-    name: props.entity ? `${props.entity}-list` : 'overview',
+  if (redirectURL.value) {
+    backTo = redirectURL.value
   }
-})
+
+  router.replace(backTo)
+}
 </script>
diff --git a/src/composables/useFormRedirect.ts b/src/composables/useFormRedirect.ts
@@ -1,18 +1,21 @@
 import { computed } from 'vue'
-import { useRoute, useRouter, type RouteLocationRaw, type MatcherLocationAsPath } from 'vue-router'
+import { useRouter } from 'vue-router'
+import type { RouteLocationRaw } from 'vue-router'
+
+import { useURLFromRouteQuery } from './useRedirect'
 
 export const useFormRedirectOnCancel = (fallback: RouteLocationRaw) => {
-  const route = useRoute()
   const router = useRouter()
-  const redirectPath = computed(() => route.query.redirect)
+  const redirectRouteLocation = useURLFromRouteQuery('redirect')
+
   const routeOnCancel = computed(() => {
-    if (redirectPath.value) {
-      return { path: redirectPath.value } as MatcherLocationAsPath
+    if (redirectRouteLocation.value) {
+      return redirectRouteLocation.value
     }
 
     const previousPath = router.options?.history?.state?.back
-    if (previousPath) {
-      return { path: previousPath } as MatcherLocationAsPath
+    if (typeof previousPath === 'string') {
+      return router.resolve(previousPath)
     }
 
     return fallback
@@ -22,13 +25,14 @@ export const useFormRedirectOnCancel = (fallback: RouteLocationRaw) => {
 }
 
 export const useFormRedirectOnUpdate = (fallback: RouteLocationRaw) => {
-  const route = useRoute()
-  const redirectPath = computed(() => route.query.redirect)
+  const redirectRouteLocation = useURLFromRouteQuery('redirect')
 
   const routeOnUpdate = computed(() => {
-    const link = redirectPath.value || route.query.returnLink || route.query.redirect
+    if (redirectRouteLocation.value) {
+      return redirectRouteLocation.value
+    }
 
-    return link ? ({ path: link } as MatcherLocationAsPath) : fallback
+    return fallback
   })
 
   return routeOnUpdate.value

diff --git a/src/composables/useListRedirect.ts b/src/composables/useListRedirect.ts
@@ -1,12 +1,13 @@
 import { computed } from 'vue'
 import { useRoute, useRouter } from 'vue-router'
-import type { LocationQueryValue } from 'vue-router'
+
+import { useURLFromRouteQuery } from './useRedirect'
 
 export const useListRedirect = () => {
   const route = useRoute()
   const router = useRouter()
 
-  const redirectPath = computed(() => route.query.redirect as LocationQueryValue)
+  const redirectPath = useURLFromRouteQuery('redirect')
   const redirectRouteQuery = computed(() => ({ redirect: redirectPath.value }))
 
   const createRedirectRouteQuery = (redirect = route.fullPath) => {

diff --git a/src/composables/useRedirect.ts b/src/composables/useRedirect.ts
@@ -0,0 +1,21 @@
+import { computed } from 'vue'
+import { useRoute } from 'vue-router'
+import type { LocationQueryValue } from 'vue-router'
+
+export const useURLFromRouteQuery = (urlKey: string) => {
+  const route = useRoute()
+
+  return computed(() => {
+    const query = route.query[urlKey]
+    let url: LocationQueryValue = null
+
+    if (Array.isArray(query)) {
+      url = query[0]
+      console.warn(`useURLFromRouteQuery: "${urlKey}" in query should not be an array, using first element this time`)
+    } else if (typeof query === 'string') {
+      url = query
+    }
+
+    return url
+  })
+}
diff --git a/src/pages/consumers/ConsumerCredentials.vue b/src/pages/consumers/ConsumerCredentials.vue
@@ -30,10 +30,12 @@ import { pluginMeta } from '@/pages/plugins/PluginMeta'
 import { useI18n } from '@/composables/useI18n'
 import { useInfoStore } from '@/stores/info'
 import CredentialList from './CredentialList.vue'
+import { useListRedirect } from '@/composables/useListRedirect'
 
 const router = useRouter()
 const { t } = useI18n()
 const infoStore = useInfoStore()
+const { createRedirectRouteQuery } = useListRedirect()
 
 const enabledPlugins = computed(() => infoStore.plugins.enabledInCluster)
 const enabledPluginsFetched = ref(false)
@@ -58,9 +60,7 @@ const hasEnabledPlugins = computed(() => credentialPlugins.some(({ pluginType })
 const navigateToPluginSelection = () => {
   router.push({
     name: 'plugin-select',
-    query: {
-      redirect: router.currentRoute.value.fullPath,
-    },
+    query: createRedirectRouteQuery(),
   })
 }
 

diff --git a/src/pages/plugins/Form.vue b/src/pages/plugins/Form.vue
@@ -113,11 +113,6 @@ export default {
       type: String,
       default: null,
     },
-
-    returnLinkPath: {
-      type: String,
-      default: null,
-    },
     useKonnectSchema: {
       type: Boolean,
       default: false,
@@ -298,18 +293,14 @@ export default {
         return this.redirectPath
       }
 
-      if (this.returnLinkPath) {
-        return this.returnLinkPath
-      }
-
       const entityId = this.resourceEndpoint.match(new RegExp(uuidRegEx))
 
       if (entityId && this.$route.query.entity_id) {
         // e.g. /consumers/e22cf8e2-795a-4eac-8da1-76b47de30b5c
         return `/${this.resourceEndpoint.split('/')[0]}/${entityId}`
       }
 
-      return this.$route.query.returnLink
+      return null
     },
 
     docsLink () {
@@ -440,9 +431,7 @@ export default {
   methods: {
     onCancel () {
       if (this.redirectPath) {
-        this.$router.push({ path: this.redirectPath })
-      } else if (this.returnLinkPath) {
-        this.$router.push({ path: this.returnLinkPath })
+        this.$router.push(this.redirectPath)
       } else if (this.$router.previous) {
         this.$router.push({ ...this.$router.previous })
       } else {
@@ -489,7 +478,7 @@ export default {
           }
 
           this.returnLink
-            ? this.$router.push({ path: this.returnLink })
+            ? this.$router.push(this.returnLink)
             : redirectOnResponseStatus(this.$router, 201, { name: 'plugin-list' })(res)
 
           return res.data
@@ -536,7 +525,7 @@ export default {
           }
 
           this.returnLink
-            ? this.$router.push({ path: this.returnLink })
+            ? this.$router.push(this.returnLink)
             : redirectOnResponseStatus(this.$router, 200, { name: 'plugin-detail', params: { id: this.id } })(res)
 
           return res.data

diff --git a/tests/playwright/specs/services/01-Service.spec.ts b/tests/playwright/specs/services/01-Service.spec.ts
@@ -107,7 +107,7 @@ test.describe('services', () => {
     await withNavigation(page, () =>
       page
         .locator('.page-header')
-        .locator('a[type="button"]:has-text("Back")')
+        .locator('[type="button"]:has-text("Back")')
         .click()
     )
     await page.waitForSelector('.kong-ui-entities-gateway-services-list')