[PATCH] Add a credential-helper for KDE

[PATCH] Add a credential-helper for KDE

[Lukas Sandström]

This Python script plugs into the credentials API
of Git to ask the user for passwords with a nice
KDE password dialog.

The password is saved in the KWallet.

Signed-off-by: Lukas Sandström <luksan@gmail.com>
---

Here is a credentials-helper for KDE. You need to have PyKDE installed to use it.

See Documentation/gitcredentials.txt for more info.

 .../git-kde-credentials-helper.py                  |  122 ++++++++++++++++++++
 1 files changed, 122 insertions(+), 0 deletions(-)
 create mode 100755 contrib/kde-credetials-helper/git-kde-credentials-helper.py

diff --git a/contrib/kde-credetials-helper/git-kde-credentials-helper.py b/contrib/kde-credetials-helper/git-kde-credentials-helper.py
new file mode 100755
index 0000000..8d3be4d
--- /dev/null
+++ b/contrib/kde-credetials-helper/git-kde-credentials-helper.py
@@ -0,0 +1,122 @@
+#!/usr/bin/env python
+# encoding=utf-8
+#
+# Copyright 2011, Lukas Sandström
+#
+# Licensed under the GPL version 2.
+
+import sys, commands
+from PyQt4.QtCore import QString
+from PyKDE4.kdecore import i18n, ki18n, KAboutData, KCmdLineArgs, KCmdLineOptions
+from PyKDE4.kdeui import KApplication, KWallet, KPasswordDialog
+
+appName     = "git-kde-credentials-helper"
+catalog     = ""
+programName = ki18n ("Git KDE credentials helper")
+version     = "0.1"
+description = ki18n ("Credentials storage helper for Git")
+license     = KAboutData.License_GPL_V2
+copyright   = ki18n ("(c) 2011 Lukas Sandström")
+text        = ki18n ("none")
+homePage    = "http://www.git-scm.com"
+bugEmail    = "luksan@gmail.com"
+
+aboutData   = KAboutData (appName, catalog, programName, version, description,
+                          license, copyright, text, homePage, bugEmail)
+
+class CredentialHelper(KApplication):
+    def __init__(self, token, username = None, desc = None, reject = False):
+        super(CredentialHelper, self).__init__()
+        self.password = None
+        self.username = username
+        self.save_password = False
+        self.token = token
+        self.desc = desc
+
+        if not self.token:
+            return
+
+        self.open_wallet()
+
+        if reject:
+            self.wallet.removeEntry(QString(token))
+            return
+
+        if not self.check_wallet():
+            self.ask_password_dialog()
+        
+        if self.save_password:
+            self.store_password()
+
+        self.output_credentials()
+
+    def output_credentials(self):
+        if self.username:
+            print "username=" + self.username
+        if self.password:
+            print "password=" + self.password
+
+    def store_password(self):
+        self.wallet.writeMap(QString(self.token),
+            {QString("username") : QString(self.username),
+             QString("password") : QString(self.password)})
+
+    def open_wallet(self):
+        self.wallet = KWallet.Wallet.openWallet(
+            KWallet.Wallet.LocalWallet(), 0, KWallet.Wallet.Synchronous)
+        if not self.wallet.isOpen():
+            return None
+        if not self.wallet.hasFolder("GitCredentials"):
+            self.wallet.createFolder("GitCredentials")
+        self.wallet.setFolder("GitCredentials")
+
+    def check_wallet(self):
+        (res, data) = self.wallet.readMap(self.token)
+        if res != 0:
+            return None
+        try:
+            self.username = data[QString("username")]
+            self.password = data[QString("password")]
+        except KeyError:
+            return None
+        return self.username and self.password
+
+    def ask_password_dialog(self):
+        dlg = KPasswordDialog(None,
+            KPasswordDialog.KPasswordDialogFlag(
+                KPasswordDialog.ShowKeepPassword |
+                KPasswordDialog.ShowUsernameLine))
+        if self.desc:
+            desc = self.desc
+        else:
+            desc = self.token
+        dlg.setPrompt(i18n("Please enter username and password for %s" % (desc)))
+        dlg.setUsername(self.username)
+        dlg.setKeepPassword(True)
+        if not dlg.exec_():
+            return
+        self.username = dlg.username()
+        self.password = dlg.password()
+        self.save_password = dlg.keepPassword()
+
+def main():    
+    KCmdLineArgs.init(sys.argv, aboutData)
+    
+    options = KCmdLineOptions()
+    options.add("unique <token>", ki18n("Unique token identifying the credential"))
+    options.add("description <desc>", ki18n("Human readable description of the credential"))
+    options.add("username <username>", ki18n("Requested username"))
+    options.add("reject", ki18n("Purge credential"))    
+    
+    KCmdLineArgs.addCmdLineOptions(options)
+    args = KCmdLineArgs.parsedArgs();
+
+    username = args.getOption("username")
+    token = args.getOption("unique")
+    desc = args.getOption("description")
+    reject = args.isSet("reject")
+
+    app = CredentialHelper(token, username, desc, reject)
+
+if __name__ == "__main__":
+    main()
-- 
1.7.6.1

