search

bitwarden / passwordless-server

Bitwarden Passwordless.dev infrastructure/backend (API, database, Docker, etc).
https://bitwarden.com/
Other
83 stars 26 forks source link

Wrong `credentialId` returned from `/signin/verify` endpoint #730

Open aliaftab612 opened 2 weeks ago

aliaftab612 commented 2 weeks ago

The /signin/verify endpoint is returning an incorrect credentialId. When hitting this endpoint with a token to verify passkey sign-in, the response contains a credentialId that does not match the expected value.

In the Admin console, the credentialId for the passkey is: gC-5YbmlbT2S9qv4uv1aIw, which I assume is correct. However, when I try to verify the sign-in, the response returns this credentialId: gC+5YbmlbT2S9qv4uv1aIw==, which differs from the one in the Admin console.

Below I have shared images for reference:

Screenshot 2024-09-12 at 3 35 59 PM

Screenshot 2024-09-12 at 3 36 14 PM

abergs commented 1 week ago

Hey @aliaftab612, I'm going to check if there's an actual issue, but just looking at this I think it's just encoded with/without base64url in the AdminConsole UI.

aliaftab612 commented 1 week ago

Hey @aliaftab612, I'm going to check if there's an actual issue, but just looking at this I think it's just encoded with/without base64url in the AdminConsole UI.

@abergs It appears that, except for this endpoint, all other endpoints return the Base64url-encoded credentialId. For example, in the credentials/delete endpoint, the encoded credentialId is used for deletion, and in credentials/list, the encoded credentialId is returned. Other endpoints related to sign and registration, also work with the encoded credentialId. This specific endpoint returns the credentialId without encoding, causing my application to fail when performing checks with the credentialId returned after verification. I believe the credentialId encoding should be consistent across all endpoints.

jonashendrickx commented 1 week ago

@abergs It looks like we don't use the Base64UrlConverter, I'm only seeing [MessagePack] attributes for the properties. So it's serializing to Base64 by default instead.

GET /credentials/list and other endpoints do Base64Url encode all byte arrays.

https://github.com/bitwarden/passwordless-server/blob/724c4bd44047c28917752af54d88ab2f1cda467b/src/Service/Models/RegisterToken.cs#L74

We probably have to fix it, but it would be a breaking change.