[epiphany/mcatanzaro/password-manager-frames: 64/64] Password manager should work in iframes
- From: Michael Catanzaro <mcatanzaro src gnome org>
- To: commits-list gnome org
- Cc:
- Subject: [epiphany/mcatanzaro/password-manager-frames: 64/64] Password manager should work in iframes
- Date: Thu, 6 Jun 2019 13:04:42 +0000 (UTC)
commit 065361ac0e76c6d7fcfab1098dced8986af41dd7
Author: Michael Catanzaro <mcatanzaro igalia com>
Date: Wed Apr 24 23:01:10 2019 -0500
Password manager should work in iframes
Currently the password manager does not work on reddit.com because we
run Ephy JS only in the main frame, never in iframes, but the login form
on reddit is an iframe. What a shame! Let's change this.
It requires a new webkit_frame_get_id() API to be able to track frames
across the process boundary, and also a new
form-controls-associated-for-frame signal since sadly we can't add new
parameters to signals without breaking API.
Note this only works if the frame's origin is the same as the main
frame's origin. If the iframe has a different security origin, then the
password will not be saved at all. This is because the UI process
ensures the save request matches the page's origin to prevent a
compromised web process from overwriting passwords for other origins.
Note also I found a minor bug here in our will-submit-form callback: the
target_frame parameter was named source_frame. That was not good, but
fortunately it wasn't being used for anything, so this didn't matter.
Probably fixes #25 and probably also fixes #26
embed/ephy-embed-shell.c | 16 +++-
embed/ephy-web-process-extension-proxy.c | 8 +-
embed/ephy-web-process-extension-proxy.h | 4 +-
.../ephy-web-process-extension.c | 88 ++++++++++++++++------
embed/web-process-extension/resources/js/ephy.js | 32 ++++----
meson.build | 2 +-
6 files changed, 102 insertions(+), 48 deletions(-)
---
diff --git a/embed/ephy-embed-shell.c b/embed/ephy-embed-shell.c
index 4632065a3..8a38d76d6 100644
--- a/embed/ephy-embed-shell.c
+++ b/embed/ephy-embed-shell.c
@@ -346,6 +346,7 @@ typedef struct {
char *origin;
gint32 promise_id;
guint64 page_id;
+ guint64 frame_id;
} PasswordManagerData;
static void
@@ -374,7 +375,7 @@ password_manager_query_finished_cb (GList *records,
data->page_id,
data->origin);
if (proxy)
- ephy_web_process_extension_proxy_password_query_response (proxy, username, password, data->promise_id,
data->page_id);
+ ephy_web_process_extension_proxy_password_query_response (proxy, username, password, data->promise_id,
data->frame_id);
password_manager_data_free (data);
}
@@ -419,6 +420,7 @@ web_process_extension_password_manager_query_received_cb (WebKitUserContentManag
g_autofree char *password_field = property_to_string_or_null (value, "passwordField");
gint32 promise_id = property_to_int32 (value, "promiseID");
guint64 page_id = property_to_uint64 (value, "pageID");
+ guint64 frame_id = property_to_uint64 (value, "frameID");
if (!origin || !target_origin || !password_field)
return;
@@ -427,7 +429,8 @@ web_process_extension_password_manager_query_received_cb (WebKitUserContentManag
data->shell = g_object_ref (shell);
data->promise_id = promise_id;
data->page_id = page_id;
- data->origin = g_strdup (origin);
+ data->frame_id = frame_id;
+ data->origin = g_steal_pointer (&origin);
ephy_password_manager_query (priv->password_manager,
NULL,
@@ -517,7 +520,10 @@ web_process_extension_password_manager_save_real (EphyEmbedShell *shell,
/* This also sanity checks that a page isn't saving websites for
* other origins. Remember the request comes from the untrusted web
- * process and we have to make sure it's not being evil here.
+ * process and we have to make sure it's not being evil here. This
+ * could also happen even without malice if the origin of a subframe
+ * doesn't match the origin of the main frame (in which case we'll
+ * refuse to save the password).
*/
view = ephy_embed_shell_get_view_for_page_id (shell, page_id, origin);
if (!view)
@@ -573,6 +579,8 @@ web_process_extension_password_manager_query_usernames_received_cb (WebKitUserCo
g_autofree char *origin = property_to_string_or_null (value, "origin");
gint32 promise_id = property_to_int32 (value, "promiseID");
guint64 page_id = property_to_uint64 (value, "pageID");
+ guint64 frame_id = property_to_uint64 (value, "frameID");
+
GList *usernames;
if (!origin)
@@ -582,7 +590,7 @@ web_process_extension_password_manager_query_usernames_received_cb (WebKitUserCo
EphyWebProcessExtensionProxy *proxy = ephy_embed_shell_get_extension_proxy_for_page_id (shell, page_id,
origin);
if (proxy)
- ephy_web_process_extension_proxy_password_query_usernames_response (proxy, usernames, promise_id,
page_id);
+ ephy_web_process_extension_proxy_password_query_usernames_response (proxy, usernames, promise_id,
frame_id);
}
static void
diff --git a/embed/ephy-web-process-extension-proxy.c b/embed/ephy-web-process-extension-proxy.c
index 5a05acc25..71c839458 100644
--- a/embed/ephy-web-process-extension-proxy.c
+++ b/embed/ephy-web-process-extension-proxy.c
@@ -298,7 +298,7 @@ void
ephy_web_process_extension_proxy_password_query_usernames_response (EphyWebProcessExtensionProxy
*web_process_extension,
GList *users,
gint32 promise_id,
- guint64 page_id)
+ guint64 frame_id)
{
if (!web_process_extension->proxy)
return;
@@ -310,7 +310,7 @@ ephy_web_process_extension_proxy_password_query_usernames_response (EphyWebProce
g_dbus_proxy_call (web_process_extension->proxy,
"PasswordQueryUsernamesResponse",
- g_variant_new ("(asit)", &builder, promise_id, page_id),
+ g_variant_new ("(asit)", &builder, promise_id, frame_id),
G_DBUS_CALL_FLAGS_NONE,
-1,
web_process_extension->cancellable,
@@ -322,14 +322,14 @@ ephy_web_process_extension_proxy_password_query_response (EphyWebProcessExtensio
const char *username,
const char *password,
gint32 promise_id,
- guint64 page_id)
+ guint64 frame_id)
{
if (!web_process_extension->proxy)
return;
g_dbus_proxy_call (web_process_extension->proxy,
"PasswordQueryResponse",
- g_variant_new ("(ssit)", username ?: "", password ?: "", promise_id, page_id),
+ g_variant_new ("(ssit)", username ?: "", password ?: "", promise_id, frame_id),
G_DBUS_CALL_FLAGS_NONE,
-1,
web_process_extension->cancellable,
diff --git a/embed/ephy-web-process-extension-proxy.h b/embed/ephy-web-process-extension-proxy.h
index 6727555ae..f4b205dfd 100644
--- a/embed/ephy-web-process-extension-proxy.h
+++ b/embed/ephy-web-process-extension-proxy.h
@@ -45,10 +45,10 @@ void ephy_web_process_extension_proxy_history_clear
void ephy_web_process_extension_proxy_password_query_usernames_response
(EphyWebProcessExtensionProxy *web_process_extension,
GList
*users,
gint32
promise_id,
- guint64
page_id);
+ guint64
frame_id);
void ephy_web_process_extension_proxy_password_query_response
(EphyWebProcessExtensionProxy *web_process_extension,
const char
*username,
const char
*password,
gint32
promise_id,
- guint64
page_id);
+ guint64
frame_id);
G_END_DECLS
diff --git a/embed/web-process-extension/ephy-web-process-extension.c
b/embed/web-process-extension/ephy-web-process-extension.c
index a40a8fab2..4bd733a9d 100644
--- a/embed/web-process-extension/ephy-web-process-extension.c
+++ b/embed/web-process-extension/ephy-web-process-extension.c
@@ -58,6 +58,8 @@ struct _EphyWebProcessExtension {
WebKitScriptWorld *script_world;
gboolean is_private_profile;
+
+ GHashTable *frames_map;
};
static const char introspection_xml[] =
@@ -88,12 +90,12 @@ static const char introspection_xml[] =
" <arg type='s' name='username' direction='in'/>"
" <arg type='s' name='password' direction='in'/>"
" <arg type='i' name='promise_id' direction='in'/>"
- " <arg type='t' name='page_id' direction='in'/>"
+ " <arg type='t' name='frame_id' direction='in'/>"
" </method>"
" <method name='PasswordQueryUsernamesResponse'>"
" <arg type='as' name='users' direction='in'/>"
" <arg type='i' name='promise_id' direction='in'/>"
- " <arg type='t' name='page_id' direction='in'/>"
+ " <arg type='t' name='frame_id' direction='in'/>"
" </method>"
" </interface>"
"</node>";
@@ -217,8 +219,8 @@ static void
web_page_will_submit_form (WebKitWebPage *web_page,
WebKitDOMHTMLFormElement *dom_form,
WebKitFormSubmissionStep step,
- WebKitFrame *frame,
WebKitFrame *source_frame,
+ WebKitFrame *target_frame,
GPtrArray *text_field_names,
GPtrArray *text_field_values)
{
@@ -242,34 +244,56 @@ web_page_will_submit_form (WebKitWebPage *web_page,
extension = ephy_web_process_extension_get ();
js_context = webkit_frame_get_js_context_for_script_world (source_frame, extension->script_world);
js_ephy = jsc_context_get_value (js_context, "Ephy");
- js_form = webkit_frame_get_js_value_for_dom_object_in_script_world (frame, WEBKIT_DOM_OBJECT (dom_form),
extension->script_world);
+ js_form = webkit_frame_get_js_value_for_dom_object_in_script_world (source_frame, WEBKIT_DOM_OBJECT
(dom_form), extension->script_world);
js_result = jsc_value_object_invoke_method (js_ephy,
"handleFormSubmission",
G_TYPE_UINT64, webkit_web_page_get_id (web_page),
+ G_TYPE_UINT64, webkit_frame_get_id (source_frame),
JSC_TYPE_VALUE, js_form,
G_TYPE_NONE);
}
static char *
-password_form_message_serializer (guint64 page_id,
+password_form_message_serializer (guint64 frame_id,
gboolean is_insecure_action)
{
GVariant *variant;
char *message;
- variant = g_variant_new ("(tb)", page_id, is_insecure_action);
+ variant = g_variant_new ("(tb)", frame_id, is_insecure_action);
message = g_variant_print (variant, FALSE);
g_variant_unref (variant);
return message;
}
+static gboolean
+remove_if_value_matches_user_data (gpointer key,
+ gpointer value,
+ gpointer user_data)
+{
+ return value == user_data;
+}
+
+static void
+frame_destroyed_notify (EphyWebExtension *extension,
+ GObject *where_the_object_was)
+{
+ /* This is awkward. We can't just remove the frame from the table
+ * directly since we don't have any way to get its ID except by
+ * checking every entry in the map....
+ */
+ g_hash_table_foreach_remove (extension->frames_map,
+ remove_if_value_matches_user_data,
+ where_the_object_was);
+}
+
static void
web_page_form_controls_associated (WebKitWebPage *web_page,
GPtrArray *elements,
+ WebKitFrame *frame,
EphyWebProcessExtension *extension)
{
- WebKitFrame *frame;
g_autoptr(GPtrArray) form_controls = NULL;
g_autoptr(JSCContext) js_context = NULL;
g_autoptr(JSCValue) js_ephy = NULL;
@@ -277,7 +301,6 @@ web_page_form_controls_associated (WebKitWebPage *web_page,
g_autoptr(JSCValue) js_result = NULL;
guint i;
- frame = webkit_web_page_get_main_frame (web_page);
js_context = webkit_frame_get_js_context_for_script_world (frame, extension->script_world);
form_controls = g_ptr_array_new_with_free_func (g_object_unref);
@@ -296,9 +319,15 @@ web_page_form_controls_associated (WebKitWebPage *web_page,
js_result = jsc_value_object_invoke_method (js_ephy,
"formControlsAssociated",
G_TYPE_UINT64, webkit_web_page_get_id (web_page),
+ G_TYPE_UINT64, webkit_frame_get_id (frame),
G_TYPE_PTR_ARRAY, form_controls,
JSC_TYPE_VALUE, js_serializer,
G_TYPE_NONE);
+
+ if (!g_hash_table_contains (extension->frames_map, GINT_TO_POINTER (webkit_frame_get_id (frame)))) {
+ g_hash_table_insert (extension->frames_map, GINT_TO_POINTER (webkit_frame_get_id (frame)), frame);
+ g_object_weak_ref (G_OBJECT (frame), (GWeakNotify)frame_destroyed_notify, extension);
+ }
}
static gboolean
@@ -315,6 +344,9 @@ web_page_context_menu (WebKitWebPage *web_page,
g_autoptr(JSCValue) js_value = NULL;
extension = ephy_web_process_extension_get ();
+ /* FIXME: this is wrong, see https://gitlab.gnome.org/GNOME/epiphany/issues/442
+ * We need a way to get the right frame to use here.
+ */
frame = webkit_web_page_get_main_frame (web_page);
js_context = webkit_frame_get_js_context_for_script_world (frame, extension->script_world);
@@ -403,25 +435,23 @@ ephy_web_process_extension_page_created_cb (EphyWebProcessExtension *extension,
g_signal_connect (web_page, "will-submit-form",
G_CALLBACK (web_page_will_submit_form),
extension);
- g_signal_connect (web_page, "form-controls-associated",
+ g_signal_connect (web_page, "form-controls-associated-for-frame",
G_CALLBACK (web_page_form_controls_associated),
extension);
}
static JSCValue *
get_password_manager (EphyWebProcessExtension *self,
- guint64 page_id)
+ guint64 frame_id)
{
- WebKitWebPage *page;
WebKitFrame *frame;
g_autoptr(JSCContext) js_context = NULL;
g_autoptr(JSCValue) js_ephy = NULL;
- page = webkit_web_extension_get_page (self->extension, page_id);
- if (page == NULL)
+ frame = g_hash_table_lookup (self->frames_map, GINT_TO_POINTER (frame_id));
+ if (!frame)
return NULL;
- frame = webkit_web_page_get_main_frame (page);
js_context = webkit_frame_get_js_context_for_script_world (frame, self->script_world);
js_ephy = jsc_context_get_value (js_context, "Ephy");
@@ -503,13 +533,13 @@ handle_method_call (GDBusConnection *connection,
g_autoptr(JSCValue) ret = NULL;
g_autoptr(JSCValue) password_manager = NULL;
gint32 promise_id;
- guint64 page_id;
+ guint64 frame_id;
users = g_variant_get_strv (g_variant_get_child_value (parameters, 0), NULL);
g_variant_get_child (parameters, 1, "i", &promise_id);
- g_variant_get_child (parameters, 2, "t", &page_id);
+ g_variant_get_child (parameters, 2, "t", &frame_id);
- password_manager = get_password_manager (extension, page_id);
+ password_manager = get_password_manager (extension, frame_id);
if (password_manager != NULL)
ret = jsc_value_object_invoke_method (password_manager, "_onQueryUsernamesResponse",
G_TYPE_STRV, users, G_TYPE_INT, promise_id, G_TYPE_NONE);
@@ -517,12 +547,12 @@ handle_method_call (GDBusConnection *connection,
const char *username;
const char *password;
gint32 promise_id;
- guint64 page_id;
+ guint64 frame_id;
g_autoptr(JSCValue) ret = NULL;
g_autoptr(JSCValue) password_manager = NULL;
- g_variant_get (parameters, "(&s&sit)", &username, &password, &promise_id, &page_id);
- password_manager = get_password_manager (extension, page_id);
+ g_variant_get (parameters, "(&s&sit)", &username, &password, &promise_id, &frame_id);
+ password_manager = get_password_manager (extension, frame_id);
if (password_manager != NULL)
ret = jsc_value_object_invoke_method (password_manager, "_onQueryResponse",
G_TYPE_STRING, username,
@@ -537,6 +567,12 @@ static const GDBusInterfaceVTable interface_vtable = {
NULL
};
+static void
+drop_frame_weak_ref (gpointer key, gpointer value, gpointer extension)
+{
+ g_object_weak_unref (G_OBJECT (value), (GWeakNotify)frame_destroyed_notify, extension);
+}
+
static void
ephy_web_process_extension_dispose (GObject *object)
{
@@ -555,6 +591,11 @@ ephy_web_process_extension_dispose (GObject *object)
g_clear_object (&extension->dbus_connection);
g_clear_object (&extension->extension);
+ if (extension->frames_map) {
+ g_hash_table_foreach (extension->frames_map, drop_frame_weak_ref, extension);
+ g_clear_pointer (&extension->frames_map, g_hash_table_unref);
+ }
+
G_OBJECT_CLASS (ephy_web_process_extension_parent_class)->dispose (object);
}
@@ -713,9 +754,6 @@ window_object_cleared_cb (WebKitScriptWorld *world,
g_autoptr(JSCValue) js_function = NULL;
g_autoptr(JSCValue) result = NULL;
- if (!webkit_frame_is_main_frame (frame))
- return;
-
js_context = webkit_frame_get_js_context_for_script_world (frame, world);
jsc_context_push_exception_handler (js_context, (JSCExceptionHandler)js_exception_handler, NULL, NULL);
@@ -772,6 +810,7 @@ window_object_cleared_cb (WebKitScriptWorld *world,
g_autoptr(JSCValue) js_password_manager_ctor = jsc_value_object_get_property (js_ephy,
"PasswordManager");
g_autoptr(JSCValue) js_password_manager = jsc_value_constructor_call (js_password_manager_ctor,
G_TYPE_UINT64,
webkit_web_page_get_id (page),
+ G_TYPE_UINT64, webkit_frame_get_id
(frame),
G_TYPE_NONE);
jsc_value_object_set_property (js_ephy, "passwordManager", js_password_manager);
@@ -854,4 +893,7 @@ ephy_web_process_extension_initialize (EphyWebProcessExtension *extension,
extension);
extension->uri_tester = ephy_uri_tester_new (adblock_data_dir);
+
+ extension->frames_map = g_hash_table_new_full (g_direct_hash, g_direct_equal,
+ NULL, NULL);
}
diff --git a/embed/web-process-extension/resources/js/ephy.js
b/embed/web-process-extension/resources/js/ephy.js
index 55556abeb..440cdf3ac 100644
--- a/embed/web-process-extension/resources/js/ephy.js
+++ b/embed/web-process-extension/resources/js/ephy.js
@@ -248,33 +248,34 @@ Ephy.PreFillUserMenu = class PreFillUserMenu
}
}
-Ephy.formControlsAssociated = function(pageID, forms, serializer)
+Ephy.formControlsAssociated = function(pageID, frameID, forms, serializer)
{
Ephy.formManagers = [];
for (let i = 0; i < forms.length; i++) {
if (!(forms[i] instanceof HTMLFormElement))
continue;
- let formManager = new Ephy.FormManager(pageID, forms[i]);
+ let formManager = new Ephy.FormManager(pageID, frameID, forms[i]);
formManager.handlePasswordForms(serializer);
formManager.preFillForms();
Ephy.formManagers.push(formManager);
}
}
-Ephy.handleFormSubmission = function(pageID, form)
+Ephy.handleFormSubmission = function(pageID, frameID, form)
{
+ // FIXME: Find out: is it really possible to have multiple frames with same window object???
let formManager = null;
for (let i = 0; i < Ephy.formManagers.length; i++) {
let manager = Ephy.formManagers[i];
- if (manager.pageID() == pageID && manager.form() == form) {
+ if (manager.frameID() == frameID && manager.form() == form) {
formManager = manager;
break;
}
}
if (!formManager) {
- formManager = new Ephy.FormManager(pageID, form);
+ formManager = new Ephy.FormManager(pageID, frameID, form);
Ephy.formManagers.push(formManager);
}
@@ -311,9 +312,10 @@ Ephy.hasModifiedForms = function()
Ephy.PasswordManager = class PasswordManager
{
- constructor(pageID)
+ constructor(pageID, frameID)
{
this._pageID = pageID;
+ this._frameID = frameID;
this._pendingPromises = [];
this._promiseCounter = 0;
}
@@ -345,7 +347,7 @@ Ephy.PasswordManager = class PasswordManager
let promiseID = this._promiseCounter++;
window.webkit.messageHandlers.passwordManagerQuery.postMessage({
origin, targetOrigin, username, usernameField, passwordField, promiseID,
- pageID: this._pageID,
+ pageID: this._pageID, frameID: this._frameID
});
this._pendingPromises.push({promiseID, resolver});
});
@@ -355,15 +357,16 @@ Ephy.PasswordManager = class PasswordManager
{
window.webkit.messageHandlers.passwordManagerSave.postMessage({
origin, targetOrigin, username, password, usernameField, passwordField, isNew,
- pageID: this._pageID,
+ pageID: this._pageID
});
}
+ // FIXME: Why is pageID a parameter here?
requestSave(origin, targetOrigin, username, password, usernameField, passwordField, isNew, pageID)
{
window.webkit.messageHandlers.passwordManagerRequestSave.postMessage({
origin, targetOrigin, username, password, usernameField, passwordField, isNew,
- pageID,
+ pageID
});
}
@@ -379,7 +382,7 @@ Ephy.PasswordManager = class PasswordManager
return new Promise((resolver, reject) => {
let promiseID = this._promiseCounter++;
window.webkit.messageHandlers.passwordManagerQueryUsernames.postMessage({
- origin, promiseID, pageID: this._pageID,
+ origin, promiseID, pageID: this._pageID, frameID: this._frameID
});
this._pendingPromises.push({promiseID, resolver});
});
@@ -388,9 +391,10 @@ Ephy.PasswordManager = class PasswordManager
Ephy.FormManager = class FormManager
{
- constructor(pageID, form)
+ constructor(pageID, frameID, form)
{
this._pageID = pageID;
+ this._frameID = frameID;
this._form = form;
this._passwordFormMessageSerializer = null;
this._preFillUserMenu = null;
@@ -399,9 +403,9 @@ Ephy.FormManager = class FormManager
// Public
- pageID()
+ frameID()
{
- return this._pageID;
+ return this._frameID;
}
form()
@@ -561,7 +565,7 @@ Ephy.FormManager = class FormManager
let url = new URL(this._form.action);
// Warning: we do not whitelist localhost because it could be redirected by DNS.
let isInsecureAction = url.protocol == 'http:' && url.hostname != "127.0.0.1" && url.hostname !=
"::1";
-
window.webkit.messageHandlers.passwordFormFocused.postMessage(this._passwordFormMessageSerializer(this._pageID,
isInsecureAction));
+
window.webkit.messageHandlers.passwordFormFocused.postMessage(this._passwordFormMessageSerializer(this._frameID,
isInsecureAction));
}
_findPasswordFields()
diff --git a/meson.build b/meson.build
index b7af3b426..701eae7c8 100644
--- a/meson.build
+++ b/meson.build
@@ -79,7 +79,7 @@ config_h = declare_dependency(
glib_requirement = '>= 2.56.0'
gtk_requirement = '>= 3.24.0'
nettle_requirement = '>= 3.4'
-webkitgtk_requirement = '>= 2.24.1'
+webkitgtk_requirement = '>= 2.25.1'
cairo_dep = dependency('cairo', version: '>= 1.2')
gcr_dep = dependency('gcr-3', version: '>= 3.5.5')
[
Date Prev][
Date Next] [
Thread Prev][
Thread Next]
[
Thread Index]
[
Date Index]
[
Author Index]
