Integration of Formlock's functionality #94
Conversation
There was a problem hiding this comment.
Is a588f8b the exact copy of the original? If so, please, mention that in the commit message.
It seams to me that 00e65a4 and 782ff96 can be squashed together (see git interactive rebase and squash).
if a588f8b is not the exact copy of the original project then, all three commits should be squashed together.
AFAIU this code is not ready to be merged and this pull request seeks comments?
chrome/formlock.js
Outdated
| @@ -0,0 +1,177 @@ | |||
| /* | |||
There was a problem hiding this comment.
- Most of the functionality seems to be generic, is it really Chrome-specific?
- The file needs a license, have you been able to learn more than:
sure, Formlock is open source - https://github.com/ostarov/Formlock>
Please cite our work properly of course
- We need to distinguish between yours code and the original
There was a problem hiding this comment.
I've made it like this for the time being, because I had some problems while running it on FIrefox
Yes, the author wrote that I could use it as long as I marked his code and mentioned his name and link to original repo in the comments
chrome/mouse_track.js
Outdated
| @@ -0,0 +1,63 @@ | |||
| /* | |||
There was a problem hiding this comment.
- Most of the functionality seems to be generic, is it really Chrome-specific?
- The file needs a license, have you been able to learn more than:
sure, Formlock is open source - https://github.com/ostarov/Formlock>
Please cite our work properly of course
- We need to distinguish between yours code and the original
polcak
force-pushed
the
master
branch
3 times, most recently
from
70c6f80 to
dfdd92d
Compare
|
I shall look into the squashing tomorrow. I have probably misunderstood the point about frequent merging and such. In the future I will create PRs when I have more things done. |
docs/permissions.md
Outdated
| @@ -6,6 +6,9 @@ | |||
| * **webRequest, webRequestBlocking, and all_urls --** *needed for modyfing JavaScript objects and APIs on all pages and also used for capturing and blocking malicious HTTP requests* | |||
| * **dns --** *used for resoluting DNS queries in Firefox version of HTTP request shield* | |||
| * **notifications--** *used for notifying user on blocked HTTP requests/hosts* | |||
| * **contextMenus --** *used for creation of a context menu allowing the user to lock/unlock a certain form* | |||
| * **browsingData --** *used for removal of data stored during the lock of a form* | |||
| * **webNavigation --** *used for adding a listener so that form locking code can be injected in a certain manner* | |||
There was a problem hiding this comment.
"in a certain manner" -> allows form locking code to inject code ...
|
While working on the Firefox update I found out that Firefox doesn't support removing localStorage data with the since option different than 0. If I understood the documentation correctly then that means all localStorage since "forever" would be removed. That sounds like quite a drastic move to make, but it's also crucial to get rid of the stored data. Should I keep it like this and add some text into a notification informing the user that all local data will be deleted upon unlock? |
|
I think that there are several options:
You can iterate through the storage with the code: for (s of [localStorage, sessionStorage]) {
for (i = 0; i < s.length; i++) {
console.log(s.getItem(s.key(i)));
}
}Of course, similar needs to be addressed with Indexed DB. By the way what happens in Chromium browsers if say key X is set originally to Y but changed to Z during locking. Does the original FormLock reload Y or removes X entirely? |
|
I see, keeping the original state would probably be best for the user. Thank you for bringing the second issue to my attention. I found out that the original key is removed entirely, therefore I shall look into restoration of the original values (same with cookies). |
|
I've added restoration of the indexed database, storages and cookies. Sadly, I didn't find any way to get database names in Firefox, so there the backup is currently impossible. |
|
I've squashed a bugfix and saving of store indexes into the data restoration commit. I will also look into improving the reactions to level changes because it currently doesn't account for inserted pages inside the main page. I'll also do some small tweaks like changing the form highlighting to nofitication and changing the way forms are detected as potentially unsafe because otherwise the notification would pop up on almost every page with search bars. |
|
I've been thinking about form detection because with the current detection even forms that have only buttons etc. are detected. Should I code a check that would recursively go through input types of forms and mark as safe forms with input of no informative value (buttons, radio, checkbox) or is that too overkill and would be counterproductive? |
Integration of code from the Formlock extension (https://github.com/ostarov/Formlock). Added comments, copyrights and minor changes like using browser instead of chrome
Differences from the Chrome version include: -Functions returning promises are handled with .then() -Alerts were replaced with notifications (Should probably be added into Chrome version as well) -clear_new_data() has changed properties in accordance with Mozilla documentation
Chrome version includes restoration of indexed databases, but Firefox doesn't due to Firefox's absence of .databases() method.
Context menu is not available on level 0 and no code is injected. If someone changes level to 0 during lock and then refreshes the page then the lock is released and url blocking doesn't continue
Also fixed accidental chrome extension check in Firefox version
The storage change listener in mouse_track.js was removed because the feature wasn't really useful and produced too many informations for the user to care about.
This commit fixes a bug when forms would be highlighted even on pages with JSR level 0. This was due to the fact that each url was checked instead of only checking the tabs main url.
Notification about suspicious forms is created when the user first opens the page and when they switch to it from other tab
This fixes a small bug when going to other page with unsafe forms wouldn't trigger a notification because it happened on the same tab
Both files formlock.js now check for existence of formlock attribute in the level retrieved by getCurrentLevelJSON. Also a check was addded in options.js to enable formlock in custom levels
Added a recursive function which throws an exception if an element or it's children are inputs with unsafe types. Safe types are: - button, reset, submit because they don't contain any real value - color because it's value would hardly be useful during a leak - hidden because the type itself is not dangerous, only when combined with other unsafe types
Now only tabs that originated from the locked tab during lock are closed. On Chromium this includes tabs that were opened by user who had focus on the locked tab
User is now notified about form safety only only when they trigger the focusin event.
Formlock now no longer contains a context menu. Unsafe forms are locked on focusIn event and are unlocked either on refresh of the tab or when the tab navigates to the form's landing page
Also improvements were added to form locking, mainly changing the way URLs were built which means mainly getting rid of using get_root_domain etc. and using functions already defined in JSR
If a form was locked and then another suspicious one was focused then Formlock changes the locked domains but keeps the first data backup. Also added small bugfixes to form unlock.
Added a variable to prevent multiple unlocks of forms and moved database restoration before page refresh to prevent blocking problems
Added unlock without refresh to deal with pages that autofocus on GET search form. Small fixes to domain checking in lock_form. Re-added context menu so that users could lock even safe forms to combat ad scripts on the same page
Removed the options limiting data removal only to locked websites because external scripts can create storages and cookies with different origin therefore everything should be cleared.
This reverts commit bafe7f1.
|
Is this done? (After conflict resolution) |
|
This is a proof of concept code. The downside is that more permissions are needed which may be scary to users. Additionally, the functionality is not perfect in any browser. So currently, there are not plans to merge the code but this decision may be revisited in the future. Also if the community is interested in the functionality even if it is not perfect, I'm willing to listen. |
|
I added an issue regarding the future of this code https://pagure.io/JShelter/webextension/issue/74 and some motivation behind adding such features. |
They are not justified in popup. Mainly paragraphs holding list of long API names like Locally generated audio look ugly. Also mentioned in Pagure issue #94
Adds the capabilities of Formlock (https://github.com/ostarov/Formlock). I shall provide fixes like compatibility with Firefox and update of levels in the follow-up commits.