Skip to content
This repository has been archived by the owner on May 27, 2019. It is now read-only.

Add feature to launch URLs & support basic auth #224

Merged
merged 10 commits into from
Mar 21, 2018
12 changes: 10 additions & 2 deletions browserpass.go
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ type Login struct {
Password string `json:"p"`
OTP string `json:"digits"`
OTPLabel string `json:"label"`
URL string `json:"url"`
}

var endianness = binary.LittleEndian
Expand Down Expand Up @@ -212,14 +213,21 @@ func parseLogin(r io.Reader) (*Login, error) {
login.Password = scanner.Text()

// Keep reading file for string in "login:", "username:" or "user:" format (case insensitive).
re := regexp.MustCompile("(?i)^(login|username|user):")
userPattern := regexp.MustCompile("(?i)^(login|username|user):")
urlPattern := regexp.MustCompile("(?i)^(url|link|website|web|site):")
for scanner.Scan() {
line := scanner.Text()
parseTotp(line, login)
replaced := re.ReplaceAllString(line, "")
replaced := userPattern.ReplaceAllString(line, "")
if len(replaced) != len(line) {
login.Username = strings.TrimSpace(replaced)
}
if (login.URL == "") {
replaced = urlPattern.ReplaceAllString(line, "")
if len(replaced) != len(line) {
login.URL = strings.TrimSpace(replaced)
}
}
}

// if an unlabelled OTP is present, label it with the username
Expand Down
38 changes: 38 additions & 0 deletions chrome/background.js
Original file line number Diff line number Diff line change
Expand Up @@ -95,6 +95,44 @@ function onMessage(request, sender, sendResponse) {
localStorage.getItem("use_fuzzy_search") != "false";
sendResponse({ use_fuzzy_search: use_fuzzy_search });
}

// spawn a new tab with pre-provided credentials
if (request.action == "launch") {
chrome.tabs.create({url: request.url}, function (tab) {
var authAttempted = false;
chrome.webRequest.onAuthRequired.addListener(
function authListener(requestDetails) {
// only supply credentials if this is the first time for this tab
if (authAttempted) {
return {};
}
authAttempted = true;
// remove event listeners once tab loading is complete
chrome.tabs.onUpdated.addListener(function statusListener(tabId, info) {
if (info.status === "complete") {
Copy link
Member

Choose a reason for hiding this comment

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

@erayd do you think this code should also compare tabId with the tab.id before removing the listeners?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It doesn't need to, because the listeners are specific to the tab anyway.

There is a security problem, but that's not it. I'm filing an issue now.

Copy link
Member

Choose a reason for hiding this comment

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

Are you sure that this doesn't register a listener that fires for all tabs? Maybe it's just a weird API, but the tab object is not used when registering the listener, and the tabId is provided as a first argument for a reason, both of these lead me to think that this will fire for all tabs...

I know that this is not about the security problem, it's just what I accidentally stumbled upon 🙂

Copy link
Contributor Author

@erayd erayd Mar 22, 2018

Choose a reason for hiding this comment

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

Ooh, good point. The onAuthRequested handler is specific to the tab, but the status listener is not. My bad.

I'm going to roll the security fix PR right now - I feel quite bad I didn't notice that in the first place; I'm normally more careful than that. I will fix this at the same time.

Copy link
Member

Choose a reason for hiding this comment

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

Thank you!

chrome.tabs.onUpdated.removeListener(statusListener);
chrome.webRequest.onAuthRequired.removeListener(authListener);
}
});
// ask the user before sending credentials over an insecure connection
if (!requestDetails.url.match(/^https:/i)) {
var message =
"You are about to send login credentials via an insecure connection!\n\n" +
"Are you sure you want to do this? If there is an attacker watching your " +
"network traffic, they may be able to see your username and password.\n\n" +
"URL: " + requestDetails.url
;
if (!confirm(message)) {
return {};
}
}
return {authCredentials: {username: request.username, password: request.password}};
},
{urls: ["*://*/*"], tabId: tab.id},
["blocking"]
);
});
}
}

