Skip to content
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.

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

Uncaught UnexpectedValueException error / unnecessary users in DB #203

Closed
iandunn opened this issue May 31, 2023 · 2 comments
Closed

Uncaught UnexpectedValueException error / unnecessary users in DB #203

iandunn opened this issue May 31, 2023 · 2 comments
Milestone

Comments

@iandunn
Copy link
Member

iandunn commented May 31, 2023

Problem

I noticed this error in the logs:

E_ERROR: Uncaught UnexpectedValueException: Unable to save the user handle to the database. in wp-content/plugins/two-factor-provider-webauthn/inc/class-webauthn-user.php:54
Uncaught UnexpectedValueException: Unable to save the user handle to the database. in wp-content/plugins/two-factor-provider-webauthn/inc/class-webauthn-user.php:54
Stack trace:
#0 wp-content/plugins/two-factor-provider-webauthn/inc/class-webauthn-credential-store.php(93): WildWolf\WordPress\TwoFactorWebAuthn\WebAuthn_User->getUserHandle()
#1 wp-content/plugins/wporg-two-factor/settings/rest-api.php(214): WildWolf\WordPress\TwoFactorWebAuthn\WebAuthn_Credential_Store::get_user_keys(Object(WP_User))
...

Full log in Slack /archives/G02QB4059/p1685257590345359. 3 other instances if you search for UnexpectedValueException.

I also noticed that the 2fa_webauthn_users table has 12k rows, but only a handful of users have created credentials so far. It doesn't seem like there should be a row for the user unless they create a credential; otherwise it could take up a lot of unnecessary space it if ends up creating an entry for every user who logs in after #153 was deployed.

Relevant code

$keys = WebAuthn_Credential_Store::get_user_keys( get_userdata( $user['id'] ) );

https://github.com/sjinks/wp-two-factor-provider-webauthn/blob/a17ed697a218741986198366217d4e73daa4403f/inc/class-webauthn-credential-store.php#L93

https://github.com/sjinks/wp-two-factor-provider-webauthn/blob/a17ed697a218741986198366217d4e73daa4403f/inc/class-webauthn-user.php#L54

Next steps

We need to understand why get_user_keys() ends up creating a handle for every user, even if they don't have a key yet. @sjinks, can you give some insight there?

https://github.com/sjinks/wp-two-factor-provider-webauthn/blob/a17ed697a218741986198366217d4e73daa4403f/inc/class-webauthn-user.php#L42-L51

If that functionality is not needed, then maybe we can short-circuit it somehow. That should avoid the error and the extra users.

If it is needed, then we need to figure out why it works most of the time, but fails every so often.

@sjinks
Copy link

sjinks commented Jun 5, 2023

The plugin created a handle for every user viewing their profile regardless of whether it was necessary.

The fix is sjinks/wp-two-factor-provider-webauthn#480

@iandunn
Copy link
Member Author

iandunn commented Jun 5, 2023

Thanks!

@iandunn iandunn closed this as completed Jun 5, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants