Skip to content

Conversation

@varjolintu
Copy link
Member

Add support for a new command for retrieving the current TOTP value.

Testing strategy

Manually using keepassxreboot/keepassxc-browser#961.

Type of change

  • ✅ New feature (change that adds functionality)

Comment on lines +475 to +487
if (!m_associated) {
return getErrorReply(action, ERROR_KEEPASS_ASSOCIATION_FAILED);
}

const QJsonObject decrypted = decryptMessage(encrypted, nonce);
if (decrypted.isEmpty()) {
return getErrorReply(action, ERROR_KEEPASS_CANNOT_DECRYPT_MESSAGE);
}

QString command = decrypted.value("action").toString();
if (command.isEmpty() || command.compare("get-totp", Qt::CaseSensitive) != 0) {
return getErrorReply(action, ERROR_KEEPASS_INCORRECT_ACTION);
}
Copy link
Member

Choose a reason for hiding this comment

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

Future improvement.... these checks are generic and should be done before any information is passed to the specific handler. The handler should just be given the decrypted QJsonObject

Copy link
Member Author

Choose a reason for hiding this comment

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

I agree.

Comment on lines +290 to +295
for (auto dbWidget : getMainWindow()->getOpenDatabases()) {
auto db = dbWidget->database();
if (db) {
databases << db;
}
}
Copy link
Member

Choose a reason for hiding this comment

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

Future improvement... we need to make it dead simple to get the current valid database objects.

Copy link
Member Author

Choose a reason for hiding this comment

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

I'll put that to my TODO list.

const QString newNonce = incrementNonce(nonce);

QJsonObject message = buildMessage(newNonce);
message["totp"] = totp;
Copy link
Member

Choose a reason for hiding this comment

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

Since we return empty string if a totp is not found we should check for that on the extension side and warn the user that no TOTP was available. This might already be done.

Copy link
Member Author

Choose a reason for hiding this comment

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

Good idea.

@droidmonkey droidmonkey merged commit 0cc2c83 into keepassxreboot:release/2.6.1 Aug 17, 2020
@varjolintu varjolintu deleted the feature/check_new_totp branch August 17, 2020 10:19
phoerious added a commit that referenced this pull request Aug 19, 2020
Added

- Add menu entries for auto-typing only username or only password [#4891]
- Browser: Add command for retrieving current TOTP [#5278]
- Improve man pages [#5010]
- Linux: Support Xfce screen lock signals [#4971]
- Linux: Add OARS metadata to AppStream markup [#5031]
- SSH Agent: Substitute tilde with %USERPROFILE% on Windows [#5116]

Changed

- Improve password generator UI and UX [#5129]
- Do not prompt to restart if switching the theme back and forth [#5084]
- Change actions for F1, F2, and F3 keys [#5082]
- Skip referenced passwords in health check report [#5056]
- Check system-wide Qt translations directory for downstream translations packaging [#5064]
- macOS: Change password visibility toggle shortcut to Ctrl+H to avoid conflict with system shortcut [#5114]
- Browser: Only display domain name in browser access confirm dialog to avoid overly wide window sizes [#5214]

Fixed

- Fix clipboard not being cleared when database is locked while timeout is still active [#5184]
- Fix list of previous databases not being cleared in some cases [#5123]
- Fix saving of non-data changes on database lock [#5210]
- Fix search results banner theming [#5197]
- Don't enforce theme palette in Classic theme mode and add hover effect for buttons [#5122,#5267]
- Fix label clipping in settings on high-DPI screens [#5227]
- Fix excessive memory usage by icons on systems with high-DPI screens [#5266]
- Fix crash if number of TOTP digits exceeds ten [#5106]
- Fix slot detection when first YubiKey is configured on the second slot [#5004]
- Prevent crash if focus widget gets deleted during saving [#5005]
- Always show buttons for opening or saving attachments [#4956]
- Update link to Auto-Type help [#5228]
- Fix build errors with Ninja [#5121]
- CLI: Fix db-info command wrongly labelled as db-show in usage listing [#5140]
- Windows: Use Classic theme by default if high-contrast mode is on [#5191]
- Linux: Add workaround for qt5ct bug, causing icons not to show up [#5011]
- Linux: Correct high-DPI display by not allowing fractional scaling [#5185]
- Browser: Consider subdomain and path when requesting only "best-matching credentials" [#4832]
- SSH Agent: Always forget all keys on lock [#5115]
aswild added a commit to aswild/keepassxc that referenced this pull request Aug 26, 2020
Release 2.6.1

Added

- Add menu entries for auto-typing only username or only password [keepassxreboot#4891]
- Browser: Add command for retrieving current TOTP [keepassxreboot#5278]
- Improve man pages [keepassxreboot#5010]
- Linux: Support Xfce screen lock signals [keepassxreboot#4971]
- Linux: Add OARS metadata to AppStream markup [keepassxreboot#5031]
- SSH Agent: Substitute tilde with %USERPROFILE% on Windows [keepassxreboot#5116]

Changed

- Improve password generator UI and UX [keepassxreboot#5129]
- Do not prompt to restart if switching the theme back and forth [keepassxreboot#5084]
- Change actions for F1, F2, and F3 keys [keepassxreboot#5082]
- Skip referenced passwords in health check report [keepassxreboot#5056]
- Check system-wide Qt translations directory for downstream translations packaging [keepassxreboot#5064]
- macOS: Change password visibility toggle shortcut to Ctrl+H to avoid conflict with system shortcut [keepassxreboot#5114]
- Browser: Only display domain name in browser access confirm dialog to avoid overly wide window sizes [keepassxreboot#5214]

Fixed

- Fix clipboard not being cleared when database is locked while timeout is still active [keepassxreboot#5184]
- Fix list of previous databases not being cleared in some cases [keepassxreboot#5123]
- Fix saving of non-data changes on database lock [keepassxreboot#5210]
- Fix search results banner theming [keepassxreboot#5197]
- Don't enforce theme palette in Classic theme mode and add hover effect for buttons [keepassxreboot#5122,keepassxreboot#5267]
- Fix label clipping in settings on high-DPI screens [keepassxreboot#5227]
- Fix excessive memory usage by icons on systems with high-DPI screens [keepassxreboot#5266]
- Fix crash if number of TOTP digits exceeds ten [keepassxreboot#5106]
- Fix slot detection when first YubiKey is configured on the second slot [keepassxreboot#5004]
- Prevent crash if focus widget gets deleted during saving [keepassxreboot#5005]
- Always show buttons for opening or saving attachments [keepassxreboot#4956]
- Update link to Auto-Type help [keepassxreboot#5228]
- Fix build errors with Ninja [keepassxreboot#5121]
- CLI: Fix db-info command wrongly labelled as db-show in usage listing [keepassxreboot#5140]
- Windows: Use Classic theme by default if high-contrast mode is on [keepassxreboot#5191]
- Linux: Add workaround for qt5ct bug, causing icons not to show up [keepassxreboot#5011]
- Linux: Correct high-DPI display by not allowing fractional scaling [keepassxreboot#5185]
- Browser: Consider subdomain and path when requesting only "best-matching credentials" [keepassxreboot#4832]
- SSH Agent: Always forget all keys on lock [keepassxreboot#5115]
@phoerious phoerious added pr: new feature Pull request adds a new feature and removed new feature labels Nov 22, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

feature: Browser pr: new feature Pull request adds a new feature

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants