-
Notifications
You must be signed in to change notification settings - Fork 9
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.
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
Treat as same site #68
base: future
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -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"; | ||
|
@@ -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!"; | ||
|
||
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"; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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 |
||
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!'; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. same thing about throw |
||
|
||
return [urlParsed.domain.split('.'), domainMatched.split('.')]; | ||
} | ||
|
||
function showSessionSetup() { | ||
|
@@ -301,7 +334,17 @@ function popup(masterkey) { | |
} | ||
|
||
window.addEventListener('load', function () { | ||
config.get(['username', 'key_id', 'defaulttype', 'pass_to_clipboard', 'pass_store', 'passwdtimeout', 'use_sync', 'defaultname']) | ||
config.get([ | ||
'username', | ||
'key_id', | ||
'defaulttype', | ||
'pass_to_clipboard', | ||
'pass_store', | ||
'passwdtimeout', | ||
'use_sync', | ||
'defaultname', | ||
'treat_as_same_site', | ||
]) | ||
.then(v=>{ | ||
return runtimeSendMessage({action: 'masterkey_get', use_pass_store: !!v.pass_store}); | ||
}) | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -87,7 +87,11 @@ button:focus, input:focus, mp-combobox:focus-within { | |
outline-style: auto; | ||
} | ||
|
||
input, select, mp-combobox { | ||
input, | ||
select, | ||
mp-combobox, | ||
textarea, | ||
no-op { | ||
background: #1b1d23; | ||
color: #d7dae0; | ||
height: 2em; | ||
|
@@ -198,7 +202,8 @@ button#siteconfig_show { | |
} | ||
#siteconfig > input, | ||
#siteconfig > select, | ||
#siteconfig > option { | ||
#siteconfig > option, | ||
no-op { | ||
font-size: 1em; | ||
font-weight: normal; | ||
width: 6em; | ||
|
@@ -302,7 +307,9 @@ button#siteconfig_show { | |
|
||
.configitem > input, | ||
.configitem > select, | ||
.configitem > option { | ||
.configitem > option, | ||
.configitem > textarea, | ||
no-op { | ||
margin-top:1.5em; | ||
margin-left:-8em; | ||
font-size: 1em; | ||
|
@@ -316,6 +323,25 @@ button#siteconfig_show { | |
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, | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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; | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -8,7 +8,6 @@ | |
div.item { | ||
break-inside: avoid; | ||
width:100%; | ||
height:3em; | ||
display:flex; | ||
justify-content: space-between; | ||
align-items: center; | ||
|
@@ -26,7 +25,9 @@ | |
|
||
input, | ||
select, | ||
option { | ||
option, | ||
textarea, | ||
no-op { | ||
display:block; | ||
border: 0; | ||
width: 14em; | ||
|
@@ -36,6 +37,11 @@ | |
input[type=checkbox] { | ||
width:1em; | ||
} | ||
#treat_as_same_site { | ||
height: 15em; | ||
white-space: nowrap; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. the nowrap is the problem.. seems like |
||
overflow: auto; | ||
} | ||
|
||
</style> | ||
</head> | ||
|
@@ -71,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"/> | ||
|
There was a problem hiding this comment.
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