Index: components/autofill/core/common/save_password_progress_logger.h |
diff --git a/components/autofill/core/common/save_password_progress_logger.h b/components/autofill/core/common/save_password_progress_logger.h |
index 9df99080152f6479ab5d61aeaee518a491ab7d68..bfea98c36c4bb4e37152465b068e799584fb7cd4 100644 |
--- a/components/autofill/core/common/save_password_progress_logger.h |
+++ b/components/autofill/core/common/save_password_progress_logger.h |
@@ -6,7 +6,9 @@ |
#define COMPONENTS_AUTOFILL_CORE_COMMON_SAVE_PASSWORD_PROGRESS_LOGGER_H_ |
#include <string> |
+#include <vector> |
+#include "base/strings/string16.h" |
#include "url/gurl.h" |
namespace base { |
@@ -32,39 +34,123 @@ struct PasswordForm; |
// to stay in autofill as well. |
class SavePasswordProgressLogger { |
public: |
- // All three possible decisions about saving a password. Call LogFinalDecision |
- // as soon as one is taken by the password management code. |
- enum Decision { DECISION_SAVE, DECISION_ASK, DECISION_DROP }; |
+ // IDs of strings allowed in the logs: for security reasons, we only pass the |
+ // IDs from the renderer, and map them to strings in the browser. |
+ // Make sure to keep this in the same order as |kIDsToMessages| in the .cc |
+ // file. |
Ilya Sherman
2014/04/12 00:00:51
Rather than requiring the order to match, please j
vabr (Chromium)
2014/04/13 20:12:30
Done.
|
+ // The code relies on them being automatically numbered, starting with 0 and |
+ // increasing by 1 (guaranteed by the C++ standard, see ISO/IEC 14882:2003 |
+ // 7.2.1). |
+ enum StringID { |
+ STRING_DECISION_ASK, |
+ STRING_DECISION_DROP, |
+ STRING_DECISION_SAVE, |
+ STRING_METHOD, |
+ STRING_METHOD_GET, |
+ STRING_METHOD_POST, |
+ STRING_METHOD_EMPTY, |
+ STRING_OTHER, |
+ STRING_SCHEME_HTML, |
+ STRING_SCHEME_BASIC, |
+ STRING_SCHEME_DIGEST, |
+ STRING_SCHEME_MESSAGE, |
+ STRING_SIGNON_REALM, |
+ STRING_ORIGINAL_SIGNON_REALM, |
+ STRING_ORIGIN, |
+ STRING_ACTION, |
+ STRING_USERNAME_ELEMENT, |
+ STRING_PASSWORD_ELEMENT, |
+ STRING_PASSWORD_AUTOCOMPLETE_SET, |
+ STRING_OLD_PASSWORD_ELEMENT, |
+ STRING_SSL_VALID, |
+ STRING_PASSWORD_GENERATED, |
+ STRING_TIMES_USED, |
+ STRING_USE_ADDITIONAL_AUTHENTICATION, |
+ STRING_PSL_MATCH, |
+ STRING_NAME_OR_ID, |
+ STRING_MESSAGE, |
+ STRING_INVALID, // Represents a string returned in a case of an error. |
+ STRING_MAX = STRING_INVALID |
+ }; |
+ |
+ // A "structured" log, as opposed to just a log string, is passed from the |
+ // place of log creation (which might be in a renderer code) to browser for |
+ // display. We need to keep it structured to allow browser-side sanitization |
+ // for security reasons. |
+ struct StructuredLog { |
+ // Each log has one of these types. Depending on the type, the valid data |
+ // members are shown in comments (in addition to |log_type| itself). |
+ enum LogType { |
+ LOG_TYPE_LABEL, // |label| |
+ LOG_TYPE_GURL, // |label|, |url| |
+ LOG_TYPE_MESSAGE, // |label|, |message| |
+ LOG_TYPE_NUMBER, // |label|, |number| |
+ LOG_TYPE_BOOL, // |label|, |truth_value| |
+ LOG_TYPE_ELEMENT_ID, // |label|, |element_id| |
+ LOG_TYPE_MAX = LOG_TYPE_ELEMENT_ID |
+ }; |
+ |
+ LogType log_type; |
+ StringID label; |
+ // The members below would ideally form a union, but cannot, because of |
+ // non-POD members. The size overhead does not seem worse than having |
+ // multiple IPC methods specialized for each possible log type. |
+ GURL url; |
+ StringID message; |
+ int number; |
+ bool truth_value; |
+ // Only use |element_id| to store an HTML element name or id; for security |
+ // reasons all characters other than alphanumeric ones will be replaced by |
+ // spaces. |
+ std::string element_id; |
+ |
+ StructuredLog(); |
+ explicit StructuredLog(StringID label); |
+ StructuredLog(StringID label, GURL url); |
+ StructuredLog(StringID label, StringID message); |
+ StructuredLog(StringID label, int number); |
+ StructuredLog(StringID label, bool truth_value); |
+ StructuredLog(StringID label, const std::string& element_id); |
+ StructuredLog(StringID label, const base::string16& element_id); |
+ ~StructuredLog(); |
+ |
+ // Equality operators for testing. |
+ bool operator==(const StructuredLog& form) const; |
+ bool operator!=(const StructuredLog& form) const; |
+ }; |
Ilya Sherman
2014/04/12 00:00:51
I'm not convinced that passing a struct rather tha
vabr (Chromium)
2014/04/12 09:11:38
My understanding is that LogFoo don't actually pre
Ilya Sherman
2014/04/12 22:25:31
I would keep the previous IPC design, i.e. just se
vabr (Chromium)
2014/04/16 20:55:36
Done.
|
SavePasswordProgressLogger(); |
virtual ~SavePasswordProgressLogger(); |
- // Logging: specialized methods (for logging forms, URLs, etc.) take care of |
- // proper removing of sensitive data where appropriate. |
- void LogPasswordForm(const std::string& message, |
+ // Call SanitizeStructuredLogs only in the browser, to get a safe-for-display |
+ // string representation of |logs|. The vector |logs| needs to be organised in |
+ // one of the two following ways: |
+ // 1) there is only one log in the vector, of type different than |
+ // LOG_TYPE_LABEL, or |
+ // 2) there are more logs, and exactly the first one has type LOG_TYPE_LABEL, |
+ // providing a label for the whole group. |
+ static std::string SanitizeStructuredLogs( |
+ const std::vector<StructuredLog>& logs); |
Ilya Sherman
2014/04/12 00:00:51
How about only making this function available in t
vabr (Chromium)
2014/04/16 20:55:36
That function is gone.
|
+ |
+ // Logging: these methods create and send a StructuredLog for sanitization and |
+ // display. |
+ void LogPasswordForm(StringID label, |
const autofill::PasswordForm& form); |
- void LogHTMLForm(const std::string& message, |
+ void LogHTMLForm(StringID label, |
const std::string& name_or_id, |
const std::string& method, |
const GURL& action); |
- void LogURL(const std::string& message, const GURL& url); |
- void LogBoolean(const std::string& message, bool value); |
- void LogNumber(const std::string& message, int value); |
- void LogNumber(const std::string& message, size_t value); |
- void LogFinalDecision(Decision decision); |
- // Do not use LogMessage when there is an appropriate specialized method |
- // above. LogMessage performs no scrubbing of sensitive data. |
- void LogMessage(const std::string& message); |
+ void LogURL(StringID label, const GURL& url); |
+ void LogBoolean(StringID label, bool value); |
+ void LogNumber(StringID label, int value); |
+ void LogNumber(StringID label, size_t value); |
+ void LogMessage(StringID message); |
protected: |
- // Sends |log| immediately for display. |
- virtual void SendLog(const std::string& log) = 0; |
+ // Sends |logs| immediately for display. |
+ virtual void SendLog(const std::vector<StructuredLog>& logs) = 0; |
private: |
- // Takes a structured |log|, converts it to a string suitable for plain text |
- // output, adds the |name| as a caption, and sends out via SendLog. |
- void LogValue(const std::string& name, const base::Value& log); |
- |
DISALLOW_COPY_AND_ASSIGN(SavePasswordProgressLogger); |
}; |