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::core::ciphers::put_cipher_share_selected does not use cipher_ids #4105

Closed
zacknewman opened this issue Nov 26, 2023 · 3 comments · Fixed by #4143
Closed

api::core::ciphers::put_cipher_share_selected does not use cipher_ids #4105

zacknewman opened this issue Nov 26, 2023 · 3 comments · Fixed by #4143
Labels
enhancement New feature or request

Comments

@zacknewman
Copy link
Contributor

This may not be a bug, but it could be if cipher_ids is actually important. Anyway, cipher_ids in api::core::ciphers::put_cipher_share_selected is never read from. Is this accidental? What's the point of pushing IDs into it without ever reading the IDs?

Possible fix if that variable is unimportant:

#[put("/ciphers/share", data = "<data>")]
async fn put_cipher_share_selected(
    data: JsonUpcase<ShareSelectedCipherData>,
    headers: Headers,
    mut conn: DbConn,
    nt: Notify<'_>,
) -> EmptyResult {
    let mut data: ShareSelectedCipherData = data.into_inner().data;

    if data.Ciphers.is_empty() {
        err!("You must select at least one cipher.")
    }

    if data.CollectionIds.is_empty() {
        err!("You must select at least one collection.")
    }

    for cipher in data.Ciphers.iter() {
        if cipher.Id.is_none() {
            err!("Request missing ids field");
        }
    }

    while let Some(cipher) = data.Ciphers.pop() {
        let mut shared_cipher_data = ShareCipherData {
            Cipher: cipher,
            CollectionIds: data.CollectionIds.clone(),
        };

        match shared_cipher_data.Cipher.Id.take() {
            Some(id) => share_cipher_by_uuid(&id, shared_cipher_data, &headers, &mut conn, &nt).await?,
            None => err!("Request missing ids field"),
        };
    }
    Ok(())
}
@BlackDex
Copy link
Collaborator

Looks like a leftover indeed. I'll check it out further but i think you are right.

@BlackDex
Copy link
Collaborator

btw strange that clippy doesn't find this. It should see that it's never read after written.

@zacknewman
Copy link
Contributor Author

zacknewman commented Nov 27, 2023 via email

BlackDex added a commit to BlackDex/vaultwarden that referenced this issue Dec 6, 2023
During some refactoring this seems to be overlooked.
This variable gets filled but isn't used at all afterwards.

Fixes dani-garcia#4105
@BlackDex BlackDex added the enhancement New feature or request label Dec 6, 2023
dani-garcia pushed a commit that referenced this issue Dec 9, 2023
* Fix BWDC when re-run with cleared cache

Using the BWDC with a cleared cache caused invited users to be converted
to accepted users.

The problem was a wrong check for the `restore` function.

Fixes #4114

* Remove useless variable

During some refactoring this seems to be overlooked.
This variable gets filled but isn't used at all afterwards.

Fixes #4105

* Check some `.git` paths to force a rebuild

When a checked-out repo switches to a specific tag, and that tag does
not have anything else changed in the files except the tag, it could
happen that the build process doesn't see any changes, while it could be
that the version string needs to be different.

This commit ensures that if some specific paths are changed within the
.git directory, cargo will be triggered to rebuild.

Fixes #4087

* Do not delete dir on file delete

Previously during a `delete_file` check we also tried to delete the
parent directory and ignored all errors, like not being empty for
example.

Since this function is called `delete_file` and does not mention
anything in regards to a directory i have removed that code and it will
now only delete the file and leave the rest as-is.

If this somehow is still needed or wanted, which i do not think we want,
then we should create a new function.

Fixes #4081

* Fix healthcheck when using an ENV file

If someone is using a `.env` file or configured the `ENV_FILE` variable
to use that as it's configuration, this was missed by the healthcheck.

So, `DOMAIN` and `ROCKET_TLS` were not seen, and not used in these cases.

This commit fixes this by checking for this file and if it exists, then
it will load those variables first.

Fixes #4112

* Add missing route

While there was a function and a derive, this endpoint wasn't part of
the routes. Since Bitwarden does have this endpoint ill add the route
instead of deleting it.

Fixes #4076
Fixes #4144

* Update crates to update the openssl crate

Because of a bug in the openssl-sys crate we pinned the version to an
older version. This issue has been fixed and was released 2 days ago.

This commit updates the openssl crates including others.
This should also fix the issues with building Vaultwarden using newer
versions of LibreSSL.

Fixes #4051
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants