Bug 113057 - kwallet popup window does not grab focus anymore
Summary: kwallet popup window does not grab focus anymore
Status: RESOLVED FIXED
Alias: None
Product: kwin
Classification: Plasma
Component: general (show other bugs)
Version: unspecified
Platform: Debian testing Linux
: NOR major
Target Milestone: ---
Assignee: KWin default assignee
URL:
Keywords:
: 86347 (view as bug list)
Depends on:
Blocks:
 
Reported: 2005-09-22 13:27 UTC by David Delbecq
Modified: 2013-08-30 03:59 UTC (History)
4 users (show)

See Also:
Latest Commit:
Version Fixed In:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description David Delbecq 2005-09-22 13:27:11 UTC
Version:            (using KDE KDE 3.4.2)
Installed from:    Debian testing/unstable Packages
OS:                Linux

When i do the following:
1) start a program requiring kwalletmanager (eg kmail or kontact)
2) switch focus to another window before program is started (like an irc chat program)
3) as part of first program launch, a walletmanager popup window appear
4) type your password, enter

Then you sadly notice despite kwalletmanager password box being in front of everything, it didn't grab keyboard focus and you just sent your kwalletmanager password to the irc channel.

I think the popup window should grab focus or not show itself on front of everything. I don't always look at screen when typing and so doesn't notice little '*' don't show in textarea.

I never sent my password to irc like that, but i guess the scenario is not so improbable. 
This dialog didn't have this kind of behaviour before upgrade.
Comment 1 George Staikos 2005-09-22 13:52:45 UTC
Funny, I didn't change anything.  Must be a kwin change.
Comment 2 Lubos Lunak 2006-01-24 16:43:50 UTC
Actually you did :). You made the dialog to be on all desktops because of bug #91940. Which means here that the app started in 1) is launched on an inactive virtual desktop and the kwallet dialog is shown for it. Since the app properly passed parent to the wallet dialog kwin recognizes they belong together and normally would move the dialog to the inactive virtual desktop too and wouldn't activate it. So it does so, however the dialog is marked to be on all desktops and as such is shown also on the current one - without focus.

I think the proper fix would be to revert the change and make it possible for the dialog to have more than one "parent" at the same time. This would first need implementing though.
Comment 3 David Delbecq 2006-01-25 09:38:31 UTC
Have to check, but maybe it possible to allow password box to still have focus if you disable kwin focus stealing prevention for this particular dialog.
Comment 4 Lubos Lunak 2006-02-28 13:19:05 UTC
*** Bug 86347 has been marked as a duplicate of this bug. ***
Comment 5 Greg Baker 2006-03-24 22:24:11 UTC
I confirm this bug.  It has my vote.
Comment 6 gmud 2006-05-03 11:56:43 UTC
Dito
Comment 7 Matthew Lanteigne 2006-05-04 11:29:12 UTC
I confirm as well. For example, I open Konversation, w/e, then I open Kontact, Kontact opens Kwallet, but KWallet is on top, but no focus, so I type password (without really looking, as it pops on top) and I end up typing my password into IRC. Not good, and to me is a major security flaw.
Comment 8 David Delbecq 2006-05-11 11:35:12 UTC
Same problem arise even more badly with kopete. Kopete starts, request password to access IM account, but the newly opened IRC chat windows by kopete steal the focus. You have to be very very carefull when you use kopete.
Comment 9 Mats Ahlgren 2006-06-23 11:56:04 UTC
I can confirm this as well. This is a very dangerous bug.

And while I have not sent my password, I've almost on three separate occasions sent it as part of a KMail message title.

I think the same behavior can also be seen with the run dialog box (alt-f2) -- really, kwin's popups should grab focus when they popup and freeze focus changing for 0.25 sec immediately afterwards (think fast interrupt, not inter-process communication).
Comment 10 Ferdinand Gassauer 2006-06-23 12:17:02 UTC
seems to be a major issue

