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

Incorrect URI concatenation with leading slash in BitwardenSecretsClient.cs #49

Open
doublean opened this issue Nov 29, 2024 · 0 comments

Comments

@doublean
Copy link

Description:

I encountered an issue in the BitwardenSecretsClient.cs file of the bitwarden/dotnet-extensions project. The code currently concatenates HTTP requests using the following pattern:

return (await _apiClient.GetFromJsonAsync($"/secrets/{secretId}",
    BitwardenSerializerContext.Default.SecretResponseModel))!;

This causes a problem when the relative URI starts with a slash (/), as it replaces the base URI path instead of appending to it. For example, if the base URI is https://vault.bitwarden.com/api and the relative URI is /secret, the request is incorrectly generated as https://vault.bitwarden.com/secret, which is not the desired result.

Problem:

  • When the relative URI starts with a slash (/), it overwrites the base URI path instead of being appended to it, which results in incorrect API calls.

Proposed Solution:

To resolve this, the relative URI should not start with a slash. Specifically, the code in BitwardenSecretsClient.cs should be updated to remove the leading slash from the relative URI, so that it properly appends to the base URI.

Here’s how the code could be modified:

// Change the relative URI from "/secrets/{secretId}" to "secrets/{secretId}" to avoid overwriting the base path.
return (await _apiClient.GetFromJsonAsync($"secrets/{secretId}",
    BitwardenSerializerContext.Default.SecretResponseModel))!;

This modification ensures that the relative URI is correctly appended to the base URI, regardless of whether the base URI ends with a slash.

Steps to Reproduce:

To reproduce the issue, use the following test code:

Uri apiUrl = new Uri("https://vault.bitwarden.com/api");
Uri path = new Uri("/secret", UriKind.RelativeOrAbsolute);

Uri result = new Uri(apiUrl, path);
Assert.Equal("https://vault.bitwarden.com/api/secret", result.AbsoluteUri);

This test demonstrates how the relative URI with a leading slash (/secret) causes an incorrect request by overwriting the base URI path, rather than appending to it.

Expected Behavior:

The combined URI should be correct, even if the base URI ends with a /. For example, a base URI of https://vault.bitwarden.com/api/ and a relative URI of secret (without the leading slash) should result in https://vault.bitwarden.com/api/secret, not https://vault.bitwarden.com/secret.

Context:

  • Project: bitwarden/dotnet-extensions
  • Affected file: BitwardenSecretsClient.cs
  • Affected functionality: URI concatenation in API calls via _apiClient.GetFromJsonAsync

Thank you for addressing this issue to improve how relative URIs are handled.

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

1 participant