Re: [PATCH] Add a credential-helper for KDE

[Jeff King]

On Sat, Aug 27, 2011 at 09:54:02PM +0200, Lukas Sandström wrote:

> This Python script plugs into the credentials API
> of Git to ask the user for passwords with a nice
> KDE password dialog.

Thanks for working on this.

>  .../git-kde-credentials-helper.py                  |  122 ++++++++++++++++++++

Can we call it git-credential-kdewallet or similar? Then users can just
do:

  git config credential.helper kdewallet

(where "kdewallet" can be whatever you think is most appropriate; the
key is naming it git-credential-*).

>  1 files changed, 122 insertions(+), 0 deletions(-)
>  create mode 100755 contrib/kde-credetials-helper/git-kde-credentials-helper.py

Minor typo in directory name.

> +    def check_wallet(self):
> +        (res, data) = self.wallet.readMap(self.token)
> +        if res != 0:
> +            return None
> +        try:
> +            self.username = data[QString("username")]
> +            self.password = data[QString("password")]
> +        except KeyError:
> +            return None
> +        return self.username and self.password

If I am reading this correctly, you look up based purely on the context
token. Which means that if I do something like this:

  $ git push https://host.com/repo.git
  [enter username: user1, password: foo]
  $ git push https://user2@host.com/other-repo.git

We will invoke the helper as:

  git credential-kdewallet --unique=https:host.com --username=user2

but the helper will ignore the "user2" bit, and return "user1 / foo".

The "cache" helper I wrote handles this situation better, by indexing
both on the token and the username. I wonder if the username should
become part of the token. Or if the token should really just become a
canonicalized URL, minus the actual path. So the first one would get:

  --unique=https://host.com

and the second would get:

  --unique=https://user2@host.com

Then helpers wouldn't need to worry about doing anything special.

What do you think? Also, any comments in general on writing a helper?
You are the first one besides me to do so. Did you find anything in the
interface or the documentation confusing? Suggestions are very welcome,
as nothing has been released yet and we're free to tweak as much as we
want.

-Peff

[PATCH v2] Add a credential-helper for KDE

[Lukas Sandström]

This Python script plugs into the credentials API
of Git to ask the user for passwords with a nice
KDE password dialog.

The password is saved in the KWallet.

Signed-off-by: Lukas Sandström <luksan@gmail.com>
---

On 2011-08-31 03:42, Jeff King wrote:
> Can we call it git-credential-kdewallet or similar? Then users can just
> do:
> 
>   git config credential.helper kdewallet
> 
> (where "kdewallet" can be whatever you think is most appropriate; the
> key is naming it git-credential-*).

Done.

[...]

