Skip to content

Commit

Permalink
Implement basic PIN dialog on SAML page
Browse files Browse the repository at this point in the history
This CL builds the missing integration between the C++ code (starting
from the handler of the PIN requests made by extensions) and the
JavaScript code on the SAML sign-in page:

* dynamically enabling/disabling this UI when the SAML sign-in
  begins/ends;
* sending the PIN request parameters from C++ to JS;
* replying with the entered PIN from JS to C++.

The proper error displaying and other UI improvements will be done in
follow-up CLs.

Bug: 964069
Test: enroll a Chromebook, configure device policies to force-install smart card middleware onto Login Screen, start logging in using a SAML account that uses smart card authentication, check that the PIN dialog appears on top of the SAML page, enter the smart card PIN, check that the login completes
Change-Id: I95c2d5e5110a967ca6b7d8f957bbc8422f62d758
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1742233
Reviewed-by: Denis Kuznetsov <antrim@chromium.org>
Reviewed-by: Achuith Bhandarkar <achuith@chromium.org>
Reviewed-by: Roman Sorokin [CET] <rsorokin@chromium.org>
Commit-Queue: Maksim Ivanov <emaxx@chromium.org>
Cr-Commit-Position: refs/heads/master@{#686151}
  • Loading branch information
Maksim Ivanov authored and Commit Bot committed Aug 12, 2019
1 parent afda951 commit cbbb33c
Show file tree
Hide file tree
Showing 5 changed files with 220 additions and 27 deletions.
16 changes: 10 additions & 6 deletions chrome/browser/resources/chromeos/login/oobe_types.js
Original file line number Diff line number Diff line change
Expand Up @@ -102,6 +102,7 @@ OobeTypes.OobeConfiguration;
/**
* Specifies the type of the information that is requested by the security token
* PIN dialog.
* Must be kept in sync with chromeos/constants/security_token_pin_types.h.
* @enum {number}
*/
OobeTypes.SecurityTokenPinDialogType = {
Expand All @@ -112,20 +113,23 @@ OobeTypes.SecurityTokenPinDialogType = {
/**
* Specifies the type of the error that is displayed in the security token PIN
* dialog.
* Must be kept in sync with chromeos/constants/security_token_pin_types.h.
* @enum {number}
*/
OobeTypes.SecurityTokenPinDialogErrorType = {
UNKNOWN_ERROR: 0,
INVALID_PIN: 1,
INVALID_PUK: 2,
MAX_ATTEMPTS_EXCEEDED: 3,
NONE: 0,
UNKNOWN: 1,
INVALID_PIN: 2,
INVALID_PUK: 3,
MAX_ATTEMPTS_EXCEEDED: 4,
};

/**
* Configuration of the security token PIN dialog.
* @typedef {{
* type: OobeTypes.SecurityTokenPinDialogType,
* errorType: (OobeTypes.SecurityTokenPinDialogErrorType|undefined),
* codeType: OobeTypes.SecurityTokenPinDialogType,
* enableUserInput: boolean,
* errorLabel: OobeTypes.SecurityTokenPinDialogErrorType,
* attemptsLeft: number,
* }}
*/
Expand Down
64 changes: 51 additions & 13 deletions chrome/browser/resources/chromeos/login/screen_gaia_signin.js
Original file line number Diff line number Diff line change
Expand Up @@ -103,6 +103,7 @@ Polymer({
isSaml_: {
type: Boolean,
value: false,
observer: 'onSamlChanged_',
},

/**
Expand All @@ -114,6 +115,7 @@ Polymer({
pinDialogParameters_: {
type: Object,
value: null,
observer: 'onPinDialogParametersChanged_',
},

/**
Expand Down Expand Up @@ -927,14 +929,28 @@ Polymer({
onAuthFlowChange_: function() {
this.isSaml_ =
this.authenticator_.authFlow == cr.login.Authenticator.AuthFlow.SAML;
},

/**
* Observer that is called when the |isSaml_| property gets changed.
* @param {number} newValue
* @param {number} oldValue
* @private
*/
onSamlChanged_: function(newValue, oldValue) {
chrome.send('samlStateChanged', [this.isSaml_]);

this.classList.toggle('saml', this.isSaml_);

if (Oobe.getInstance().currentScreen.id == 'gaia-signin') {
Oobe.getInstance().updateScreenSize(this);
}
// Skip these updates in the initial observer run, which is happening during
// the property initialization.
if (oldValue !== undefined) {
if (Oobe.getInstance().currentScreen.id == 'gaia-signin') {
Oobe.getInstance().updateScreenSize(this);
}

this.updateGuestButtonVisibility_();
this.updateGuestButtonVisibility_();
}
},

/**
Expand Down Expand Up @@ -1440,25 +1456,47 @@ Polymer({
showPinDialog: function(parameters) {
assert(parameters);

// If currently shown, reset and send the cancellation result if not yet.
this.closePinDialog();
this.$.pinDialog.reset();

// Note that this must be done before updating |pinDialogResultReported_|,
// since the observer will notify the handler about the cancellation of the
// previous dialog depending on this flag.
this.pinDialogParameters_ = parameters;

this.$.pinDialog.reset();
this.pinDialogResultReported_ = false;
},

/**
* Closes the PIN dialog (that was previously opened using showPinDialog()).
* Does nothing if the dialog is not shown.
*/
closePinDialog: function() {
if (this.pinDialogParameters_ && !this.pinDialogResultReported_) {
this.pinDialogResultReported_ = true;
// TODO(crbug.com/964069): Send the "canceled" result to the C++ side.
}
// Note that the update triggers the observer, that notifies the handler
// about the closing.
this.pinDialogParameters_ = null;
},

/**
* Observer that is called when the |pinDialogParameters_| property gets
* changed.
* @param {number} newValue
* @param {number} oldValue
* @private
*/
onPinDialogParametersChanged_: function(newValue, oldValue) {
if (oldValue === undefined) {
// Don't do anything on the initial call, triggered by the property
// initialization.
return;
}
if ((oldValue !== null && newValue === null) ||
(oldValue !== null && newValue !== null &&
!this.pinDialogResultReported_)) {
// Report the cancellation result if the dialog got closed or got reused
// before reporting the result.
chrome.send('securityTokenPinEntered', [/*user_input=*/ '']);
}
},

/**
* Invoked when the user cancels the PIN dialog.
* @param {!CustomEvent} e
Expand All @@ -1473,7 +1511,7 @@ Polymer({
*/
onPinDialogCompleted_: function(e) {
this.pinDialogResultReported_ = true;
// TODO(crbug.com/964069): Send the PIN to the C++ side.
chrome.send('securityTokenPinEntered', [/*user_input=*/ e.detail]);
},

});
Expand Down
111 changes: 111 additions & 0 deletions chrome/browser/ui/webui/chromeos/login/gaia_screen_handler.cc
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,9 @@
#include "base/values.h"
#include "chrome/browser/browser_process.h"
#include "chrome/browser/chromeos/authpolicy/authpolicy_helper.h"
#include "chrome/browser/chromeos/certificate_provider/certificate_provider_service.h"
#include "chrome/browser/chromeos/certificate_provider/certificate_provider_service_factory.h"
#include "chrome/browser/chromeos/certificate_provider/pin_dialog_manager.h"
#include "chrome/browser/chromeos/language_preferences.h"
#include "chrome/browser/chromeos/login/lock_screen_utils.h"
#include "chrome/browser/chromeos/login/reauth_stats.h"
Expand Down Expand Up @@ -59,6 +62,7 @@
#include "chromeos/constants/chromeos_features.h"
#include "chromeos/constants/chromeos_switches.h"
#include "chromeos/constants/devicetype.h"
#include "chromeos/constants/security_token_pin_types.h"
#include "chromeos/dbus/util/version_loader.h"
#include "chromeos/login/auth/challenge_response/cert_utils.h"
#include "chromeos/login/auth/cryptohome_key_constants.h"
Expand Down Expand Up @@ -255,6 +259,27 @@ bool GaiaActionButtonsEnabled() {
return base::FeatureList::IsEnabled(chromeos::features::kGaiaActionButtons);
}

PinDialogManager* GetLoginScreenPinDialogManager() {
DCHECK(ProfileHelper::IsSigninProfileInitialized());
CertificateProviderService* certificate_provider_service =
CertificateProviderServiceFactory::GetForBrowserContext(
ProfileHelper::GetSigninProfile());
return certificate_provider_service->pin_dialog_manager();
}

base::Value MakeSecurityTokenPinDialogParameters(
SecurityTokenPinCodeType code_type,
bool enable_user_input,
SecurityTokenPinErrorLabel error_label,
int attempts_left) {
base::Value params(base::Value::Type::DICTIONARY);
params.SetIntKey("codeType", static_cast<int>(code_type));
params.SetBoolKey("enableUserInput", enable_user_input);
params.SetIntKey("errorLabel", static_cast<int>(error_label));
params.SetIntKey("attemptsLeft", attempts_left);
return params;
}

} // namespace

constexpr StaticOobeScreenId GaiaView::kScreenId;
Expand Down Expand Up @@ -299,6 +324,8 @@ GaiaScreenHandler::GaiaScreenHandler(
GaiaScreenHandler::~GaiaScreenHandler() {
if (network_portal_detector_)
network_portal_detector_->RemoveObserver(this);
if (is_security_token_pin_enabled_)
GetLoginScreenPinDialogManager()->RemovePinDialogHost(this);
}

void GaiaScreenHandler::MaybePreloadAuthExtension() {
Expand Down Expand Up @@ -633,6 +660,9 @@ void GaiaScreenHandler::RegisterMessages() {
AddCallback("updateSigninUIState",
&GaiaScreenHandler::HandleUpdateSigninUIState);
AddCallback("showGuestInOobe", &GaiaScreenHandler::HandleShowGuestInOobe);
AddCallback("samlStateChanged", &GaiaScreenHandler::HandleSamlStateChanged);
AddCallback("securityTokenPinEntered",
&GaiaScreenHandler::HandleSecurityTokenPinEntered);

// Allow UMA metrics collection from JS.
web_ui()->AddMessageHandler(std::make_unique<MetricsHandler>());
Expand Down Expand Up @@ -950,6 +980,48 @@ void GaiaScreenHandler::HandleShowGuestInOobe(bool show) {
ash::LoginScreen::Get()->ShowGuestButtonInOobe(show);
}

void GaiaScreenHandler::HandleSamlStateChanged(bool is_saml) {
if (is_saml == is_security_token_pin_enabled_) {
// We're already in the needed |is_security_token_pin_enabled_| state.
return;
}
// Enable ourselves as a security token PIN dialog host during the SAML
// sign-in, so that when the SAML page requires client authentication (e.g.,
// against a smart card), this PIN request is embedded into the SAML login UI.
if (is_saml) {
GetLoginScreenPinDialogManager()->AddPinDialogHost(this);
} else {
security_token_pin_entered_callback_.Reset();
security_token_pin_dialog_closed_callback_.Reset();
GetLoginScreenPinDialogManager()->RemovePinDialogHost(this);
}
is_security_token_pin_enabled_ = is_saml;
}

void GaiaScreenHandler::HandleSecurityTokenPinEntered(
const std::string& user_input) {
// Invariant: when the pin_entered_callback is present, the closed_callback
// must be present as well.
DCHECK(!security_token_pin_entered_callback_ ||
security_token_pin_dialog_closed_callback_);

if (!security_token_pin_dialog_closed_callback_) {
// The PIN request has already been canceled on the handler side.
return;
}

if (user_input.empty()) {
security_token_pin_entered_callback_.Reset();
std::move(security_token_pin_dialog_closed_callback_).Run();
} else {
// The callback must be non-null, since the UI implementation should not
// send multiple non-empty results.
std::move(security_token_pin_entered_callback_).Run(user_input);
// Keep |security_token_pin_dialog_closed_callback_|, in order to be able to
// notify about the dialog closing afterwards.
}
}

void GaiaScreenHandler::OnShowAddUser() {
signin_screen_handler_->is_account_picker_showing_first_time_ = false;
lock_screen_utils::EnforcePolicyInputMethods(std::string());
Expand Down Expand Up @@ -1125,6 +1197,45 @@ void GaiaScreenHandler::ShowSigninScreenForTest(const std::string& username,
}
}

void GaiaScreenHandler::ShowSecurityTokenPinDialog(
const std::string& /*caller_extension_name*/,
SecurityTokenPinCodeType code_type,
bool enable_user_input,
SecurityTokenPinErrorLabel error_label,
int attempts_left,
const base::Optional<AccountId>& /*authenticating_user_account_id*/,
SecurityTokenPinEnteredCallback pin_entered_callback,
SecurityTokenPinDialogClosedCallback pin_dialog_closed_callback) {
DCHECK(is_security_token_pin_enabled_);
// There must be either no active PIN dialog, or the active dialog for which
// the PIN has already been entered.
DCHECK(!security_token_pin_entered_callback_);

security_token_pin_entered_callback_ = std::move(pin_entered_callback);
// Note that this overwrites the previous closed_callback in the case where
// the dialog was already shown. This is intended, since the closing callback
// should only be used to notify that the dialog got canceled, which imposes a
// stricter quota on the PIN request caller.
security_token_pin_dialog_closed_callback_ =
std::move(pin_dialog_closed_callback);

CallJS("login.GaiaSigninScreen.showPinDialog",
MakeSecurityTokenPinDialogParameters(code_type, enable_user_input,
error_label, attempts_left));
}

void GaiaScreenHandler::CloseSecurityTokenPinDialog() {
DCHECK(is_security_token_pin_enabled_);
// Invariant: when the pin_entered_callback is present, the closed_callback
// must be present as well.
DCHECK(!security_token_pin_entered_callback_ ||
security_token_pin_dialog_closed_callback_);

security_token_pin_entered_callback_.Reset();
security_token_pin_dialog_closed_callback_.Reset();
CallJS("login.GaiaSigninScreen.closePinDialog");
}

bool GaiaScreenHandler::IsOfflineLoginActive() const {
return (screen_mode_ == GAIA_SCREEN_MODE_OFFLINE) || offline_login_is_active_;
}
Expand Down
38 changes: 37 additions & 1 deletion chrome/browser/ui/webui/chromeos/login/gaia_screen_handler.h
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
#include "base/macros.h"
#include "base/memory/ref_counted.h"
#include "chrome/browser/chromeos/authpolicy/authpolicy_helper.h"
#include "chrome/browser/chromeos/certificate_provider/security_token_pin_dialog_host.h"
#include "chrome/browser/chromeos/login/login_client_cert_usage_observer.h"
#include "chrome/browser/ui/webui/chromeos/login/base_screen_handler.h"
#include "chrome/browser/ui/webui/chromeos/login/core_oobe_handler.h"
Expand Down Expand Up @@ -75,7 +76,8 @@ class GaiaView {
// A class that handles WebUI hooks in Gaia screen.
class GaiaScreenHandler : public BaseScreenHandler,
public GaiaView,
public NetworkPortalDetector::Observer {
public NetworkPortalDetector::Observer,
public SecurityTokenPinDialogHost {
public:
using TView = GaiaView;

Expand Down Expand Up @@ -117,6 +119,18 @@ class GaiaScreenHandler : public BaseScreenHandler,
const std::string& password,
const std::string& services) override;

// SecurityTokenPinDialogHost:
void ShowSecurityTokenPinDialog(
const std::string& caller_extension_name,
SecurityTokenPinCodeType code_type,
bool enable_user_input,
SecurityTokenPinErrorLabel error_label,
int attempts_left,
const base::Optional<AccountId>& authenticating_user_account_id,
SecurityTokenPinEnteredCallback pin_entered_callback,
SecurityTokenPinDialogClosedCallback pin_dialog_closed_callback) override;
void CloseSecurityTokenPinDialog() override;

// Returns true if offline login mode was either required, or reported by the
// WebUI (i.e. WebUI mignt not have completed transition to the new mode).
bool IsOfflineLoginActive() const;
Expand Down Expand Up @@ -222,6 +236,12 @@ class GaiaScreenHandler : public BaseScreenHandler,
// OOBE.
void HandleShowGuestInOobe(bool show);

// Called to notify whether the SAML sign-in is currently happening.
void HandleSamlStateChanged(bool is_saml);
// Called to deliver the result of the security token PIN request. Called with
// an empty string when the request is canceled.
void HandleSecurityTokenPinEntered(const std::string& user_input);

void OnShowAddUser();

// Really handles the complete login message.
Expand Down Expand Up @@ -397,6 +417,22 @@ class GaiaScreenHandler : public BaseScreenHandler,
std::unique_ptr<LoginClientCertUsageObserver>
extension_provided_client_cert_usage_observer_;

// State of the security token PIN dialogs:

// Whether this instance is currently registered as a host for showing the
// security token PIN dialogs. (See PinDialogManager for the default host.)
bool is_security_token_pin_enabled_ = false;
// The callback to run when the user submits a non-empty input to the security
// token PIN dialog.
// Is non-empty iff the dialog is active and the input wasn't sent yet.
SecurityTokenPinEnteredCallback security_token_pin_entered_callback_;
// The callback to run when the security token PIN dialog gets closed - either
// due to the user canceling the dialog or the whole sign-in attempt being
// aborted.
// Is non-empty iff the dialog is active.
SecurityTokenPinDialogClosedCallback
security_token_pin_dialog_closed_callback_;

base::WeakPtrFactory<GaiaScreenHandler> weak_factory_;

DISALLOW_COPY_AND_ASSIGN(GaiaScreenHandler);
Expand Down

0 comments on commit cbbb33c

Please sign in to comment.