Skip to content

Commit 2442f6b

Browse files
susnuxsusnux
committed
fix(theming): Ensure to only send valid URLs to backend
Signed-off-by: Ferdinand Thiessen <opensource@fthiessen.de>
1 parent 931ad03 commit 2442f6b

File tree

2 files changed

+178
-4
lines changed

2 files changed

+178
-4
lines changed

‎apps/theming/src/mixins/admin/TextValueMixin.js

+35-4
Original file line numberDiff line numberDiff line change
@@ -21,25 +21,56 @@ export default {
2121

2222
data() {
2323
return {
24+
/** @type {string|boolean} */
2425
localValue: this.value,
2526
}
2627
},
2728

29+
computed: {
30+
valueToPost() {
31+
if (this.type === 'url') {
32+
// if this is already encoded just make sure there is no doublequote (HTML XSS)
33+
// otherwise simply URL encode
34+
return this.isUrlEncoded(this.localValue)
35+
? this.localValue.replaceAll('"', '%22')
36+
: encodeURI(this.localValue)
37+
}
38+
// Convert boolean to string as server expects string value
39+
if (typeof this.localValue === 'boolean') {
40+
return this.localValue ? 'yes' : 'no'
41+
}
42+
return this.localValue
43+
},
44+
},
45+
2846
methods: {
47+
/**
48+
* Check if URL is percent-encoded
49+
* @param {string} url The URL to check
50+
* @return {boolean}
51+
*/
52+
isUrlEncoded(url) {
53+
try {
54+
return decodeURI(url) !== url
55+
} catch {
56+
return false
57+
}
58+
},
59+
2960
async save() {
3061
this.reset()
3162
const url = generateUrl('/apps/theming/ajax/updateStylesheet')
32-
// Convert boolean to string as server expects string value
33-
const valueToPost = this.localValue === true ? 'yes' : this.localValue === false ? 'no' : this.localValue
63+
3464
try {
3565
await axios.post(url, {
3666
setting: this.name,
37-
value: valueToPost,
67+
value: this.valueToPost,
3868
})
3969
this.$emit('update:value', this.localValue)
4070
this.handleSuccess()
4171
} catch (e) {
42-
this.errorMessage = e.response.data.data?.message
72+
console.error('Failed to save changes', e)
73+
this.errorMessage = e.response?.data.data?.message
4374
}
4475
},
4576

‎cypress/e2e/theming/admin-settings_urls.cy.ts

+143
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,143 @@
1+
/*!
2+
* SPDX-FileCopyrightText: 2025 Nextcloud GmbH and Nextcloud contributors
3+
* SPDX-License-Identifier: AGPL-3.0-or-later
4+
*/
5+
import { User } from '@nextcloud/cypress'
6+
7+
const admin = new User('admin', 'admin')
8+
9+
describe('Admin theming: Setting custom project URLs', function() {
10+
this.beforeEach(() => {
11+
// Just in case previous test failed
12+
cy.resetAdminTheming()
13+
cy.login(admin)
14+
cy.visit('/settings/admin/theming')
15+
cy.intercept('POST', '**/apps/theming/ajax/updateStylesheet').as('updateTheming')
16+
})
17+
18+
it('Setting the web link', () => {
19+
cy.findByRole('textbox', { name: /web link/i })
20+
.and('have.attr', 'type', 'url')
21+
.as('input')
22+
.scrollIntoView()
23+
cy.get('@input')
24+
.should('be.visible')
25+
.type('{selectAll}http://example.com/path?query#fragment{enter}')
26+
27+
cy.wait('@updateTheming')
28+
29+
cy.logout()
30+
31+
cy.visit('/')
32+
cy.contains('a', 'Nextcloud')
33+
.should('be.visible')
34+
.and('have.attr', 'href', 'http://example.com/path?query#fragment')
35+
})
36+
37+
it('Setting the legal notice link', () => {
38+
cy.findByRole('textbox', { name: /legal notice link/i })
39+
.should('exist')
40+
.and('have.attr', 'type', 'url')
41+
.as('input')
42+
.scrollIntoView()
43+
cy.get('@input')
44+
.type('http://example.com/path?query#fragment{enter}')
45+
46+
cy.wait('@updateTheming')
47+
48+
cy.logout()
49+
50+
cy.visit('/')
51+
cy.contains('a', /legal notice/i)
52+
.should('be.visible')
53+
.and('have.attr', 'href', 'http://example.com/path?query#fragment')
54+
})
55+
56+
it('Setting the privacy policy link', () => {
57+
cy.findByRole('textbox', { name: /privacy policy link/i })
58+
.should('exist')
59+
.as('input')
60+
.scrollIntoView()
61+
cy.get('@input')
62+
.should('have.attr', 'type', 'url')
63+
.type('http://privacy.local/path?query#fragment{enter}')
64+
65+
cy.wait('@updateTheming')
66+
67+
cy.logout()
68+
69+
cy.visit('/')
70+
cy.contains('a', /privacy policy/i)
71+
.should('be.visible')
72+
.and('have.attr', 'href', 'http://privacy.local/path?query#fragment')
73+
})
74+
75+
})
76+
77+
describe('Admin theming: Web link corner cases', function() {
78+
this.beforeEach(() => {
79+
// Just in case previous test failed
80+
cy.resetAdminTheming()
81+
cy.login(admin)
82+
cy.visit('/settings/admin/theming')
83+
cy.intercept('POST', '**/apps/theming/ajax/updateStylesheet').as('updateTheming')
84+
})
85+
86+
it('Already URL encoded', () => {
87+
cy.findByRole('textbox', { name: /web link/i })
88+
.and('have.attr', 'type', 'url')
89+
.as('input')
90+
.scrollIntoView()
91+
cy.get('@input')
92+
.should('be.visible')
93+
.type('{selectAll}http://example.com/%22path%20with%20space%22{enter}')
94+
95+
cy.wait('@updateTheming')
96+
97+
cy.logout()
98+
99+
cy.visit('/')
100+
cy.contains('a', 'Nextcloud')
101+
.should('be.visible')
102+
.and('have.attr', 'href', 'http://example.com/%22path%20with%20space%22')
103+
})
104+
105+
it('URL with double quotes', () => {
106+
cy.findByRole('textbox', { name: /web link/i })
107+
.and('have.attr', 'type', 'url')
108+
.as('input')
109+
.scrollIntoView()
110+
cy.get('@input')
111+
.should('be.visible')
112+
.type('{selectAll}http://example.com/"path"{enter}')
113+
114+
cy.wait('@updateTheming')
115+
116+
cy.logout()
117+
118+
cy.visit('/')
119+
cy.contains('a', 'Nextcloud')
120+
.should('be.visible')
121+
.and('have.attr', 'href', 'http://example.com/%22path%22')
122+
})
123+
124+
it('URL with double quotes and already encoded', () => {
125+
cy.findByRole('textbox', { name: /web link/i })
126+
.and('have.attr', 'type', 'url')
127+
.as('input')
128+
.scrollIntoView()
129+
cy.get('@input')
130+
.should('be.visible')
131+
.type('{selectAll}http://example.com/"the%20path"{enter}')
132+
133+
cy.wait('@updateTheming')
134+
135+
cy.logout()
136+
137+
cy.visit('/')
138+
cy.contains('a', 'Nextcloud')
139+
.should('be.visible')
140+
.and('have.attr', 'href', 'http://example.com/%22the%20path%22')
141+
})
142+
143+
})

0 commit comments

Comments
 (0)