Skip to content

Commit

Permalink
feat(storage): Respect custom endpoint for SignedUrl (#14179)
Browse files Browse the repository at this point in the history
  • Loading branch information
bajajneha27 committed May 24, 2024
1 parent 45ed21b commit 3ce6715
Show file tree
Hide file tree
Showing 11 changed files with 201 additions and 25 deletions.
18 changes: 17 additions & 1 deletion google/cloud/storage/client.cc
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@
#include "google/cloud/internal/make_status.h"
#include "google/cloud/internal/opentelemetry.h"
#include "google/cloud/log.h"
#include "absl/strings/str_split.h"
#include <fstream>
#include <memory>
#include <thread>
Expand Down Expand Up @@ -389,7 +390,7 @@ StatusOr<std::string> Client::SignUrlV2(
std::string signature = curl.MakeEscapedString(encoded).get();

std::ostringstream os;
os << "https://storage.googleapis.com/" << request.bucket_name();
os << Endpoint() << '/' << request.bucket_name();
if (!request.object_name().empty()) {
os << '/' << curl.MakeEscapedString(request.object_name()).get();
}
Expand Down Expand Up @@ -481,6 +482,21 @@ std::string CreateRandomPrefixName(std::string const& prefix) {
"abcdefghijklmnopqrstuvwxyz");
}

std::string Client::Endpoint() const {
return connection_->options().get<RestEndpointOption>();
}

// This can be optimized to not have a lot of string copies.
// But the code is rarely used and not in any critical path.
std::string Client::EndpointAuthority() const {
auto endpoint = Endpoint();
auto endpoint_authority = absl::string_view(endpoint);
if (!absl::ConsumePrefix(&endpoint_authority, "https://")) {
absl::ConsumePrefix(&endpoint_authority, "http://");
}
return std::string(endpoint_authority);
}

namespace internal {

Client ClientImplDetails::CreateWithDecorations(
Expand Down
12 changes: 10 additions & 2 deletions google/cloud/storage/client.h
Original file line number Diff line number Diff line change
Expand Up @@ -2996,7 +2996,8 @@ class Client {
google::cloud::internal::OptionsSpan const span(
SpanOptions(std::forward<Options>(options)...));
internal::V4SignUrlRequest request(std::move(verb), std::move(bucket_name),
std::move(object_name));
std::move(object_name),
EndpointAuthority());
request.set_multiple_options(std::forward<Options>(options)...);
return SignUrlV4(std::move(request));
}
Expand Down Expand Up @@ -3082,7 +3083,8 @@ class Client {
PolicyDocumentV4 document, Options&&... options) {
google::cloud::internal::OptionsSpan const span(
SpanOptions(std::forward<Options>(options)...));
internal::PolicyDocumentV4Request request(std::move(document));
internal::PolicyDocumentV4Request request(std::move(document),
EndpointAuthority());
request.set_multiple_options(std::forward<Options>(options)...);
return SignPolicyDocumentV4(std::move(request));
}
Expand Down Expand Up @@ -3492,6 +3494,12 @@ class Client {
StatusOr<PolicyDocumentV4Result> SignPolicyDocumentV4(
internal::PolicyDocumentV4Request request);

// The configured endpoint, including any scheme and port.
std::string Endpoint() const;

// The hostname:port part of the configured endpoint.
std::string EndpointAuthority() const;

std::shared_ptr<internal::StorageConnection> connection_;
};

Expand Down
20 changes: 17 additions & 3 deletions google/cloud/storage/client_sign_policy_document_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@ using ::google::cloud::internal::CurrentOptions;
using ::google::cloud::storage::testing::canonical_errors::TransientError;
using ::testing::HasSubstr;
using ::testing::Return;
using ::testing::StartsWith;

constexpr char kJsonKeyfileContents[] = R"""({
"type": "service_account",
Expand All @@ -58,9 +59,12 @@ std::string Dec64(std::string const& s) {
return std::string(res.begin(), res.end());
};

Client CreateClientForTest() {
return Client(Options{}.set<UnifiedCredentialsOption>(
MakeServiceAccountCredentials(kJsonKeyfileContents)));
Client CreateClientForTest(
std::string endpoint = "https://storage.googleapis.com") {
return Client(Options{}
.set<UnifiedCredentialsOption>(
MakeServiceAccountCredentials(kJsonKeyfileContents))
.set<RestEndpointOption>(std::move(endpoint)));
}

/**
Expand Down Expand Up @@ -255,6 +259,16 @@ TEST(CreateSignedPolicyDocTest, SignV4VirtualHostname) {
EXPECT_EQ("https://test-bucket.storage.googleapis.com/", actual->url);
}

TEST(CreateSignedPolicyDocTest, SignV4CustomEndpoint) {
auto const custom_endpoint = std::string{"https://storage.mydomain.com"};
auto client = CreateClientForTest(custom_endpoint);
auto actual =
client.GenerateSignedPostPolicyV4(CreatePolicyDocumentV4ForTest());

ASSERT_STATUS_OK(actual);
EXPECT_THAT(actual->url, StartsWith(custom_endpoint));
}

} // namespace
GOOGLE_CLOUD_CPP_INLINE_NAMESPACE_END
} // namespace storage
Expand Down
76 changes: 76 additions & 0 deletions google/cloud/storage/client_sign_url_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@
#include "google/cloud/storage/testing/canonical_errors.h"
#include "google/cloud/storage/testing/client_unit_test.h"
#include "google/cloud/testing_util/status_matchers.h"
#include "google/cloud/universe_domain_options.h"
#include <gmock/gmock.h>

namespace google {
Expand All @@ -28,9 +29,11 @@ namespace {

using ::google::cloud::internal::CurrentOptions;
using ::google::cloud::storage::testing::canonical_errors::TransientError;
using ::google::cloud::testing_util::IsOkAndHolds;
using ::google::cloud::testing_util::StatusIs;
using ::testing::HasSubstr;
using ::testing::Return;
using ::testing::StartsWith;

constexpr char kJsonKeyfileContents[] = R"""({
"type": "service_account",
Expand Down Expand Up @@ -107,6 +110,36 @@ TEST_F(CreateSignedUrlTest, V2SignRemote) {
EXPECT_THAT(*actual, HasSubstr(expected_signed_blob_safe));
}

/// @test Verify that CreateV2SignedUrl() respects the custom endpoint.
TEST_F(CreateSignedUrlTest, V2SignCustomEndpoint) {
auto const custom_endpoint = std::string{"https://storage.mydomain.com"};

Options options = Options{}
.set<UnifiedCredentialsOption>(
MakeServiceAccountCredentials(kJsonKeyfileContents))
.set<RestEndpointOption>(custom_endpoint);
Client client(options);
StatusOr<std::string> actual =
client.CreateV2SignedUrl("GET", "test-bucket", "test-object");
EXPECT_THAT(actual, IsOkAndHolds(StartsWith(custom_endpoint)));
}

/// @test Verify that CreateV2SignedUrl() respects the custom universe domain.
TEST_F(CreateSignedUrlTest, V2SignCustomUniverseDomain) {
auto const custom_ud = std::string{"mydomain.com"};

Options options =
Options{}
.set<UnifiedCredentialsOption>(
MakeServiceAccountCredentials(kJsonKeyfileContents))
.set<google::cloud::internal::UniverseDomainOption>(custom_ud);
Client client(options);

StatusOr<std::string> actual =
client.CreateV2SignedUrl("GET", "test-bucket", "test-object");
EXPECT_THAT(actual, IsOkAndHolds(HasSubstr(custom_ud)));
}

// This is a placeholder service account JSON file that is inactive. It's fine
// for it to be public.
constexpr char kJsonKeyfileContentsForV4[] = R"""({
Expand Down Expand Up @@ -231,6 +264,49 @@ TEST_F(CreateSignedUrlTest, V4SignRemote) {
EXPECT_THAT(*actual, HasSubstr(expected_signed_blob_hex));
}

/// @test Verify that CreateV4SignedUrl() respects the custom endpoint.
TEST_F(CreateSignedUrlTest, V4SignCustomEndpoint) {
auto const custom_endpoint = std::string{"https://storage.mydomain.com"};
std::string const bucket_name = "test-bucket";
std::string const object_name = "test-object";
std::string const date = "2019-02-01T09:00:00Z";
auto const valid_for = std::chrono::seconds(10);

Options options =
Options{}
.set<UnifiedCredentialsOption>(
MakeServiceAccountCredentials(kJsonKeyfileContentsForV4))
.set<RestEndpointOption>(custom_endpoint);
Client client(options);

auto actual = client.CreateV4SignedUrl(
"GET", bucket_name, object_name,
SignedUrlTimestamp(google::cloud::internal::ParseRfc3339(date).value()),
SignedUrlDuration(valid_for));
EXPECT_THAT(actual, IsOkAndHolds(StartsWith(custom_endpoint)));
}

/// @test Verify that CreateV4SignUrl() respects the custom universe domain.
TEST_F(CreateSignedUrlTest, V4SignCustomUniverseDomain) {
auto const custom_ud = std::string{"mydomain.com"};
std::string const bucket_name = "test-bucket";
std::string const object_name = "test-object";
std::string const date = "2019-02-01T09:00:00Z";
auto const valid_for = std::chrono::seconds(10);

Options options =
Options{}
.set<UnifiedCredentialsOption>(
MakeServiceAccountCredentials(kJsonKeyfileContentsForV4))
.set<google::cloud::internal::UniverseDomainOption>(custom_ud);
Client client(options);
auto actual = client.CreateV4SignedUrl(
"GET", bucket_name, object_name,
SignedUrlTimestamp(google::cloud::internal::ParseRfc3339(date).value()),
SignedUrlDuration(valid_for));
EXPECT_THAT(actual, IsOkAndHolds(HasSubstr(custom_ud)));
}

TEST_F(CreateSignedUrlTest, V4SignRemoteNoSigningEmail) {
EXPECT_CALL(*mock_, SignBlob).Times(0);
auto client = ClientForMock();
Expand Down
11 changes: 6 additions & 5 deletions google/cloud/storage/internal/policy_document_request.cc
Original file line number Diff line number Diff line change
Expand Up @@ -275,11 +275,11 @@ std::string PolicyDocumentV4Request::Url() const {
return scheme_ + "://" + *bucket_bound_domain_ + "/";
}
if (virtual_host_name_) {
return scheme_ + "://" + policy_document().bucket +
".storage.googleapis.com/";
return scheme_ + "://" + policy_document().bucket + "." +
endpoint_authority_ + "/";
}
return scheme_ + "://storage.googleapis.com/" + policy_document().bucket +
"/";
return scheme_ + "://" + endpoint_authority_ + "/" +
policy_document().bucket + "/";
}

std::string PolicyDocumentV4Request::Credentials() const {
Expand Down Expand Up @@ -342,7 +342,8 @@ std::map<std::string, std::string> PolicyDocumentV4Request::RequiredFormFields()
}

std::ostream& operator<<(std::ostream& os, PolicyDocumentV4Request const& r) {
return os << "PolicyDocumentRequest={" << r.StringToSign() << "}";
return os << "PolicyDocumentRequest={endpoint_authority="
<< r.endpoint_authority() << "," << r.StringToSign() << "}";
}

} // namespace internal
Expand Down
13 changes: 12 additions & 1 deletion google/cloud/storage/internal/policy_document_request.h
Original file line number Diff line number Diff line change
Expand Up @@ -104,7 +104,15 @@ std::ostream& operator<<(std::ostream& os, PolicyDocumentRequest const& r);

class PolicyDocumentV4Request {
public:
PolicyDocumentV4Request() : scheme_("https") {}
PolicyDocumentV4Request()
: PolicyDocumentV4Request("storage.googleapis.com") {}
explicit PolicyDocumentV4Request(std::string endpoint_authority)
: scheme_("https"), endpoint_authority_(std::move(endpoint_authority)) {}
PolicyDocumentV4Request(PolicyDocumentV4 document,
std::string endpoint_authority)
: PolicyDocumentV4Request(std::move(endpoint_authority)) {
document_ = std::move(document);
}
explicit PolicyDocumentV4Request(PolicyDocumentV4 document)
: PolicyDocumentV4Request() {
document_ = std::move(document);
Expand All @@ -125,6 +133,8 @@ class PolicyDocumentV4Request {
return signing_account_delegates_;
}

std::string endpoint_authority() const { return endpoint_authority_; }

void SetOption(SigningAccount const& o) { signing_account_ = o; }

void SetOption(SigningAccountDelegates const& o) {
Expand Down Expand Up @@ -191,6 +201,7 @@ class PolicyDocumentV4Request {
absl::optional<std::string> bucket_bound_domain_;
std::string scheme_;
bool virtual_host_name_{false};
std::string endpoint_authority_;
};

std::ostream& operator<<(std::ostream& os, PolicyDocumentV4Request const& r);
Expand Down
25 changes: 24 additions & 1 deletion google/cloud/storage/internal/policy_document_request_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ namespace {
using ::google::cloud::testing_util::IsOkAndHolds;
using ::google::cloud::testing_util::StatusIs;
using ::testing::ElementsAre;
using ::testing::StartsWith;

TEST(PolicyDocumentRequest, SigningAccount) {
PolicyDocumentRequest request;
Expand Down Expand Up @@ -119,7 +120,8 @@ TEST(PolicyDocumentV4Request, Printing) {
std::stringstream stream;
stream << req;
EXPECT_EQ(
"PolicyDocumentRequest={{\"conditions\":[{\"bucket\":\"test-bucket\"},{"
"PolicyDocumentRequest={endpoint_authority=storage.googleapis.com,"
"{\"conditions\":[{\"bucket\":\"test-bucket\"},{"
"\"key\":\"test-object\"},{\"x-goog-date\":\"20100616T111111Z\"},{\"x-"
"goog-credential\":\"/20100616/auto/storage/"
"goog4_request\"},{\"x-goog-algorithm\":\"GOOG4-RSA-SHA256\"}],"
Expand Down Expand Up @@ -153,6 +155,27 @@ TEST(PolicyDocumentV4Request, RequiredFormFields) {
EXPECT_EQ(expected_fields, req.RequiredFormFields());
}

TEST(PolicyDocumentV4Request, Url) {
PolicyDocumentV4 doc;
doc.bucket = "test-bucket";
auto const custom_endpoint_authority = std::string{"storage.mydomain.com"};
PolicyDocumentV4Request request(doc, custom_endpoint_authority);
EXPECT_THAT(request.Url(),
StartsWith("https://" + custom_endpoint_authority));
}

TEST(PolicyDocumentV4Request, UrlWithVirtualHostName) {
PolicyDocumentV4 doc;
doc.bucket = "test-bucket";
auto const custom_endpoint_authority = std::string{"storage.mydomain.com"};
PolicyDocumentV4Request request(doc, custom_endpoint_authority);
request.SetOption(VirtualHostname(true));

auto const expected_url = std::string{"https://" + doc.bucket + "." +
custom_endpoint_authority + "/"};
EXPECT_EQ(request.Url(), expected_url);
}

} // namespace
} // namespace internal
GOOGLE_CLOUD_CPP_INLINE_NAMESPACE_END
Expand Down
10 changes: 6 additions & 4 deletions google/cloud/storage/internal/signed_url_requests.cc
Original file line number Diff line number Diff line change
Expand Up @@ -100,7 +100,8 @@ std::string V2SignUrlRequest::StringToSign() const {
}

std::ostream& operator<<(std::ostream& os, V2SignUrlRequest const& r) {
return os << "SingUrlRequest={" << r.StringToSign() << "}";
return os << "SingUrlRequest={" << r.endpoint_authority() << ","
<< r.StringToSign() << "}";
}

namespace {
Expand Down Expand Up @@ -235,13 +236,14 @@ Status V4SignUrlRequest::Validate() {
}

std::string V4SignUrlRequest::Hostname() {
auto endpoint_authority = common_request_.endpoint_authority();
if (virtual_host_name_) {
return common_request_.bucket_name() + ".storage.googleapis.com";
return common_request_.bucket_name() + "." + endpoint_authority;
}
if (domain_named_bucket_) {
return *domain_named_bucket_;
}
return "storage.googleapis.com";
return endpoint_authority;
}

std::string V4SignUrlRequest::HostnameWithBucket() {
Expand Down Expand Up @@ -318,7 +320,7 @@ std::string V4SignUrlRequest::PayloadHashValue() const {
}

std::ostream& operator<<(std::ostream& os, V4SignUrlRequest const& r) {
return os << "V4SignUrlRequest={"
return os << "V4SignUrlRequest={" << r.endpoint_authority() << ","
<< r.CanonicalRequest("placeholder-client-id") << ","
<< r.StringToSign("placeholder-client-id") << "}";
}
Expand Down

0 comments on commit 3ce6715

Please sign in to comment.