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

API endpoint /accounts/prelogin to be removed from the official Bitwarden server #190

Closed
dezeroku opened this issue Jun 26, 2024 · 3 comments

Comments

@dezeroku
Copy link
Contributor

bitwarden/server#4206

After this change goes live on the prod instance of Bitwarden all new rbw login calls will likely fail due to our usage of this endpoint.
Information about KDF, iterations, etc. is now returned as part of the /connect/token response and should be parsed from there.

There's only one place where we use this call: https://github.com/doy/rbw/blob/main/src/actions.rs#L31
Probably the best idea here is to first calculate hash of the master password, then perform the login flow and only create the Identity at the end.

It's also a good moment to think about making master_password_hash an Option in call to client.login() (and the Identity struct), it's only really used in the email+password auth flow and isn't needed for SSO/apikey. With this in place we could not ask for master password for these flows when rbw login is run.

@dezeroku dezeroku changed the title API endpoint /accounts/prelogin is deprecated API endpoint /accounts/prelogin to be removed from the official Bitwarden server Jun 26, 2024
@doy
Copy link
Owner

doy commented Jun 26, 2024

from what i can tell from that pr, it doesn't look like the structure of the flow itself is changing, it looks like it's just that the api endpoint for prelogin is moving from the main api to the identity api - am i missing something else? the entire point of the prelogin call is that you can't calculate the password hash ahead of time without knowing the kdf to use, so there's no way to make the initial call to /connect/token without it. this should be easy to fix though.

@doy
Copy link
Owner

doy commented Jun 26, 2024

i think a06655c should be sufficient, let me know if i'm missing anything. thanks for the heads up!

@doy doy closed this as completed Jun 26, 2024
@dezeroku
Copy link
Contributor Author

You're right, it should be enough. Some kind of panic mode kicked in on my end 😅

And yup, this looks like a proper fix, thanks!

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