> If I am reading this correctly, you look up based purely on the context
> token. Which means that if I do something like this:
> 
>   $ git push https://host.com/repo.git
>   [enter username: user1, password: foo]
>   $ git push https://user2@host.com/other-repo.git
> 
> We will invoke the helper as:
> 
>   git credential-kdewallet --unique=https:host.com --username=user2
> 
> but the helper will ignore the "user2" bit, and return "user1 / foo".
> 
> The "cache" helper I wrote handles this situation better, by indexing
> both on the token and the username. I wonder if the username should
> become part of the token. Or if the token should really just become a
> canonicalized URL, minus the actual path. So the first one would get:
> 
>   --unique=https://host.com
> 
> and the second would get:
> 
>   --unique=https://user2@host.com
> 
> Then helpers wouldn't need to worry about doing anything special.
> 
> What do you think? Also, any comments in general on writing a helper?
> You are the first one besides me to do so. Did you find anything in the
> interface or the documentation confusing? Suggestions are very welcome,
> as nothing has been released yet and we're free to tweak as much as we
> want.
> 
> -Peff

Right. Multiple usernames per "unique" context is supported in this version.
I looked at the git-credential-storage helper when I wrote the first patch,
which didn't have obvious support for multiple usernames per unique context.

Keeping the username outside the token is probably a good thing, but perhaps it
should be clarified in the api-docs that multiple usernames has to be supported.

Also; what about rejecting credentials. This code currently deletes just a 
username/password pair if a username is specified, and all credentials associated
with the token if only --unique and --reject is specified. Is this correct/expected
behavior?

When I first wrote the helper I tried to immediately ask for a new password if a
credential was rejected, but this didn't work with the HTTP auth code, since it
doesn't retry the auth with the new credentials after a reject. I think it would
be better if we asked for a new password instead of just saying "auth failed" and 
having the user retry the fetch/pull when the stored credentials are incorrect.

/Lkas

 .../git-credential-kdewallet.py                    |  137 ++++++++++++++++++++
 1 files changed, 137 insertions(+), 0 deletions(-)
 create mode 100755 contrib/git-credential-kdewallet/git-credential-kdewallet.py

