Skip to content

WebAuthn: prefer discoverable credentials with legacy fallback #57140

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
swissbit-eis-admin wants to merge 1 commit into from
Closed

WebAuthn: prefer discoverable credentials with legacy fallback #57140

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion apps/settings/lib/Controller/WebAuthnController.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,8 @@ public function __construct(
public function startRegistration(): JSONResponse {
$this->logger->debug('Starting WebAuthn registration');

$credentialOptions = $this->manager->startRegistration($this->userSession->getUser(), $this->request->getServerHost());
$discoverable = $this->request->getParam('discoverable', '1') !== '0';
$credentialOptions = $this->manager->startRegistration($this->userSession->getUser(), $this->request->getServerHost(), $discoverable);

// Set this in the session since we need it on finish
$this->session->set(self::WEBAUTHN_REGISTRATION, $credentialOptions);
Expand Down
21 changes: 18 additions & 3 deletions apps/settings/src/service/WebAuthnRegistrationSerice.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,16 +16,19 @@ import logger from '../logger'
* Start registering a new device
* @return The device attributes
*/
export async function startRegistration() {
const url = generateUrl('/settings/api/personal/webauthn/registration')

export async function startRegistration(discoverable = true): Promise<RegistrationResponseJSON> {
const url = generateUrl('/settings/api/personal/webauthn/registration') + (discoverable ? '' : '?discoverable=0')
try {
logger.debug('Fetching webauthn registration data')
const { data } = await axios.get<PublicKeyCredentialCreationOptionsJSON>(url)
logger.debug('Start webauthn registration')
const attrs = await registerWebAuthn({ optionsJSON: data })
return attrs
} catch (e) {
if (shouldFallbackToLegacy(e) && discoverable) {
logger.debug('WebAuthn discoverable registration failed, falling back to legacy mode')
return await startRegistration(false)
}
logger.error(e as Error)
if (isAxiosError(e)) {
throw new Error(t('settings', 'Could not register device: Network error'))
Expand All @@ -36,6 +39,18 @@ export async function startRegistration() {
}
}

function shouldFallbackToLegacy(error: unknown): boolean {
if (error instanceof Error) {
if (error.name === 'ConstraintError' || error.name === 'NotSupportedError') {
return true
}
if (error.message.includes('Discoverable credentials were required')) {
return true
}
}
return false
}

/**
* @param name Name of the device
* @param data Device attributes
Expand Down
36 changes: 23 additions & 13 deletions core/Controller/WebAuthnController.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -43,21 +43,28 @@ public function __construct(
#[PublicPage]
#[UseSession]
#[FrontpageRoute(verb: 'POST', url: 'login/webauthn/start')]
public function startAuthentication(string $loginName): JSONResponse {
public function startAuthentication(?string $loginName = null): JSONResponse {
$this->logger->debug('Starting WebAuthn login');

$this->logger->debug('Converting login name to UID');
$uid = $loginName;
Util::emitHook(
'\OCA\Files_Sharing\API\Server2Server',
'preLoginNameUsedAsUserName',
['uid' => &$uid]
);
$this->logger->debug('Got UID: ' . $uid);
$uid = null;
if ($loginName !== null && $loginName !== '') {
$this->logger->debug('Converting login name to UID');
$uid = $loginName;
Util::emitHook(
'\OCA\Files_Sharing\API\Server2Server',
'preLoginNameUsedAsUserName',
['uid' => &$uid]
);
$this->logger->debug('Got UID: ' . $uid);
}

$publicKeyCredentialRequestOptions = $this->webAuthnManger->startAuthentication($uid, $this->request->getServerHost());
$this->session->set(self::WEBAUTHN_LOGIN, json_encode($publicKeyCredentialRequestOptions));
$this->session->set(self::WEBAUTHN_LOGIN_UID, $uid);
if ($uid !== null && $uid !== '') {
$this->session->set(self::WEBAUTHN_LOGIN_UID, $uid);
} else {
$this->session->remove(self::WEBAUTHN_LOGIN_UID);
}

return new JSONResponse($publicKeyCredentialRequestOptions);
}
Expand All @@ -68,15 +75,18 @@ public function startAuthentication(string $loginName): JSONResponse {
public function finishAuthentication(string $data): JSONResponse {
$this->logger->debug('Validating WebAuthn login');

if (!$this->session->exists(self::WEBAUTHN_LOGIN) || !$this->session->exists(self::WEBAUTHN_LOGIN_UID)) {
if (!$this->session->exists(self::WEBAUTHN_LOGIN)) {
$this->logger->debug('Trying to finish WebAuthn login without session data');
return new JSONResponse([], Http::STATUS_BAD_REQUEST);
}

// Obtain the publicKeyCredentialOptions from when we started the registration
$publicKeyCredentialRequestOptions = PublicKeyCredentialRequestOptions::createFromString($this->session->get(self::WEBAUTHN_LOGIN));
$uid = $this->session->get(self::WEBAUTHN_LOGIN_UID);
$this->webAuthnManger->finishAuthentication($publicKeyCredentialRequestOptions, $data, $uid);
$uidFromSession = $this->session->get(self::WEBAUTHN_LOGIN_UID);
$this->session->remove(self::WEBAUTHN_LOGIN);
$this->session->remove(self::WEBAUTHN_LOGIN_UID);
$publicKeyCredentialSource = $this->webAuthnManger->finishAuthentication($publicKeyCredentialRequestOptions, $data, $uidFromSession);
$uid = $uidFromSession ?? $publicKeyCredentialSource->getUserHandle();

//TODO: add other parameters
$loginData = new LoginData(
Expand Down
71 changes: 53 additions & 18 deletions core/src/components/login/PasswordLessLoginForm.vue
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,18 +14,25 @@
{{ t('core', 'Log in with a device') }}
</h2>

<NcTextField required
:value="user"
:autocomplete="autoCompleteAllowed ? 'on' : 'off'"
:error="!validCredentials"
:label="t('core', 'Login or email')"
:placeholder="t('core', 'Login or email')"
:helper-text="!validCredentials ? t('core', 'Your account is not setup for passwordless login.') : ''"
@update:value="changeUsername" />
<div v-if="manualFlow" class="password-less-login-form__manual">
<NcTextField required
:value="user"
:autocomplete="autoCompleteAllowed ? 'on' : 'off'"
:error="!validCredentials"
:label="t('core', 'Login or email')"
:placeholder="t('core', 'Login or email')"
:helper-text="!validCredentials ? t('core', 'Your account is not setup for passwordless login.') : ''"
@update:value="changeUsername" />

<LoginButton v-if="validCredentials"
:loading="loading"
@click="authenticate" />
<LoginButton v-if="validCredentials"
:loading="loading"
@click="authenticate" />
</div>
<div v-else class="password-less-login-form__discoverable">
<LoginButton :loading="loading"
:value="t('core', 'Log in with a device')"
:value-loading="t('core', 'Logging in …')" />
</div>
</form>

<NcEmptyContent v-else-if="!isHttps && !isLocalhost"
Expand Down Expand Up @@ -105,9 +112,26 @@ export default defineComponent({
user: this.username,
loading: false,
validCredentials: true,
manualFlow: false,
}
},
mounted() {
if ((this.isHttps || this.isLocalhost) && this.supportsWebauthn) {
this.tryDiscoverableAuthentication()
}
},
methods: {
async tryDiscoverableAuthentication() {
this.loading = true
try {
const params = await startAuthentication()
await this.completeAuthentication(params)
} catch (error) {
logger.debug(error)
this.loading = false
this.manualFlow = true
}
},
async authenticate() {
// check required fields
if (!this.$refs.loginForm.checkValidity()) {
Expand All @@ -116,10 +140,12 @@ export default defineComponent({

console.debug('passwordless login initiated')

this.loading = true
try {
const params = await startAuthentication(this.user)
await this.completeAuthentication(params)
} catch (error) {
this.loading = false
if (error instanceof NoValidCredentials) {
this.validCredentials = false
return
Expand All @@ -129,6 +155,7 @@ export default defineComponent({
},
changeUsername(username) {
this.user = username
this.validCredentials = true
this.$emit('update:username', this.user)
},
completeAuthentication(challenge) {
Expand All @@ -143,20 +170,28 @@ export default defineComponent({
.catch(error => {
console.debug('GOT AN ERROR WHILE SUBMITTING CHALLENGE!')
console.debug(error) // Example: timeout, interaction refused...
this.loading = false
this.manualFlow = true
})
},
submit() {
// noop
if (this.manualFlow && !this.loading) {
void this.authenticate()
}
},
},
})
</script>

<style lang="scss" scoped>
.password-less-login-form {
display: flex;
flex-direction: column;
gap: 0.5rem;
margin: 0;
}
.password-less-login-form {
display: flex;
flex-direction: column;
gap: 0.5rem;
margin: 0;

&__discoverable {
align-self: flex-start;
}
}
</style>
13 changes: 8 additions & 5 deletions core/src/services/WebAuthnAuthenticationService.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,17 +17,20 @@ export class NoValidCredentials extends Error {}
* Start webautn authentication
* This loads the challenge, connects to the authenticator and returns the repose that needs to be sent to the server.
*
* @param loginName Name to login
* @param loginName Name to login (optional for discoverable credentials)
*/
export async function startAuthentication(loginName: string) {
export async function startAuthentication(loginName?: string) {
const url = generateUrl('/login/webauthn/start')

const { data } = await Axios.post<PublicKeyCredentialRequestOptionsJSON>(url, { loginName })
if (!data.allowCredentials || data.allowCredentials.length === 0) {
const body = loginName ? { loginName } : undefined
const { data } = await Axios.post<PublicKeyCredentialRequestOptionsJSON>(url, body)
if (loginName && (!data.allowCredentials || data.allowCredentials.length === 0)) {
logger.error('No valid credentials returned for webauthn')
throw new NoValidCredentials()
}
return await startWebauthnAuthentication({ optionsJSON: data })
return await startWebauthnAuthentication({
optionsJSON: data,
})
}

/**
Expand Down
4 changes: 2 additions & 2 deletions dist/core-login.js
View file
Open in desktop

Large diffs are not rendered by default.

2 changes: 1 addition & 1 deletion dist/core-login.js.map
View file
Open in desktop

Large diffs are not rendered by default.

4 changes: 2 additions & 2 deletions dist/settings-vue-settings-personal-webauthn.js
View file
Open in desktop

Large diffs are not rendered by default.

2 changes: 1 addition & 1 deletion dist/settings-vue-settings-personal-webauthn.js.map
View file
Open in desktop

Large diffs are not rendered by default.

53 changes: 32 additions & 21 deletions lib/private/Authentication/WebAuthn/Manager.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@
use Webauthn\PublicKeyCredentialParameters;
use Webauthn\PublicKeyCredentialRequestOptions;
use Webauthn\PublicKeyCredentialRpEntity;
use Webauthn\PublicKeyCredentialSource;
use Webauthn\PublicKeyCredentialUserEntity;
use Webauthn\TokenBinding\TokenBindingNotSupportedHandler;

Expand Down Expand Up @@ -61,7 +62,7 @@ public function __construct(
$this->config = $config;
}

public function startRegistration(IUser $user, string $serverHost): PublicKeyCredentialCreationOptions {
public function startRegistration(IUser $user, string $serverHost, bool $requireDiscoverable = true): PublicKeyCredentialCreationOptions {
$rpEntity = new PublicKeyCredentialRpEntity(
'Nextcloud', //Name
$this->stripPort($serverHost), //ID
Expand All @@ -87,12 +88,20 @@ public function startRegistration(IUser $user, string $serverHost): PublicKeyCre
$excludedPublicKeyDescriptors = [
];

$authenticatorSelectionCriteria = new AuthenticatorSelectionCriteria(
AuthenticatorSelectionCriteria::AUTHENTICATOR_ATTACHMENT_NO_PREFERENCE,
AuthenticatorSelectionCriteria::USER_VERIFICATION_REQUIREMENT_PREFERRED,
null,
false,
);
if ($requireDiscoverable) {
$authenticatorSelectionCriteria = new AuthenticatorSelectionCriteria(
AuthenticatorSelectionCriteria::AUTHENTICATOR_ATTACHMENT_NO_PREFERENCE,
AuthenticatorSelectionCriteria::USER_VERIFICATION_REQUIREMENT_REQUIRED,
AuthenticatorSelectionCriteria::RESIDENT_KEY_REQUIREMENT_REQUIRED,
);
} else {
$authenticatorSelectionCriteria = new AuthenticatorSelectionCriteria(
AuthenticatorSelectionCriteria::AUTHENTICATOR_ATTACHMENT_NO_PREFERENCE,
AuthenticatorSelectionCriteria::USER_VERIFICATION_REQUIREMENT_PREFERRED,
AuthenticatorSelectionCriteria::RESIDENT_KEY_REQUIREMENT_NO_PREFERENCE,
false,
);
}

return new PublicKeyCredentialCreationOptions(
$rpEntity,
Expand Down Expand Up @@ -159,19 +168,21 @@ private function stripPort(string $serverHost): string {
return preg_replace('/(:\d+$)/', '', $serverHost);
}

public function startAuthentication(string $uid, string $serverHost): PublicKeyCredentialRequestOptions {
// List of registered PublicKeyCredentialDescriptor classes associated to the user
public function startAuthentication(?string $uid, string $serverHost): PublicKeyCredentialRequestOptions {
$userVerificationRequirement = AuthenticatorSelectionCriteria::USER_VERIFICATION_REQUIREMENT_REQUIRED;
$registeredPublicKeyCredentialDescriptors = array_map(function (PublicKeyCredentialEntity $entity) use (&$userVerificationRequirement) {
if ($entity->getUserVerification() !== true) {
$userVerificationRequirement = AuthenticatorSelectionCriteria::USER_VERIFICATION_REQUIREMENT_DISCOURAGED;
}
$credential = $entity->toPublicKeyCredentialSource();
return new PublicKeyCredentialDescriptor(
$credential->type,
$credential->publicKeyCredentialId,
);
}, $this->credentialMapper->findAllForUid($uid));
$registeredPublicKeyCredentialDescriptors = [];
if ($uid !== null && $uid !== '') {
$registeredPublicKeyCredentialDescriptors = array_map(function (PublicKeyCredentialEntity $entity) use (&$userVerificationRequirement) {
if ($entity->getUserVerification() !== true) {
$userVerificationRequirement = AuthenticatorSelectionCriteria::USER_VERIFICATION_REQUIREMENT_DISCOURAGED;
}
$credential = $entity->toPublicKeyCredentialSource();
return new PublicKeyCredentialDescriptor(
$credential->type,
$credential->publicKeyCredentialId,
);
}, $this->credentialMapper->findAllForUid($uid));
}

// Public Key Credential Request Options
return new PublicKeyCredentialRequestOptions(
Expand All @@ -183,7 +194,7 @@ public function startAuthentication(string $uid, string $serverHost): PublicKeyC
);
}

public function finishAuthentication(PublicKeyCredentialRequestOptions $publicKeyCredentialRequestOptions, string $data, string $uid) {
public function finishAuthentication(PublicKeyCredentialRequestOptions $publicKeyCredentialRequestOptions, string $data, ?string $uid = null): PublicKeyCredentialSource {
$attestationStatementSupportManager = new AttestationStatementSupportManager();
$attestationStatementSupportManager->add(new NoneAttestationStatementSupport());

Expand Down Expand Up @@ -231,7 +242,7 @@ public function finishAuthentication(PublicKeyCredentialRequestOptions $publicKe
throw $e;
}

return true;
return $publicKeyCredentialSource;
}

public function deleteRegistration(IUser $user, int $id): void {
Expand Down