Merge pull request #51521 from nextcloud/fix/webauthn

fix(webauthn): adjust for updated library and add tests
pull/51587/head
Ferdinand Thiessen 7 months ago committed by GitHub
commit 37f8beb967
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
  1. 5
      .eslintrc.js
  2. 3
      apps/settings/lib/Settings/Personal/Security/WebAuthn.php
  3. 4
      apps/settings/lib/SetupChecks/PhpModules.php
  4. 6
      apps/settings/src/service/WebAuthnRegistrationSerice.ts
  5. 14
      core/src/components/login/PasswordLessLoginForm.vue
  6. 2
      core/src/services/WebAuthnAuthenticationService.ts
  7. 14
      cypress/dockerNode.ts
  8. 41
      cypress/e2e/files/LivePhotosUtils.ts
  9. 22
      cypress/e2e/files_external/files-user-credentials.cy.ts
  10. 25
      cypress/e2e/files_versions/version_deletion.cy.ts
  11. 27
      cypress/e2e/files_versions/version_download.cy.ts
  12. 81
      cypress/e2e/files_versions/version_naming.cy.ts
  13. 29
      cypress/e2e/files_versions/version_restoration.cy.ts
  14. 152
      cypress/e2e/login/webauth.cy.ts
  15. 4
      dist/core-login.js
  16. 2
      dist/core-login.js.map
  17. 4
      dist/settings-vue-settings-personal-webauthn.js
  18. 2
      dist/settings-vue-settings-personal-webauthn.js.map
  19. 8
      lib/private/Authentication/WebAuthn/Manager.php

@ -35,6 +35,9 @@ module.exports = {
jsdoc: {
mode: 'typescript',
},
'import/resolver': {
typescript: {}, // this loads <rootdir>/tsconfig.json to eslint
},
},
overrides: [
// Allow any in tests
@ -43,6 +46,6 @@ module.exports = {
rules: {
'@typescript-eslint/no-explicit-any': 'warn',
},
}
},
],
}