diff --git a/contrib/git-credential-kdewallet/git-credential-kdewallet.py b/contrib/git-credential-kdewallet/git-credential-kdewallet.py
new file mode 100755
index 0000000..29c4ae1
--- /dev/null
+++ b/contrib/git-credential-kdewallet/git-credential-kdewallet.py
@@ -0,0 +1,137 @@
+#!/usr/bin/env python
+# encoding=utf-8
+#
+# Copyright 2011, Lukas Sandström
+#
+# Licensed under the GPL version 2.
+
+import sys
+from PyQt4.QtCore import QString
+from PyKDE4.kdecore import i18n, ki18n, KAboutData, KCmdLineArgs, KCmdLineOptions
+from PyKDE4.kdeui import KApplication, KWallet, KPasswordDialog
+
+appName     = "git-credential-kdewallet"
+catalog     = ""
+programName = ki18n ("Git KDE credentials helper")
+version     = "0.1"
+description = ki18n ("Credentials storage helper for Git")
+license     = KAboutData.License_GPL_V2
+copyright   = ki18n ("(c) 2011 Lukas Sandström")
+text        = ki18n ("none")
+homePage    = "http://www.git-scm.com"
+bugEmail    = "luksan@gmail.com"
+
+aboutData   = KAboutData (appName, catalog, programName, version, description,
+                          license, copyright, text, homePage, bugEmail)
+
+class CredentialHelper(KApplication):
+    def __init__(self, token, username = None, desc = None, reject = False):
+        super(CredentialHelper, self).__init__()
+        self.password = None
+        self.username = username
+        self.save_password = False
+        self.token = token
+        self.desc = desc
+
+        if not self.token:
+            return
+
+        self.open_wallet()
+
+        if reject:
+            self.reject_credential()
+            return
+
+        if not self.check_wallet():
+            self.ask_password_dialog()
+
+        if self.save_password:
+            self.store_password()
+
+        self.output_credentials()
+
+    def output_credentials(self):
+        if self.username:
+            print "username=" + self.username
+        if self.password:
+            print "password=" + self.password
+
+    def reject_credential(self):
+        (res, data) = self.wallet.readMap(self.token)
+        if self.username:
+            try:
+                del data[QString(self.username)]
+            except KeyError:
+                pass
+            self.wallet.writeMap(self.token, data)
+        else:
+            self.wallet.removeEntry(self.token)
+
+    def store_password(self):
+        (res, data) = self.wallet.readMap(self.token)
+        data[QString(self.username)] = QString(self.password)
+        self.wallet.writeMap(QString(self.token), data)
+
+    def open_wallet(self):
+        self.wallet = KWallet.Wallet.openWallet(
+            KWallet.Wallet.LocalWallet(), 0, KWallet.Wallet.Synchronous)
+        if not self.wallet.isOpen():
+            return None
+        if not self.wallet.hasFolder("GitCredentials"):
+            self.wallet.createFolder("GitCredentials")
+        self.wallet.setFolder("GitCredentials")
+
+    def check_wallet(self):
+        (res, data) = self.wallet.readMap(self.token)
+        if res != 0:
+            return None
+        for u, p in data.iteritems():
+            # Pick the first complete credential if no username is specified
+            if not self.username and u and p:
+                self.username = u
+                self.password = p
+                return True
+            if self.username == u:
+                self.password = p
+                return True
+        return None
+
+    def ask_password_dialog(self):
+        dlg = KPasswordDialog(None,
+            KPasswordDialog.KPasswordDialogFlag(
+                KPasswordDialog.ShowKeepPassword |
+                KPasswordDialog.ShowUsernameLine))
+        if self.desc:
+            desc = self.desc
+        else:
+            desc = self.token
+        dlg.setPrompt(i18n("Please enter username and password for %s" % (desc)))
+        dlg.setUsername(self.username)
+        dlg.setKeepPassword(True)
+        if not dlg.exec_():
+            return
+        self.username = dlg.username()
+        self.password = dlg.password()
+        self.save_password = dlg.keepPassword()
+
+def main():
+    KCmdLineArgs.init(sys.argv, aboutData)
+
+    options = KCmdLineOptions()
+    options.add("unique <token>", ki18n("Unique token identifying the credential"))
+    options.add("description <desc>", ki18n("Human readable description of the credential"))
+    options.add("username <username>", ki18n("Requested username"))
+    options.add("reject", ki18n("Purge credential"))
+
+    KCmdLineArgs.addCmdLineOptions(options)
+    args = KCmdLineArgs.parsedArgs();
+
+    username = args.getOption("username")
+    token = args.getOption("unique")
+    desc = args.getOption("description")
+    reject = args.isSet("reject")
+
+    app = CredentialHelper(token, username, desc, reject)
+
+if __name__ == "__main__":
+    main()
-- 
1.7.6.1

Re: [PATCH v2] Add a credential-helper for KDE

[Jeff King]

On Sun, Sep 18, 2011 at 04:52:58PM +0200, Lukas Sandström wrote:

> Right. Multiple usernames per "unique" context is supported in this version.
> I looked at the git-credential-storage helper when I wrote the first patch,
> which didn't have obvious support for multiple usernames per unique context.

Yeah, sorry about that. The -cache helper is much more fully fleshed out
(though I have improved the -store helper in the past few days to handle
multiple usernames better).

> Keeping the username outside the token is probably a good thing, but perhaps it
> should be clarified in the api-docs that multiple usernames has to be supported.

OK, I'll try to write up a clarification.

> Also; what about rejecting credentials. This code currently deletes just a 
> username/password pair if a username is specified, and all credentials associated
> with the token if only --unique and --reject is specified. Is this correct/expected
> behavior?

Yes, that's what I think should happen, and what both of my helpers do.
In practice, I don't think it will be called that way by git, which
will always be rejecting a username we just tried. But I wanted to leave
things flexible in case a user wants to manually remove a credential
from a store.

