Skip to content

Commit

Permalink
Add support for disabling CUP with a script parameter
Browse files Browse the repository at this point in the history
Bug: b/231699389
Change-Id: I9214e9b67a3c327217d300675d3be9f3377dcaa8
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3629421
Commit-Queue: Sergio Vila Prado <sergiovila@google.com>
Reviewed-by: Clemens Arbesser <arbesser@google.com>
Cr-Commit-Position: refs/heads/main@{#1002551}
  • Loading branch information
Sergio-Vila authored and Chromium LUCI CQ committed May 12, 2022
1 parent 471d8aa commit 9cb0434
Show file tree
Hide file tree
Showing 17 changed files with 88 additions and 1 deletion.
4 changes: 4 additions & 0 deletions components/autofill_assistant/browser/controller.cc
Original file line number Diff line number Diff line change
Expand Up @@ -766,6 +766,10 @@ void Controller::InitFromParameters() {
}

user_model_.SetCurrentURL(GetCurrentURL());

GetService()->SetDisableRpcSigning(
trigger_context_->GetScriptParameters().GetDisableRpcSigning().value_or(
false));
}

void Controller::Track(std::unique_ptr<TriggerContext> trigger_context,
Expand Down
7 changes: 7 additions & 0 deletions components/autofill_assistant/browser/script_parameters.cc
Original file line number Diff line number Diff line change
Expand Up @@ -107,6 +107,9 @@ const char kSourceParameterName[] = "SOURCE";
// Parameter to specify experiments.
const char kExperimentsParameterName[] = "EXPERIMENT_IDS";

// Parameter to disable CUP RPC signing. Intended for internal use only.
const char kDisableRpcSigningParamaterName[] = "DISABLE_RPC_SIGNING";

// The list of non sensitive script parameters that client requests are allowed
// to send to the backend i.e., they do not require explicit approval in the
// autofill-assistant onboarding. Even so, please always reach out to Chrome
Expand Down Expand Up @@ -277,6 +280,10 @@ std::vector<std::string> ScriptParameters::GetExperiments() const {
base::SplitResult::SPLIT_WANT_NONEMPTY);
}

absl::optional<bool> ScriptParameters::GetDisableRpcSigning() const {
return GetTypedParameter<bool>(parameters_, kDisableRpcSigningParamaterName);
}

absl::optional<bool> ScriptParameters::GetDetailsShowInitial() const {
return GetTypedParameter<bool>(parameters_, kDetailsShowInitialParameterName);
}
Expand Down
1 change: 1 addition & 0 deletions components/autofill_assistant/browser/script_parameters.h
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,7 @@ class ScriptParameters {
absl::optional<int> GetCaller() const;
absl::optional<int> GetSource() const;
std::vector<std::string> GetExperiments() const;
absl::optional<bool> GetDisableRpcSigning() const;

// Details parameters.
absl::optional<bool> GetDetailsShowInitial() const;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -108,6 +108,7 @@ TEST(ScriptParametersTest, SpecialScriptParameters) {
{"CALLER", "3"},
{"SOURCE", "4"},
{"EXPERIMENT_IDS", "123,456,789"},
{"DISABLE_RPC_SIGNING", "true"},
{"DETAILS_SHOW_INITIAL", "true"},
{"DETAILS_TITLE", "title"},
{"DETAILS_DESCRIPTION_LINE_1", "line1"},
Expand All @@ -134,6 +135,7 @@ TEST(ScriptParametersTest, SpecialScriptParameters) {
EXPECT_THAT(
parameters.GetExperiments(),
UnorderedElementsAreArray(std::vector<std::string>{"123", "456", "789"}));
EXPECT_THAT(parameters.GetDisableRpcSigning(), Eq(true));
EXPECT_THAT(parameters.GetDetailsShowInitial(), Eq(true));
EXPECT_THAT(parameters.GetDetailsTitle(), Eq("title"));
EXPECT_THAT(parameters.GetDetailsDescriptionLine1(), Eq("line1"));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -53,4 +53,6 @@ void JavaServiceRequestSender::OnResponse(
std::move(callback_).Run(http_status, response, ResponseInfo{});
}

void JavaServiceRequestSender::SetDisableRpcSigning(bool disable_rpc_signing) {}

} // namespace autofill_assistant
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,8 @@ class JavaServiceRequestSender : public ServiceRequestSender {
jint http_status,
const base::android::JavaParamRef<jbyteArray>& jresponse);

void SetDisableRpcSigning(bool disable_rpc_signing) override;

private:
ResponseCallback callback_;
base::android::ScopedJavaGlobalRef<jobject> jservice_request_sender_;
Expand Down
4 changes: 4 additions & 0 deletions components/autofill_assistant/browser/service/mock_service.h
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,10 @@ class MockService : public Service {
const UserData* user_data,
ServiceRequestSender::ResponseCallback callback),
(override));
MOCK_METHOD(void,
SetDisableRpcSigning,
(bool disable_rpc_signing),
(override));
};

} // namespace autofill_assistant
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,11 @@ class MockServiceRequestSender : public ServiceRequestSender {
const std::string& request_body,
ResponseCallback& callback,
RpcType rpc_type));

MOCK_METHOD(void,
SetDisableRpcSigning,
(bool disable_rpc_signing),
(override));
};

} // namespace autofill_assistant
Expand Down
2 changes: 2 additions & 0 deletions components/autofill_assistant/browser/service/service.h
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,8 @@ class Service {
const UserData* user_data,
ServiceRequestSender::ResponseCallback callback) = 0;

virtual void SetDisableRpcSigning(bool disable_rpc_signing) {}