workaround - put kmail into autostart, so it will open kwalletmanager at the beginning of the KDE session where it is highly unlikely that other keyboard input occurs.
Comment 11 George Staikos 2006-06-23 12:29:08 UTC
Tired of hearing about this.  It does what I was told to do in order to grab the focus with the popup dialog.  The rest is up to the window manager to deal with.  If I did something wrong, please reassign to me with the details.
Comment 12 Mats Ahlgren 2006-07-22 08:06:46 UTC
I've thought about this for awhile, and have come up with the exact solution we need:

PROBLEM:
This dangerous problem stems from two situations: 1) the user starts typing his password without realizing the popup dialog doesn't have focus (the big problem), and/or 2) while he's typing his password, another program (e.g. chat client) does something which changes window focus, like launching a chat window.

SOLUTION:
Either of these proposals will remedy the problem. I propose a simple solution which can be implemented relatively easily, and (alternatively) below that I detail how to fix the current system to keep it in line with the original intent of the KDE developers.

I've thought very carefully about this, and the following pseudocode takes into account very subtle situations. Even minor deviations from what I've written may create dangerous loopholes. Please feel free to comment and check my work.

[BEGIN SIMPLE SOLUTION]

All password prompts are unconditionally on top of all other windows, always, no matter what.

Pseudocode: "All new password windows start on top. If a change would be made to the window stacking order, then instead do the following as an atomic action: make that change and put the password window(s) on top while keeping their relative order."

[END SIMPLE SOLUTION]
[BEGIN ELEGANT/FIXED SOLUTION]

WHEN { Requesting-Application requests Password-Prompt }
DO {
-- IF {Requesting-App is in foreground} THEN
-- -- {Spawn the password prompt WITH special window-stacking rules (see below).}
-- ELSE
-- -- {Spawn modified* password prompt WITH same special window-stacking rules.}
}

Note: it is very important that this executes ATOMICally -- i.e. when it executes, it must execute COMPLETELY and UNINTERRUPTED by other processes or threads.

* The modified password prompt has no password entry-field, but instead says "The program [Requesting-App] needs your password to continue. Click this window to enter your password." When you click the window and it become active, the field appears and is given focus so you can start typing. If the prompt loses focus, then the prompt reverts back to the "Click here" window.

Special Rules for Password Prompt Window Stacking Order:
(1) The Post-It Note Rule: "This prompt behaves like a Post-It note attached to its master program. (This is a simple rule, but it has some implications, so I'll give some examples to illustrate what I mean: When the program would have focus, instead the prompt gets focus. When the program loses focus, the prompt loses focus. When the program is hidden, the prompt is hidden. If the password prompt is launched by a program hidden behind some windows, the password prompt will spawn above the requesting program, but below all those other windows. When the program changes its z-order, so does the prompt.)"
(2) The First-Things-First Rule: "It is impossible to click on or otherwise affect the requesting program until the password has been entered or the 'Cancel' button has been pressed; the only effect these clicks should have is to bring the Note-and-Application window-complex to the foreground."
(3) The Don't-Interrupt Rule: "If this prompt has focus, then no program - even the requesting program - may usurp focus from it. As a corollary, if any program tries to create a new window and the prompt is in the foreground, that new window will instead be created below the password prompt's master program."
(4) The One-at-a-Time Rule: "If the requesting program attempt to launch two password prompts at the same time, then put the other one on hold until the first one has resolved."

Reasoning:
1) This is elegant and I think it can be implemented in a snap by using KWin's toolbar or whatever functionality. It's also what I think the original intent of the password prompts was. However, it might not be necessary.
2) Necessary: This prevents the situation where you're clicking in the program's GUI, and the password prompt launches, and you didn't realize you clicked after the launch of the prompt giving focus back to the program. An alternative to this rule is to implement a 0.5 second delay during which changes to window stacking order is frozen, to give the user a chance to react.
3) Necessary: This prevents the situation where your email client, after taking 10 seconds to load, pops up while you're typing your password to another program, and you end up sending your password to a mailing list.
4) This prevents unnecessary complications.

[END ELEGANT/FIXED SOLUTION]
Comment 13 Mats Ahlgren 2006-07-22 08:12:13 UTC
How odd -- I think I reinvented Mac OS X's dialog boxes... Y'know, the ones that roll out from the window title bar like a printer spitting out a piece of paper, and roll back in when finished, and are "physically attached" to the window as you move the window around and change window focus?

