Skip to content

Commit f61636b

Browse files
Merge pull request #51578 from nextcloud/backport/51521/stable31
[stable31] fix(webauthn): adjust for updated library and add tests
2 parents a988387 + 6c371c7 commit f61636b

File tree

19 files changed

+358
-169
lines changed

19 files changed

+358
-169
lines changed

.eslintrc.js

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -35,6 +35,9 @@ module.exports = {
3535
jsdoc: {
3636
mode: 'typescript',
3737
},
38+
'import/resolver': {
39+
typescript: {}, // this loads <rootdir>/tsconfig.json to eslint
40+
},
3841
},
3942
overrides: [
4043
// Allow any in tests
@@ -43,6 +46,6 @@ module.exports = {
4346
rules: {
4447
'@typescript-eslint/no-explicit-any': 'warn',
4548
},
46-
}
49+
},
4750
],
4851
}

apps/settings/lib/Settings/Personal/Security/WebAuthn.php

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -40,8 +40,7 @@ public function getForm() {
4040
$this->mapper->findAllForUid($this->userId)
4141
);
4242

43-
return new TemplateResponse('settings', 'settings/personal/security/webauthn', [
44-
]);
43+
return new TemplateResponse('settings', 'settings/personal/security/webauthn');
4544
}
4645

4746
public function getSection(): ?string {

apps/settings/lib/SetupChecks/PhpModules.php

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -32,7 +32,6 @@ class PhpModules implements ISetupCheck {
3232
'zlib',
3333
];
3434
protected const RECOMMENDED_MODULES = [
35-
'bcmath',
3635
'exif',
3736
'gmp',
3837
'intl',
@@ -58,8 +57,7 @@ protected function getRecommendedModuleDescription(string $module): string {
5857
return match($module) {
5958
'intl' => $this->l10n->t('increases language translation performance and fixes sorting of non-ASCII characters'),
6059
'sodium' => $this->l10n->t('for Argon2 for password hashing'),
61-
'bcmath' => $this->l10n->t('for WebAuthn passwordless login'),
62-
'gmp' => $this->l10n->t('for WebAuthn passwordless login, and SFTP storage'),
60+
'gmp' => $this->l10n->t('required for SFTP storage and recommended for WebAuthn performance'),
6361
'exif' => $this->l10n->t('for picture rotation in server and metadata extraction in the Photos app'),
6462
default => '',
6563
};

apps/settings/src/service/WebAuthnRegistrationSerice.ts

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@
33
* SPDX-License-Identifier: AGPL-3.0-or-later
44
*/
55

6-
import type { RegistrationResponseJSON } from '@simplewebauthn/types'
6+
import type { PublicKeyCredentialCreationOptionsJSON, RegistrationResponseJSON } from '@simplewebauthn/types'
77

88
import { translate as t } from '@nextcloud/l10n'
99
import { generateUrl } from '@nextcloud/router'
@@ -21,9 +21,9 @@ export async function startRegistration() {
2121

2222
try {
2323
logger.debug('Fetching webauthn registration data')
24-
const { data } = await axios.get(url)
24+
const { data } = await axios.get<PublicKeyCredentialCreationOptionsJSON>(url)
2525
logger.debug('Start webauthn registration')
26-
const attrs = await registerWebAuthn(data)
26+
const attrs = await registerWebAuthn({ optionsJSON: data })
2727
return attrs
2828
} catch (e) {
2929
logger.error(e as Error)

core/src/components/login/PasswordLessLoginForm.vue

Lines changed: 32 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -5,24 +5,27 @@
55
<template>
66
<form v-if="(isHttps || isLocalhost) && supportsWebauthn"
77
ref="loginForm"
8+
aria-labelledby="password-less-login-form-title"
9+
class="password-less-login-form"
810
method="post"
911
name="login"
1012
@submit.prevent="submit">
11-
<h2>{{ t('core', 'Log in with a device') }}</h2>
12-
<fieldset>
13-
<NcTextField required
14-
:value="user"
15-
:autocomplete="autoCompleteAllowed ? 'on' : 'off'"
16-
:error="!validCredentials"
17-
:label="t('core', 'Login or email')"
18-
:placeholder="t('core', 'Login or email')"
19-
:helper-text="!validCredentials ? t('core', 'Your account is not setup for passwordless login.') : ''"
20-
@update:value="changeUsername" />
13+
<h2 id="password-less-login-form-title">
14+
{{ t('core', 'Log in with a device') }}
15+
</h2>
2116

22-
<LoginButton v-if="validCredentials"
23-
:loading="loading"
24-
@click="authenticate" />
25-
</fieldset>
17+
<NcTextField required
18+
:value="user"
19+
:autocomplete="autoCompleteAllowed ? 'on' : 'off'"
20+
:error="!validCredentials"
21+
:label="t('core', 'Login or email')"
22+
:placeholder="t('core', 'Login or email')"
23+
:helper-text="!validCredentials ? t('core', 'Your account is not setup for passwordless login.') : ''"
24+
@update:value="changeUsername" />
25+
26+
<LoginButton v-if="validCredentials"
27+
:loading="loading"
28+
@click="authenticate" />
2629
</form>
2730
<div v-else-if="!supportsWebauthn" class="update">
2831
<InformationIcon size="70" />
@@ -40,9 +43,11 @@
4043
</div>
4144
</template>
4245

43-
<script>
46+
<script type="ts">
4447
import { browserSupportsWebAuthn } from '@simplewebauthn/browser'
48+
import { defineComponent } from 'vue'
4549
import {
50+
NoValidCredentials,
4651
startAuthentication,
4752
finishAuthentication,
4853
} from '../../services/WebAuthnAuthenticationService.ts'
@@ -52,7 +57,7 @@ import LockOpenIcon from 'vue-material-design-icons/LockOpen.vue'
5257
import NcTextField from '@nextcloud/vue/dist/Components/NcTextField.js'
5358
import logger from '../../logger'
5459
55-
export default {
60+
export default defineComponent({
5661
name: 'PasswordLessLoginForm',
5762
components: {
5863
LoginButton,
@@ -138,21 +143,21 @@ export default {
138143
// noop
139144
},
140145
},
141-
}
146+
})
142147
</script>
143148

144149
<style lang="scss" scoped>
145-
fieldset {
146-
display: flex;
147-
flex-direction: column;
148-
gap: 0.5rem;
150+
.password-less-login-form {
151+
display: flex;
152+
flex-direction: column;
153+
gap: 0.5rem;
149154
150-
:deep(label) {
151-
text-align: initial;
152-
}
155+
:deep(label) {
156+
text-align: initial;
153157
}
158+
}
154159
155-
.update {
156-
margin: 0 auto;
157-
}
160+
.update {
161+
margin: 0 auto;
162+
}
158163
</style>

core/src/services/WebAuthnAuthenticationService.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,7 @@ export async function startAuthentication(loginName: string) {
2727
logger.error('No valid credentials returned for webauthn')
2828
throw new NoValidCredentials()
2929
}
30-
return await startWebauthnAuthentication(data)
30+
return await startWebauthnAuthentication({ optionsJSON: data })
3131
}
3232

3333
/**

cypress/dockerNode.ts

Lines changed: 12 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -88,6 +88,12 @@ export const startNextcloud = async function(branch: string = getCurrentGitBranc
8888
Type: 'tmpfs',
8989
ReadOnly: false,
9090
}],
91+
PortBindings: {
92+
'80/tcp': [{
93+
HostIP: '0.0.0.0',
94+
HostPort: '8083',
95+
}],
96+
},
9197
},
9298
Env: [
9399
`BRANCH=${branch}`,
@@ -242,11 +248,15 @@ export const getContainerIP = async function(
242248
while (ip === '' && tries < 10) {
243249
tries++
244250

245-
await container.inspect(function(err, data) {
251+
container.inspect(function(err, data) {
246252
if (err) {
247253
throw err
248254
}
249-
ip = data?.NetworkSettings?.IPAddress || ''
255+
if (data?.HostConfig.PortBindings?.['80/tcp']?.[0]?.HostPort) {
256+
ip = `localhost:${data.HostConfig.PortBindings['80/tcp'][0].HostPort}`
257+
} else {
258+
ip = data?.NetworkSettings?.IPAddress || ''
259+
}
250260
})
251261

252262
if (ip !== '') {

cypress/e2e/files/LivePhotosUtils.ts

Lines changed: 16 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -14,34 +14,25 @@ type SetupInfo = {
1414
}
1515

1616
/**
17-
*
18-
* @param user
19-
* @param fileName
20-
* @param domain
21-
* @param requesttoken
22-
* @param metadata
2317
*/
2418
function setMetadata(user: User, fileName: string, requesttoken: string, metadata: object) {
25-
cy.url().then(url => {
26-
const hostname = new URL(url).hostname
27-
cy.request({
28-
method: 'PROPPATCH',
29-
url: `http://${hostname}/remote.php/dav/files/${user.userId}/${fileName}`,
30-
auth: { user: user.userId, pass: user.password },
31-
headers: {
32-
requesttoken,
33-
},
34-
body: `<?xml version="1.0"?>
35-
<d:propertyupdate xmlns:d="DAV:" xmlns:nc="http://nextcloud.org/ns">
36-
<d:set>
37-
<d:prop>
38-
${Object.entries(metadata).map(([key, value]) => `<${key}>${value}</${key}>`).join('\n')}
39-
</d:prop>
40-
</d:set>
41-
</d:propertyupdate>`,
42-
})
19+
const base = Cypress.config('baseUrl')!.replace(/\/index\.php\/?/, '')
20+
cy.request({
21+
method: 'PROPPATCH',
22+
url: `${base}/remote.php/dav/files/${user.userId}/${fileName}`,
23+
auth: { user: user.userId, pass: user.password },
24+
headers: {
25+
requesttoken,
26+
},
27+
body: `<?xml version="1.0"?>
28+
<d:propertyupdate xmlns:d="DAV:" xmlns:nc="http://nextcloud.org/ns">
29+
<d:set>
30+
<d:prop>
31+
${Object.entries(metadata).map(([key, value]) => `<${key}>${value}</${key}>`).join('\n')}
32+
</d:prop>
33+
</d:set>
34+
</d:propertyupdate>`,
4335
})
44-
4536
}
4637

4738
/**

cypress/e2e/files_external/files-user-credentials.cy.ts

Lines changed: 38 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -15,15 +15,16 @@ describe('Files user credentials', { testIsolation: true }, () => {
1515
let user2: User
1616
let storageUser: User
1717

18-
beforeEach(() => {
19-
})
20-
2118
before(() => {
2219
cy.runOccCommand('app:enable files_external')
2320

2421
// Create some users
25-
cy.createRandomUser().then((user) => user1 = user)
26-
cy.createRandomUser().then((user) => user2 = user)
22+
cy.createRandomUser().then((user) => {
23+
user1 = user
24+
})
25+
cy.createRandomUser().then((user) => {
26+
user2 = user
27+
})
2728

2829
// This user will hold the webdav storage
2930
cy.createRandomUser().then((user) => {
@@ -34,7 +35,7 @@ describe('Files user credentials', { testIsolation: true }, () => {
3435

3536
after(() => {
3637
// Cleanup global storages
37-
cy.runOccCommand(`files_external:list --output=json`).then(({stdout}) => {
38+
cy.runOccCommand('files_external:list --output=json').then(({ stdout }) => {
3839
const list = JSON.parse(stdout)
3940
list.forEach((storage) => cy.runOccCommand(`files_external:delete --yes ${storage.mount_id}`), { failOnNonZeroExit: false })
4041
})
@@ -43,8 +44,10 @@ describe('Files user credentials', { testIsolation: true }, () => {
4344
})
4445

4546
it('Create a user storage with user credentials', () => {
46-
const url = Cypress.config('baseUrl') + '/remote.php/dav/files/' + storageUser.userId
47-
createStorageWithConfig(storageUser.userId, StorageBackend.DAV, AuthBackend.UserProvided, { host: url.replace('index.php/', ''), 'secure': 'false' })
47+
// Its not the public server address but the address so the server itself can connect to it
48+
const base = 'http://localhost'
49+
const host = `${base}/remote.php/dav/files/${storageUser.userId}`
50+
createStorageWithConfig(storageUser.userId, StorageBackend.DAV, AuthBackend.UserProvided, { host, secure: 'false' })
4851

4952
cy.login(user1)
5053
cy.visit('/apps/files/extstoragemounts')
@@ -72,6 +75,7 @@ describe('Files user credentials', { testIsolation: true }, () => {
7275

7376
// Auth dialog should be closed and the set credentials button should be gone
7477
authDialog.should('not.exist', { timeout: 2000 })
78+
7579
getActionEntryForFile(storageUser.userId, ACTION_CREDENTIALS_EXTERNAL_STORAGE).should('not.exist')
7680

7781
// Finally, the storage should be accessible
@@ -81,8 +85,10 @@ describe('Files user credentials', { testIsolation: true }, () => {
8185
})
8286

8387
it('Create a user storage with GLOBAL user credentials', () => {
84-
const url = Cypress.config('baseUrl') + '/remote.php/dav/files/' + storageUser.userId
85-
createStorageWithConfig('storage1', StorageBackend.DAV, AuthBackend.UserGlobalAuth, { host: url.replace('index.php/', ''), 'secure': 'false' })
88+
// Its not the public server address but the address so the server itself can connect to it
89+
const base = 'http://localhost'
90+
const host = `${base}/remote.php/dav/files/${storageUser.userId}`
91+
createStorageWithConfig('storage1', StorageBackend.DAV, AuthBackend.UserGlobalAuth, { host, secure: 'false' })
8692

8793
cy.login(user2)
8894
cy.visit('/apps/files/extstoragemounts')
@@ -93,23 +99,32 @@ describe('Files user credentials', { testIsolation: true }, () => {
9399
triggerInlineActionForFile('storage1', ACTION_CREDENTIALS_EXTERNAL_STORAGE)
94100

95101
// See credentials dialog
96-
const storageDialog = cy.findByRole('dialog', { name: 'Storage credentials' })
97-
storageDialog.should('be.visible')
98-
storageDialog.findByRole('textbox', { name: 'Login' }).type(storageUser.userId)
99-
storageDialog.get('input[type="password"]').type(storageUser.password)
100-
storageDialog.get('button').contains('Confirm').click()
101-
storageDialog.should('not.exist')
102+
cy.findByRole('dialog', { name: 'Storage credentials' })
103+
.as('storageDialog')
104+
cy.get('@storageDialog')
105+
.should('be.visible')
106+
.findByRole('textbox', { name: 'Login' })
107+
.type(storageUser.userId)
108+
cy.get('@storageDialog')
109+
.find('input[type="password"]')
110+
.type(storageUser.password)
111+
cy.get('@storageDialog')
112+
.contains('button', 'Confirm')
113+
.click()
114+
cy.get('@storageDialog')
115+
.should('not.exist')
102116

103117
// Storage dialog now closed, the user auth dialog should be visible
104-
const authDialog = cy.findByRole('dialog', { name: 'Confirm your password' })
105-
authDialog.should('be.visible')
118+
cy.findByRole('dialog', { name: 'Confirm your password' })
119+
.as('authDialog')
120+
.should('be.visible')
106121
handlePasswordConfirmation(user2.password)
107122

108123
// Wait for the credentials to be set
109124
cy.wait('@setCredentials')
110125

111126
// Auth dialog should be closed and the set credentials button should be gone
112-
authDialog.should('not.exist', { timeout: 2000 })
127+
cy.get('@authDialog').should('not.exist', { timeout: 2000 })
113128
getActionEntryForFile('storage1', ACTION_CREDENTIALS_EXTERNAL_STORAGE).should('not.exist')
114129

115130
// Finally, the storage should be accessible
@@ -119,8 +134,10 @@ describe('Files user credentials', { testIsolation: true }, () => {
119134
})
120135

121136
it('Create another user storage while reusing GLOBAL user credentials', () => {
122-
const url = Cypress.config('baseUrl') + '/remote.php/dav/files/' + storageUser.userId
123-
createStorageWithConfig('storage2', StorageBackend.DAV, AuthBackend.UserGlobalAuth, { host: url.replace('index.php/', ''), 'secure': 'false' })
137+
// Its not the public server address but the address so the server itself can connect to it
138+
const base = 'http://localhost'
139+
const host = `${base}/remote.php/dav/files/${storageUser.userId}`
140+
createStorageWithConfig('storage2', StorageBackend.DAV, AuthBackend.UserGlobalAuth, { host, secure: 'false' })
124141

125142
cy.login(user2)
126143
cy.visit('/apps/files/extstoragemounts')

0 commit comments

Comments
 (0)