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

Select group when adding new credentials #396

Merged
merged 1 commit into from
Mar 23, 2019
Merged

Select group when adding new credentials #396

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
36 changes: 36 additions & 0 deletions keepassxc-browser/_locales/en/messages.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -307,6 +307,26 @@
"message": "Please choose the credentials you want to update.",
"description": "A popup message shown choosing what credentials user wants to update."
},
"popupRememberChooseGroup": {
"message": "Please choose the group you want to add the new credentials.",
"description": "A popup message shown choosing what group user wants to use for new credentials."
},
"popupRememberErrorDefaultGroupNotFound": {
"message": "Error: Specified default group not found. Creating all necessary groups.",
varjolintu marked this conversation as resolved.
Show resolved Hide resolved
"description": "Error message shown when default group set cannot be found."
},
"popupRememberErrorCreatingNewGroup": {
"message": "New group(s) cannot be created.",
varjolintu marked this conversation as resolved.
Show resolved Hide resolved
"description": "Error message shown when new groups cannot be created."
},
"popupRememberErrorPasswordNotChanged": {
"message": "Error: Credentials not updated. The password has not been changed.",
"description": "Error message shown when credential password is not changed."
},
"popupRememberErrorCannotSaveCredentials": {
"message": "Error: Credentials cannot be saved or updated.",
"description": "Error message shown when credentials cannot be saved or updated."
},
"popupLoginText": {
"message": "Select the login information you would like to get entered into the page.",
"description": "A popup message shown when one or multiple credentials are present."
Expand Down Expand Up @@ -447,6 +467,22 @@
"message": "Maximum (Number of) Redirects:",
"description": "RMaximum (Number of) Redirects options text."
},
"optionsLabelDefaultGroup": {
"message": "Default group for saving new passwords:",
"description": "Default group options text."
},
"optionsDefaultGroupHelpText": {
"message": "Separate the group with slashes, for example: Group/ChildGroup.",
"description": "Default group help text."
},
"optionsLabelDefaultGroupCheckboxText": {
"message": "Always ask where to save new credentials",
"description": "Default group checkbox help text."
},
"optionsLabelDefaultGroupCheckboxHelpText": {
"message": "When enabled, the popup shows a group listing.",
"description": "Default checkbox explanation help text."
},
"optionsCheckboxUsePasswordGenerator": {
"message": "Activate password generator.",
"description": "Activate password generator checkbox text."
Expand Down
10 changes: 5 additions & 5 deletions keepassxc-browser/background/browserAction.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -167,13 +167,13 @@ browserAction.stackPop = function(tabId) {

browserAction.stackPush = function(data, tabId) {
const id = tabId || page.currentTabId;
browserAction.removeLevelFromStack(null, {'id': id}, data.level, '<=', true);
browserAction.removeLevelFromStack(null, { 'id': id }, data.level, '<=', true);
page.tabs[id].stack.push(data);
};

browserAction.stackUnshift = function(data, tabId) {
const id = tabId || page.currentTabId;
browserAction.removeLevelFromStack(null, {'id': id}, data.level, '<=', true);
browserAction.removeLevelFromStack(null, { 'id': id }, data.level, '<=', true);
page.tabs[id].stack.unshift(data);
};

Expand All @@ -193,7 +193,7 @@ browserAction.removeRememberPopup = function(callback, tab, removeImmediately) {
const currentMS = Date.now();
if (removeImmediately || (data.visibleForPageUpdates <= 0 && data.redirectOffset > 0)) {
browserAction.stackPop(tab.id);
browserAction.show(null, {"id": tab.id});
browserAction.show(null, { 'id': tab.id });
page.clearCredentials(tab.id);
} else if (!isNaN(data.visibleForPageUpdates) && data.redirectOffset > 0 && currentMS >= data.redirectOffset) {
data.visibleForPageUpdates -= 1;
Expand All @@ -202,7 +202,7 @@ browserAction.removeRememberPopup = function(callback, tab, removeImmediately) {
};

browserAction.setRememberPopup = function(tabId, username, password, url, usernameExists, credentialsList) {
browser.storage.local.get({'settings': {}}).then(function(item) {
browser.storage.local.get({ 'settings': {} }).then(function(item) {
const settings = item.settings;

// Don't show anything if the site is in the ignore
Expand Down Expand Up @@ -233,7 +233,7 @@ browserAction.setRememberPopup = function(tabId, username, password, url, userna
index: 0,
counter: 0,
max: 2,
icons: ['icon_remember_red_background_19x19.png', 'icon_remember_red_lock_19x19.png']
icons: [ 'icon_remember_red_background_19x19.png', 'icon_remember_red_lock_19x19.png' ]
},
icon: 'icon_remember_red_background_19x19.png',
popup: 'popup_remember.html'
Expand Down
2 changes: 2 additions & 0 deletions keepassxc-browser/background/event.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -284,8 +284,10 @@ kpxcEvent.messageHandlers = {
'add_credentials': keepass.addCredentials,
'associate': keepass.associate,
'check_update_keepassxc': kpxcEvent.onCheckUpdateKeePassXC,
'create_new_group': keepass.createNewGroup,
'generate_password': keepass.generatePassword,
'get_connected_database': kpxcEvent.onGetConnectedDatabase,
'get_database_groups': keepass.getDatabaseGroups,
'get_keepassxc_versions': kpxcEvent.onGetKeePassXCVersions,
'get_status': kpxcEvent.onGetStatus,
'get_tab_information': kpxcEvent.onGetTabInformation,
Expand Down
144 changes: 140 additions & 4 deletions keepassxc-browser/background/keepass.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,9 @@ const kpActions = {
CHANGE_PUBLIC_KEYS: 'change-public-keys',
LOCK_DATABASE: 'lock-database',
DATABASE_LOCKED: 'database-locked',
DATABASE_UNLOCKED: 'database-unlocked'
DATABASE_UNLOCKED: 'database-unlocked',
GET_DATABASE_GROUPS: 'get-database-groups',
CREATE_NEW_GROUP: 'create-new-group'
};

const kpErrors = {
Expand Down Expand Up @@ -127,11 +129,11 @@ keepass.sendNativeMessage = function(request, enableTimeout = false) {
});
};

keepass.addCredentials = function(callback, tab, username, password, url) {
keepass.updateCredentials(callback, tab, null, username, password, url);
keepass.addCredentials = function(callback, tab, username, password, url, group, groupUuid) {
keepass.updateCredentials(callback, tab, null, username, password, url, group, groupUuid);
};

keepass.updateCredentials = function(callback, tab, entryId, username, password, url) {
keepass.updateCredentials = function(callback, tab, entryId, username, password, url, group, groupUuid) {
page.debug('keepass.updateCredentials(callback, {1}, {2}, {3}, [password], {4})', tab.id, entryId, username, url);
if (tab && page.tabs[tab.id]) {
page.tabs[tab.id].errorMessage = null;
Expand Down Expand Up @@ -162,6 +164,11 @@ keepass.updateCredentials = function(callback, tab, entryId, username, password,
messageData.uuid = entryId;
}

if (group && groupUuid) {
messageData.group = group;
messageData.groupUuid = groupUuid;
}

const request = {
action: kpAction,
message: keepass.encrypt(messageData, nonce),
Expand Down Expand Up @@ -678,6 +685,135 @@ keepass.lockDatabase = function(tab) {
});
};

keepass.getDatabaseGroups = function(callback, tab) {
keepass.testAssociation((taResponse) => {
if (!taResponse) {
browserAction.showDefault(null, tab);
callback([]);
return;
}

if (tab && page.tabs[tab.id]) {
page.tabs[tab.id].errorMessage = null;
}

if (!keepass.isConnected) {
callback([]);
return;
}

let groups = [];
const kpAction = kpActions.GET_DATABASE_GROUPS;
const nonce = keepass.getNonce();
const incrementedNonce = keepass.incrementedNonce(nonce);

const messageData = {
action: kpAction
};

const request = {
action: kpAction,
message: keepass.encrypt(messageData, nonce),
nonce: nonce,
clientID: keepass.clientID
};

keepass.sendNativeMessage(request).then((response) => {
if (response.message && response.nonce) {
const res = keepass.decrypt(response.message, response.nonce);
if (!res) {
keepass.handleError(tab, kpErrors.CANNOT_DECRYPT_MESSAGE);
callback([]);
return;
}

const message = nacl.util.encodeUTF8(res);
const parsed = JSON.parse(message);

if (keepass.verifyResponse(parsed, incrementedNonce)) {
groups = parsed.groups;
groups.defaultGroup = page.settings.defaultGroup;
groups.defaultGroupAlwaysAsk = page.settings.defaultGroupAlwaysAsk;
keepass.updateLastUsed(keepass.databaseHash);
callback(groups);
} else {
console.log('getDatabaseGroups rejected');
Copy link
Member

Choose a reason for hiding this comment

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

Would you want to show a proper error at this point? Would the CANNOT_DECRYPT_MESSAGE be appropriate here?

Copy link
Member Author

Choose a reason for hiding this comment

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

It's not appropriate here. The actual decryption is made much earlier. The extension actually doesn't have an error message for failed verification. I'll add it as a separate PR later, because that is something that every protocol command will need.

Copy link
Member

Choose a reason for hiding this comment

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

Something like "Invalid or corrupt message sent from KeePassXC"

Copy link
Member Author

Choose a reason for hiding this comment

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

Something like that yes. Currently the verifyResponse() returns false from multiple reasons, so the message must be a general one. The one you proposed or something like: "Message sent from KeePassXC cannot be verified".

callback([]);
}
} else if (response.error && response.errorCode) {
keepass.handleError(tab, response.errorCode, response.error);
callback([]);
} else {
browserAction.showDefault(null, tab);
callback([]);
}
});
}, tab, false);
};

keepass.createNewGroup = function(callback, tab, groupName) {
keepass.testAssociation((taResponse) => {
if (!taResponse) {
browserAction.showDefault(null, tab);
callback([]);
return;
}

if (tab && page.tabs[tab.id]) {
page.tabs[tab.id].errorMessage = null;
}

if (!keepass.isConnected) {
callback([]);
return;
}

const kpAction = kpActions.CREATE_NEW_GROUP;
const nonce = keepass.getNonce();
const incrementedNonce = keepass.incrementedNonce(nonce);

const messageData = {
action: kpAction,
groupName: groupName
};

const request = {
action: kpAction,
message: keepass.encrypt(messageData, nonce),
nonce: nonce,
clientID: keepass.clientID
};

keepass.sendNativeMessage(request).then((response) => {
if (response.message && response.nonce) {
const res = keepass.decrypt(response.message, response.nonce);
if (!res) {
keepass.handleError(tab, kpErrors.CANNOT_DECRYPT_MESSAGE);
callback([]);
return;
}

const message = nacl.util.encodeUTF8(res);
const parsed = JSON.parse(message);

if (keepass.verifyResponse(parsed, incrementedNonce)) {
keepass.updateLastUsed(keepass.databaseHash);
callback(parsed);
} else {
console.log('getDatabaseGroups rejected');
callback([]);
}
} else if (response.error && response.errorCode) {
keepass.handleError(tab, response.errorCode, response.error);
callback([]);
} else {
browserAction.showDefault(null, tab);
callback([]);
}
});
}, tab, false);
};

keepass.generateNewKeyPair = function() {
keepass.keyPair = nacl.box.keyPair();
//console.log(nacl.util.encodeBase64(keepass.keyPair.publicKey) + ' ' + nacl.util.encodeBase64(keepass.keyPair.secretKey));
Expand Down
12 changes: 10 additions & 2 deletions keepassxc-browser/background/page.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,9 @@ const defaultSettings = {
autoRetrieveCredentials: true,
showNotifications: true,
showLoginNotifications: true,
saveDomainOnly: true
saveDomainOnly: true,
defaultGroup: '',
defaultGroupAlwaysAsk: false
};

var page = {};
Expand Down Expand Up @@ -49,7 +51,13 @@ page.initSettings = function() {
if (!('saveDomainOnly' in page.settings)) {
page.settings.saveDomainOnly = defaultSettings.saveDomainOnly;
}
browser.storage.local.set({'settings': page.settings});
if (!('defaultGroup' in page.settings)) {
page.settings.defaultGroup = defaultSettings.defaultGroup;
}
if (!('defaultGroupAlwaysAsk' in page.settings)) {
page.settings.defaultGroupAlwaysAsk = defaultSettings.defaultGroupAlwaysAsk;
}
browser.storage.local.set({ 'settings': page.settings });
resolve(page.settings);
});
});
Expand Down
4 changes: 4 additions & 0 deletions keepassxc-browser/options/options.css
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -119,6 +119,10 @@ h2+hr {
width: 75%;
}

#defaultGroup {
width: 50%;
}

tr.clone {
display: none;
}
Expand Down
24 changes: 24 additions & 0 deletions keepassxc-browser/options/options.html
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -87,6 +87,30 @@ <h3 data-i18n="optionsKeyboardShortcutsHeader"></h3>
</div>
</p>
<hr />
<p>
<div class="form-group">
<label for="defaultGroup" data-i18n="optionsLabelDefaultGroup"></label>
<div class="control-group">
<div class="input-append">
<input type="text" id="defaultGroup" placeholder="KeePassXC-Browser Passwords">
<button class="btn btn-sm btn-primary" id="defaultGroupButton" type="button"><span class="glyphicon glyphicon-floppy-disk"></span> <span data-i18n="optionsButtonSave"/></button>
<button class="btn btn-sm btn-danger" id="defaultGroupButtonReset"><span class="glyphicon glyphicon-remove-sign"></span> <span data-i18n="optionsButtonReset"></span></button>
</div>
</div>
<span class="help-block">
<span data-i18n="optionsDefaultGroupHelpText"></span>
<br />
<span data-i18n="optionsDefault"></span> KeePassXC-Browser Passwords
</span>
<div class="checkbox">
<label class="checkbox">
<input type="checkbox" name="defaultGroupAlwaysAsk" value="true" /><span data-i18n="optionsLabelDefaultGroupCheckboxText"></span>
</label>
<span class="help-block" data-i18n="optionsLabelDefaultGroupCheckboxHelpText"></span>
</div>
</div>
</p>
<hr />
<p>
<div class="checkbox">
<label class="checkbox">
Expand Down
Loading