@ -40,8 +40,7 @@ class WebAuthn implements ISettings {
$this->mapper->findAllForUid($this->userId)
);
return new TemplateResponse('settings', 'settings/personal/security/webauthn', [
]);
return new TemplateResponse('settings', 'settings/personal/security/webauthn');
}
public function getSection(): ?string {

@ -32,7 +32,6 @@ class PhpModules implements ISetupCheck {
'zlib',
];
protected const RECOMMENDED_MODULES = [
'bcmath',
'exif',
'gmp',
'intl',
@ -58,8 +57,7 @@ class PhpModules implements ISetupCheck {
return match($module) {
'intl' => $this->l10n->t('increases language translation performance and fixes sorting of non-ASCII characters'),
'sodium' => $this->l10n->t('for Argon2 for password hashing'),
'bcmath' => $this->l10n->t('for WebAuthn passwordless login'),
'gmp' => $this->l10n->t('for WebAuthn passwordless login, and SFTP storage'),
'gmp' => $this->l10n->t('required for SFTP storage and recommended for WebAuthn performance'),
'exif' => $this->l10n->t('for picture rotation in server and metadata extraction in the Photos app'),
default => '',
};

@ -3,7 +3,7 @@
* SPDX-License-Identifier: AGPL-3.0-or-later
*/
import type { RegistrationResponseJSON } from '@simplewebauthn/types'
import type { PublicKeyCredentialCreationOptionsJSON, RegistrationResponseJSON } from '@simplewebauthn/types'
import { translate as t } from '@nextcloud/l10n'
import { generateUrl } from '@nextcloud/router'
@ -21,9 +21,9 @@ export async function startRegistration() {
try {
logger.debug('Fetching webauthn registration data')
const { data } = await axios.get(url)
const { data } = await axios.get<PublicKeyCredentialCreationOptionsJSON>(url)
logger.debug('Start webauthn registration')
const attrs = await registerWebAuthn(data)
const attrs = await registerWebAuthn({ optionsJSON: data })
return attrs
} catch (e) {
logger.error(e as Error)

@ -5,11 +5,15 @@
<template>
<form v-if="(isHttps || isLocalhost) && supportsWebauthn"
ref="loginForm"
aria-labelledby="password-less-login-form-title"
class="password-less-login-form"
method="post"
name="login"
@submit.prevent="submit">
<h2>{{ t('core', 'Log in with a device') }}</h2>
<h2 id="password-less-login-form-title">
{{ t('core', 'Log in with a device') }}
</h2>
<NcTextField required
:value="user"
:autocomplete="autoCompleteAllowed ? 'on' : 'off'"
@ -41,9 +45,11 @@
</NcEmptyContent>
</template>
<script>
<script type="ts">
import { browserSupportsWebAuthn } from '@simplewebauthn/browser'
import { defineComponent } from 'vue'
import {
NoValidCredentials,
startAuthentication,
finishAuthentication,
} from '../../services/WebAuthnAuthenticationService.ts'
@ -56,7 +62,7 @@ import LoginButton from './LoginButton.vue'
import LockOpenIcon from 'vue-material-design-icons/LockOpen.vue'
import logger from '../../logger'
export default {
export default defineComponent({
name: 'PasswordLessLoginForm',
components: {
LoginButton,
@ -143,7 +149,7 @@ export default {
// noop
},
},
}
})
</script>
<style lang="scss" scoped>

@ -27,7 +27,7 @@ export async function startAuthentication(loginName: string) {
logger.error('No valid credentials returned for webauthn')
throw new NoValidCredentials()
}
return await startWebauthnAuthentication(data)
return await startWebauthnAuthentication({ optionsJSON: data })
}
/**

@ -88,6 +88,12 @@ export const startNextcloud = async function(branch: string = getCurrentGitBranc
Type: 'tmpfs',
ReadOnly: false,
}],
PortBindings: {
'80/tcp': [{
HostIP: '0.0.0.0',
HostPort: '8083',
}],
},
},
Env: [
`BRANCH=${branch}`,
@ -242,11 +248,15 @@ export const getContainerIP = async function(
while (ip === '' && tries < 10) {
tries++
await container.inspect(function(err, data) {
container.inspect(function(err, data) {
if (err) {
throw err
}
ip = data?.NetworkSettings?.IPAddress || ''
if (data?.HostConfig.PortBindings?.['80/tcp']?.[0]?.HostPort) {
ip = `localhost:${data.HostConfig.PortBindings['80/tcp'][0].HostPort}`
} else {
ip = data?.NetworkSettings?.IPAddress || ''
}
})
if (ip !== '') {

@ -14,34 +14,25 @@ type SetupInfo = {
}
/**
*
* @param user
* @param fileName
* @param domain
* @param requesttoken
* @param metadata
*/
function setMetadata(user: User, fileName: string, requesttoken: string, metadata: object) {
cy.url().then(url => {
const hostname = new URL(url).hostname
cy.request({
method: 'PROPPATCH',
url: `http://${hostname}/remote.php/dav/files/${user.userId}/${fileName}`,
auth: { user: user.userId, pass: user.password },
headers: {
requesttoken,
},
body: `<?xml version="1.0"?>
<d:propertyupdate xmlns:d="DAV:" xmlns:nc="http://nextcloud.org/ns">
<d:set>
<d:prop>
${Object.entries(metadata).map(([key, value]) => `<${key}>${value}</${key}>`).join('\n')}
</d:prop>
</d:set>
</d:propertyupdate>`,
})
const base = Cypress.config('baseUrl')!.replace(/\/index\.php\/?/, '')
cy.request({
method: 'PROPPATCH',
url: `${base}/remote.php/dav/files/${user.userId}/${fileName}`,
auth: { user: user.userId, pass: user.password },
headers: {
requesttoken,
},
body: `<?xml version="1.0"?>
<d:propertyupdate xmlns:d="DAV:" xmlns:nc="http://nextcloud.org/ns">
<d:set>
<d:prop>
${Object.entries(metadata).map(([key, value]) => `<${key}>${value}</${key}>`).join('\n')}
</d:prop>
</d:set>
</d:propertyupdate>`,
})
}
/**

@ -15,9 +15,6 @@ describe('Files user credentials', { testIsolation: true }, () => {
let user2: User
let storageUser: User
beforeEach(() => {
})
before(() => {
cy.runOccCommand('app:enable files_external')
@ -43,8 +40,10 @@ describe('Files user credentials', { testIsolation: true }, () => {
})
it('Create a user storage with user credentials', () => {
const url = Cypress.config('baseUrl') + '/remote.php/dav/files/' + storageUser.userId
createStorageWithConfig(storageUser.userId, StorageBackend.DAV, AuthBackend.UserProvided, { host: url.replace('index.php/', ''), secure: 'false' })
// Its not the public server address but the address so the server itself can connect to it
const base = 'http://localhost'
const host = `${base}/remote.php/dav/files/${storageUser.userId}`
createStorageWithConfig(storageUser.userId, StorageBackend.DAV, AuthBackend.UserProvided, { host, secure: 'false' })
cy.login(user1)
cy.visit('/apps/files/extstoragemounts')
@ -72,6 +71,7 @@ describe('Files user credentials', { testIsolation: true }, () => {
// Auth dialog should be closed and the set credentials button should be gone
cy.get('@authDialog').should('not.exist', { timeout: 2000 })
getActionEntryForFile(storageUser.userId, ACTION_CREDENTIALS_EXTERNAL_STORAGE).should('not.exist')
// Finally, the storage should be accessible
@ -81,8 +81,10 @@ describe('Files user credentials', { testIsolation: true }, () => {
})
it('Create a user storage with GLOBAL user credentials', () => {
const url = Cypress.config('baseUrl') + '/remote.php/dav/files/' + storageUser.userId
createStorageWithConfig('storage1', StorageBackend.DAV, AuthBackend.UserGlobalAuth, { host: url.replace('index.php/', ''), secure: 'false' })
// Its not the public server address but the address so the server itself can connect to it
const base = 'http://localhost'
const host = `${base}/remote.php/dav/files/${storageUser.userId}`
createStorageWithConfig('storage1', StorageBackend.DAV, AuthBackend.UserGlobalAuth, { host, secure: 'false' })
cy.login(user2)
cy.visit('/apps/files/extstoragemounts')
@ -119,8 +121,10 @@ describe('Files user credentials', { testIsolation: true }, () => {
})
it('Create another user storage while reusing GLOBAL user credentials', () => {
const url = Cypress.config('baseUrl') + '/remote.php/dav/files/' + storageUser.userId
createStorageWithConfig('storage2', StorageBackend.DAV, AuthBackend.UserGlobalAuth, { host: url.replace('index.php/', ''), secure: 'false' })
// Its not the public server address but the address so the server itself can connect to it
const base = 'http://localhost'
const host = `${base}/remote.php/dav/files/${storageUser.userId}`
createStorageWithConfig('storage2', StorageBackend.DAV, AuthBackend.UserGlobalAuth, { host, secure: 'false' })
cy.login(user2)
cy.visit('/apps/files/extstoragemounts')

@ -59,7 +59,6 @@ describe('Versions restoration', () => {
})
it('Does not work without delete permission through direct API access', () => {
let hostname: string
let fileId: string|undefined
let versionId: string|undefined
@ -68,24 +67,30 @@ describe('Versions restoration', () => {
navigateToFolder(folderName)
openVersionsPanel(randomFilePath)
cy.url().then(url => { hostname = new URL(url).hostname })
getRowForFile(randomFileName).invoke('attr', 'data-cy-files-list-row-fileid').then(_fileId => { fileId = _fileId })
cy.get('[data-files-versions-version]').eq(1).invoke('attr', 'data-files-versions-version').then(_versionId => { versionId = _versionId })
getRowForFile(randomFileName)
.should('be.visible')
.invoke('attr', 'data-cy-files-list-row-fileid')
.then(($fileId) => { fileId = $fileId })
cy.get('[data-files-versions-version]')
.eq(1)
.invoke('attr', 'data-files-versions-version')
.then(($versionId) => { versionId = $versionId })
cy.logout()
cy.then(() => {
cy.logout()
cy.request({
const base = Cypress.config('baseUrl')!.replace(/\/index\.php\/?$/, '')
return cy.request({
method: 'DELETE',
url: `${base}/remote.php/dav/versions/${recipient.userId}/versions/${fileId}/${versionId}`,
auth: { user: recipient.userId, pass: recipient.password },
headers: {
cookie: '',
},
url: `http://${hostname}/remote.php/dav/versions/${recipient.userId}/versions/${fileId}/${versionId}`,
failOnStatusCode: false,
})
.then(({ status }) => {
expect(status).to.equal(403)
})
}).then(({ status }) => {
expect(status).to.equal(403)
})
})
})

@ -52,31 +52,36 @@ describe('Versions download', () => {
})
it('Does not work without download permission through direct API access', () => {
let hostname: string
let fileId: string|undefined
let versionId: string|undefined
setupTestSharedFileFromUser(user, randomFileName, { download: false })
.then(recipient => {
.then((recipient) => {
openVersionsPanel(randomFileName)
cy.url().then(url => { hostname = new URL(url).hostname })
getRowForFile(randomFileName).invoke('attr', 'data-cy-files-list-row-fileid').then(_fileId => { fileId = _fileId })
cy.get('[data-files-versions-version]').eq(1).invoke('attr', 'data-files-versions-version').then(_versionId => { versionId = _versionId })
getRowForFile(randomFileName)
.should('be.visible')
.invoke('attr', 'data-cy-files-list-row-fileid')
.then(($fileId) => { fileId = $fileId })
cy.get('[data-files-versions-version]')
.eq(1)
.invoke('attr', 'data-files-versions-version')
.then(($versionId) => { versionId = $versionId })
cy.logout()
cy.then(() => {
cy.logout()
cy.request({
const base = Cypress.config('baseUrl')!.replace(/\/index\.php\/?$/, '')
return cy.request({
url: `${base}/remote.php/dav/versions/${recipient.userId}/versions/${fileId}/${versionId}`,
auth: { user: recipient.userId, pass: recipient.password },
headers: {
cookie: '',
},
url: `http://${hostname}/remote.php/dav/versions/${recipient.userId}/versions/${fileId}/${versionId}`,
failOnStatusCode: false,
})
.then(({ status }) => {
expect(status).to.equal(403)
})
}).then(({ status }) => {
expect(status).to.equal(403)
})
})
})

@ -69,10 +69,17 @@ describe('Versions naming', () => {
})
context('without edit permission', () => {
it('Does not show action', () => {
let recipient: User
beforeEach(() => {
setupTestSharedFileFromUser(user, randomFileName, { update: false })
openVersionsPanel(randomFileName)
.then(($recipient) => {
recipient = $recipient
openVersionsPanel(randomFileName)
})
})
it('Does not show action', () => {
cy.get('[data-files-versions-version]').eq(0).find('.action-item__menutoggle').should('not.exist')
cy.get('[data-files-versions-version]').eq(0).get('[data-cy-version-action="label"]').should('not.exist')
@ -81,45 +88,45 @@ describe('Versions naming', () => {
})
it('Does not work without update permission through direct API access', () => {
let hostname: string
let fileId: string|undefined
let versionId: string|undefined
setupTestSharedFileFromUser(user, randomFileName, { update: false })
.then(recipient => {
openVersionsPanel(randomFileName)
cy.url().then(url => { hostname = new URL(url).hostname })
getRowForFile(randomFileName).invoke('attr', 'data-cy-files-list-row-fileid').then(_fileId => { fileId = _fileId })
cy.get('[data-files-versions-version]').eq(1).invoke('attr', 'data-files-versions-version').then(_versionId => { versionId = _versionId })
cy.then(() => {
cy.logout()
cy.request({
method: 'PROPPATCH',
auth: { user: recipient.userId, pass: recipient.password },
headers: {
cookie: '',
},
body: `<?xml version="1.0"?>
<d:propertyupdate xmlns:d="DAV:"
xmlns:oc="http://owncloud.org/ns"
xmlns:nc="http://nextcloud.org/ns"
xmlns:ocs="http://open-collaboration-services.org/ns">
<d:set>
<d:prop>
<nc:version-label>not authorized labeling</nc:version-label>
</d:prop>
</d:set>
</d:propertyupdate>`,
url: `http://${hostname}/remote.php/dav/versions/${recipient.userId}/versions/${fileId}/${versionId}`,
failOnStatusCode: false,
})
.then(({ status }) => {
expect(status).to.equal(403)
})
})
getRowForFile(randomFileName)
.should('be.visible')
.invoke('attr', 'data-cy-files-list-row-fileid')
.then(($fileId) => { fileId = $fileId })
cy.get('[data-files-versions-version]')
.eq(1)
.invoke('attr', 'data-files-versions-version')
.then(($versionId) => { versionId = $versionId })
cy.logout()
cy.then(() => {
const base = Cypress.config('baseUrl')!.replace(/index\.php\/?/, '')
return cy.request({
method: 'PROPPATCH',
url: `${base}/remote.php/dav/versions/${recipient.userId}/versions/${fileId}/${versionId}`,
auth: { user: recipient.userId, pass: recipient.password },
headers: {
cookie: '',
},
body: `<?xml version="1.0"?>
<d:propertyupdate xmlns:d="DAV:"
xmlns:oc="http://owncloud.org/ns"
xmlns:nc="http://nextcloud.org/ns"
xmlns:ocs="http://open-collaboration-services.org/ns">
<d:set>
<d:prop>
<nc:version-label>not authorized labeling</nc:version-label>
</d:prop>
</d:set>
</d:propertyupdate>`,
failOnStatusCode: false,
})
}).then(({ status }) => {
expect(status).to.equal(403)
})
})
})
})

@ -77,33 +77,38 @@ describe('Versions restoration', () => {
})
it('Does not work without update permission through direct API access', () => {
let hostname: string
let fileId: string|undefined
let versionId: string|undefined
setupTestSharedFileFromUser(user, randomFileName, { update: false })
.then(recipient => {
.then((recipient) => {
openVersionsPanel(randomFileName)
cy.url().then(url => { hostname = new URL(url).hostname })
getRowForFile(randomFileName).invoke('attr', 'data-cy-files-list-row-fileid').then(_fileId => { fileId = _fileId })
cy.get('[data-files-versions-version]').eq(1).invoke('attr', 'data-files-versions-version').then(_versionId => { versionId = _versionId })
getRowForFile(randomFileName)
.should('be.visible')
.invoke('attr', 'data-cy-files-list-row-fileid')
.then(($fileId) => { fileId = $fileId })
cy.get('[data-files-versions-version]')
.eq(1)
.invoke('attr', 'data-files-versions-version')
.then(($versionId) => { versionId = $versionId })
cy.logout()
cy.then(() => {
cy.logout()
cy.request({
const base = Cypress.config('baseUrl')!.replace(/\/index\.php\/?$/, '')
return cy.request({
method: 'MOVE',
url: `${base}/remote.php/dav/versions/${recipient.userId}/versions/${fileId}/${versionId}`,
auth: { user: recipient.userId, pass: recipient.password },
headers: {
cookie: '',
Destination: `http://${hostname}/remote.php/dav/versions/${recipient.userId}/restore/target`,
Destination: `${base}}/remote.php/dav/versions/${recipient.userId}/restore/target`,
},
url: `http://${hostname}/remote.php/dav/versions/${recipient.userId}/versions/${fileId}/${versionId}`,
failOnStatusCode: false,
})
.then(({ status }) => {
expect(status).to.equal(403)
})
}).then(({ status }) => {
expect(status).to.equal(403)
})
})
})

@ -0,0 +1,152 @@
/**
* SPDX-FileCopyrightText: 2025 Nextcloud GmbH and Nextcloud contributors
* SPDX-License-Identifier: AGPL-3.0-or-later
*/
import type { User } from '@nextcloud/cypress'
interface IChromeVirtualAuthenticator {
authenticatorId: string
}
/**
* Create a virtual authenticator using chrome debug protocol
*/
async function createAuthenticator(): Promise<IChromeVirtualAuthenticator> {
await Cypress.automation('remote:debugger:protocol', {
command: 'WebAuthn.enable',
})
const authenticator = await Cypress.automation('remote:debugger:protocol', {
command: 'WebAuthn.addVirtualAuthenticator',
params: {
options: {
protocol: 'ctap2',
ctap2Version: 'ctap2_1',
hasUserVerification: true,
transport: 'usb',
automaticPresenceSimulation: true,
isUserVerified: true,
},
},
})
return authenticator
}
/**
* Delete a virtual authenticator using chrome devbug protocol
*
* @param authenticator the authenticator object
*/
async function deleteAuthenticator(authenticator: IChromeVirtualAuthenticator) {
await Cypress.automation('remote:debugger:protocol', {
command: 'WebAuthn.removeVirtualAuthenticator',
params: {
...authenticator,
},
})
}
describe('Login using WebAuthn', () => {
let authenticator: IChromeVirtualAuthenticator
let user: User
afterEach(() => {
cy.deleteUser(user)
.then(() => deleteAuthenticator(authenticator))
})
beforeEach(() => {
cy.createRandomUser()
.then(($user) => {
user = $user
cy.login(user)
})
.then(() => createAuthenticator())
.then(($authenticator) => {
authenticator = $authenticator
cy.log('Created virtual authenticator')
})
})
it('add and delete WebAuthn', () => {
cy.intercept('**/settings/api/personal/webauthn/registration').as('webauthn')
cy.visit('/settings/user/security')
cy.contains('[role="note"]', /No devices configured/i).should('be.visible')
cy.findByRole('button', { name: /Add WebAuthn device/i })
.should('be.visible')
.click()
cy.wait('@webauthn')
cy.findByRole('textbox', { name: /Device name/i })
.should('be.visible')
.type('test device{enter}')
cy.wait('@webauthn')
cy.contains('[role="note"]', /No devices configured/i).should('not.exist')
cy.findByRole('list', { name: /following devices are configured for your account/i })
.should('be.visible')
.contains('li', 'test device')
.should('be.visible')
.findByRole('button', { name: /Actions/i })
.click()
cy.findByRole('menuitem', { name: /Delete/i })
.should('be.visible')
.click()
cy.contains('[role="note"]', /No devices configured/i).should('be.visible')
cy.findByRole('list', { name: /following devices are configured for your account/i })
.should('not.exist')
cy.reload()
cy.contains('[role="note"]', /No devices configured/i).should('be.visible')
})
it('add WebAuthn and login', () => {
cy.intercept('GET', '**/settings/api/personal/webauthn/registration').as('webauthnSetupInit')
cy.intercept('POST', '**/settings/api/personal/webauthn/registration').as('webauthnSetupDone')
cy.intercept('POST', '**/login/webauthn/start').as('webauthnLogin')
cy.visit('/settings/user/security')
cy.findByRole('button', { name: /Add WebAuthn device/i })
.should('be.visible')
.click()
cy.wait('@webauthnSetupInit')
cy.findByRole('textbox', { name: /Device name/i })
.should('be.visible')
.type('test device{enter}')
cy.wait('@webauthnSetupDone')
cy.findByRole('list', { name: /following devices are configured for your account/i })
.should('be.visible')
.findByText('test device')
.should('be.visible')
cy.logout()
cy.visit('/login')
cy.findByRole('button', { name: /Log in with a device/i })
.should('be.visible')
.click()
cy.findByRole('form', { name: /Log in with a device/i })
.should('be.visible')
.findByRole('textbox', { name: /Login or email/i })
.should('be.visible')
.type(`{selectAll}${user.userId}`)
cy.findByRole('button', { name: /Log in/i })
.click()
cy.wait('@webauthnLogin')
// Then I see that the current page is the Files app
cy.url().should('match', /apps\/dashboard(\/|$)/)
})
})

File diff suppressed because one or more lines are too long

File diff suppressed because one or more lines are too long

File diff suppressed because one or more lines are too long

File diff suppressed because one or more lines are too long

@ -246,14 +246,6 @@ class Manager {
}
public function isWebAuthnAvailable(): bool {
if (!extension_loaded('bcmath')) {
return false;
}
if (!extension_loaded('gmp')) {
return false;
}
if (!$this->config->getSystemValueBool('auth.webauthn.enabled', true)) {
return false;
}

Loading…
Cancel
Save