OAuth 2.0 (RFC 6749) violations #1192

New Issue

MrUnknownDE · 2026-04-06T01:41:24+02:00

MrUnknownDE commented

2026-04-06 01:41:24 +02:00

Originally created by @zacknewman on 3/13/2024

Sections 10.3 and 10.4 of RFC 6749 requires the authorization server to use TLS when exchanging the access and refresh tokens; however Vaultwarden—which acts as both the resource and authorization servers—currently does not force TLS, meaning deployments without TLS violate the RFC.

Additionally Section 6 requires the refresh token to be uniquely linked to the client; however Vaultwarden does not have a UNIQUE CONSTRAINT defined on devices.refresh_token which means it's possible, albeit unlikely, for multiple clients to have the same refresh token.

If adherence to RFC 6749 is not a goal, I do believe a UNIQUE CONSTRAINT should still be defined on devices.refresh_token since it will ensure the token is truly unique. Furthermore Vaultwarden already (incorrectly) assumes/hopes the refresh token is unique; and with an actual UNIQUE CONSTRAINT defined—which will be backed by a UNIQUE INDEX with any real-world RDBMS I am aware of—the performance will be even better (and most importantly correct).

With this change, it would also be nice if refresh token compromise were detected as well forcing all clients associated to a user to re-authenticate via password and whatever 2-factor authentication is hopefully configured.

*Originally created by @zacknewman on 3/13/2024* Sections [10.3](https://www.rfc-editor.org/rfc/rfc6749#section-10.3) and [10.4](https://www.rfc-editor.org/rfc/rfc6749#section-10.4) of RFC 6749 requires the authorization server to use TLS when exchanging the access and refresh tokens; however Vaultwarden—which acts as both the resource and authorization servers—currently does not force TLS, meaning deployments without TLS violate the RFC. Additionally [Section 6](https://www.rfc-editor.org/rfc/rfc6749#section-6) requires the refresh token to be uniquely linked to the client; however Vaultwarden does not have a `UNIQUE CONSTRAINT` defined on `devices.refresh_token` which means it's possible, albeit unlikely, for multiple clients to have the same refresh token. If adherence to RFC 6749 is not a goal, I do believe a `UNIQUE CONSTRAINT` should still be defined on `devices.refresh_token` since it will ensure the token is truly unique. Furthermore [Vaultwarden already (incorrectly) assumes/hopes the refresh token is unique](https://github.com/dani-garcia/vaultwarden/blob/main/src/db/models/device.rs#L201); and with an actual `UNIQUE CONSTRAINT` defined—which will be backed by a `UNIQUE INDEX` with any real-world RDBMS I am aware of—the performance will be even better (and most importantly correct). With this change, it would also be nice if refresh token compromise were detected as well forcing all clients associated to a user to re-authenticate via password and whatever 2-factor authentication is hopefully configured.

MrUnknownDE closed this issue

2026-04-06 01:41:24 +02:00

MrUnknownDE added the enhancement low priority enhancement enhancement enhancement enhancement enhancement enhancement enhancement enhancement enhancement enhancement enhancement enhancement enhancement enhancement enhancement low priority low priority low priority low priority low priority low priority low priority labels 2026-04-06 01:41:28 +02:00

Sign in to join this conversation.

No Label enhancement enhancement enhancement enhancement enhancement enhancement enhancement enhancement enhancement enhancement enhancement enhancement enhancement enhancement enhancement enhancement low priority low priority low priority low priority low priority low priority low priority low priority

1 Participants

Notifications

Due Date

No due date set.

Dependencies

No dependencies set.

Reference: github/vaultwarden#1192