Mailing List Archive

[PATCH pinentry] qt: Present option to save password in password manager
* qt/main.cpp: Pass pinentry info to PinEntryDialog constructor. Set
save passphrase checkbox text from pinentry_t->default_pwmngr.
* qt/pinentrydialog.cpp, qt/pinentrydialog.h: Dialog now accepts
pinentry info in the constructor and removed unneeded setter for
pinentry info. Add save passphrase checkbox.

--

This patch adds functionality to save key passphrases with pinentry-qt
that already exists in pinentry-gtk-2.

A "save passphrase" checkbox is shown when libsecret is available,
the external password cache is enabled, and there is valid data in
pinentry_t->keyinfo. When checked, the pinentry info is updated to allow
the underlying implementation in pinentry/pinentry.c and
pinentry/password-cache.c to cache the password using libsecret.

Signed-off-by: Jason Carrete <jasoncarrete5@gmail.com>
---
qt/main.cpp | 7 +++++--
qt/pinentrydialog.cpp | 37 +++++++++++++++++++++++++++++--------
qt/pinentrydialog.h | 9 +++++----
3 files changed, 39 insertions(+), 14 deletions(-)

diff --git a/qt/main.cpp b/qt/main.cpp
index 8c8ab48..bc87e1a 100644
--- a/qt/main.cpp
+++ b/qt/main.cpp
@@ -212,12 +212,14 @@ qt_cmd_handler(pinentry_t pe)
const QString generateTT = pe->genpin_tt ? from_utf8(pe->genpin_tt) :
QString();

+ const QString savePassphraseText =
+ pe->default_pwmngr ? escape_accel(from_utf8(pe->default_pwmngr)) :
+ QLatin1String("Save passphrase using libsecret");

if (want_pass) {
- PinEntryDialog pinentry(nullptr, 0, pe->timeout, true,
!!pe->quality_bar,
+ PinEntryDialog pinentry(pe, nullptr, 0, true,
repeatString, visibilityTT, hideTT);
setup_foreground_window(&pinentry, pe->parent_wid);
- pinentry.setPinentryInfo(pe);
pinentry.setPrompt(escape_accel(from_utf8(pe->prompt)));
pinentry.setDescription(from_utf8(pe->description));
pinentry.setRepeatErrorText(repeatError);
@@ -233,6 +235,7 @@ qt_cmd_handler(pinentry_t pe)
from_utf8(pe->constraints_hint_long),
from_utf8(pe->constraints_error_title)
});
+ pinentry.setSavePassphraseCBText(savePassphraseText);

if (!title.isEmpty()) {
pinentry.setWindowTitle(title);
diff --git a/qt/pinentrydialog.cpp b/qt/pinentrydialog.cpp
index 515576b..259fd6a 100644
--- a/qt/pinentrydialog.cpp
+++ b/qt/pinentrydialog.cpp
@@ -96,13 +96,14 @@ void PinEntryDialog::slotTimeout()
reject();
}

