[WebID] Move auth response to a separate mojo service

Almost always a renderer is either using the request or response side of
the federated auth service. Having two mojo interfaces for each one is
cleaner, more flexible, and more efficient as it also avoids wasting
resource by keeping unused state around.

Test: manually using existing demo
      `./chrome  --enable-features=WebID https://webid-demo.glitch.me`

Bug: 1141125
Change-Id: I2a41cd57022775ef5348e74991650dd5be90fa3e
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2636946Reviewed-by: Ken Buchanan <kenrb@chromium.org>
Reviewed-by: Dave Tapuska <dtapuska@chromium.org>
Commit-Queue: Majid Valipour <majidvp@chromium.org>
Cr-Commit-Position: refs/heads/master@{#846195}

content/browser/BUILD.gn

@@ -1866,12 +1866,16 @@ source_set("browser") {
     "webauth/webauth_request_security_checker.h",
     "webid/federated_auth_request_impl.cc",
     "webid/federated_auth_request_impl.h",
+    "webid/federated_auth_response_impl.cc",
+    "webid/federated_auth_response_impl.h",
     "webid/flags.cc",
     "webid/flags.h",
     "webid/id_token_request_callback_data.cc",
     "webid/id_token_request_callback_data.h",
     "webid/idp_network_request_manager.cc",
     "webid/idp_network_request_manager.h",
+    "webid/webid_utils.cc",
+    "webid/webid_utils.h",
     "webrtc/webrtc_internals.cc",
     "webrtc/webrtc_internals.h",
     "webrtc/webrtc_internals_connections_observer.h",

content/browser/browser_interface_binders.cc

@@ -130,6 +130,7 @@
 #include "third_party/blink/public/mojom/webauthn/authenticator.mojom.h"
 #include "third_party/blink/public/mojom/webauthn/virtual_authenticator.mojom.h"
 #include "third_party/blink/public/mojom/webid/federated_auth_request.mojom.h"
+#include "third_party/blink/public/mojom/webid/federated_auth_response.mojom.h"
 #include "third_party/blink/public/mojom/websockets/websocket_connector.mojom.h"
 #include "third_party/blink/public/mojom/webtransport/quic_transport_connector.mojom.h"
 #include "third_party/blink/public/mojom/worker/dedicated_worker_host_factory.mojom.h"
@@ -659,6 +660,9 @@ void PopulateFrameBinders(RenderFrameHostImpl* host, mojo::BinderMap* map) {
     map->Add<blink::mojom::FederatedAuthRequest>(base::BindRepeating(
         &RenderFrameHostImpl::BindFederatedAuthRequestReceiver,
         base::Unretained(host)));
+    map->Add<blink::mojom::FederatedAuthResponse>(base::BindRepeating(
+        &RenderFrameHostImpl::BindFederatedAuthResponseReceiver,
+        base::Unretained(host)));
   }
 
   map->Add<blink::mojom::WebUsbService>(base::BindRepeating(

content/browser/renderer_host/render_frame_host_impl.cc

@@ -129,6 +129,7 @@
 #include "content/browser/webauth/authenticator_impl.h"
 #include "content/browser/webauth/webauth_request_security_checker.h"
 #include "content/browser/webid/federated_auth_request_impl.h"
+#include "content/browser/webid/federated_auth_response_impl.h"
 #include "content/browser/webid/flags.h"
 #include "content/browser/websockets/websocket_connector_impl.h"
 #include "content/browser/webtransport/quic_transport_connector_impl.h"
@@ -8136,6 +8137,12 @@ void RenderFrameHostImpl::BindFederatedAuthRequestReceiver(
   FederatedAuthRequestImpl::Create(this, std::move(receiver));
 }
 
+void RenderFrameHostImpl::BindFederatedAuthResponseReceiver(
+    mojo::PendingReceiver<blink::mojom::FederatedAuthResponse> receiver) {
+  DCHECK(base::FeatureList::IsEnabled(features::kWebID));
+  FederatedAuthResponseImpl::Create(this, std::move(receiver));
+}
+
 void RenderFrameHostImpl::BindRestrictedCookieManager(
     mojo::PendingReceiver<network::mojom::RestrictedCookieManager> receiver) {
   static_cast<StoragePartitionImpl*>(GetProcess()->GetStoragePartition())

content/browser/renderer_host/render_frame_host_impl.h

@@ -141,6 +141,7 @@
 #include "third_party/blink/public/mojom/webauthn/authenticator.mojom.h"
 #include "third_party/blink/public/mojom/webauthn/virtual_authenticator.mojom.h"
 #include "third_party/blink/public/mojom/webid/federated_auth_request.mojom-forward.h"
+#include "third_party/blink/public/mojom/webid/federated_auth_response.mojom-forward.h"
 #include "third_party/blink/public/mojom/websockets/websocket_connector.mojom.h"
 #include "third_party/blink/public/mojom/webtransport/quic_transport_connector.mojom.h"
 #include "third_party/blink/public/mojom/worker/dedicated_worker_host_factory.mojom.h"
@@ -1444,6 +1445,9 @@ class CONTENT_EXPORT RenderFrameHostImpl
   void BindFederatedAuthRequestReceiver(
       mojo::PendingReceiver<blink::mojom::FederatedAuthRequest> receiver);
 
+  void BindFederatedAuthResponseReceiver(
+      mojo::PendingReceiver<blink::mojom::FederatedAuthResponse> receiver);
+
   void BindRestrictedCookieManager(
       mojo::PendingReceiver<network::mojom::RestrictedCookieManager> receiver);
 

content/browser/webid/federated_auth_request_impl.cc

@@ -8,43 +8,14 @@
 #include "base/strings/string_piece.h"
 #include "content/browser/renderer_host/render_frame_host_impl.h"
 #include "content/browser/webid/id_token_request_callback_data.h"
+#include "content/browser/webid/webid_utils.h"
 #include "content/public/common/content_client.h"
 #include "url/url_constants.h"
 
-using blink::mojom::ProvideIdTokenStatus;
 using blink::mojom::RequestIdTokenStatus;
 
 namespace content {
 
-namespace {
-
-// Determines whether |host| is same-origin with all of its ancestors in the
-// frame tree. Returns false if not.
-// |origin| is provided because it is not considered safe to use
-// host->GetLastCommittedOrigin() at some times, so FrameServiceBase::origin()
-// should be used to obtain the frame's origin.
-bool IsSameOriginWithAncestors(RenderFrameHost* host,
-                               const url::Origin& origin) {
-  RenderFrameHost* parent = host->GetParent();
-  while (parent) {
-    if (!parent->GetLastCommittedOrigin().IsSameOriginWith(origin)) {
-      return false;
-    }
-    parent = parent->GetParent();
-  }
-  return true;
-}
-
-// Checks requirements for URLs received from the IDP.
-bool IdpUrlIsValid(const GURL& url) {
-  if (!url.is_valid() || !url.SchemeIs(url::kHttpsScheme))
-    return false;
-
-  return true;
-}
-
-}  // namespace
-
 FederatedAuthRequestImpl::FederatedAuthRequestImpl(
     RenderFrameHost* host,
     mojo::PendingReceiver<blink::mojom::FederatedAuthRequest> receiver)
@@ -291,49 +262,4 @@ void FederatedAuthRequestImpl::CompleteRequest(
     std::move(callback_).Run(status, id_token);
 }
 
-// ---- Provider logic -----
-
-void FederatedAuthRequestImpl::ProvideIdToken(
-    const std::string& id_token,
-    ProvideIdTokenCallback idp_callback) {
-  // The ptr below is actually the same as |idp_web_contents_| but because this
-  // is a different instance of |FederatedAuthRequestImpl| for which
-  // |idp_web_contents_| has not been initialized.
-  //
-  // TODO(majidvp): We should have two separate mojo service for request and
-  // response sides would have make this more obvious. http://crbug.com/1141125
-  WebContents* idp_web_contents =
-      content::WebContents::FromRenderFrameHost(render_frame_host());
-  auto* request_callback_data =
-      IdTokenRequestCallbackData::Get(idp_web_contents);
-
-  // TODO(majidvp): This may happen if the page is not loaded by the browser's
-  // WebID machinery. We need a way for IDP logic to detect that and not provide
-  // a token. The current plan is to send a special header but we may also need
-  // to not expose this in JS somehow. Investigate this further.
-  // http://crbug.com/1141125
-  if (!request_callback_data) {
-    std::move(idp_callback).Run(ProvideIdTokenStatus::kError);
-    return;
-  }
-
-  // After running the RP done callback the IDP sign-in page gets closed and its
-  // web contents cleared in `FederatedAuthRequestImpl::CompleteRequest()`. So
-  // we should not access |idp_web_contents| or any of its associated objects
-  // as it may already be destructed. This is why we first run any logic that
-  // needs to touch the IDP web contents and then run the RP done callback.
-
-  auto rp_done_callback = request_callback_data->TakeDoneCallback();
-  IdTokenRequestCallbackData::Remove(idp_web_contents);
-
-  if (!rp_done_callback) {
-    std::move(idp_callback).Run(ProvideIdTokenStatus::kErrorTooManyResponses);
-    return;
-  }
-  std::move(idp_callback).Run(ProvideIdTokenStatus::kSuccess);
-
-  std::move(rp_done_callback).Run(id_token);
-  // Don't access |idp_web_contents| passed this point.
-}
-
 }  // namespace content

content/browser/webid/federated_auth_request_impl.h

@@ -46,9 +46,6 @@ class CONTENT_EXPORT FederatedAuthRequestImpl
                       const std::string& id_request,
                       RequestIdTokenCallback) override;
 
-  void ProvideIdToken(const std::string& id_token,
-                      ProvideIdTokenCallback) override;
-
  private:
   FederatedAuthRequestImpl(
       RenderFrameHost*,

content/browser/webid/federated_auth_response_impl.cc

+// Copyright 2021 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#include "content/browser/webid/federated_auth_response_impl.h"
+
+#include "base/callback.h"
+#include "content/browser/webid/id_token_request_callback_data.h"
+#include "content/browser/webid/webid_utils.h"
+#include "content/public/browser/render_frame_host.h"
+#include "content/public/browser/web_contents.h"
+#include "url/origin.h"
+
+using blink::mojom::ProvideIdTokenStatus;
+
+namespace content {
+
+FederatedAuthResponseImpl::FederatedAuthResponseImpl(
+    RenderFrameHost* host,
+    mojo::PendingReceiver<blink::mojom::FederatedAuthResponse> receiver)
+    : FrameServiceBase(host, std::move(receiver)) {}
+
+// TODO(majidvp): We should reject any pending promise here.
+// http://crbug.com/1141125
+FederatedAuthResponseImpl::~FederatedAuthResponseImpl() = default;
+
+// static
+void FederatedAuthResponseImpl::Create(
+    RenderFrameHost* host,
+    mojo::PendingReceiver<blink::mojom::FederatedAuthResponse> receiver) {
+  DCHECK(host);
+
+  // TODO(kenrb): This should also be verified in the renderer process before
+  // the mojo method is invoked, causing the promise to be rejected.
+  // https://crbug.com/1141125
+  // It is safe to access host->GetLastCommittedOrigin during construction
+  // but FrameServiceBase::origin() should be used thereafter.
+  if (!IsSameOriginWithAncestors(host, host->GetLastCommittedOrigin())) {
+    mojo::ReportBadMessage(
+        "WebID cannot be invoked from within cross-origin iframes.");
+    return;
+  }
+
+  // FederatedAuthRequestImpl owns itself. It will self-destruct when a mojo
+  // interface error occurs, the render frame host is deleted, or the render
+  // frame host navigates to a new document.
+  new FederatedAuthResponseImpl(host, std::move(receiver));
+}
+
+void FederatedAuthResponseImpl::ProvideIdToken(
+    const std::string& id_token,
+    ProvideIdTokenCallback idp_callback) {
+  // The ptr below is actually the same as |idp_web_contents_| but because this
+  // is a different instance of |FederatedAuthRequestImpl| for which
+  // |idp_web_contents_| has not been initialized.
+  //
+  // TODO(majidvp): We should have two separate mojo service for request and
+  // response sides would have make this more obvious. http://crbug.com/1141125
+  WebContents* idp_web_contents =
+      content::WebContents::FromRenderFrameHost(render_frame_host());
+  auto* request_callback_data =
+      IdTokenRequestCallbackData::Get(idp_web_contents);
+
+  // TODO(majidvp): This may happen if the page is not loaded by the browser's
+  // WebID machinery. We need a way for IDP logic to detect that and not provide
+  // a token. The current plan is to send a special header but we may also need
+  // to not expose this in JS somehow. Investigate this further.
+  // http://crbug.com/1141125
+  if (!request_callback_data) {
+    std::move(idp_callback).Run(ProvideIdTokenStatus::kError);
+    return;
+  }
+
+  // After running the RP done callback the IDP sign-in page gets closed and its
+  // web contents cleared in `FederatedAuthRequestImpl::CompleteRequest()`. So
+  // we should not access |idp_web_contents| or any of its associated objects
+  // as it may already be destructed. This is why we first run any logic that
+  // needs to touch the IDP web contents and then run the RP done callback.
+
+  auto rp_done_callback = request_callback_data->TakeDoneCallback();
+  IdTokenRequestCallbackData::Remove(idp_web_contents);
+
+  if (!rp_done_callback) {
+    std::move(idp_callback).Run(ProvideIdTokenStatus::kErrorTooManyResponses);
+    return;
+  }
+  std::move(idp_callback).Run(ProvideIdTokenStatus::kSuccess);
+
+  std::move(rp_done_callback).Run(id_token);
+  // Don't access |idp_web_contents| passed this point.
+}
+
+}  // namespace content

content/browser/webid/federated_auth_response_impl.h

+// Copyright 2021 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#ifndef CONTENT_BROWSER_WEBID_FEDERATED_AUTH_RESPONSE_IMPL_H_
+#define CONTENT_BROWSER_WEBID_FEDERATED_AUTH_RESPONSE_IMPL_H_
+
+#include <memory>
+#include <string>
+
+#include "base/callback_forward.h"
+#include "base/macros.h"
+#include "content/common/content_export.h"
+#include "content/public/browser/frame_service_base.h"
+#include "mojo/public/cpp/bindings/pending_receiver.h"
+#include "third_party/blink/public/mojom/webid/federated_auth_response.mojom.h"
+
+namespace content {
+
+class RenderFrameHost;
+
+// FederatedAuthResponseImpl handles mojo connections from the renderer to
+// fulfill WebID-related response by an IDP.
+class FederatedAuthResponseImpl
+    : public FrameServiceBase<blink::mojom::FederatedAuthResponse> {
+ public:
+  // Creates a self-managed instance of FederatedAuthResponseImpl and binds it
+  // to the receiver.
+  static void Create(
+      RenderFrameHost*,
+      mojo::PendingReceiver<blink::mojom::FederatedAuthResponse>);
+
+  FederatedAuthResponseImpl(const FederatedAuthResponseImpl&) = delete;
+  FederatedAuthResponseImpl& operator=(const FederatedAuthResponseImpl&) =
+      delete;
+  ~FederatedAuthResponseImpl() override;
+
+  void ProvideIdToken(const std::string& id_token,
+                      ProvideIdTokenCallback) override;
+
+ private:
+  FederatedAuthResponseImpl(
+      RenderFrameHost*,
+      mojo::PendingReceiver<blink::mojom::FederatedAuthResponse>);
+};
+
+}  // namespace content
+
+#endif  // CONTENT_BROWSER_WEBID_FEDERATED_AUTH_RESPONSE_IMPL_H_
\ No newline at end of file

content/browser/webid/webid_utils.cc

+// Copyright 2021 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#include "content/browser/webid/webid_utils.h"
+
+#include "content/public/browser/render_frame_host.h"
+
+namespace content {
+
+bool IsSameOriginWithAncestors(RenderFrameHost* host,
+                               const url::Origin& origin) {
+  RenderFrameHost* parent = host->GetParent();
+  while (parent) {
+    if (!parent->GetLastCommittedOrigin().IsSameOriginWith(origin)) {
+      return false;
+    }
+    parent = parent->GetParent();
+  }
+  return true;
+}
+
+bool IdpUrlIsValid(const GURL& url) {
+  if (!url.is_valid() || !url.SchemeIs(url::kHttpsScheme))
+    return false;
+
+  return true;
+}
+
+}  // namespace content
\ No newline at end of file

content/browser/webid/webid_utils.h

+// Copyright 2021 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#ifndef CONTENT_BROWSER_WEBID_WEBID_UTILS_H_
+#define CONTENT_BROWSER_WEBID_WEBID_UTILS_H_
+
+#include "url/gurl.h"
+#include "url/origin.h"
+
+namespace content {
+class RenderFrameHost;
+
+// Determines whether |host| is same-origin with all of its ancestors in the
+// frame tree. Returns false if not.
+// |origin| is provided because it is not considered safe to use
+// host->GetLastCommittedOrigin() at some times, so FrameServiceBase::origin()
+// should be used to obtain the frame's origin.
+bool IsSameOriginWithAncestors(RenderFrameHost* host,
+                               const url::Origin& origin);
+
+// Checks requirements for URLs received from the IDP.
+bool IdpUrlIsValid(const GURL& url);
+
+}  // namespace content
+#endif  // CONTENT_BROWSER_WEBID_WEBID_UTILS_H_
\ No newline at end of file

third_party/blink/public/mojom/BUILD.gn

@@ -184,6 +184,7 @@ mojom("mojom_platform") {
     "webaudio/audio_context_manager.mojom",
     "webdatabase/web_database.mojom",
     "webid/federated_auth_request.mojom",
+    "webid/federated_auth_response.mojom",
     "webpreferences/web_preferences.mojom",
     "websockets/websocket_connector.mojom",
     "webtransport/quic_transport_connector.mojom",

third_party/blink/public/mojom/webid/federated_auth_request.mojom

@@ -22,13 +22,6 @@ enum RequestIdTokenStatus {
   kError,
 };
 
-enum ProvideIdTokenStatus {
-  kSuccess,
-  kErrorTooManyResponses,
-  kError,
-};
-
-
 // Create a federated sign-in request using the specified provider.
 // This interface is called from a renderer process and implemented in the
 // browser process.
@@ -36,16 +29,5 @@ interface FederatedAuthRequest {
   // Requests an IdToken to be generated, given an IDP URL and an OAuth request.
   // Returns the raw content of the IdToken.
   RequestIdToken(url.mojom.Url provider, string id_request) => (RequestIdTokenStatus status, string? id_token);
-
-  // TODO(majidvp): Consider creating a whole new interface for the response
-  // This is cleaner and more flexible. It also avoids wasting resource by
-  // keeping unused state around given that we mostly expect each frame to use
-  // only one of these interfaces. http://crbug.com/1141125
-
-  // Provides an IdToken that is passed to the pending request. This is meant to
-  // be used by the IDP generating the token. Empty string resolves the pending
-  // request with an error.
-  // Returns a status indicating if the the pending RP request was resolved.
-  ProvideIdToken(string id_token) => (ProvideIdTokenStatus status);
 };
 

third_party/blink/public/mojom/webid/federated_auth_response.mojom

+// Copyright 2021 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+module blink.mojom;
+
+// Implementation of the proposed WebId API.
+//
+// Proposal: https://github.com/WICG/WebID
+
+enum ProvideIdTokenStatus {
+  kSuccess,
+  kErrorTooManyResponses,
+  kError,
+};
+
+
+// Creates a federated sign-in response.
+// This interface is called from the provider renderer process and is
+// implemented in the browser process.
+interface FederatedAuthResponse {
+  // Provides an IdToken that is passed to the pending request. This is meant to
+  // be used by the IDP generating the token. Empty string resolves the pending
+  // request with an error.
+  // Returns a status indicating if the the pending RP request was resolved.
+  ProvideIdToken(string id_token) => (ProvideIdTokenStatus status);
+};
+

third_party/blink/renderer/modules/webid/web_id.cc

@@ -90,7 +90,9 @@ void OnProvideIdToken(ScriptPromiseResolver* resolver,
 }  // namespace
 
 WebID::WebID(ExecutionContext& context)
-    : ExecutionContextClient(&context), auth_request_(&context) {}
+    : ExecutionContextClient(&context),
+      auth_request_(&context),
+      auth_response_(&context) {}
 
 ScriptPromise WebID::get(ScriptState* script_state,
                          const WebIDRequestOptions* options,
@@ -115,7 +117,7 @@ ScriptPromise WebID::get(ScriptState* script_state,
     return ScriptPromise();
   }
 
-  BindAuthRequest();
+  BindRemote(auth_request_);
 
   auto* resolver = MakeGarbageCollected<ScriptPromiseResolver>(script_state);
   ScriptPromise promise = resolver->Promise();
@@ -128,29 +130,30 @@ ScriptPromise WebID::get(ScriptState* script_state,
 }
 
 ScriptPromise WebID::provide(ScriptState* script_state, String id_token) {
-  BindAuthRequest();
+  BindRemote(auth_response_);
 
   auto* resolver = MakeGarbageCollected<ScriptPromiseResolver>(script_state);
   ScriptPromise promise = resolver->Promise();
 
-  auth_request_->ProvideIdToken(
+  auth_response_->ProvideIdToken(
       id_token, WTF::Bind(&OnProvideIdToken, WrapPersistent(resolver)));
 
   return promise;
 }
 
-void WebID::BindAuthRequest() {
+template <typename Interface>
+void WebID::BindRemote(HeapMojoRemote<Interface>& remote) {
   auto* context = GetExecutionContext();
 
-  if (auth_request_.is_bound())
+  if (remote.is_bound())
     return;
 
   // TODO(kenrb): Work out whether kUserInteraction is the best task type
   // here. It might be appropriate to create a new one.
   context->GetBrowserInterfaceBroker().GetInterface(
-      auth_request_.BindNewPipeAndPassReceiver(
+      remote.BindNewPipeAndPassReceiver(
           context->GetTaskRunner(TaskType::kUserInteraction)));
-  auth_request_.set_disconnect_handler(
+  remote.set_disconnect_handler(
       WTF::Bind(&WebID::OnConnectionError, WrapWeakPersistent(this)));
 }
 
@@ -158,10 +161,15 @@ void WebID::Trace(blink::Visitor* visitor) const {
   ScriptWrappable::Trace(visitor);
   ExecutionContextClient::Trace(visitor);
   visitor->Trace(auth_request_);
+  visitor->Trace(auth_response_);
 }
 
 void WebID::OnConnectionError() {
   auth_request_.reset();
+  // TODO(majidvp): We should handle connection errors for request and response
+  // separately.
+  auth_response_.reset();
+
   // TODO(kenrb): Cache the resolver and resolve the promise with an
   // appropriate error message.
 }

third_party/blink/renderer/modules/webid/web_id.h

@@ -7,6 +7,7 @@
 
 #include "mojo/public/cpp/bindings/binding.h"
 #include "third_party/blink/public/mojom/webid/federated_auth_request.mojom-blink.h"
+#include "third_party/blink/public/mojom/webid/federated_auth_response.mojom-blink.h"
 #include "third_party/blink/renderer/core/execution_context/execution_context_lifecycle_observer.h"
 #include "third_party/blink/renderer/platform/bindings/script_wrappable.h"
 #include "third_party/blink/renderer/platform/heap/heap_allocator.h"
@@ -33,10 +34,12 @@ class WebID final : public ScriptWrappable, public ExecutionContextClient {
   void Trace(blink::Visitor*) const override;
 
  private:
-  void BindAuthRequest();
+  template <typename Interface>
+  void BindRemote(HeapMojoRemote<Interface>& remote);
   void OnConnectionError();
 
   HeapMojoRemote<mojom::blink::FederatedAuthRequest> auth_request_;
+  HeapMojoRemote<mojom::blink::FederatedAuthResponse> auth_response_;
 };
 
 }  // namespace blink