(Except OS X doesn't use those for password boxes.)
Comment 14 David Delbecq 2006-07-24 09:18:06 UTC
Quote George Staikos:
"Tired of hearing about this.  It does what I was told to do in order to grab the focus with the popup dialog.  The rest is up to the window manager to deal with.  If I did something wrong, please reassign to me with the details."

And as user, i am tired of seeing this bug not solved. After 10 months of confirmed existence of this security related bug, nothing seems to have been done to solve it. Despite number of people confirming the bug, it's state has never been switched to confirmed. Despite the kwin developper being assigned this bug, there has been no message from them in this report. Do the kwin developper *know* they have a major security issue on the hand? 

Kwallet is now an important part of kde, and a lot of kde application developpers do use it now as a security tool. However, as a matter of facts, kwalletmanager is now a dangerous tool instead of a security tool. 

And what was bound to happen, happened. This week-end i sent my kwallet password on IRC, using kopete. This app was requestiong my wallet password and at the same time did open IRC windows. Those in background got keyboard focus.

It is really time something is done about it. If kwin developper don't seems to take this problem into account, do some action, contact them, ask them why they don't take it into account, move the issue to critical, security related and shake them a bit, this is really a dangerous situation on a security software and it has not been solved in 10 month, this, am sorry to have to say this to a free software, something that is unacceptable. If it were not because so much apps depend on kwallet i would have already removed this package from my system.

But unfortunately, this bugged security software is at the core of kde now...
Comment 15 gmud 2006-07-24 10:31:57 UTC
*** This bug has been confirmed by popular vote. ***
Comment 16 Lubos Lunak 2006-08-29 17:09:50 UTC
SVN commit 578532 by lunakl:

Replace the wallet dialog hacks that were supposed to work around #91940
and caused #113057 with slightly better hacks. If somebody notices
some problems with kwallet dialogs after this change please yell.
CCMAIL: kde-core-devel@kde.org
BUG: 113057



 M  +50 -26    kwalletd.cpp  
 M  +7 -2      kwalletd.h  


--- branches/KDE/3.5/kdelibs/kio/misc/kwalletd/kwalletd.cpp #578531:578532
@@ -50,10 +50,6 @@
 
 #include <assert.h>
 
-#ifdef Q_WS_X11
-#include <X11/Xlib.h>
-#endif
-
 extern "C" {
    KDE_EXPORT KDEDModule *create_kwalletd(const QCString &name) {
 	   return new KWalletD(name);
@@ -67,6 +63,7 @@
 			tType = Unknown;
 			transaction = 0L;
 			client = 0L;
+			modal = false;
 		}
 
 		~KWalletTransaction() {
@@ -83,6 +80,7 @@
 		QCString appid;
 		uint wId;
 		QString wallet;
+		bool modal;
 };
 
 
@@ -149,7 +147,7 @@
 
 		switch (xact->tType) {
 			case KWalletTransaction::Open:
-				res = doTransactionOpen(xact->appid, xact->wallet, xact->wId);
+				res = doTransactionOpen(xact->appid, xact->wallet, xact->wId, xact->modal);
 				replyType = "int";
 				if (!xact->returnObject.isEmpty()) {
 					DCOPRef(xact->rawappid, xact->returnObject).send("walletOpenResult", res);
@@ -232,6 +230,7 @@
 	DCOPRef(appid, returnObject).send("walletOpenResult", 0);
 
 	QTimer::singleShot(0, this, SLOT(processTransactions()));
+	checkActiveDialog();
 }
 
 
@@ -266,18 +265,53 @@
 	xact->wallet = wallet;
 	xact->wId = wId;
 	xact->tType = KWalletTransaction::Open;
+	xact->modal = true; // mark dialogs as modal, the app has blocking wait
 	QTimer::singleShot(0, this, SLOT(processTransactions()));
+	checkActiveDialog();
 	return 0; // process later
 }
 
 
-int KWalletD::doTransactionOpen(const QCString& appid, const QString& wallet, uint wId) {
+// Sets up a dialog that will be shown by kwallet.
+void KWalletD::setupDialog( QWidget* dialog, WId wId, const QCString& appid, bool modal ) {
+	if( wId != 0 ) 
+		KWin::setMainWindow( dialog, wId ); // correct, set dialog parent
+	else {
+		if( appid.isEmpty())
+			kdWarning() << "Using kwallet without parent window!" << endl;
+		else
+			kdWarning() << "Application '" << appid << "' using kwallet without parent window!" << endl;
+		// allow dialog activation even if it interrupts, better than trying hacks
+		// with keeping the dialog on top or on all desktops
+		kapp->updateUserTimestamp();
+	}
+	if( modal )
+		KWin::setState( dialog->winId(), NET::Modal );
+	else
+		KWin::clearState( dialog->winId(), NET::Modal );
+	activeDialog = dialog;
+}
+
+// If there's a dialog already open and another application tries some operation that'd lead to
+// opening a dialog, that application will be blocked by this dialog. A proper solution would
+// be to set the second application's window also as a parent for the active dialog, so that
+// KWin properly handles focus changes and so on, but there's currently no support for multiple
+// dialog parents. Hopefully to be done in KDE4, for now just use all kinds of bad hacks to make
+//  sure the user doesn't overlook the active dialog.
+void KWalletD::checkActiveDialog() {
+	if( !activeDialog || !activeDialog->isShown())
+		return;
+	kapp->updateUserTimestamp();
+	KWin::setState( activeDialog->winId(), NET::KeepAbove );
+	KWin::setOnAllDesktops( activeDialog->winId(), true );
+	KWin::forceActiveWindow( activeDialog->winId());
+}
+
+int KWalletD::doTransactionOpen(const QCString& appid, const QString& wallet, uint wId, bool modal) {
 	if (_firstUse && !wallets().contains(KWallet::Wallet::LocalWallet())) {
 		// First use wizard
 		KWalletWizard *wiz = new KWalletWizard(0);
-#ifdef Q_WS_X11
-		XSetTransientForHint(qt_xdisplay(), wiz->winId(), wId);
-#endif
+		setupDialog( wiz, wId, appid, modal );
 		int rc = wiz->exec();
 		if (rc == QDialog::Accepted) {
 			KConfig cfg("kwalletrc");
@@ -317,12 +351,12 @@
 		cfg.sync();
 	}
 
-	int rc = internalOpen(appid, wallet, false, wId);
+	int rc = internalOpen(appid, wallet, false, wId, modal);
 	return rc;
 }
 
 
-int KWalletD::internalOpen(const QCString& appid, const QString& wallet, bool isPath, WId w) {
+int KWalletD::internalOpen(const QCString& appid, const QString& wallet, bool isPath, WId w, bool modal) {
 	int rc = -1;
 	bool brandNew = false;
 
@@ -402,11 +436,7 @@
 		const char *p = 0L;
 		while (!b->isOpen()) {
 			assert(kpd); // kpd can't be null if isOpen() is false
-#ifdef Q_WS_X11
-			XSetTransientForHint(qt_xdisplay(), kpd->winId(), w);
-#endif
-			KWin::setState( kpd->winId(), NET::KeepAbove );
-			KWin::setOnAllDesktops(kpd->winId(), true);
+			setupDialog( kpd, w, appid, modal );
 			if (kpd->exec() == KDialog::Accepted) {
 				p = kpd->password();
 				int rc = b->open(QByteArray().duplicate(p, strlen(p)));
@@ -489,12 +519,7 @@
 		} else {
 			dialog->setLabel(i18n("<qt>The application '<b>%1</b>' has requested access to the open wallet '<b>%2</b>'.").arg(QStyleSheet::escape(QString(appid))).arg(QStyleSheet::escape(wallet)));
 		}
-#ifdef Q_WS_X11
-		XSetTransientForHint(qt_xdisplay(), dialog->winId(), w);
-#endif
-		KWin::setState(dialog->winId(), NET::KeepAbove);
-		KWin::setOnAllDesktops(dialog->winId(), true);
-
+		setupDialog( dialog, w, appid, false );
 		response = dialog->exec();
 		delete dialog;
 	}
@@ -560,6 +585,7 @@
 	_transactions.append(xact);
 
 	QTimer::singleShot(0, this, SLOT(processTransactions()));
+	checkActiveDialog();
 }
 
 
@@ -576,7 +602,7 @@
 	}
 
 	if (!it.current()) {
-		handle = doTransactionOpen(appid, wallet, wId);
+		handle = doTransactionOpen(appid, wallet, wId,false);
 		if (-1 == handle) {
 			KMessageBox::sorryWId(wId, i18n("Unable to open wallet. The wallet must be opened in order to change the password."), i18n("KDE Wallet Service"));
 			return;
@@ -596,9 +622,7 @@
 	kpd->setPrompt(i18n("<qt>Please choose a new password for the wallet '<b>%1</b>'.").arg(QStyleSheet::escape(wallet)));
 	kpd->setCaption(i18n("KDE Wallet Service"));
 	kpd->setAllowEmptyPasswords(true);
-#ifdef Q_WS_X11
-	XSetTransientForHint(qt_xdisplay(), kpd->winId(), wId);
-#endif
+	setupDialog( kpd, wId, appid, false );
 	if (kpd->exec() == KDialog::Accepted) {
 		const char *p = kpd->password();
 		if (p) {
--- branches/KDE/3.5/kdelibs/kio/misc/kwalletd/kwalletd.h #578531:578532
@@ -26,6 +26,7 @@
 #include <qintdict.h>
 #include <qstring.h>
 #include <qwidget.h>
+#include <qguardedptr.h>
 #include "kwalletbackend.h"
 
 #include <time.h>
@@ -150,7 +151,7 @@
 		void processTransactions();
 
 	private:
-		int internalOpen(const QCString& appid, const QString& wallet, bool isPath = false, WId w = 0);
+		int internalOpen(const QCString& appid, const QString& wallet, bool isPath = false, WId w = 0, bool modal = false);
 		bool isAuthorizedApp(const QCString& appid, const QString& wallet, WId w);
 		// This also validates the handle.  May return NULL.
 		KWallet::Backend* getWallet(const QCString& appid, int handle);
@@ -169,8 +170,11 @@
 		QCString friendlyDCOPPeerName();
 
 		void doTransactionChangePassword(const QCString& appid, const QString& wallet, uint wId);
-		int doTransactionOpen(const QCString& appid, const QString& wallet, uint wId);
+		int doTransactionOpen(const QCString& appid, const QString& wallet, uint wId, bool modal);
 
+		void setupDialog( QWidget* dialog, WId wId, const QCString& appid, bool modal );
+		void checkActiveDialog();
+
 		QIntDict<KWallet::Backend> _wallets;
 		QMap<QCString,QValueList<int> > _handles;
 		QMap<QString,QCString> _passwords;
@@ -184,6 +188,7 @@
 		KTimeout *_timeouts;
 
 		QPtrList<KWalletTransaction> _transactions;
+		QGuardedPtr< QWidget > activeDialog;
 };
 
 
Comment 17 Daniel Danner 2007-10-29 19:07:01 UTC
I found this bug via Google because I'm (still) experiencing this problem.

I'm using Gentoo with all kde packages installed version 3.5.8 and my window manager is dwm. I'm pretty sure this is not a problem of my WM since x11-ssh-askpass (for example) does a great job totally grabbing my input.
Comment 18 Jonatan Cloutier 2013-08-30 02:23:14 UTC
Well, problem still exist on 4.11
Comment 19 Thomas Lübking 2013-08-30 03:59:30 UTC
(In reply to comment #18)
> Well, problem still exist on 4.11
No idea whether anything broke in the meantime, but if there's an issue rather "once again" than "still" - and this severely depends on your local focus setting and stealing configuration as well.

Please leave this corpse alone, file a new bug, attach the output of "qdbus org.kde.kwin /KWin supportInformation" and the output of "xprop" on the window that kwallet is opened for as well as the kwallet dialog.
(For the latter two maybe briefly scan them for private data eg. in the title string or so)