Skip to content

Commit

Permalink
fix issue where query-params get replicated (for example oidc-topic) …
Browse files Browse the repository at this point in the history
…after logout or in the buildUrl helper, allowed params have to be whitelisted now.
  • Loading branch information
fschade committed Mar 23, 2023
1 parent 61d3e50 commit 7f35d0d
Show file tree
Hide file tree
Showing 4 changed files with 23 additions and 7 deletions.
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
Bugfix: Broken re-login after logout fails
Bugfix: Broken re-login after logout

After a user logged out, it was no longer possible to login without reloading the ocis root domain,
this has now been fixed and only whitelisted query-params are taken into account in the logout url-replace.
this has now been fixed and only allowed query-params are taken into account.

https://github.com/owncloud/web/pull/8694
16 changes: 15 additions & 1 deletion packages/web-runtime/src/router/helpers.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import { base, router } from './index'
import { RouteLocation, RouteParams, Router, RouteRecordNormalized } from 'vue-router'
import pick from 'lodash-es/pick'
import {
AuthContext,
authContextValues,
Expand All @@ -10,7 +11,20 @@ import {
export const buildUrl = (pathname) => {
const isHistoryMode = !!base
const baseUrl = new URL(window.location.href.split('#')[0])
baseUrl.search = window.location.search

// searchParams defines a set of search parameters which should be part of new url.
// The resulting querystring only contains the properties listed here.
const searchParams = new URLSearchParams(
pick(Object.fromEntries(new URLSearchParams(window.location.search).entries()), [
// needed for private links
'details'
])
).toString()

baseUrl.search = searchParams
console.log('###')
console.log(baseUrl.search)
console.log('###')
if (isHistoryMode) {
// in history mode we can't determine the base path, it must be provided by the document
baseUrl.pathname = new URL(base.href).pathname
Expand Down
7 changes: 3 additions & 4 deletions packages/web-runtime/src/services/auth/authService.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import { extractPublicLinkToken, isPublicLinkContext, isUserContext } from '../.
import { unref } from 'vue'
import { Ability } from 'web-pkg/src/utils'
import { Language } from 'vue3-gettext'
import pick from 'lodash-es/pick'

export class AuthService {
private clientService: ClientService
Expand Down Expand Up @@ -175,12 +176,10 @@ export class AuthService {

// transportQuery defines a set of query parameters which should be part of router route replace.
// The resulting object only contains the properties listed here.
const transportQuery = [
const transportQuery = pick(currentQuery, [
// needed for private links
'details'
]
.filter((key) => key in currentQuery)
.reduce((acc, key) => ((acc[key] = currentQuery[key]), acc), {})
])

return this.router.replace({
path: redirectUrl,
Expand Down
3 changes: 3 additions & 0 deletions packages/web-runtime/tests/unit/router/index.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,9 @@ describe('buildUrl', () => {
${'https://localhost:9200/files/list/all'} | ${'/foo'} | ${'/login/foo'} | ${'https://localhost:9200/foo/login/foo'}
${'https://localhost:9200/files/list/all'} | ${'/'} | ${'/bar.html'} | ${'https://localhost:9200/bar.html'}
${'https://localhost:9200/files/list/all'} | ${'/foo'} | ${'/bar.html'} | ${'https://localhost:9200/foo/bar.html'}
${'https://localhost:9200/files/list/all?details=91953740'} | ${'/foo'} | ${'/bar.html'} | ${'https://localhost:9200/foo/bar.html?details=91953740'}
${'https://localhost:9200/files/list/all?details=91953740&unknown=87987987'} | ${'/foo'} | ${'/bar.html'} | ${'https://localhost:9200/foo/bar.html?details=91953740'}
${'https://localhost:9200/files/list/all?unknown=87987987'} | ${'/foo'} | ${'/bar.html'} | ${'https://localhost:9200/foo/bar.html'}
`('$path -> $expected', async ({ location, base, path, expected }) => {
delete window.location
window.location = new URL(location) as any
Expand Down

0 comments on commit 7f35d0d

Please sign in to comment.