On Dec 3, 2005, at 7:11 AM, mark benedetto king wrote:
> On Fri, Dec 02, 2005 at 03:03:54PM -0800, Wilfredo S?nchez Vega wrote:
>> If there is code somewhere to split up the realmstring passed to
>> the auth functions (we'd need to pull out protocol, host(+port), and
>> realm name), then we can use internet passwords instead of generic
>> passwords, which would allow us to use the same credentials as Safari
>> (and other clients) is using and vice versa.
>
> We don't (AFAIK) have a realm-string parsing API.
>
> I looked at the Internet API before settling on Generic passwords;
> it wasn't clear to me that we could use "svn" as a protocol (it's
> not one of the enumerated SecProtocolType values). Maybe you can
> shed some light on that.
I don't know the KeyChain API all that well (not particularly fond
of it so far), so I just assumed that we could use any protocol we
like. If that's not the case, then we may be SOL, but I'll also file
a bug on that at Apple.
> Rework the simple auth provider internals to allow for more control
> over
> the storage of the password component of the credentials (in
> preparation
> for adding Mac OS X Keychain support).
>
> So my implementation added a third simple_provider ("darwin") that
> used the same helper system to drive the config-file storage of
> the
> username. This enabled me to call
> SecKeychainFindGenericPassword()
> only once, using the username from the config-file as the key,
> rather than two calls, one with a "username" prompt and one
> with a "password" prompt.
>
> Perhaps this wasn't the best strategy; perhaps I should have made
> a first-class provider as you have done, and stored the username
> and password as separate entries. That might be more consistent
> with other OS X application behaviour.
>
> I'm not sentimentially attached to those changes, and I'll revert
> them if they serve no purpose, but I'd like to make sure
> that we're moving in the right direction when I do. Do you have
> an opinion on this?
Well, I'm a bit wary of trying to overload the simple provider too
much, I used a first-class provider to avoid polluting the simple
provider. I have no objection to leveraging the simple provider's
helper, though.
I do like the single entry in the KeyChain better than two
entries. I picked that up from Ken's patch and didn't have a better
idea. Using the config-file storage for the username is better; I
think the username some come from the subversion client and not the
KeyChain.
> 2.) Configury.
>
>
> +if test "$enable_keychain" = "yes"; then
> + AC_MSG_RESULT([yes])
> + SVN_KEYCHAIN_INCLUDES=""
> + SVN_KEYCHAIN_LIBS="-framework Security -framework CoreFoundation
> -framework CoreServices"
> + AC_DEFINE([SVN_HAVE_KEYCHAIN_SERVICES], [1], [Is Mac OS KeyChain
> support enabled?])
> + AC_SUBST(SVN_KEYCHAIN_INCLUDES)
> + AC_SUBST(SVN_KEYCHAIN_LIBS)
> +else
> + AC_MSG_RESULT([no])
> +fi
>
> Yay, configury! But won't this break the build on non-OS-X
> systems, or
> at least systems whose ld can't ignore -framework?
The -framework flag should only get used by systems with the
Security framework. Enabling KeyChain support on a non-OS X box will
break, yes, so we should make sure it's present.
> And is it sufficient?
>
> I had code like
>
> #ifdef DARWIN
> #include <AvailabilityMacros.h>
>
> #if (MAC_OS_X_VERSION_MAX_ALLOWED >= MAC_OS_X_VERSION_10_2)
> #define SVN_HAVE_KEYCHAIN_SERVICES 1
> #endif
>
> to test for the presence of the API. We could do this test at
> configure-time. But is this the right test?
I think a config-time test in 'if test "$enable_keychain" =
"yes"; ... fi' would be appropriate.
We could just test for the presence of the Security framework at
the right path (/System/Library/Frameworks/Security.framework).
A more autoconf-like test would attempt to build a test program
which uses the Security framework, but I'm not aware of a case where
the path test would fail to find it. It would be nice if autoconf
had an AC_CHECK_FRAMEWORK() peer to AC_CHECK_LIB(). Without that,
I'm not sure it's really worth the bother.
I guess I don't know whether the KeyChain API we're using is in
all versions of the Security framework, but I think it has been, so I
don't think that we need a OS version test as well.
> 3.) Support for --non-interactive.
>
> Is surrounding the SecKeychain calls with calls to
> SecKeychainSetUserInteractionAllowed(FALSE) and then
> SecKeychainSetUserInteractionAllowed(TRUE) the right way to
> implement
> --non-interactive? Could it be that easy?
That sounds right, but again I'm also new to the KeyChain API.
-wsv
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Received on Sat Dec 3 20:01:13 2005