Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Treat as same site #68

Open
wants to merge 2 commits into
base: future
Choose a base branch
from
Open

Treat as same site #68

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
4584247
Some formatting tidyups to help merging in future.
jmorgannz Nov 24, 2021
e6488c2
* Add new sync'd option 'treat_as_same_site', which allows configurat…
jmorgannz Nov 24, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
66 changes: 50 additions & 16 deletions ext/webextension/src/browser_action/main_popup.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@

import {SiteStore} from "../lib/sitestore.js";
import {Site} from "../lib/sites.js";
import {defer, copy_to_clipboard} from "../lib/utils.js";
import {defer, copy_to_clipboard, regexpEscape} from "../lib/utils.js";
import {parseUri} from "../lib/uritools.js";
import config from "../lib/config.js";
import {ui} from "./ui.js";
Expand Down Expand Up @@ -241,21 +241,54 @@ function updateUIForDomainSettings(combined)
}

function extractDomainFromUrl(url) {
if (!url || url.startsWith('about:')
|| url.startsWith('resource:')
|| url.startsWith('moz-extension:')
|| url.startsWith('chrome-extension:')
|| url.startsWith('chrome:'))
url = '';
let domain_parts = parseUri(url).domain.split(".");
let significant_parts = 2;
const common_slds = ['co','com','gov','govt','net','org','edu','priv','ac'];
let second_level_domain = domain_parts[domain_parts.length-2].toLowerCase();
if (domain_parts.length > 2 && common_slds.includes(second_level_domain))
significant_parts = 3;

let significant_domain = domain_parts.slice(-significant_parts)
return [domain_parts, significant_domain];
if (!url)
throw "extractDomainFromUrl: No url provided!";
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Throwing here is not such a great Idea.. everything that follows extractDomainFromUrl will be skipped, in particular loadSites and updateUIForDomainSettings.
So instead of having access to all your sites, it appears to have lost all data.

probably better to return some sane default


var urlParsed = parseUri(url);
{
let protocolsIgnored = ["about", "resource", "moz-extension", "chrome-extension", "chrome", "edge", "brave"];
if (protocolsIgnored.includes(urlParsed.protocol))
throw "extractDomainFromUrl: Invalid url protocol provided";
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Especially here. about:blank is a perfectly legal page to open to just quickly lookup a password.

}

let rxDomainMatchers = [
/([^\.]+\.[^\.]+)$/i, // Default matcher matches two-part domain names (.* pattern) i.e. github.com
];

config.treat_as_same_site.split('\n').forEach(function(pattern) {
Copy link
Owner

@ttyridal ttyridal Nov 24, 2021 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I know I'm often guilty of this myself, but hey, now I'm in review mode and trying to understand someone else's code ;)

Could it be better to make it a named function and do something like
let matchRegEx = config.treat_as_same_site.split('\n').map(translate_match_string_to_regex); rxDomainMatchers.push(...matchRegEx.map(pattern => new RegExp(pattern, 'i')))

pattern = pattern.trim();
if (pattern.length == 0) return;
if (!pattern.startsWith('.')) return;
if (pattern.endsWith('.')) return;
//FIXME: More strict domain character parsing
// (should actually be config done at entry time)
pattern = '*' + pattern;
pattern = regexpEscape(pattern);
pattern = pattern.replace(/\\\*/g, '[^\.]+');
rxDomainMatchers.push(new RegExp(pattern, 'i'));
});

let domainMatched = null;

rxDomainMatchers.forEach(function(rx) {
let rxResult = rx.exec(urlParsed.domain);
if (!rxResult) return;

if (domainMatched == null)
return domainMatched = rxResult[0];

// Select domain with most parts that match
let domainMatchedParts = domainMatched.split('.');
let domainThisParts = rxResult[0].split('.');

if (domainThisParts.length > domainMatchedParts.length)
domainMatched = rxResult[0];
});

if (domainMatched == null)
throw 'extractDomainFromUrl: Could not match any sites! Check your configuration settings!';
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

same thing about throw


return [urlParsed.domain.split('.'), domainMatched.split('.')];
}

