You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@subversion.apache.org by Alexander Thomas <al...@collab.net> on 2008/09/25 05:40:58 UTC
[PATCH] Issue 2489: requests keyring password and unlocks the
keyring.
[[[
If the default keyring is already locked, prompt for keyring
password and unlock the keyring.
* subversion/libsvn_auth_gnome_keyring/gnome_keyring.c
(): Include svn_pools.h
(gnome_keyring_unlock_keyring): New static function to unlock keyring.
Patch by: Alexander Thomas <al...@collab.net>
]]]
-Alexander Thomas(AT)
RE: [PATCH] Issue 2489: requests keyring password and unlocks the
keyring.
Posted by Alexander Thomas <al...@collab.net>.
Thanks Bert
On Thu, 2008-09-25 at 09:26 +0200, Bert Huijben wrote:
> Hi
>
> > -----Original Message-----
> > From: Alexander Thomas [mailto:alexander@collab.net]
> > Sent: donderdag 25 september 2008 7:41
> > To: Subversion-Dev
> > Subject: [PATCH] Issue 2489: requests keyring password and unlocks the
> > keyring.
> >
> > [[[
> > If the default keyring is already locked, prompt for keyring
> > password and unlock the keyring.
> >
> > * subversion/libsvn_auth_gnome_keyring/gnome_keyring.c
> > (): Include svn_pools.h
> > (gnome_keyring_unlock_keyring): New static function to unlock
> > keyring.
> >
> > Patch by: Alexander Thomas <al...@collab.net>
> > ]]]
> >
> >
> > -Alexander Thomas(AT)
>
> > Index: subversion/libsvn_auth_gnome_keyring/gnome_keyring.c
> > ===================================================================
> > --- subversion/libsvn_auth_gnome_keyring/gnome_keyring.c (revision 33273)
> > +++ subversion/libsvn_auth_gnome_keyring/gnome_keyring.c (working copy)
> > @@ -26,6 +26,7 @@
> > #include "svn_auth.h"
> > #include "svn_auth_dso.h"
> > #include "svn_error.h"
> > +#include "svn_pools.h"
> >
> > #include "private/svn_auth_private.h"
> >
> > @@ -40,6 +41,48 @@
> > /* GNOME Keyring simple provider, puts passwords in GNOME Keyring */
> > /*-----------------------------------------------------------------------*/
> >
> > +/* If the default keyring is already locked, prompts for keyring
> > + * password and unlocks the keyring. Returns default keyring
> > + * name if successfully unlocked, else NULL. */
> > +static const char *
> > +gnome_keyring_unlock_keyring(apr_pool_t *pool)
> > +{
> > + GnomeKeyringResult result;
> > + char *def = NULL;
> > + char *def_dup = NULL;
> > + GnomeKeyringInfo *keyring_info=NULL;
> > +
> > + /* Finds default keyring. */
> > + result = gnome_keyring_get_default_keyring_sync(&def);
> > + if (result != GNOME_KEYRING_RESULT_OK || def == NULL)
> > + return NULL;
> > +
> > + /* Get details about the default keyring. */
> > + gnome_keyring_get_info_sync(def, &keyring_info);
> > +
> > + /* Check if default keyring is locked. */
> > + if (keyring_info && gnome_keyring_info_get_is_locked(keyring_info))
> > + {
> > + char *passwd;
> > + char *prompt_text;
> > + svn_auth_cred_simple_t *cred;
> > + apr_pool_t *subpool = svn_pool_create(pool);
> > +
> > + prompt_text = apr_psprintf(subpool, "Keyring [%s]", def);
> > + svn_cmdline_auth_simple_prompt(&cred, NULL, NULL, prompt_text,
> > + TRUE, subpool);
>
> This code assumes the keyring support always runs on the cmdline, but the code is in a library that is also used by gui clients. (e.g. Eclipse)
>
> The gnome keyring support should define a prompt handler like the other authentication handlers. This allows the client to provide the prompt: dialog, cmdline, etc.
> This prompt handler can also handle --non-interactive support for you, if that isn't handled higher up. (I didn't check if keyring support is enabled at all in --non-interactive mode)
>
I understood what you are saying. My understanding on the auth module is
very limited, Still I will try to address your concerns with my next
patch. Can you give me some clues how to implement this?
> > + gnome_keyring_unlock_sync(def, cred->password);
> > + svn_pool_destroy(subpool);
> > + }
> > +
> > + /* Cleanup. */
> > + gnome_keyring_info_free(keyring_info);
> > + def_dup = apr_pstrdup(pool, def);
> > + if (def)
> > + free(def);
> > + return def_dup;
> > +}
> > +
> > /* Implementation of password_get_t that retrieves the password
> > from GNOME Keyring. */
> > static svn_boolean_t
> > @@ -65,6 +108,9 @@
> > return FALSE;
> > }
> >
> > + if (!gnome_keyring_unlock_keyring(pool))
> > + return FALSE;
> > +
> > GnomeKeyringResult result;
> > GList *items;
> > svn_boolean_t ret = FALSE;
> > @@ -117,6 +163,9 @@
> > return FALSE;
> > }
> >
> > + if (!gnome_keyring_unlock_keyring(pool))
> > + return FALSE;
> > +
> > GnomeKeyringResult result;
> > guint32 item_id;
> >
>
> These last two changes are not documented in your log message.
>
Sorry, forgot to add in log message.
-AT
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
RE: [PATCH] Issue 2489: requests keyring password and unlocks the keyring.
Posted by Bert Huijben <b....@competence.biz>.
Hi
> -----Original Message-----
> From: Alexander Thomas [mailto:alexander@collab.net]
> Sent: donderdag 25 september 2008 7:41
> To: Subversion-Dev
> Subject: [PATCH] Issue 2489: requests keyring password and unlocks the
> keyring.
>
> [[[
> If the default keyring is already locked, prompt for keyring
> password and unlock the keyring.
>
> * subversion/libsvn_auth_gnome_keyring/gnome_keyring.c
> (): Include svn_pools.h
> (gnome_keyring_unlock_keyring): New static function to unlock
> keyring.
>
> Patch by: Alexander Thomas <al...@collab.net>
> ]]]
>
>
> -Alexander Thomas(AT)
> Index: subversion/libsvn_auth_gnome_keyring/gnome_keyring.c
> ===================================================================
> --- subversion/libsvn_auth_gnome_keyring/gnome_keyring.c (revision 33273)
> +++ subversion/libsvn_auth_gnome_keyring/gnome_keyring.c (working copy)
> @@ -26,6 +26,7 @@
> #include "svn_auth.h"
> #include "svn_auth_dso.h"
> #include "svn_error.h"
> +#include "svn_pools.h"
>
> #include "private/svn_auth_private.h"
>
> @@ -40,6 +41,48 @@
> /* GNOME Keyring simple provider, puts passwords in GNOME Keyring */
> /*-----------------------------------------------------------------------*/
>
> +/* If the default keyring is already locked, prompts for keyring
> + * password and unlocks the keyring. Returns default keyring
> + * name if successfully unlocked, else NULL. */
> +static const char *
> +gnome_keyring_unlock_keyring(apr_pool_t *pool)
> +{
> + GnomeKeyringResult result;
> + char *def = NULL;
> + char *def_dup = NULL;
> + GnomeKeyringInfo *keyring_info=NULL;
> +
> + /* Finds default keyring. */
> + result = gnome_keyring_get_default_keyring_sync(&def);
> + if (result != GNOME_KEYRING_RESULT_OK || def == NULL)
> + return NULL;
> +
> + /* Get details about the default keyring. */
> + gnome_keyring_get_info_sync(def, &keyring_info);
> +
> + /* Check if default keyring is locked. */
> + if (keyring_info && gnome_keyring_info_get_is_locked(keyring_info))
> + {
> + char *passwd;
> + char *prompt_text;
> + svn_auth_cred_simple_t *cred;
> + apr_pool_t *subpool = svn_pool_create(pool);
> +
> + prompt_text = apr_psprintf(subpool, "Keyring [%s]", def);
> + svn_cmdline_auth_simple_prompt(&cred, NULL, NULL, prompt_text,
> + TRUE, subpool);
This code assumes the keyring support always runs on the cmdline, but the code is in a library that is also used by gui clients. (e.g. Eclipse)
The gnome keyring support should define a prompt handler like the other authentication handlers. This allows the client to provide the prompt: dialog, cmdline, etc.
This prompt handler can also handle --non-interactive support for you, if that isn't handled higher up. (I didn't check if keyring support is enabled at all in --non-interactive mode)
> + gnome_keyring_unlock_sync(def, cred->password);
> + svn_pool_destroy(subpool);
> + }
> +
> + /* Cleanup. */
> + gnome_keyring_info_free(keyring_info);
> + def_dup = apr_pstrdup(pool, def);
> + if (def)
> + free(def);
> + return def_dup;
> +}
> +
> /* Implementation of password_get_t that retrieves the password
> from GNOME Keyring. */
> static svn_boolean_t
> @@ -65,6 +108,9 @@
> return FALSE;
> }
>
> + if (!gnome_keyring_unlock_keyring(pool))
> + return FALSE;
> +
> GnomeKeyringResult result;
> GList *items;
> svn_boolean_t ret = FALSE;
> @@ -117,6 +163,9 @@
> return FALSE;
> }
>
> + if (!gnome_keyring_unlock_keyring(pool))
> + return FALSE;
> +
> GnomeKeyringResult result;
> guint32 item_id;
>
These last two changes are not documented in your log message.
Bert
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: [PATCH] Issue 2489: requests keyring password and unlocks the keyring.
Posted by Mark Phippard <ma...@gmail.com>.
On Thu, Sep 25, 2008 at 3:26 AM, Bert Huijben <b....@competence.biz> wrote:
> This code assumes the keyring support always runs on the cmdline, but the code is in a library
> that is also used by gui clients. (e.g. Eclipse)
In the current code, if you are running a GUI client, then GNOME
itself prompts you to unlock the keyring. So it is possible this
would not be an issue? That said, I imagine someone could be writing
a non-GUI tool using JavaHL or the other bindings where they would
need this to surface as a callback. So I do agree it is necessary to
go through the callbacks that exist.
--
Thanks
Mark Phippard
http://markphip.blogspot.com/
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org