Commit f840e4e3 authored by Tomasz Garbus's avatar Tomasz Garbus Committed by Commit Bot

Implemented CredentialManager methods + tests

Implemented CredentialManager methods:
 * HandleScriptCommandCallback
 * SendGetResponse
 * SendStoreResponse
 * SendPreventSilentAccessResponse
 * constructor (register script command callback)
 * destructor (remove script command callback)

Added unit tests for this class, which test it via public
API i.e. exposed JavaScript interface. Tests are meant to
ensure that:
 * calling exposed navigator.credentials method
|get|, |store| or |preventSilentAccess| results in calling
proper method on CredentialManagerImpl
 * that arguments are validated before that happens
 * that calls from insecure context are ignored
 * that Promises returned by navigator.credentials methods
|get|, |store|, |preventSilentAccess| resolve with correct
type and correct value or reject with error of right type.

Change-Id: Ia82c0ec29b9011fc763abc952b888c7483b4b840
Reviewed-on: https://chromium-review.googlesource.com/649610
Commit-Queue: Tomasz Garbus <tgarbus@chromium.org>
Reviewed-by: default avatarEugene But <eugenebut@chromium.org>
Reviewed-by: default avatarVasilii Sukhanov <vasilii@chromium.org>
Cr-Commit-Position: refs/heads/master@{#501235}
parent f09ab023
...@@ -7,6 +7,10 @@ ...@@ -7,6 +7,10 @@
#include "components/password_manager/core/browser/credential_manager_impl.h" #include "components/password_manager/core/browser/credential_manager_impl.h"
namespace web {
class WebState;
}
namespace credential_manager { namespace credential_manager {
// Owned by PasswordController. It is responsible for registering and handling // Owned by PasswordController. It is responsible for registering and handling
...@@ -25,7 +29,8 @@ namespace credential_manager { ...@@ -25,7 +29,8 @@ namespace credential_manager {
// website. // website.
class CredentialManager { class CredentialManager {
public: public:
explicit CredentialManager(password_manager::PasswordManagerClient* client); CredentialManager(password_manager::PasswordManagerClient* client,
web::WebState* web_state);
~CredentialManager(); ~CredentialManager();
private: private:
...@@ -46,6 +51,7 @@ class CredentialManager { ...@@ -46,6 +51,7 @@ class CredentialManager {
void SendStoreResponse(int promise_id); void SendStoreResponse(int promise_id);
password_manager::CredentialManagerImpl impl_; password_manager::CredentialManagerImpl impl_;
web::WebState* web_state_;
DISALLOW_COPY_AND_ASSIGN(CredentialManager); DISALLOW_COPY_AND_ASSIGN(CredentialManager);
}; };
......
...@@ -5,7 +5,10 @@ ...@@ -5,7 +5,10 @@
#import "ios/chrome/browser/passwords/credential_manager.h" #import "ios/chrome/browser/passwords/credential_manager.h"
#import "base/mac/bind_objc_block.h" #import "base/mac/bind_objc_block.h"
#include "base/strings/utf_string_conversions.h"
#include "ios/chrome/browser/passwords/credential_manager_util.h" #include "ios/chrome/browser/passwords/credential_manager_util.h"
#include "ios/chrome/browser/passwords/js_credential_manager.h"
#import "ios/web/public/web_state/web_state.h"
#if !defined(__has_feature) || !__has_feature(objc_arc) #if !defined(__has_feature) || !__has_feature(objc_arc)
#error "This file requires ARC support." #error "This file requires ARC support."
...@@ -18,46 +21,80 @@ using password_manager::CredentialMediationRequirement; ...@@ -18,46 +21,80 @@ using password_manager::CredentialMediationRequirement;
namespace credential_manager { namespace credential_manager {
namespace {
// Script command prefix for CM API calls. Possible commands to be sent from
// injected JS are 'credentials.get', 'credentials.store' and
// 'credentials.preventSilentAccess'.
constexpr char kCommandPrefix[] = "credentials";
} // namespace
CredentialManager::CredentialManager( CredentialManager::CredentialManager(
password_manager::PasswordManagerClient* client) password_manager::PasswordManagerClient* client,
: impl_(client) { web::WebState* web_state)
// TODO(crbug.com/435047): Register a script command callback for prefix : impl_(client), web_state_(web_state) {
// "credentials" and HandleScriptCommand method. web_state_->AddScriptCommandCallback(
base::Bind(&CredentialManager::HandleScriptCommand,
base::Unretained(this)),
kCommandPrefix);
} }
CredentialManager::~CredentialManager() {} CredentialManager::~CredentialManager() {
web_state_->RemoveScriptCommandCallback(kCommandPrefix);
}
bool CredentialManager::HandleScriptCommand(const base::DictionaryValue& json, bool CredentialManager::HandleScriptCommand(const base::DictionaryValue& json,
const GURL& origin_url, const GURL& origin_url,
bool user_is_interacting) { bool user_is_interacting) {
// TODO(crbug.com/435047): Check if context is secure. double promise_id_double = -1;
std::string command; // |promiseId| field should be an integer value, but since JavaScript has only
if (!json.GetString("command", &command)) { // one type for numbers (64-bit float), all numbers in the messages are sent
DLOG(ERROR) << "RECEIVED BAD json - NO VALID 'command' FIELD"; // as doubles.
if (!json.GetDouble("promiseId", &promise_id_double)) {
DLOG(ERROR) << "Received bad json - no valid 'promiseId' field";
return false; return false;
} }
int promise_id = static_cast<int>(promise_id_double);
int promise_id; if (!WebStateContentIsSecureHtml(web_state_)) {
if (!json.GetInteger("promiseId", &promise_id)) { RejectPromiseWithInvalidStateError(
DLOG(ERROR) << "RECEIVED BAD json - NO VALID 'promiseId' FIELD"; web_state_, promise_id,
base::ASCIIToUTF16(
"Credential Manager API called from insecure context"));
return true;
}
std::string command;
if (!json.GetString("command", &command)) {
DLOG(ERROR) << "Received bad json - no valid 'command' field";
return false; return false;
} }
if (command == "credentials.get") { if (command == "credentials.get") {
CredentialMediationRequirement mediation; CredentialMediationRequirement mediation;
if (!ParseMediationRequirement(json, &mediation)) { if (!ParseMediationRequirement(json, &mediation)) {
// TODO(crbug.com/435047): Reject promise with a TypeError. RejectPromiseWithTypeError(
return false; web_state_, promise_id,
base::ASCIIToUTF16(
"CredentialRequestOptions: Invalid 'mediation' value."));
return true;
} }
bool include_passwords; bool include_passwords;
if (!ParseIncludePasswords(json, &include_passwords)) { if (!ParseIncludePasswords(json, &include_passwords)) {
// TODO(crbug.com/435047): Reject promise with a TypeError. RejectPromiseWithTypeError(
return false; web_state_, promise_id,
base::ASCIIToUTF16(
"CredentialRequestOptions: Invalid 'password' value."));
return true;
} }
std::vector<GURL> federations; std::vector<GURL> federations;
if (!ParseFederations(json, &federations)) { if (!ParseFederations(json, &federations)) {
// TODO(crbug.com/435047): Reject promise with a TypeError. RejectPromiseWithTypeError(
return false; web_state_, promise_id,
base::ASCIIToUTF16(
"CredentialRequestOptions: invalid 'providers' value."));
return true;
} }
impl_.Get(mediation, include_passwords, federations, impl_.Get(mediation, include_passwords, federations,
base::BindOnce(&CredentialManager::SendGetResponse, base::BindOnce(&CredentialManager::SendGetResponse,
...@@ -67,7 +104,11 @@ bool CredentialManager::HandleScriptCommand(const base::DictionaryValue& json, ...@@ -67,7 +104,11 @@ bool CredentialManager::HandleScriptCommand(const base::DictionaryValue& json,
if (command == "credentials.store") { if (command == "credentials.store") {
CredentialInfo credential; CredentialInfo credential;
if (!ParseCredentialDictionary(json, &credential)) { if (!ParseCredentialDictionary(json, &credential)) {
// TODO(crbug.com/435047): Reject promise with a TypeError. // TODO(crbug.com/435047): Refactor ParseCredentialDictionary method to
// provide more meaningful error message.
RejectPromiseWithTypeError(
web_state_, promise_id,
base::ASCIIToUTF16("Invalid Credential object."));
return false; return false;
} }
impl_.Store(credential, impl_.Store(credential,
...@@ -87,10 +128,39 @@ bool CredentialManager::HandleScriptCommand(const base::DictionaryValue& json, ...@@ -87,10 +128,39 @@ bool CredentialManager::HandleScriptCommand(const base::DictionaryValue& json,
void CredentialManager::SendGetResponse( void CredentialManager::SendGetResponse(
int promise_id, int promise_id,
CredentialManagerError error, CredentialManagerError error,
const base::Optional<CredentialInfo>& info) {} const base::Optional<CredentialInfo>& info) {
switch (error) {
case CredentialManagerError::SUCCESS:
ResolvePromiseWithCredentialInfo(web_state_, promise_id, info);
break;
case CredentialManagerError::DISABLED:
RejectPromiseWithInvalidStateError(
web_state_, promise_id,
base::ASCIIToUTF16("Credential Manager is disabled."));
break;
case CredentialManagerError::PENDINGREQUEST:
RejectPromiseWithInvalidStateError(
web_state_, promise_id,
base::ASCIIToUTF16("Pending 'get()' request."));
break;
case CredentialManagerError::PASSWORDSTOREUNAVAILABLE:
RejectPromiseWithNotSupportedError(
web_state_, promise_id,
base::ASCIIToUTF16("Password store is unavailable."));
break;
case CredentialManagerError::UNKNOWN:
RejectPromiseWithNotSupportedError(
web_state_, promise_id,
base::ASCIIToUTF16("Unknown error has occurred."));
}
}
void CredentialManager::SendPreventSilentAccessResponse(int promise_id) {} void CredentialManager::SendPreventSilentAccessResponse(int promise_id) {
ResolvePromiseWithUndefined(web_state_, promise_id);
}
void CredentialManager::SendStoreResponse(int promise_id) {} void CredentialManager::SendStoreResponse(int promise_id) {
ResolvePromiseWithUndefined(web_state_, promise_id);
}
} // namespace credential_manager } // namespace credential_manager
...@@ -40,20 +40,20 @@ security_state::SecurityLevel GetSecurityLevelForWebState( ...@@ -40,20 +40,20 @@ security_state::SecurityLevel GetSecurityLevelForWebState(
} // namespace } // namespace
const char kCredentialIdKey[] = "id"; const char kCredentialIdKey[] = "_id";
const char kCredentialTypeKey[] = "type"; const char kCredentialTypeKey[] = "_type";
const char kCredentialNameKey[] = "name"; const char kCredentialNameKey[] = "_name";
const char kCredentialIconKey[] = "iconURL"; const char kCredentialIconKey[] = "_iconURL";
const char kPasswordCredentialPasswordKey[] = "password"; const char kPasswordCredentialPasswordKey[] = "_password";
const char kFederatedCredentialProviderKey[] = "provider"; const char kFederatedCredentialProviderKey[] = "_provider";
const char kCredentialRequestMediationKey[] = "mediation"; const char kCredentialRequestMediationKey[] = "mediation";
const char kCredentialRequestPasswordKey[] = "password"; const char kCredentialRequestPasswordKey[] = "password";
const char kCredentialRequestProvidersKey[] = "providers"; const char kCredentialRequestProvidersKey[] = "providers";
const char kMediationRequirementSilent[] = "silent"; const char kMediationRequirementSilent[] = "silent";
const char kMediationRequirementRequired[] = "required"; const char kMediationRequirementRequired[] = "required";
const char kMediationRequirementOptional[] = "optional"; const char kMediationRequirementOptional[] = "optional";
const char kCredentialTypePassword[] = "PasswordCredential"; const char kCredentialTypePassword[] = "password";
const char kCredentialTypeFederated[] = "FederatedCredential"; const char kCredentialTypeFederated[] = "federated";
bool ParseMediationRequirement(const base::DictionaryValue& json, bool ParseMediationRequirement(const base::DictionaryValue& json,
CredentialMediationRequirement* mediation) { CredentialMediationRequirement* mediation) {
......
...@@ -40,7 +40,7 @@ std::string CredentialInfoToJsCredential( ...@@ -40,7 +40,7 @@ std::string CredentialInfoToJsCredential(
base::GetQuotedJSONString(info.icon.spec()).c_str(), base::GetQuotedJSONString(info.icon.spec()).c_str(),
base::GetQuotedJSONString(info.password).c_str()); base::GetQuotedJSONString(info.password).c_str());
} }
NOTREACHED() << "Invalid CredentialType"; /* if (info.type == CREDENTIAL_TYPE_EMPTY) */
return std::string(); return std::string();
} }
......
...@@ -183,10 +183,10 @@ function PasswordCredential(init) { ...@@ -183,10 +183,10 @@ function PasswordCredential(init) {
// Perform following steps: // Perform following steps:
// https://w3c.github.io/webappsec-credential-management/#abstract-opdef-create-a-passwordcredential-from-passwordcredentialdata // https://w3c.github.io/webappsec-credential-management/#abstract-opdef-create-a-passwordcredential-from-passwordcredentialdata
if (!data.id) { if (!data.id || typeof data.id != 'string') {
throw new TypeError('id must be a non-empty string'); throw new TypeError('id must be a non-empty string');
} }
if (!data.password) { if (!data.password || typeof data.password != 'string') {
throw new TypeError('password must be a non-empty string'); throw new TypeError('password must be a non-empty string');
} }
if (data.iconURL && !data.iconURL.startsWith('https://')) { if (data.iconURL && !data.iconURL.startsWith('https://')) {
...@@ -260,10 +260,10 @@ Object.defineProperty(PasswordCredential.prototype, Symbol.toStringTag, ...@@ -260,10 +260,10 @@ Object.defineProperty(PasswordCredential.prototype, Symbol.toStringTag,
* @constructor * @constructor
*/ */
function FederatedCredential(init) { function FederatedCredential(init) {
if (!init.id) { if (!init.id || typeof init.id != 'string') {
throw new TypeError('id must be a non-empty string'); throw new TypeError('id must be a non-empty string');
} }
if (!init.provider) { if (!init.provider || typeof init.provider != 'string') {
throw new TypeError('provider must be a non-empty string'); throw new TypeError('provider must be a non-empty string');
} }
if (!init.provider.startsWith('https://') && if (!init.provider.startsWith('https://') &&
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment