Greg Hudson wrote:
> There have been a lot of calls for ra_svn authentication in daemon
> mode. I have some technical issues with the Cyrus library, so I
> decided that since we're nearing 1.0, I should cut bait and make a
> simple implementation of one of the shared-secret SASL mechanisms.
>
> I have working code, but here are some notes:
>
> * I am a bit lost on how to integrate this on the client side. Our
> current auth framework is based on an HTTP model where you try all
> operations anonymously and retry them with authentication (in a
> specific realm) if the server asks for it. (Corollary: if a
> server allows anonymous commits, but you want to authenticate
> anyway to get your username recorded, you can't do that.) I
> didn't design the ra_svn protocol and implementation with the HTTP
> model in mind, so I'm a little stuck. The client integration in
> this patch is inflexible and broken; it's just enough to let me
> exercise the code.
Yeah, when I was playing around with this kind of stuff I couldn't come
up with a really good way to implement this either. There's just a huge
difference between the way authentication is done in ra_svn vs http.
> * On the server, the natural place to put the password database is
> inside the repository. But you can't, because authentication
> happens before a repository is selected. (Perhaps that's a
> mistake.) So instead, this patch has you put the password
> database wherever you want and tell svnserve where that is with
> the -p flag.
Seems as good a solution as any.
> Comments appreciated.
Just a few...
It seems like this should be returning an error, in case the file can't
be open or something...
> +static const char *lookup_password(const char *pwfile, const char *user,
> + apr_pool_t *pool)
> +{
> + FILE *fp;
> + char line[2048];
> + const char *sep;
> + int ulen = strlen(user), len;
> +
> + fp = fopen(pwfile, "r");
> + if (!fp)
> + return FALSE;
Returning FALSE as a char *? ick. And using a FILE* instead of an
apr_file_t is kind of odd, most of the rest of subversion uses the APR
interfaces for this sort of thing.
> + while (fgets(line, sizeof(line), fp) != NULL)
> + {
> + len = strlen(line);
> + if (len > 0 && line[len - 1] == '\n')
> + line[--len] = '\0';
> + sep = strchr(line, ':');
> + if (sep && sep - line == ulen && memcmp(line, user, ulen) == 0)
> + {
> + fclose(fp);
> + return apr_pstrdup(pool, sep + 1);
> + }
> + }
> + fclose(fp);
> + return FALSE;
> +}
Shouldn't there be a patch for svnserve somewhere around here? Kind of
hard to play with it without that.
Anyway, thanks a lot for taking a look at this. Even a simple
authentication system will go a long way to making ra_svn more usable in
many situations.
-garrett
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Received on Wed Oct 15 00:13:11 2003