-PinEntryDialog::PinEntryDialog(QWidget *parent, const char *name,
- int timeout, bool modal, bool
enable_quality_bar,
+PinEntryDialog::PinEntryDialog(pinentry_t pe, QWidget *parent, const
char *name,
+ bool modal,
const QString &repeatString,
const QString &visibilityTT,
const QString &hideTT)
: QDialog{parent}
- , _have_quality_bar{enable_quality_bar}
+ , _have_quality_bar{!!pe->quality_bar}
+ , _pinentry_info{pe}
, mVisibilityTT{visibilityTT}
, mHideTT{hideTT}
{
@@ -232,7 +233,7 @@ PinEntryDialog::PinEntryDialog(QWidget *parent,
const char *name,
grid->addWidget(mRepeatError, row, 2);
}

- if (enable_quality_bar) {
+ if (_have_quality_bar) {
row++;
_quality_bar_label = new QLabel(this);
_quality_bar_label->setTextFormat(Qt::PlainText);
@@ -246,6 +247,19 @@ PinEntryDialog::PinEntryDialog(QWidget *parent,
const char *name,
grid->addWidget(_quality_bar, row, 2);
}

+ ++row;
+ mSavePassphraseCB = new QCheckBox{this};
+ mSavePassphraseCB->setVisible(false);
+ mSavePassphraseCB->setCheckState(!!_pinentry_info->may_cache_password
+ ? Qt::Checked
+ : Qt::Unchecked);
+#ifdef HAVE_LIBSECRET
+ if (_pinentry_info->allow_external_password_cache &&
_pinentry_info->keyinfo) {
+ mSavePassphraseCB->setVisible(true);
+ }
+#endif
+ grid->addWidget(mSavePassphraseCB, row, 1, 1, 2);
+
hbox->addLayout(grid, 1);
mainLayout->addLayout(hbox);

@@ -263,10 +277,10 @@ PinEntryDialog::PinEntryDialog(QWidget *parent,
const char *name,
mainLayout->addWidget(buttons);
mainLayout->setSizeConstraint(QLayout::SetFixedSize);

- if (timeout > 0) {
+ if (_pinentry_info->timeout > 0) {
_timer = new QTimer(this);
connect(_timer, &QTimer::timeout, this,
&PinEntryDialog::slotTimeout);
- _timer->start(timeout * 1000);
+ _timer->start(_pinentry_info->timeout * 1000);
}

connect(buttons, &QDialogButtonBox::accepted,
@@ -295,6 +309,8 @@ PinEntryDialog::PinEntryDialog(QWidget *parent,
const char *name,
connect(mRepeat, &QLineEdit::textChanged,
this, &PinEntryDialog::textChanged);
}
+ connect(mSavePassphraseCB, &QCheckBox::stateChanged,
+ this, &PinEntryDialog::toggleSavePassphrase);

auto capsLockWatcher = new CapsLockWatcher{this};
connect(capsLockWatcher, &CapsLockWatcher::stateChanged,
@@ -510,6 +526,11 @@ void PinEntryDialog::toggleFormattedPassphrase()
}
}

+void PinEntryDialog::toggleSavePassphrase()
+{
+ _pinentry_info->may_cache_password =
!_pinentry_info->may_cache_password;
+}
+
void PinEntryDialog::onBackspace()
{
cancelTimeout();
@@ -552,9 +573,9 @@ void PinEntryDialog::updateQuality(const QString &txt)
}
}

-void PinEntryDialog::setPinentryInfo(pinentry_t peinfo)
+void PinEntryDialog::setSavePassphraseCBText(const QString &text)
{
- _pinentry_info = peinfo;
+ mSavePassphraseCB->setText(text);
}

void PinEntryDialog::focusChanged(QWidget *old, QWidget *now)
diff --git a/qt/pinentrydialog.h b/qt/pinentrydialog.h
index 60161c5..6baa5ba 100644
--- a/qt/pinentrydialog.h
+++ b/qt/pinentrydialog.h
@@ -72,9 +72,8 @@ public:
QString errorTitle;
};

- explicit PinEntryDialog(QWidget *parent = 0, const char *name = 0,
- int timeout = 0, bool modal = false,
- bool enable_quality_bar = false,
+ explicit PinEntryDialog(pinentry_t pe, QWidget *parent = 0, const
char *name = 0,
+ bool modal = false,
const QString &repeatString = QString(),
const QString &visibiltyTT = QString(),
const QString &hideTT = QString());
@@ -110,7 +109,7 @@ public:

void setConstraintsOptions(const ConstraintsOptions &options);

- void setPinentryInfo(pinentry_t);
+ void setSavePassphraseCBText(const QString &text);

bool timedOut() const;

@@ -123,6 +122,7 @@ protected Q_SLOTS:
void onBackspace();
void generatePin();
void toggleFormattedPassphrase();
+ void toggleSavePassphrase();

protected:
void keyPressEvent(QKeyEvent *event) override;
@@ -176,6 +176,7 @@ private:
QLabel *mCapsLockHint = nullptr;
QLabel *mConstraintsHint = nullptr;
QString mConstraintsErrorTitle;
+ QCheckBox *mSavePassphraseCB = nullptr;
};

#endif // __PINENTRYDIALOG_H__
--
2.42.1
Re: [PATCH pinentry] qt: Present option to save password in password manager [ In reply to ]
Hi Jason,

thanks for your work! Nowadays we mostly use https://dev.gnupg.org for such
things but patches per Mail are also always welcome. Using libsecret for
password storage is indeed something that would be good for pinentry-qt.

I have created https://dev.gnupg.org/T6801 for this. Could you maybe attach
your patch through a differential https://dev.gnupg.org/differential/diff/create/
?

The problem is that even if I extract the raw Base64 from your mail the patch
is already word wrapped and that breaks it in many places. I could fix it up
manually but as you will have this as a patch file somwhere it would be easier
if you could just upload it.

Best Regards,
Andre

On Monday, 06 November 2023 09:09:47 CET Jason Carrete via Gnupg-devel wrote:
> * qt/main.cpp: Pass pinentry info to PinEntryDialog constructor. Set
> save passphrase checkbox text from pinentry_t->default_pwmngr.
> * qt/pinentrydialog.cpp, qt/pinentrydialog.h: Dialog now accepts
> pinentry info in the constructor and removed unneeded setter for
> pinentry info. Add save passphrase checkbox.
>
> --
>
> This patch adds functionality to save key passphrases with pinentry-qt
> that already exists in pinentry-gtk-2.
>
> A "save passphrase" checkbox is shown when libsecret is available,
> the external password cache is enabled, and there is valid data in
> pinentry_t->keyinfo. When checked, the pinentry info is updated to allow
> the underlying implementation in pinentry/pinentry.c and
> pinentry/password-cache.c to cache the password using libsecret.
>
> Signed-off-by: Jason Carrete <jasoncarrete5@gmail.com>
> ---
> qt/main.cpp | 7 +++++--
> qt/pinentrydialog.cpp | 37 +++++++++++++++++++++++++++++--------
> qt/pinentrydialog.h | 9 +++++----
> 3 files changed, 39 insertions(+), 14 deletions(-)
>
> diff --git a/qt/main.cpp b/qt/main.cpp
> index 8c8ab48..bc87e1a 100644
> --- a/qt/main.cpp
> +++ b/qt/main.cpp
> @@ -212,12 +212,14 @@ qt_cmd_handler(pinentry_t pe)
> const QString generateTT = pe->genpin_tt ? from_utf8(pe->genpin_tt) :
> QString();
>
> + const QString savePassphraseText =
> + pe->default_pwmngr ? escape_accel(from_utf8(pe->default_pwmngr)) :
> + QLatin1String("Save passphrase using libsecret");
>
> if (want_pass) {
> - PinEntryDialog pinentry(nullptr, 0, pe->timeout, true,
> !!pe->quality_bar,
> + PinEntryDialog pinentry(pe, nullptr, 0, true,
> repeatString, visibilityTT, hideTT);
> setup_foreground_window(&pinentry, pe->parent_wid);
> - pinentry.setPinentryInfo(pe);
> pinentry.setPrompt(escape_accel(from_utf8(pe->prompt)));
> pinentry.setDescription(from_utf8(pe->description));
> pinentry.setRepeatErrorText(repeatError);
> @@ -233,6 +235,7 @@ qt_cmd_handler(pinentry_t pe)
> from_utf8(pe->constraints_hint_long),
> from_utf8(pe->constraints_error_title)
> });
> + pinentry.setSavePassphraseCBText(savePassphraseText);
>
> if (!title.isEmpty()) {
> pinentry.setWindowTitle(title);
> diff --git a/qt/pinentrydialog.cpp b/qt/pinentrydialog.cpp
> index 515576b..259fd6a 100644
> --- a/qt/pinentrydialog.cpp
> +++ b/qt/pinentrydialog.cpp
> @@ -96,13 +96,14 @@ void PinEntryDialog::slotTimeout()
> reject();
> }
>
> -PinEntryDialog::PinEntryDialog(QWidget *parent, const char *name,
> - int timeout, bool modal, bool
> enable_quality_bar,
> +PinEntryDialog::PinEntryDialog(pinentry_t pe, QWidget *parent, const
> char *name,
> + bool modal,
> const QString &repeatString,
> const QString &visibilityTT,
> const QString &hideTT)
> : QDialog{parent}
> - , _have_quality_bar{enable_quality_bar}
> + , _have_quality_bar{!!pe->quality_bar}
> + , _pinentry_info{pe}
> , mVisibilityTT{visibilityTT}
> , mHideTT{hideTT}
> {
> @@ -232,7 +233,7 @@ PinEntryDialog::PinEntryDialog(QWidget *parent,
> const char *name,
> grid->addWidget(mRepeatError, row, 2);
> }
>
> - if (enable_quality_bar) {
> + if (_have_quality_bar) {
> row++;
> _quality_bar_label = new QLabel(this);
> _quality_bar_label->setTextFormat(Qt::PlainText);
> @@ -246,6 +247,19 @@ PinEntryDialog::PinEntryDialog(QWidget *parent,
> const char *name,
> grid->addWidget(_quality_bar, row, 2);
> }
>
> + ++row;
> + mSavePassphraseCB = new QCheckBox{this};
> + mSavePassphraseCB->setVisible(false);
> + mSavePassphraseCB->setCheckState(!!_pinentry_info->may_cache_password
> + ? Qt::Checked
> + : Qt::Unchecked);
> +#ifdef HAVE_LIBSECRET
> + if (_pinentry_info->allow_external_password_cache &&
> _pinentry_info->keyinfo) {
> + mSavePassphraseCB->setVisible(true);
> + }
> +#endif
> + grid->addWidget(mSavePassphraseCB, row, 1, 1, 2);
> +
> hbox->addLayout(grid, 1);
> mainLayout->addLayout(hbox);
>
> @@ -263,10 +277,10 @@ PinEntryDialog::PinEntryDialog(QWidget *parent,
> const char *name,
> mainLayout->addWidget(buttons);
> mainLayout->setSizeConstraint(QLayout::SetFixedSize);
>
> - if (timeout > 0) {
> + if (_pinentry_info->timeout > 0) {
> _timer = new QTimer(this);
> connect(_timer, &QTimer::timeout, this,
> &PinEntryDialog::slotTimeout);
> - _timer->start(timeout * 1000);
> + _timer->start(_pinentry_info->timeout * 1000);
> }
>
> connect(buttons, &QDialogButtonBox::accepted,
> @@ -295,6 +309,8 @@ PinEntryDialog::PinEntryDialog(QWidget *parent,
> const char *name,
> connect(mRepeat, &QLineEdit::textChanged,
> this, &PinEntryDialog::textChanged);
> }
> + connect(mSavePassphraseCB, &QCheckBox::stateChanged,
> + this, &PinEntryDialog::toggleSavePassphrase);
>
> auto capsLockWatcher = new CapsLockWatcher{this};
> connect(capsLockWatcher, &CapsLockWatcher::stateChanged,
> @@ -510,6 +526,11 @@ void PinEntryDialog::toggleFormattedPassphrase()
> }
> }
>
> +void PinEntryDialog::toggleSavePassphrase()
> +{
> + _pinentry_info->may_cache_password =
> !_pinentry_info->may_cache_password;
> +}
> +
> void PinEntryDialog::onBackspace()
> {
> cancelTimeout();
> @@ -552,9 +573,9 @@ void PinEntryDialog::updateQuality(const QString &txt)
> }
> }
>
> -void PinEntryDialog::setPinentryInfo(pinentry_t peinfo)
> +void PinEntryDialog::setSavePassphraseCBText(const QString &text)
> {
> - _pinentry_info = peinfo;
> + mSavePassphraseCB->setText(text);
> }
>
> void PinEntryDialog::focusChanged(QWidget *old, QWidget *now)
> diff --git a/qt/pinentrydialog.h b/qt/pinentrydialog.h
> index 60161c5..6baa5ba 100644
> --- a/qt/pinentrydialog.h
> +++ b/qt/pinentrydialog.h
> @@ -72,9 +72,8 @@ public:
> QString errorTitle;
> };
>
> - explicit PinEntryDialog(QWidget *parent = 0, const char *name = 0,
> - int timeout = 0, bool modal = false,
> - bool enable_quality_bar = false,
> + explicit PinEntryDialog(pinentry_t pe, QWidget *parent = 0, const
> char *name = 0,
> + bool modal = false,
> const QString &repeatString = QString(),
> const QString &visibiltyTT = QString(),
> const QString &hideTT = QString());
> @@ -110,7 +109,7 @@ public:
>
> void setConstraintsOptions(const ConstraintsOptions &options);
>
> - void setPinentryInfo(pinentry_t);
> + void setSavePassphraseCBText(const QString &text);
>
> bool timedOut() const;
>
> @@ -123,6 +122,7 @@ protected Q_SLOTS:
> void onBackspace();
> void generatePin();
> void toggleFormattedPassphrase();
> + void toggleSavePassphrase();
>
> protected:
> void keyPressEvent(QKeyEvent *event) override;
> @@ -176,6 +176,7 @@ private:
> QLabel *mCapsLockHint = nullptr;
> QLabel *mConstraintsHint = nullptr;
> QString mConstraintsErrorTitle;
> + QCheckBox *mSavePassphraseCB = nullptr;
> };
>
> #endif // __PINENTRYDIALOG_H__
> --
> 2.42.1
>


--
GnuPG.com - a brand of g10 Code, the GnuPG experts.

g10 Code GmbH, Erkrath/Germany, AG Wuppertal HRB14459
GF Werner Koch, USt-Id DE215605608, www.g10code.com.

GnuPG e.V., Rochusstr. 44, D-40479 D?sseldorf. VR 11482 D?sseldorf
Vorstand: W.Koch, B.Reiter, A.Heinecke Mail: board@gnupg.org
Finanzamt D-Altstadt, St-Nr: 103/5923/1779. Tel: +49-211-28010702
Re: [PATCH pinentry] qt: Present option to save password in password manager [ In reply to ]
> I have created https://dev.gnupg.org/T6801 for this. Could you maybe attach
> your patch through a differential https://dev.gnupg.org/differential/diff/create/
> ?

I have created a diff here (https://dev.gnupg.org/differential/diff/1543/) and left a comment on https://dev.gnupg.org/T6801#178117

I'm not too familiar with phabricator software so I wasn't sure how to connect the diff together with the ticket you created.