> When I first wrote the helper I tried to immediately ask for a new password if a
> credential was rejected, but this didn't work with the HTTP auth code, since it
> doesn't retry the auth with the new credentials after a reject. I think it would
> be better if we asked for a new password instead of just saying "auth failed" and 
> having the user retry the fetch/pull when the stored credentials are incorrect.

Yeah, I had a patch early on to retry authentication a few times before
exiting, but I wondered how helpful it was. It's usually pretty easy to
retry your command again via shell history, and sometimes looping on
asking for authentication can be annoying (because things like askpass
will actually grab the keyboard focus).

So I dunno what is best. I don't consider it a big deal, but maybe
others do.

Even if we did do the retry from git, the helper shouldn't ask
immediately for the new credential inside a --reject. It should wait to
be invoked again asking for the password. I know this may be an extra
fork/exec/startup cycle, but it keeps the interface to the helper simple
and flexible.

-Peff

Re: [PATCH v2] Add a credential-helper for KDE

[Jeff King]

On Sun, Sep 18, 2011 at 04:52:58PM +0200, Lukas Sandström wrote:

> This Python script plugs into the credentials API
> of Git to ask the user for passwords with a nice
> KDE password dialog.
> 
> The password is saved in the KWallet.

So I managed to play with this a bit tonight. Overall, it seems pretty
nice.

Initially, it seemed somewhat clumsy. It asked me to open the wallet
(using a password) each time git ran. Which is about as annoying as just
typing my git password each time. :)

The magic trick was to configure kwallet to "keep the wallet open for 10
minutes after the last use" instead of "close when no applications have
the wallet open". Since git runs as many small programs, kwallet has no
real idea of how long a git session is.

This is totally not a kwallet thing, and nothing to do with your helper.
But since the helper is so annoyingly useless without that config, it
might be worth mentioning it in a README.

> Right. Multiple usernames per "unique" context is supported in this version.
> I looked at the git-credential-storage helper when I wrote the first patch,
> which didn't have obvious support for multiple usernames per unique context.

This part passed my tests just fine. Very nice.

> +class CredentialHelper(KApplication):
> +    def __init__(self, token, username = None, desc = None, reject = False):
> +        super(CredentialHelper, self).__init__()
> +        self.password = None
> +        self.username = username
> +        self.save_password = False
> +        self.token = token
> +        self.desc = desc
> +
> +        if not self.token:
> +            return

My tests complained about doing nothing when there is no token. As I've
mentioned elsewhere, this doesn't matter now (as git never invokes the
helper that way), but it would be nice to future-proof the helper by
just ignoring the wallet, but still doing the nice password dialog.

> +    def open_wallet(self):
> +        self.wallet = KWallet.Wallet.openWallet(
> +            KWallet.Wallet.LocalWallet(), 0, KWallet.Wallet.Synchronous)
> +        if not self.wallet.isOpen():
> +            return None
> +        if not self.wallet.hasFolder("GitCredentials"):
> +            self.wallet.createFolder("GitCredentials")
> +        self.wallet.setFolder("GitCredentials")

I peeked around the KWallet manager. There's a "passwords" folder in the
wallet, and I was surprised that the passwords didn't go there. But when
I tried using konqueror to store a password, I found that it also made
its own folder, and then stored a map within it for each URL.

So I'm not really sure if you're following kwallet best practices or
not, as I'm clearly confused about what the "passwords" folder is for.
;)

> +    def check_wallet(self):
> +        (res, data) = self.wallet.readMap(self.token)

So you're just using the token as a big blob. Which is how I
anticipated, but is the complete opposite of what OS X Keychain wants.
Which is leading me to think we should really just hand helpers both
forms: the information broken down by item (e.g., --host=github.com),
and a full URL (e.g., --url=https://github.com/). And then the helpers
can use whatever they like (where you would use "url" instead of the
current "unique").

-Peff