Add policies for logout on smart card removal

This CL implements the core part of SecurityTokenSessionController.
The controller observes security tokens and triggers lock / logout when
a token that was used for login is removed, alongside UI implemented in
previous CLs.

This behavior can be controlled by the SecurityTokenSessionBehavior and
SecurityTokenSessionNotificationSeconds policies, which this CL
activates.

This CL also adds browser tests for both policies. In doing so, it also
adds a test for SecurityTokenRestrictionView.

The CL renames the should_fail_certificate_requests property in
TestCertificateProviderExtension to should_provide_certificates,
which more accurately reflects its purpose.

Fixed: 1131450
Change-Id: I8cf4e25fbbcb8ae4ab607ace254a85197f594f5a
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2632990
Commit-Queue: Fabian Sommer <fabiansommer@chromium.org>
Reviewed-by: Xiyuan Xia <xiyuan@chromium.org>
Reviewed-by: Maksim Ivanov <emaxx@chromium.org>
Cr-Commit-Position: refs/heads/master@{#846276}

chrome/browser/chromeos/certificate_provider/test_certificate_provider_extension.cc

@@ -205,7 +205,7 @@ void TestCertificateProviderExtension::TriggerSetCertificates() {
   base::Value message_data(base::Value::Type::DICTIONARY);
   message_data.SetStringKey("name", "setCertificates");
   base::Value cert_info_values(base::Value::Type::LIST);
-  if (!should_fail_certificate_requests_)
+  if (should_provide_certificates_)
     cert_info_values.Append(MakeClientCertificateInfoValue(*certificate_));
   message_data.SetKey("certificateInfoList", std::move(cert_info_values));
 
@@ -266,7 +266,7 @@ void TestCertificateProviderExtension::HandleCertificatesRequest(
     ReplyToJsCallback callback) {
   ++certificate_request_count_;
   base::Value cert_info_values(base::Value::Type::LIST);
-  if (!should_fail_certificate_requests_)
+  if (should_provide_certificates_)
     cert_info_values.Append(MakeClientCertificateInfoValue(*certificate_));
   std::move(callback).Run(cert_info_values);
 }

chrome/browser/chromeos/certificate_provider/test_certificate_provider_extension.h

@@ -73,11 +73,10 @@ class TestCertificateProviderExtension final
     remaining_pin_attempts_ = remaining_pin_attempts;
   }
 
-  // Sets whether the extension should respond with a failure to the
-  // onCertificatesRequested requests.
-  void set_should_fail_certificate_requests(
-      bool should_fail_certificate_requests) {
-    should_fail_certificate_requests_ = should_fail_certificate_requests;
+  // Sets whether the extension should return any certificates in response to a
+  // onCertificatesRequested request or a TriggerSetCertificates() call.
+  void set_should_provide_certificates(bool should_provide_certificates) {
+    should_provide_certificates_ = should_provide_certificates;
   }
 
   // Sets whether the extension should respond with a failure to the
@@ -113,7 +112,7 @@ class TestCertificateProviderExtension final
   // When equal to zero, signature requests will be failed immediately; when is
   // negative, infinite number of attempts is allowed.
   int remaining_pin_attempts_ = -1;
-  bool should_fail_certificate_requests_ = false;
+  bool should_provide_certificates_ = true;
   bool should_fail_sign_digest_requests_ = false;
   content::NotificationRegistrar notification_registrar_;
 

chrome/browser/chromeos/dbus/cryptohome_key_delegate_service_provider_browsertest.cc

@@ -250,7 +250,7 @@ IN_PROC_BROWSER_TEST_F(CryptohomeKeyDelegateServiceProviderTest,
 // by the test provider anymore.
 IN_PROC_BROWSER_TEST_F(CryptohomeKeyDelegateServiceProviderTest,
                        SignatureErrorKeyRemoved) {
-  certificate_provider_extension()->set_should_fail_certificate_requests(true);
+  certificate_provider_extension()->set_should_provide_certificates(false);
   RefreshCertsFromCertProviders();
 
   std::vector<uint8_t> signature;

chrome/browser/chromeos/login/security_token_session_controller.h

@@ -5,13 +5,29 @@
 #ifndef CHROME_BROWSER_CHROMEOS_LOGIN_SECURITY_TOKEN_SESSION_CONTROLLER_H_
 #define CHROME_BROWSER_CHROMEOS_LOGIN_SECURITY_TOKEN_SESSION_CONTROLLER_H_
 
+#include <string>
+#include <vector>
+
+#include "base/containers/flat_map.h"
+#include "base/containers/flat_set.h"
+#include "base/memory/weak_ptr.h"
+#include "base/timer/timer.h"
+#include "chrome/browser/chromeos/certificate_provider/certificate_provider_service.h"
 #include "components/keyed_service/core/keyed_service.h"
 #include "components/prefs/pref_change_registrar.h"
 #include "components/prefs/pref_registry_simple.h"
 #include "components/prefs/pref_service.h"
 #include "components/user_manager/user.h"
+#include "extensions/common/extension_id.h"
+
+namespace views {
+class Widget;
+}
 
 namespace chromeos {
+
+class CertificateProvider;
+
 namespace login {
 
 // A controller that implements the combined behavior of the
@@ -21,13 +37,17 @@ namespace login {
 // certificate ceases to be present while the user is logged in.
 // SecurityTokenSessionNotificationSeconds determines if and how long the user
 // is getting informed what is going to happen when the certificate vanishes.
-class SecurityTokenSessionController : public KeyedService {
+class SecurityTokenSessionController
+    : public KeyedService,
+      public CertificateProviderService::Observer {
  public:
   enum class Behavior { kIgnore, kLogout, kLock };
 
-  SecurityTokenSessionController(PrefService* local_state,
-                                 PrefService* profile_prefs,
-                                 const user_manager::User* user);
+  SecurityTokenSessionController(
+      PrefService* local_state,
+      PrefService* profile_prefs,
+      const user_manager::User* user,
+      CertificateProviderService* certificate_provider_service);
   SecurityTokenSessionController(const SecurityTokenSessionController& other) =
       delete;
   SecurityTokenSessionController& operator=(
@@ -45,13 +65,25 @@ class SecurityTokenSessionController : public KeyedService {
   // happens for a user on a device.
   static void MaybeDisplayLoginScreenNotification();
 
+  // CertificateProviderService::Observer
+  void OnCertificatesUpdated(
+      const std::string& extension_id,
+      const std::vector<certificate_provider::CertificateInfo>&
+          certificate_infos) override;
+
  private:
   Behavior GetBehaviorFromPref() const;
   void UpdateBehaviorPref();
   void UpdateNotificationPref();
 
+  void ExtensionProvidesAllRequiredCertificates(
+      const extensions::ExtensionId& extension_id);
+  void ExtensionStopsProvidingCertificate(
+      const extensions::ExtensionId& extension_id);
+  void TriggerAction();
   void AddLockNotification() const;
   void ScheduleLogoutNotification() const;
+  void Reset();
 
   PrefService* const local_state_;
   PrefService* const profile_prefs_;
@@ -59,6 +91,17 @@ class SecurityTokenSessionController : public KeyedService {
   PrefChangeRegistrar pref_change_registrar_;
   Behavior behavior_ = Behavior::kIgnore;
   base::TimeDelta notification_seconds_;
+  base::flat_set<extensions::ExtensionId> observed_extensions_;
+  base::flat_map<extensions::ExtensionId, std::vector<std::string>>
+      extension_to_spkis_;
+  base::flat_set<extensions::ExtensionId>
+      extensions_missing_required_certificates_;
+  views::Widget* fullscreen_notification_ = nullptr;
+  base::OneShotTimer action_timer_;
+  CertificateProviderService* certificate_provider_service_ = nullptr;
+  std::unique_ptr<CertificateProvider> certificate_provider_;
+
+  base::WeakPtrFactory<SecurityTokenSessionController> weak_ptr_factory_{this};
 };
 
 }  // namespace login

chrome/browser/chromeos/login/security_token_session_controller_factory.cc

@@ -6,11 +6,13 @@
 
 #include "chrome/browser/browser_process.h"
 #include "chrome/browser/chromeos/certificate_provider/certificate_provider_service_factory.h"
+#include "chrome/browser/chromeos/login/challenge_response_auth_keys_loader.h"
 #include "chrome/browser/chromeos/login/security_token_session_controller.h"
 #include "chrome/browser/chromeos/profiles/profile_helper.h"
 #include "chrome/browser/profiles/incognito_helpers.h"
 #include "chrome/browser/profiles/profile.h"
 #include "components/keyed_service/content/browser_context_dependency_manager.h"
+#include "components/user_manager/user.h"
 
 namespace chromeos {
 namespace login {
@@ -52,10 +54,21 @@ KeyedService* SecurityTokenSessionControllerFactory::BuildServiceInstanceFor(
     // This can happen in tests that do not have local state.
     return nullptr;
   }
+
+  // The service is only relevant for users who authenticate with a security
+  // token used for a challenge-response flow.
+  // TODO(crbug.com/1164373): This check produces false negatives for ephemeral
+  // users.
   user_manager::User* user = ProfileHelper::Get()->GetUserByProfile(
       Profile::FromBrowserContext(context));
+  if (!ChallengeResponseAuthKeysLoader::CanAuthenticateUser(
+          user->GetAccountId()))
+    return nullptr;
+
+  CertificateProviderService* certificate_provider_service =
+      CertificateProviderServiceFactory::GetForBrowserContext(context);
   return new SecurityTokenSessionController(local_state, profile->GetPrefs(),
-                                            user);
+                                            user, certificate_provider_service);
 }
 
 content::BrowserContext*

chrome/browser/ui/ash/security_token_session_restriction_view.cc

@@ -22,6 +22,7 @@
 #include "ui/views/controls/label.h"
 #include "ui/views/layout/box_layout.h"
 #include "ui/views/layout/layout_provider.h"
+#include "ui/views/metadata/metadata_impl_macros.h"
 #include "ui/views/widget/widget.h"
 #include "ui/views/window/dialog_delegate.h"
 
@@ -112,7 +113,6 @@ base::string16 GetDialogText(
 SecurityTokenSessionRestrictionView::SecurityTokenSessionRestrictionView(
     base::TimeDelta duration,
     base::OnceClosure accept_callback,
-    base::OnceClosure window_closing_callback,
     chromeos::login::SecurityTokenSessionController::Behavior behavior,
     const std::string& domain)
     : AppDialogView(GetImage()),
@@ -125,7 +125,6 @@ SecurityTokenSessionRestrictionView::SecurityTokenSessionRestrictionView(
   SetTitle(GetTitle(behavior));
 
   SetAcceptCallback(std::move(accept_callback));
-  RegisterWindowClosingCallback(std::move(window_closing_callback));
 
   InitializeView(/*heading_text=*/base::string16());
   UpdateLabel();
@@ -144,3 +143,6 @@ void SecurityTokenSessionRestrictionView::UpdateLabel() {
     update_timer_.Stop();
   }
 }
+
+BEGIN_METADATA(SecurityTokenSessionRestrictionView, AppDialogView);
+END_METADATA

chrome/browser/ui/ash/security_token_session_restriction_view.h

@@ -10,11 +10,14 @@
 #include "base/timer/timer.h"
 #include "chrome/browser/chromeos/login/security_token_session_controller.h"
 #include "chrome/browser/ui/views/apps/app_dialog/app_dialog_view.h"
+#include "ui/views/metadata/metadata_header_macros.h"
 
 // The dialog informing the user they are about to be logged out or locked
 // because they removed their security token (smart card).
 class SecurityTokenSessionRestrictionView : public AppDialogView {
  public:
+  METADATA_HEADER(SecurityTokenSessionRestrictionView);
+
   // Creates the dialog.
   // `duration`: Initial countdown time, will be displayed in seconds.
   // `accept_callback`: Callback when the user accepts the dialog.
@@ -26,7 +29,6 @@ class SecurityTokenSessionRestrictionView : public AppDialogView {
   SecurityTokenSessionRestrictionView(
       base::TimeDelta duration,
       base::OnceClosure accept_callback,
-      base::OnceClosure window_closing_callback,
       chromeos::login::SecurityTokenSessionController::Behavior behavior,
       const std::string& domain);
   SecurityTokenSessionRestrictionView(

components/policy/resources/policy_templates.json

@@ -7679,7 +7679,7 @@
           'caption': '''Lock the current session.''',
         },
       ],
-      'future_on': ['chrome_os'],
+      'supported_on': ['chrome_os:90-'],
       'features': {
         'dynamic_refresh': True,
         'per_profile': False,
@@ -7695,7 +7695,7 @@
       'owners': ['fabiansommer@chromium.org, emaxx@chromium.org'],
       'type': 'int',
       'schema': { 'type': 'integer', 'minimum': 0, 'maximum': 9999 },
-      'future_on': ['chrome_os'],
+      'supported_on': ['chrome_os:90-'],
       'features': {
         'dynamic_refresh': True,
         'per_profile': False,