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

fix(api): use accepted value for "Bitwarden-Client-Name" header #219

Closed
wants to merge 7 commits into from

Conversation

davla
Copy link
Contributor

@davla davla commented Dec 1, 2024

As mentioned in #175, the official Bitwarden server needs some extra HTTP headers to accept authentication requests.

Back then the value in the headers didn't matter, but apparently the time has come when it does. I keep receiving 429 HTTP responses with the header "Bitwarden-Client-Name: rbw".

Changing the value of the header to "cli" solves the issue. "cli" seems to be the most appropriate out of the list of values the official server accepts.

During my investigation I also ran across the Device-Type header, that is added by the official Bitwarden SDK. I see it very likely that it'll become mandatory in the future, so I added it.

@m0ar
Copy link

m0ar commented Dec 13, 2024

@doy any change you could have a look at this? 🙏 💘

rbw packages seems broken atm: #218

@ymatsiuk
Copy link

For all of my nixos fellows waiting for the next release:

          rbw = prev.rbw.overrideAttrs (_: {
            src = prev.fetchFromGitHub {
              owner = "davla";
              repo = "rbw";
              rev = "fix/client-name-header";
              sha256 = "sha256-Sgs+qjKdtS5i7zF86TLSZMVKTDoeYhIgKEwjUUXw/cc=";
            };
            cargoDeps = prev.rustPlatform.importCargoLock {
              lockFile = (
                prev.fetchurl {
                  url = "https://raw.githubusercontent.com/davla/rbw/dd6b65427de3a4b38d27350d8ad7ebacb29e97ff/Cargo.lock";
                  hash = "sha256-bAELLBb0x0BOGPMLBRX/s0qxqN8XOxUW9OUa55WjeaA=";
                }
              );
              allowBuiltinFetchGit = true;
            };
          });

Even though this PR addresses the issue and works like a charm (thanks @davla 🙇‍♂️ ) IMHO the solution should look slightly differently -> my thoughts

@adrianschlatter
Copy link

This PR solves #218 when I drop commit 1de97ec.

With that commit, I get a compile error on ubuntu in WSL2:

» cargo install --locked -Znext-lockfile-bump --path rbw
  Installing rbw v1.12.1 (/home/adrian/projects/rbw)
    Updating crates.io index
   Compiling futures-core v0.3.31
   Compiling futures-sink v0.3.31
.
.
.
   Compiling axum v0.7.5
   Compiling rbw v1.12.1 (/home/adrian/projects/rbw)
error[E0658]: use of unstable library feature 'result_option_inspect'
    --> src/bin/rbw/commands.rs:1646:31
     |
1646 |     crate::actions::version().inspect_err(|_e| {
     |                               ^^^^^^^^^^^
     |
     = note: see issue #91345 <https://github.com/rust-lang/rust/issues/91345> for more information

For more information about this error, try `rustc --explain E0658`.
error: could not compile `rbw` (bin "rbw") due to previous error
warning: build failed, waiting for other jobs to finish...
error: failed to compile `rbw v1.12.1 (/home/adrian/projects/rbw)`, intermediate artifacts can be found at `/home/adrian/projects/rbw/target`.
To reuse those artifacts with a future compilation, set the environment variable `CARGO_TARGET_DIR` to that path.
> uname -a
Linux ######## 5.15.167.4-microsoft-standard-WSL2 #1 SMP Tue Nov 5 00:21:55 UTC 2024 x86_64 x86_64 x86_64 GNU/Linux

@saghm
Copy link

saghm commented Dec 14, 2024

@adrianschlatter I don't get that error when I try compiling it. I'm not super familiar with WSL, but my understanding is that it defaults to Ubuntu, which as a non-rolling distro sometimes doesn't have the latest versions of certain packages. What version of rustc are you using? You can find this out by running rustc --version.

From using cargo-msrv, it seems like 1.78 is the lowest version that will build this PR successfully. If you have something lower than that, you'll need to get something newer in order to compile the PR. Using rustup to manage Rust installations is considered best practice, so if you're on Ubuntu, you can install the apt pacakge for it and then run rustup default stable to get the latest stable version of the toolchain and have it configured to be used by default (i.e. when you invoke cargo or rustc without specifying any other toolchain). You might also need to update your shell config to source ~/.cargo/env; I'm not sure whether the apt package provides shims somewhere by default like /usr/bin, but if not, sourcing that file will ensure that the toolchain binaries will be available on your PATH. Once you have that set up, running rustup update will let you update the stable toolchain whenever you want to in the future; new minor versions of the Rust toolchain come out on a six-week cadence, which is more often than most other languages, so being able to update with rustup is super helpful!

@adrianschlatter
Copy link

Thanks @saghm: It compiles with rustc 1.83.0.

@saghm
Copy link

saghm commented Dec 16, 2024

Since it's not clear to me when this upgrade will get merged, and then it might take some extra time for the new package to get released, I made an AUR package in case any other Arch users want to get this update in a way that doesn't require manually building/installing it: https://aur.archlinux.org/packages/rbw-client-header-fix

@doy
Copy link
Owner

doy commented Dec 27, 2024

applied, thanks! (and sorry for the delay here!)

@doy doy closed this Dec 27, 2024
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

Successfully merging this pull request may close these issues.

6 participants