function onTabUpdated(tabId, changeInfo, tab) {
Expand Down
89 changes: 89 additions & 0 deletions chrome/icon-globe.svg
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
4 changes: 3 additions & 1 deletion chrome/manifest.json
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@
"author": "Danny van Kooten",
"homepage_url": "https://github.com/dannyvankooten/browserpass",
"background": {
"persistent": false,
"persistent": true,
Copy link
Member

Choose a reason for hiding this comment

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

Could you comment on why this is needed?

Copy link
Contributor Author

@erayd erayd Mar 21, 2018

Choose a reason for hiding this comment

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

Chrome doesn't allow non-persistent event pages to use the webRequest API. If you have this set to false, you'll get a massive warning when you try to load the extension telling you that this isn't possible, and then Chrome will refuse to load the extension.

"scripts": [
"background.js"
]
Expand All @@ -28,6 +28,8 @@
"nativeMessaging",
"notifications",
"storage",
"webRequest",
"webRequestBlocking",
"http://*/*",
"https://*/*"
],
Expand Down
77 changes: 71 additions & 6 deletions chrome/script.browserify.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@

var m = require("mithril");
var FuzzySort = require("fuzzysort");
var Tldjs = require("tldjs");
var app = "com.dannyvankooten.browserpass";
var activeTab;
var searching = false;
Expand Down Expand Up @@ -45,6 +46,10 @@ function view() {

return m("div.entry", [
m(selector, options, login),
m("button.launch.url", {
onclick: launchURL.bind({ entry: login, what: "url" }),
tabindex: -1
}),
m("button.copy.username", {
onclick: loginToClipboard.bind({ entry: login, what: "username" }),
tabindex: -1
Expand Down Expand Up @@ -76,7 +81,7 @@ function view() {
type: "text",
id: "search-field",
name: "s",
placeholder: "Search passwords..",
placeholder: "Search passwords...",
autocomplete: "off",
autofocus: "on",
oninput: filterLogins
Expand Down Expand Up @@ -150,7 +155,6 @@ function showFilterHint(show=true) {

function submitSearchForm(e) {
e.preventDefault();

if (fillOnSubmit && logins.length > 0) {
// fill using the first result
getLoginData.bind(logins[0])();
Expand Down Expand Up @@ -232,6 +236,50 @@ function getFaviconUrl(domain) {
return null;
}

function launchURL() {
var what = this.what;
var entry = this.entry;
chrome.runtime.sendNativeMessage(
app,
{ action: "get", entry: this.entry },
function(response) {
if (chrome.runtime.lastError) {
error = chrome.runtime.lastError.message;
m.redraw();
Copy link
Member

Choose a reason for hiding this comment

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

nitpick: could you please reduce the nesting?
It's a little difficult to see which "else" belongs to which "if" 🙂

if (error) {
  ...
  return; 
}

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sure.

return;
}
// get url from login path if not available in the host app response
if (!response.hasOwnProperty("url") || response.url.length == 0) {
var parts = entry.split(/\//).reverse();
for (var i in parts) {
var part = parts[i];
var info = Tldjs.parse(part);
if (info.isValid && info.tldExists && info.domain !== null && info.hostname === part) {
response.url = part;
break;
}
}
}
// if a url is present, then launch a new tab via the background script
if (response.hasOwnProperty("url") && response.url.length > 0) {
var url = response.url.match(/^([a-z]+:)?\/\//i) ? response.url : "http://" + response.url;
chrome.runtime.sendMessage({action: "launch", url: url, username: response.u, password: response.p});
window.close();
return;
}
// no url available
if (!response.hasOwnProperty("url")) {
resetWithError(
"Unable to determine the URL for this entry. If you have defined one in the password file, " +
"your host application must be at least v2.0.14 for this to be usable."
);
} else {
resetWithError("Unable to determine the URL for this entry.");
}
}
);
}

function getLoginData() {
searching = true;
logins = resultLogins = [];
Expand Down Expand Up @@ -298,16 +346,18 @@ function keyHandler(e) {
break;
case "c":
if (e.target.id != "search-field" && e.ctrlKey) {
document.activeElement["nextElementSibling"][
"nextElementSibling"
].click();
document.activeElement.parentNode.querySelector("button.copy.password").click();
}
break;
case "C":
if (e.target.id != "search-field") {
document.activeElement["nextElementSibling"].click();
document.activeElement.parentNode.querySelector("button.copy.username").click();
}
break;
case "g":
if (e.target.id != "search-field") {
document.activeElement.parentNode.querySelector("button.launch.url").click();
}
}
}

Expand Down Expand Up @@ -336,3 +386,18 @@ function oncreate() {
document.getElementById("search-field").focus();
}, 100);
}

function resetWithError(errMsg) {
domain = '';
logins = resultLogins = [];
fillOnSubmit = false;
searching = false;
var filterSearch = document.getElementById("filter-search");
filterSearch.style.display = "none";
filterSearch.textContent = '';
var searchField = document.getElementById("search-field");
searchField.setAttribute("placeholder", "Search passwords...");
error = errMsg;
m.redraw();
searchField.focus();
}
7 changes: 6 additions & 1 deletion chrome/styles.css
Original file line number Diff line number Diff line change
Expand Up @@ -58,12 +58,17 @@ body {
border-bottom: 1px dotted #ccc;
}

.copy {
.copy, .launch {
width: 32px;
border: 0;
cursor: pointer;
}

.url {
background: no-repeat url('icon-globe.svg') center;
background-size: 16px 16px;
}

.username {
background: no-repeat url('icon-user.svg') center;
background-size: 16px 16px;
Expand Down
4 changes: 3 additions & 1 deletion firefox/manifest.json
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@
"page": "options.html"
},
"background": {
"persistent": false,
"persistent": true,
"scripts": [
"background.js"
]
Expand All @@ -26,6 +26,8 @@
"nativeMessaging",
"notifications",
"storage",
"webRequest",
"webRequestBlocking",
"http://*/*",
"https://*/*"
],
Expand Down
3 changes: 2 additions & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
"dependencies": {
"browserify": "^14.4.0",
"mithril": "^1.1.4",
"fuzzysort": "^1.1.0"
"fuzzysort": "^1.1.0",
"tldjs": "^2.3.1"
}
}
8 changes: 7 additions & 1 deletion yarn.lock
Original file line number Diff line number Diff line change
Expand Up @@ -638,7 +638,7 @@ punycode@1.3.2:
version "1.3.2"
resolved "https://registry.yarnpkg.com/punycode/-/punycode-1.3.2.tgz#9653a036fb7c1ee42342f2325cceefea3926c48d"

punycode@^1.3.2:
punycode@^1.3.2, punycode@^1.4.1:
version "1.4.1"
resolved "https://registry.yarnpkg.com/punycode/-/punycode-1.4.1.tgz#c0d5a63b2718800ad8e1eb0fa5269c84dd41845e"

Expand Down Expand Up @@ -800,6 +800,12 @@ timers-browserify@^1.0.1:
dependencies:
process "~0.11.0"

tldjs@^2.3.1:
version "2.3.1"
resolved "https://registry.yarnpkg.com/tldjs/-/tldjs-2.3.1.tgz#cf09c3eb5d7403a9e214b7d65f3cf9651c0ab039"
dependencies:
punycode "^1.4.1"

to-arraybuffer@^1.0.0:
version "1.0.1"
resolved "https://registry.yarnpkg.com/to-arraybuffer/-/to-arraybuffer-1.0.1.tgz#7d229b1fcc637e466ca081180836a7aabff83f43"
Expand Down