protected:
Service() = default;
};
Expand Down
4 changes: 4 additions & 0 deletions components/autofill_assistant/browser/service/service_impl.cc
Original file line number Diff line number Diff line change
Expand Up @@ -183,6 +183,10 @@ void ServiceImpl::GetUserData(const CollectUserDataOptions& options,
preexisting_payment_instrument_ids, std::move(callback), std::string());
}

void ServiceImpl::SetDisableRpcSigning(bool disable_rpc_signing) {
request_sender_->SetDisableRpcSigning(disable_rpc_signing);
}

void ServiceImpl::SendUserDataRequest(
uint64_t run_id,
bool request_name,
Expand Down
2 changes: 2 additions & 0 deletions components/autofill_assistant/browser/service/service_impl.h
Original file line number Diff line number Diff line change
Expand Up @@ -91,6 +91,8 @@ class ServiceImpl : public Service {
const UserData* user_data,
ServiceRequestSender::ResponseCallback callback) override;

void SetDisableRpcSigning(bool disable_rpc_signing) override;

private:
void SendUserDataRequest(
uint64_t run_id,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,8 @@ class ServiceRequestSender {
AuthMode auth_mode,
ResponseCallback response_callback,
RpcType rpc_type) = 0;

virtual void SetDisableRpcSigning(bool disable_rpc_signing) = 0;
};

} // namespace autofill_assistant
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -207,7 +207,7 @@ void ServiceRequestSenderImpl::SendRequest(
max_retries = kMaxRetriesGetUserData;
}

if (!cup::IsRpcTypeSupported(rpc_type)) {
if (!cup::IsRpcTypeSupported(rpc_type) || disable_rpc_signing_) {
InternalSendRequest(url, request_body, auth_mode, max_retries,
std::move(callback));
return;
Expand Down Expand Up @@ -341,4 +341,8 @@ bool ServiceRequestSenderImpl::OAuthEnabled(
!failed_to_fetch_oauth_token_);
}

void ServiceRequestSenderImpl::SetDisableRpcSigning(bool disable_rpc_signing) {
disable_rpc_signing_ = disable_rpc_signing;
}

} // namespace autofill_assistant
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,11 @@ class ServiceRequestSenderImpl : public ServiceRequestSender {
ResponseCallback callback,
RpcType rpc_type) override;

// Sets the value of the |disable_rpc_signing| field. If |true| CUP signing
// and verification will be bypassed even if it is enabled and the RPC type
// supported. Intended for internal use only.
void SetDisableRpcSigning(bool disable_rpc_signing) override;

private:
// Unlike |ServiceRequestSenderImpl::SendRequest|, assumes that any necessary
// CUP signing and validation is already done or accounted for in the
Expand Down Expand Up @@ -101,6 +106,9 @@ class ServiceRequestSenderImpl : public ServiceRequestSender {
// API key to add to the URL of unauthenticated requests.
std::string api_key_;

// Disable CUP RPC signing. Intended for internal use only.
bool disable_rpc_signing_ = false;

// Getting the OAuth token failed. For requests with auth mode allowing to
// fall back to API key, it will not be retried. For requests forcing auth,
// the OAuth token will tried to be re-fetched.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -664,6 +664,39 @@ TEST_F(ServiceRequestSenderImplTest, DoesNotRecordCupEventForNonSupportedRpcs) {
Metrics::CupRpcVerificationEvent::VERIFICATION_DISABLED, 0);
}

TEST_F(ServiceRequestSenderImplTest,
DoesNotPerformCupSigningIfRpcSigningDisabled) {
InitCupFeatures(true, true);
auto loader_factory =
std::make_unique<NiceMock<MockSimpleURLLoaderFactory>>();
auto loader = std::make_unique<NiceMock<MockURLLoader>>();
auto response_info = CreateResponseInfo(net::HTTP_OK, "OK");
EXPECT_CALL(*loader_factory, OnCreateLoader)
.WillOnce([&](::network::ResourceRequest* resource_request,
const ::net::NetworkTrafficAnnotationTag& annotation_tag) {
return std::move(loader);
});
EXPECT_CALL(*loader, DownloadToStringOfUnboundedSizeUntilCrashAndDie)
.WillOnce(RunOnceCallback<1>(std::make_unique<std::string>("response")));
EXPECT_CALL(*loader, ResponseInfo)
.WillRepeatedly(Return(response_info.get()));
EXPECT_CALL(mock_response_callback_, Run(net::HTTP_OK, "response", _));

auto cup_factory =
std::make_unique<NiceMock<autofill_assistant::cup::MockCUPFactory>>();
EXPECT_CALL(*cup_factory, CreateInstance).Times(0);

ServiceRequestSenderImpl request_sender{
&context_,
/* access_token_fetcher = */ nullptr, std::move(cup_factory),
std::move(loader_factory), std::string("fake_api_key")};
request_sender.SetDisableRpcSigning(true);
request_sender.SendRequest(
GURL("https://www.example.com"), std::string("request"),
ServiceRequestSender::AuthMode::API_KEY, mock_response_callback_.Get(),
autofill_assistant::RpcType::GET_ACTIONS);
}

// TODO(b/170934170): Add tests for full unit test coverage of
// service_request_sender.

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,4 +25,7 @@ void ServiceRequestSenderLocalImpl::SendRequest(
/* response_info = */ {});
}

void ServiceRequestSenderLocalImpl::SetDisableRpcSigning(
bool disable_rpc_signing) {}

} // namespace autofill_assistant
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,8 @@ class ServiceRequestSenderLocalImpl : public ServiceRequestSender {
ResponseCallback callback,
RpcType rpc_type) override;

void SetDisableRpcSigning(bool disable_rpc_signing) override;

private:
std::string response_;
};
Expand Down

0 comments on commit 9cb0434

Please sign in to comment.