-
-
Notifications
You must be signed in to change notification settings - Fork 2k
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
Add support for MFA with Duo's Universal Prompt #4637
Conversation
Thanks @0x0fbc. On first glance it looks like you need to rebase already, but besides that it looks ok. No deep checking done. The only thing that worries me is the collation for the Isn't there a way to not have this as a key? Or less larger in size. |
The column size can be scaled back, absolutely. I set the state and nonce column sizes in postgres and maria so that someone could change the constant dictating the state length to the largest Duo would accept (1024 characters) without having to mess with the database schema. That constant is set to 64 right now, so the large size is only useful to support this specific future proofing case. If you've seen issues with collations/charsets in the past I'd rather not tempt fate, so I'm in favor of just cutting the size down and dropping the overridden character set/collation for the columns in MariaDB. I'll take care of that and rebase when I get a chance later today. |
965221e
to
8c02577
Compare
@BlackDex, I reduced the column sizes and removed the collation override I had for MariaDB. I also added the changes suggested by Clippy, did another rustfmt, synced my fork, and rebased. |
8c02577
to
f02c892
Compare
This is an upstream client bug and unrelated to vaultwarden. Both snap and appimage builds have (different) issues with the url handlers at the moment. If I recall correctly from debugging, on AppImage the desktop file does not get created anymore. It needs to be created manually. |
f02c892
to
8389bcd
Compare
The new native android app still seems to send PascalCase entries for Email 2FA. Added aliasses for these keys. Tested all other 2FA's (Except Duo, which might be fixable via dani-garcia#4637) and they all work fine using the Native Android Beta v2024.7.0 version. Fixes dani-garcia#4713
8389bcd
to
ef3a739
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Overall looks nice, and it works fine too.
Just some small items i saw from my side.
btw @0x0fbc, sorry for the delay in testing. Also forgive me for pushing some changes, but i forgot the reqwest changes and those didn't popped up as a merge conflict, so i wanted to fix those. |
No worries! I figured dealing with the beta client API changes over the last month was much more urgent. Thanks for taking care of the reqwest change. I'll get right on your callouts. |
I completed the requested changes and verified that everything is still working fine. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good too me.
Seems to work fine during my tests.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Overall LGTM though I haven't had time to test that it works, just some small comments.
One question, seeing as the Duo traditional prompt is end of life and this is the replacement, is there any special case where someone might want to keep using the old iframe variant?
Just trying to think for how long we want to support the iframe version in Vaultwarden. If this is a drop in replacement with no downsides, I imagine we can keep support for the traditional prompt for a couple of versions until all the kinks are ironed out and then drop it.
Retesting the old iframe that still works until somewhere in August if I'm correct. Or maybe a bit longer. |
Duo hasn't been clear about when they're truly dropping the iframe prompt. For some Duo tenancies (mine included), it's continuing to work; for others, it's not. There's obviously no case to support it after Duo shuts their iframe prompt endpoints down, but I haven't seen any particular date for when they're removing or restricting the endpoints. September 30th, 2024, is the date Duo mentions dropping support for all exceptions that don't have more specific transition dates in the future. I think the Vaultwarden version releasing after that date would be a good time to drop the support. The special case I had in mind when choosing to leave in the existing iframe prompt functionality is fairly niche. If I'm not mistaken, the iframe prompt works without Vaultwarden needing to communicate with Duo's API directly, but the new OIDC prompt does not. Someone who's deployed Vaultwarden in a restrictive environment may need to set up firewall exceptions or change other configuration to transition. If these changes would be time-consuming for them, I wanted to avoid forcing them to choose between disabling Duo for their Vaultwarden deployment or downgrading to restore their service while they made whatever arrangements they needed to get the OIDC prompt working. |
d32e90f
to
5caa4a7
Compare
Co-authored-by: Daniel García <[email protected]>
5caa4a7
to
973ae47
Compare
Also, I've synced my fork and rebased. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM! Thanks @0x0fbc for the explanations
Once merged, are these automatically in the testing tag already? Additionally, I see this version in server diagnostics, is there somewhere here on git I can compare this number to..to know if I’m grabbing the latest test? |
Once merged it takes mostly less then 1 hour to build all the architectures and create a container. I just noticed that the alpine builds failed because GitHub killed that container. Those are building right now. |
Sorry just realized I didn’t post my version. Server diagnostics has this version: 1.31.0- .b4b27019 Is there a place here to go to find out if .b4b27019 is latest testing? **found it under actions. 👊💪 |
I'm not sure this is complete, b/c Duo is still asking for an update. I can login without 2FA, which is obviously wrong. I didnt install Duo until after the old Duo was shut down for new users. |
That is not something we can do, you need to configure that via your duo admin panel. |
Works for me. There is something on duo admin side that you have to do. There’s also an option to use new universal prompt there so make sure that is on. |
…1.32.0@71668d2 by renovate (#25023) This PR contains the following updates: | Package | Update | Change | |---|---|---| | [docker.io/vaultwarden/server](https://togithub.com/dani-garcia/vaultwarden) | minor | `1.31.0` -> `1.32.0` | --- > [!WARNING] > Some dependencies could not be looked up. Check the Dependency Dashboard for more information. --- ### Release Notes <details> <summary>dani-garcia/vaultwarden (docker.io/vaultwarden/server)</summary> ### [`v1.32.0`](https://togithub.com/dani-garcia/vaultwarden/releases/tag/1.32.0) [Compare Source](https://togithub.com/dani-garcia/vaultwarden/compare/1.31.0...1.32.0) #### Security Fixes This release has several CVE Reports fixed and we recommend everybody to update to the latest version as soon as possible. - [CVE-2024-39924](https://cve.mitre.org/cgi-bin/cvename.cgi?name=CVE-2024-39924) Fixed via [#​4715](https://togithub.com/dani-garcia/vaultwarden/issues/4715) - [CVE-2024-39925](https://cve.mitre.org/cgi-bin/cvename.cgi?name=CVE-2024-39925) Fixed via [#​4837](https://togithub.com/dani-garcia/vaultwarden/issues/4837) - [CVE-2024-39926](https://cve.mitre.org/cgi-bin/cvename.cgi?name=CVE-2024-39926) Fixed via [#​4737](https://togithub.com/dani-garcia/vaultwarden/issues/4737) #### Other changes - Updated web-vault to v2024.6.2 - Fixed issues with password reset enrollment by rolling back a web-vault commit #### What's Changed - use a custom plan of enterprise tier to fix limits by [@​stefan0xC](https://togithub.com/stefan0xC) in [https://github.com/dani-garcia/vaultwarden/pull/4726](https://togithub.com/dani-garcia/vaultwarden/pull/4726) - chore: Dockerfile to Remove port 3012 by [@​calvin-li-developer](https://togithub.com/calvin-li-developer) in [https://github.com/dani-garcia/vaultwarden/pull/4725](https://togithub.com/dani-garcia/vaultwarden/pull/4725) - Fix bug where secureNotes is empty by [@​cobyge](https://togithub.com/cobyge) in [https://github.com/dani-garcia/vaultwarden/pull/4730](https://togithub.com/dani-garcia/vaultwarden/pull/4730) - Improved HTTP client by [@​dani-garcia](https://togithub.com/dani-garcia) in [https://github.com/dani-garcia/vaultwarden/pull/4740](https://togithub.com/dani-garcia/vaultwarden/pull/4740) - Update admin interface by [@​BlackDex](https://togithub.com/BlackDex) in [https://github.com/dani-garcia/vaultwarden/pull/4737](https://togithub.com/dani-garcia/vaultwarden/pull/4737) - Fix for RSA Keys which are read only by [@​BlackDex](https://togithub.com/BlackDex) in [https://github.com/dani-garcia/vaultwarden/pull/4744](https://togithub.com/dani-garcia/vaultwarden/pull/4744) - Fix Email 2FA login on native app by [@​BlackDex](https://togithub.com/BlackDex) in [https://github.com/dani-garcia/vaultwarden/pull/4762](https://togithub.com/dani-garcia/vaultwarden/pull/4762) - Update crates & fix crate vulnerability by [@​dfunkt](https://togithub.com/dfunkt) in [https://github.com/dani-garcia/vaultwarden/pull/4771](https://togithub.com/dani-garcia/vaultwarden/pull/4771) - Fix Dockerfile linter warnings by [@​dfunkt](https://togithub.com/dfunkt) in [https://github.com/dani-garcia/vaultwarden/pull/4763](https://togithub.com/dani-garcia/vaultwarden/pull/4763) - allow re-invitations of existing users by [@​stefan0xC](https://togithub.com/stefan0xC) in [https://github.com/dani-garcia/vaultwarden/pull/4768](https://togithub.com/dani-garcia/vaultwarden/pull/4768) - Allow to override log level for specific target by [@​Timshel](https://togithub.com/Timshel) in [https://github.com/dani-garcia/vaultwarden/pull/4305](https://togithub.com/dani-garcia/vaultwarden/pull/4305) - Add support for MFA with Duo's Universal Prompt by [@​0x0fbc](https://togithub.com/0x0fbc) in [https://github.com/dani-garcia/vaultwarden/pull/4637](https://togithub.com/dani-garcia/vaultwarden/pull/4637) - Allow to increase the note size to 100\_000 by [@​BlackDex](https://togithub.com/BlackDex) in [https://github.com/dani-garcia/vaultwarden/pull/4772](https://togithub.com/dani-garcia/vaultwarden/pull/4772) - Update Rust, Crates and GHA by [@​BlackDex](https://togithub.com/BlackDex) in [https://github.com/dani-garcia/vaultwarden/pull/4783](https://togithub.com/dani-garcia/vaultwarden/pull/4783) - Duo: use the formatted db email by [@​Timshel](https://togithub.com/Timshel) in [https://github.com/dani-garcia/vaultwarden/pull/4779](https://togithub.com/dani-garcia/vaultwarden/pull/4779) - Update rust-toolchain.toml to 1.80.0 by [@​dfunkt](https://togithub.com/dfunkt) in [https://github.com/dani-garcia/vaultwarden/pull/4784](https://togithub.com/dani-garcia/vaultwarden/pull/4784) - fix issue with adding ciphers to organizations on native ios app by [@​stefan0xC](https://togithub.com/stefan0xC) in [https://github.com/dani-garcia/vaultwarden/pull/4800](https://togithub.com/dani-garcia/vaultwarden/pull/4800) - Rewrite the Push Notifications section in the configuration template by [@​dfunkt](https://togithub.com/dfunkt) in [https://github.com/dani-garcia/vaultwarden/pull/4805](https://togithub.com/dani-garcia/vaultwarden/pull/4805) - Secure send file uploads by [@​BlackDex](https://togithub.com/BlackDex) in [https://github.com/dani-garcia/vaultwarden/pull/4810](https://togithub.com/dani-garcia/vaultwarden/pull/4810) - make access_all optional by [@​stefan0xC](https://togithub.com/stefan0xC) in [https://github.com/dani-garcia/vaultwarden/pull/4812](https://togithub.com/dani-garcia/vaultwarden/pull/4812) - Remove lowercase conversion for featureStates by [@​dfunkt](https://togithub.com/dfunkt) in [https://github.com/dani-garcia/vaultwarden/pull/4820](https://togithub.com/dani-garcia/vaultwarden/pull/4820) - Fix mail::send_incomplete\_2fa_login panic issue by [@​dfunkt](https://togithub.com/dfunkt) in [https://github.com/dani-garcia/vaultwarden/pull/4792](https://togithub.com/dani-garcia/vaultwarden/pull/4792) - Update crates, web-vault and fixes by [@​BlackDex](https://togithub.com/BlackDex) in [https://github.com/dani-garcia/vaultwarden/pull/4823](https://togithub.com/dani-garcia/vaultwarden/pull/4823) - Updated web-vault to v2024.6.2b by [@​BlackDex](https://togithub.com/BlackDex) in [https://github.com/dani-garcia/vaultwarden/pull/4826](https://togithub.com/dani-garcia/vaultwarden/pull/4826) - Update Rust to 1.80.1 by [@​dfunkt](https://togithub.com/dfunkt) in [https://github.com/dani-garcia/vaultwarden/pull/4831](https://togithub.com/dani-garcia/vaultwarden/pull/4831) - Fix data disclosure on organization endpoints by [@​BlackDex](https://togithub.com/BlackDex) in [https://github.com/dani-garcia/vaultwarden/pull/4837](https://togithub.com/dani-garcia/vaultwarden/pull/4837) #### New Contributors - [@​cobyge](https://togithub.com/cobyge) made their first contribution in [https://github.com/dani-garcia/vaultwarden/pull/4730](https://togithub.com/dani-garcia/vaultwarden/pull/4730) - [@​0x0fbc](https://togithub.com/0x0fbc) made their first contribution in [https://github.com/dani-garcia/vaultwarden/pull/4637](https://togithub.com/dani-garcia/vaultwarden/pull/4637) **Full Changelog**: dani-garcia/vaultwarden@1.31.0...1.32.0 </details> --- ### Configuration 📅 **Schedule**: Branch creation - At any time (no schedule defined), Automerge - At any time (no schedule defined). 🚦 **Automerge**: Enabled. ♻ **Rebasing**: Whenever PR becomes conflicted, or you tick the rebase/retry checkbox. 🔕 **Ignore**: Close this PR and you won't be reminded about this update again. --- - [ ] <!-- rebase-check -->If you want to rebase/retry this PR, check this box --- This PR has been generated by [Renovate Bot](https://togithub.com/renovatebot/renovate). <!--renovate-debug:eyJjcmVhdGVkSW5WZXIiOiIzOC4yNS4wIiwidXBkYXRlZEluVmVyIjoiMzguMjUuMCIsInRhcmdldEJyYW5jaCI6Im1hc3RlciIsImxhYmVscyI6WyJhdXRvbWVyZ2UiLCJ1cGRhdGUvZG9ja2VyL2dlbmVyYWwvbm9uLW1ham9yIl19-->
[](https://renovatebot.com) This PR contains the following updates: | Package | Update | Change | |---|---|---| | [vaultwarden/server](https://togithub.com/dani-garcia/vaultwarden) | minor | `1.31.0` -> `1.32.0` | --- ### Release Notes <details> <summary>dani-garcia/vaultwarden (vaultwarden/server)</summary> ### [`v1.32.0`](https://togithub.com/dani-garcia/vaultwarden/releases/tag/1.32.0) [Compare Source](https://togithub.com/dani-garcia/vaultwarden/compare/1.31.0...1.32.0) #### Security Fixes This release has several CVE Reports fixed and we recommend everybody to update to the latest version as soon as possible. - [CVE-2024-39924](https://cve.mitre.org/cgi-bin/cvename.cgi?name=CVE-2024-39924) Fixed via [#​4715](https://togithub.com/dani-garcia/vaultwarden/issues/4715) - [CVE-2024-39925](https://cve.mitre.org/cgi-bin/cvename.cgi?name=CVE-2024-39925) Fixed via [#​4837](https://togithub.com/dani-garcia/vaultwarden/issues/4837) - [CVE-2024-39926](https://cve.mitre.org/cgi-bin/cvename.cgi?name=CVE-2024-39926) Fixed via [#​4737](https://togithub.com/dani-garcia/vaultwarden/issues/4737) #### Other changes - Updated web-vault to v2024.6.2 - Fixed issues with password reset enrollment by rolling back a web-vault commit #### What's Changed - use a custom plan of enterprise tier to fix limits by [@​stefan0xC](https://togithub.com/stefan0xC) in [https://github.com/dani-garcia/vaultwarden/pull/4726](https://togithub.com/dani-garcia/vaultwarden/pull/4726) - chore: Dockerfile to Remove port 3012 by [@​calvin-li-developer](https://togithub.com/calvin-li-developer) in [https://github.com/dani-garcia/vaultwarden/pull/4725](https://togithub.com/dani-garcia/vaultwarden/pull/4725) - Fix bug where secureNotes is empty by [@​cobyge](https://togithub.com/cobyge) in [https://github.com/dani-garcia/vaultwarden/pull/4730](https://togithub.com/dani-garcia/vaultwarden/pull/4730) - Improved HTTP client by [@​dani-garcia](https://togithub.com/dani-garcia) in [https://github.com/dani-garcia/vaultwarden/pull/4740](https://togithub.com/dani-garcia/vaultwarden/pull/4740) - Update admin interface by [@​BlackDex](https://togithub.com/BlackDex) in [https://github.com/dani-garcia/vaultwarden/pull/4737](https://togithub.com/dani-garcia/vaultwarden/pull/4737) - Fix for RSA Keys which are read only by [@​BlackDex](https://togithub.com/BlackDex) in [https://github.com/dani-garcia/vaultwarden/pull/4744](https://togithub.com/dani-garcia/vaultwarden/pull/4744) - Fix Email 2FA login on native app by [@​BlackDex](https://togithub.com/BlackDex) in [https://github.com/dani-garcia/vaultwarden/pull/4762](https://togithub.com/dani-garcia/vaultwarden/pull/4762) - Update crates & fix crate vulnerability by [@​dfunkt](https://togithub.com/dfunkt) in [https://github.com/dani-garcia/vaultwarden/pull/4771](https://togithub.com/dani-garcia/vaultwarden/pull/4771) - Fix Dockerfile linter warnings by [@​dfunkt](https://togithub.com/dfunkt) in [https://github.com/dani-garcia/vaultwarden/pull/4763](https://togithub.com/dani-garcia/vaultwarden/pull/4763) - allow re-invitations of existing users by [@​stefan0xC](https://togithub.com/stefan0xC) in [https://github.com/dani-garcia/vaultwarden/pull/4768](https://togithub.com/dani-garcia/vaultwarden/pull/4768) - Allow to override log level for specific target by [@​Timshel](https://togithub.com/Timshel) in [https://github.com/dani-garcia/vaultwarden/pull/4305](https://togithub.com/dani-garcia/vaultwarden/pull/4305) - Add support for MFA with Duo's Universal Prompt by [@​0x0fbc](https://togithub.com/0x0fbc) in [https://github.com/dani-garcia/vaultwarden/pull/4637](https://togithub.com/dani-garcia/vaultwarden/pull/4637) - Allow to increase the note size to 100\_000 by [@​BlackDex](https://togithub.com/BlackDex) in [https://github.com/dani-garcia/vaultwarden/pull/4772](https://togithub.com/dani-garcia/vaultwarden/pull/4772) - Update Rust, Crates and GHA by [@​BlackDex](https://togithub.com/BlackDex) in [https://github.com/dani-garcia/vaultwarden/pull/4783](https://togithub.com/dani-garcia/vaultwarden/pull/4783) - Duo: use the formatted db email by [@​Timshel](https://togithub.com/Timshel) in [https://github.com/dani-garcia/vaultwarden/pull/4779](https://togithub.com/dani-garcia/vaultwarden/pull/4779) - Update rust-toolchain.toml to 1.80.0 by [@​dfunkt](https://togithub.com/dfunkt) in [https://github.com/dani-garcia/vaultwarden/pull/4784](https://togithub.com/dani-garcia/vaultwarden/pull/4784) - fix issue with adding ciphers to organizations on native ios app by [@​stefan0xC](https://togithub.com/stefan0xC) in [https://github.com/dani-garcia/vaultwarden/pull/4800](https://togithub.com/dani-garcia/vaultwarden/pull/4800) - Rewrite the Push Notifications section in the configuration template by [@​dfunkt](https://togithub.com/dfunkt) in [https://github.com/dani-garcia/vaultwarden/pull/4805](https://togithub.com/dani-garcia/vaultwarden/pull/4805) - Secure send file uploads by [@​BlackDex](https://togithub.com/BlackDex) in [https://github.com/dani-garcia/vaultwarden/pull/4810](https://togithub.com/dani-garcia/vaultwarden/pull/4810) - make access_all optional by [@​stefan0xC](https://togithub.com/stefan0xC) in [https://github.com/dani-garcia/vaultwarden/pull/4812](https://togithub.com/dani-garcia/vaultwarden/pull/4812) - Remove lowercase conversion for featureStates by [@​dfunkt](https://togithub.com/dfunkt) in [https://github.com/dani-garcia/vaultwarden/pull/4820](https://togithub.com/dani-garcia/vaultwarden/pull/4820) - Fix mail::send_incomplete\_2fa_login panic issue by [@​dfunkt](https://togithub.com/dfunkt) in [https://github.com/dani-garcia/vaultwarden/pull/4792](https://togithub.com/dani-garcia/vaultwarden/pull/4792) - Update crates, web-vault and fixes by [@​BlackDex](https://togithub.com/BlackDex) in [https://github.com/dani-garcia/vaultwarden/pull/4823](https://togithub.com/dani-garcia/vaultwarden/pull/4823) - Updated web-vault to v2024.6.2b by [@​BlackDex](https://togithub.com/BlackDex) in [https://github.com/dani-garcia/vaultwarden/pull/4826](https://togithub.com/dani-garcia/vaultwarden/pull/4826) - Update Rust to 1.80.1 by [@​dfunkt](https://togithub.com/dfunkt) in [https://github.com/dani-garcia/vaultwarden/pull/4831](https://togithub.com/dani-garcia/vaultwarden/pull/4831) - Fix data disclosure on organization endpoints by [@​BlackDex](https://togithub.com/BlackDex) in [https://github.com/dani-garcia/vaultwarden/pull/4837](https://togithub.com/dani-garcia/vaultwarden/pull/4837) #### New Contributors - [@​cobyge](https://togithub.com/cobyge) made their first contribution in [https://github.com/dani-garcia/vaultwarden/pull/4730](https://togithub.com/dani-garcia/vaultwarden/pull/4730) - [@​0x0fbc](https://togithub.com/0x0fbc) made their first contribution in [https://github.com/dani-garcia/vaultwarden/pull/4637](https://togithub.com/dani-garcia/vaultwarden/pull/4637) **Full Changelog**: dani-garcia/vaultwarden@1.31.0...1.32.0 </details> --- ### Configuration 📅 **Schedule**: Branch creation - "after 10pm every weekday,every weekend,before 5am every weekday" in timezone America/New_York, Automerge - At any time (no schedule defined). 🚦 **Automerge**: Enabled. ♻ **Rebasing**: Whenever PR is behind base branch, or you tick the rebase/retry checkbox. 🔕 **Ignore**: Close this PR and you won't be reminded about this update again. --- - [ ] <!-- rebase-check -->If you want to rebase/retry this PR, check this box --- This PR was generated by [Mend Renovate](https://www.mend.io/free-developer-tools/renovate/). View the [repository job log](https://developer.mend.io/github/NorkzYT/Wolflith). <!--renovate-debug:eyJjcmVhdGVkSW5WZXIiOiIzOC4yNi4xIiwidXBkYXRlZEluVmVyIjoiMzguMjYuMSIsInRhcmdldEJyYW5jaCI6InN0YWdpbmciLCJsYWJlbHMiOlsiZGVwZW5kZW5jaWVzIiwibWlub3IiLCJyZW5vdmF0ZSJdfQ==--> Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
Overview
This adds support for MFA using Duo's "Universal Prompt".
On March 30th, 2024, the Duo MFA integration that Vaultwarden uses, the 'Traditional Prompt' was end-of-lifed. While Duo's API continues to allow the traditional prompt to be used in some cases, it is entirely unsupported, and it will eventually stop working. Some Vaultwarden users have already begun reporting that their Duo API keys are no longer functional in Vaultwarden. Bitwarden introduced Universal Prompt support in release 2024.2.3 on March 5th.
Duo's Universal Prompt uses the OIDC Authorization Code flow. This flow requires us to pack information about the authentication we want to be prompted for MFA into an authorization request (a JWT signed with a secret key provided by Duo) and send the user to their service, where MFA is performed. The user is then returned to our service with a code that we use to call Duo's API and obtain the result of the MFA. Once we validate the information passed back by the user's client and returned by Duo's service, we can log the user in.
Detailed documentation is located at https://duo.com/docs/oauthapi
The web vault handles receiving the redirection from Duo. It has a 'connector' page responsible for communicating the authorization code back to the user's true client so it can be provided in a call to Vaultwarden's API.
Major additions/changes
Specific Review Items
Beyond what you'd normally check, I'd like to highlight a few decisions I made that you should probably consider in your review.
device_identifier
reported by the client, and hashed to produce the OIDC nonce sent to Duo in the authorization request. Then, thedevice_identifier
reported by the user's client when providing the authorization code is hashed with the saved nonce, and the resulting hash is compared to the OIDC nonce Duo reports when we call their API for the MFA result.get_duo_keys_email
function, which fetches the Duo keys from the database/config for a given user, I set its visibility topub(crate)
and re-used it.Testing
Tested on the following clients; everything is working well:
The only major client issue I found was with the unpackaged AppImage Linux desktop client. When authenticating users on desktop clients, the connector in the web vault opens a
bitwarden://
link, which my system refused to use the AppImage to open. It looks like the AppImage client either doesn't or can't register itself as thex-scheme-handler
forbitwarden
, preventing the redirect connector from handing off to the client. It worked fine with the Flatpak packaged Bitwarden client, so I'm chalking it up to my not configuring the AppImage client correctly.There is also a known issue upstream in the web vault where the redirect connector shows 'successful authentication', does not automatically close, and the JS 'close' button does not work. See bitwarden/clients#8554
I also tested this while running PostgreSQL and MariaDB using the versions packaged in Debian 12; no issues.
Fixes #4529