function showSessionSetup() {
Expand Down Expand Up @@ -310,6 +343,7 @@ window.addEventListener('load', function () {
'passwdtimeout',
'use_sync',
'defaultname',
'treat_as_same_site',
])
.then(v=>{
return runtimeSendMessage({action: 'masterkey_get', use_pass_store: !!v.pass_store});
Expand Down
21 changes: 21 additions & 0 deletions ext/webextension/src/css/mpwd.css
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -90,6 +90,7 @@ button:focus, input:focus, mp-combobox:focus-within {
input,
select,
mp-combobox,
textarea,
no-op {
background: #1b1d23;
color: #d7dae0;
Expand Down Expand Up @@ -307,6 +308,7 @@ no-op {
.configitem > input,
.configitem > select,
.configitem > option,
.configitem > textarea,
no-op {
margin-top:1.5em;
margin-left:-8em;
Expand All @@ -321,6 +323,25 @@ no-op {
margin-top:0.2em;
}

.configitem > textarea#treat_as_same_site {
width: 11.5em;
height: 13em;
font-size: 0.7em;
margin-left: -11.5em;
margin-top: 2em;
white-space: nowrap;
}

/* FIXME: I don't understand how the left alignment of inputs works here,
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thats alright :) makes me remember I want to fix this cleanly with flexbox (which was very new back in 2015)

using negative margins and wrapping - so I just manually margin-left
by eye to line up - however it would be better to align this p
the same way as the input it is annotating */
.configitem > textarea#treat_as_same_site + p {
font-size: 0.6em;
margin-left: 6.7em;
line-height: normal;
}

#stored_sites {
width:100%;
text-align: center;
Expand Down
5 changes: 4 additions & 1 deletion ext/webextension/src/lib/config.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,9 @@ class Config {
get passwdtimeout() { if (typeof this._cache.passwdtimeout === 'undefined')
throw new Error("need get(['passwdtimeout'])");
else return this._cache.passwdtimeout; }
get treat_as_same_site() { if (typeof this._cache.treat_as_same_site === 'undefined')
throw new Error("need get(['treat_as_same_site'])");
else return this._cache.treat_as_same_site; }
get use_sync() { if (typeof this._cache.use_sync === 'undefined')
throw new Error("need get(['use_sync'])");
else return this._cache.use_sync; }
Expand Down Expand Up @@ -137,7 +140,7 @@ class Config {
if (lst.includes('username')) result.username = result.username || '';
if (lst.includes('pass_store')) result.pass_store = !!result.pass_store;
if (lst.includes('passwdtimeout')) result.passwdtimeout = isNaN(result.passwdtimeout) ? -1 : result.passwdtimeout;

if (lst.includes('treat_as_same_site')) result.treat_as_same_site = result.treat_as_same_site || [ '.ac.*', '.co.*', '.com.*', '.edu.*', '.geek.*', '.gov.*', '.govt.*', '.net.*', '.org.*', '.school.*' ].join('\n');
Object.assign(this._cache, result);

return singlekey ? result[lst[0]] : result;
Expand Down
5 changes: 5 additions & 0 deletions ext/webextension/src/lib/utils.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -46,3 +46,8 @@ export function copy_to_clipboard(mimetype, data) {
document.execCommand("Copy", false, null);
document.oncopy=null;
}

export function regexpEscape(string) {
// https://stackoverflow.com/a/6969486
return string.replace(/[.*+?^${}()|[\]\\]/g, '\\$&'); // $& means the whole matched string
}
5 changes: 5 additions & 0 deletions ext/webextension/src/options/globaloptions.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,9 @@ document.querySelector('#auto_submit_username').addEventListener('change', funct
document.querySelector('#pass_store').addEventListener('change', function() {
config.set({pass_store: this.checked});
});
document.querySelector('#treat_as_same_site').addEventListener('change', function() {
config.set({treat_as_same_site: this.value});
});
document.querySelector('#use_sync').addEventListener('change', async function() {
let oldstore = (this.checked?chrome.storage.local:chrome.storage.sync);
let newstore = (!this.checked?chrome.storage.local:chrome.storage.sync);
Expand Down Expand Up @@ -158,6 +161,7 @@ window.addEventListener('load', function() {
'auto_submit_pass',
'auto_submit_username',
'pass_store',
'treat_as_same_site',
'use_sync',
])
.then(data => {
Expand All @@ -172,6 +176,7 @@ window.addEventListener('load', function() {
document.querySelector('#auto_submit_pass').checked = data.auto_submit_pass;
document.querySelector('#auto_submit_username').checked = data.auto_submit_username;
document.querySelector('#pass_store').checked = data.pass_store;
document.querySelector('#treat_as_same_site').value = data.treat_as_same_site;
document.querySelector('#use_sync').checked = data.use_sync;
});
});
Expand Down
14 changes: 14 additions & 0 deletions ext/webextension/src/options/index.html
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,20 @@ <h2 class="accordion_toggle">Global settings</h2>
<label class="w3-label w3-small w3-text-white" for="defaultname">Default username:</label>
<input class="w3-select w3-small w3-border" id="defaultname">
</div>
<div class="configitem">
<label class="w3-label w3-small w3-text-white" for="treat_as_same_site">Override as same site</label>
<textarea class="w3-label w3-small w3-text-white" id="treat_as_same_site"></textarea>
<p class="w3-label w3-small w3-text-white">
- Enter overrides above, one per line<br />
- lines must start with a . (period)<br />
- lines that cannot be parsed are ignored<br />
- <b>*</b> is wildcard, will match anything except . (period)<br />
- Example:<br />
&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;<b>.net.*</b> will treat <i>x.net.nz</i> as a site,<br />
&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;instead of ignoring <i>x</i> and using <i>net.nz</i> as the site
</p>
</div>

</div>
<div class="configsubgroup right">
<div class="configitem">
Expand Down
12 changes: 11 additions & 1 deletion ext/webextension/src/options/options.html
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,6 @@
div.item {
break-inside: avoid;
width:100%;
height:3em;
display:flex;
justify-content: space-between;
align-items: center;
Expand All @@ -27,6 +26,7 @@
input,
select,
option,
textarea,
no-op {
display:block;
border: 0;
Expand All @@ -37,6 +37,11 @@
input[type=checkbox] {
width:1em;
}
#treat_as_same_site {
height: 15em;
white-space: nowrap;
Copy link
Owner

@ttyridal ttyridal Nov 24, 2021 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the nowrap is the problem.. seems like pre would be the correct(?) choice
ref https://bugzilla.mozilla.org/show_bug.cgi?id=1137650

overflow: auto;
}

</style>
</head>
Expand Down Expand Up @@ -72,6 +77,11 @@
<label for="defaultname">Default username</label>
<input id="defaultname">
</div>
<div class="item">
<label for="treat_as_same_site">Override as same site</label>
<textarea id="treat_as_same_site"></textarea>

</div>
<div class="item">
<label for="pass_to_clipboard">Copy password to clipboard</label>
<input id="pass_to_clipboard" type="checkbox"/>
Expand Down