You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@subversion.apache.org by Senthil Kumaran S <se...@collab.net> on 2008/04/01 08:07:32 UTC

Re: [PATCH] Fix issue #2324

Hi Dave,

David Glasser wrote:
> 2008/3/25 Senthil Kumaran S <se...@collab.net>:
>>
>>  I am attaching a patch along with this email which will fix issue #2324. This
>>  adds a new parameter 'username' to the subversion config file.
> 
> I think this would go better in ~/.subversion/servers.

Can the username parameter be something like the following in 
~/.subversion/servers?

<snip>
[groups]
group1 = *.collab.net
othergroup = repository.blarggitywhoomph.com

### Information for the first group:
[group1]
http-proxy-host = proxy1.some-domain-name.com
http-proxy-port = 80
http-proxy-username = blah
http-proxy-password = doubleblah
http-timeout = 60
http-auth-types = basic;digest;negotiate
neon-debug-mask = 130
username = stylesen

### Information for the second group:
[othergroup]
http-proxy-host = proxy2.some-domain-name.com
http-proxy-port = 9000
username = harry

[global]
http-proxy-exceptions = *.exception.com, www.internal-site.org
http-proxy-host = defaultproxy.whatever.com
http-proxy-port = 7000
http-proxy-username = defaultusername
http-proxy-password = defaultpassword
username = sally
</snip>

-- 
Senthil Kumaran S
http://www.stylesen.org/

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org

Re: [PATCH] Fix issue #2324

Posted by Senthil Kumaran S <se...@collab.net>.
-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

C. Michael Pilato wrote:
> I've not tested the patch *extensively*, but simple testing shows stuff
> working as expected.  +1 to commit.

Committed in r32403.

Thank You.
- --
Senthil Kumaran S
http://www.stylesen.org/
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.6 (GNU/Linux)

iD8DBQFIm+vY9o1G+2zNQDgRAvHgAJwO9YDKtqd2IAjjYflTaE2i7cedhACcCNMD
6qRUF9xGt1EVW2Eq1q9nIjU=
=Wh1d
-----END PGP SIGNATURE-----

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org

Re: [PATCH] Fix issue #2324

Posted by "C. Michael Pilato" <cm...@collab.net>.
I've not tested the patch *extensively*, but simple testing shows stuff 
working as expected.  +1 to commit.


Senthil Kumaran S wrote:
> Hi Mike,
> 
> Thanks for your review, my comments inline.
> 
> C. Michael Pilato wrote:
>> I have a question, though -- should the 'servers' file "win" over the
>> auth cache in terms of priority?  Seems to me that the auth cache --
>> which typically is populated with known-good credentials, should be
>> given precedence over the 'servers' file.  That would allow me to put
>> "cmpilato" in the [global] section of my servers file and yet not have
>> that contradict creds in the auth cache which use a different username.
> 
>> In fact, I guess I kinda expected to see code something like this:
> 
>>    creds = check-auth-cache
>>    if (! creds)
>>       creds = check-servers-file
>>    if (! creds)
>>       creds = apr_getusername()
> 
>> With such an ordering, the servers file effectively becomes just a way
>> to override the username assumption.
> 
> This sounds like a good ordering to me. I ve updated the patch accordingly and
> attached along with this email.
> 
>> But maybe this discussion was already played out previously and I missed
>> it?
> 
> No we didnt had a discussion about this. We had discussions whether to have
> username option in config file or the servers file.
> 
> [[[
> Fix issue #2324:  Disable username assumption.
> 
> Rather than disabling the username assumption, this takes the
> different approach of consulting the runtime configuration for a
> default username before falling back to other methods (which include
> using the current user id's name).
> 
> The order in which default username is determined is as follows:
>         1. username supplied in command line
>         2. username in the auth cache
>         3. username given in servers file
>         4. current user id's name
> 
> * subversion/include/svn_config.h
>   (SVN_CONFIG_OPTION_USERNAME): New config option.
> 
> * subversion/libsvn_subr/config_file.c
>   (svn_config_ensure): Add text about the new config file parameter 'username'.
> 
> * subversion/libsvn_subr/simple_providers.c
>   (simple_first_creds_helper, prompt_for_simple_creds): Try to find a
>    username in the 'servers' file.
> 
> Patch by: stylesen
> Suggested by: glasser
>               cmpilato
> Review by: cmpilato
> ]]]
> 

-- 
C. Michael Pilato <cm...@collab.net>
CollabNet   <>   www.collab.net   <>   Distributed Development On Demand


Re: [PATCH] Fix issue #2324

Posted by Senthil Kumaran S <se...@collab.net>.
-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

Hi Mike,

Thanks for your review, my comments inline.

C. Michael Pilato wrote:
> I have a question, though -- should the 'servers' file "win" over the
> auth cache in terms of priority?  Seems to me that the auth cache --
> which typically is populated with known-good credentials, should be
> given precedence over the 'servers' file.  That would allow me to put
> "cmpilato" in the [global] section of my servers file and yet not have
> that contradict creds in the auth cache which use a different username.
> 
> In fact, I guess I kinda expected to see code something like this:
> 
>    creds = check-auth-cache
>    if (! creds)
>       creds = check-servers-file
>    if (! creds)
>       creds = apr_getusername()
> 
> With such an ordering, the servers file effectively becomes just a way
> to override the username assumption.

This sounds like a good ordering to me. I ve updated the patch accordingly and
attached along with this email.

> But maybe this discussion was already played out previously and I missed
> it?

No we didnt had a discussion about this. We had discussions whether to have
username option in config file or the servers file.

[[[
Fix issue #2324:  Disable username assumption.

Rather than disabling the username assumption, this takes the
different approach of consulting the runtime configuration for a
default username before falling back to other methods (which include
using the current user id's name).

The order in which default username is determined is as follows:
        1. username supplied in command line
        2. username in the auth cache
        3. username given in servers file
        4. current user id's name

* subversion/include/svn_config.h
  (SVN_CONFIG_OPTION_USERNAME): New config option.

* subversion/libsvn_subr/config_file.c
  (svn_config_ensure): Add text about the new config file parameter 'username'.

* subversion/libsvn_subr/simple_providers.c
  (simple_first_creds_helper, prompt_for_simple_creds): Try to find a
   username in the 'servers' file.

Patch by: stylesen
Suggested by: glasser
              cmpilato
Review by: cmpilato
]]]

- --
Senthil Kumaran S
http://www.stylesen.org/
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.6 (GNU/Linux)

iD8DBQFImr8O9o1G+2zNQDgRAvYhAKCXVsetTWAmJtDDt5L7wN7EptkEKACgl7jL
4QNSHO/DPjwEszlvd1j5srA=
=Bzcw
-----END PGP SIGNATURE-----

Re: [PATCH] Fix issue #2324

Posted by "C. Michael Pilato" <cm...@collab.net>.
Senthil, I've made some minor changes to your patch:

    * Adding a trailing period, and losing an extra blank line from the
      default 'servers' file content.
    * Tweaking comments in the code so that neighboring comments make sense.

I have a question, though -- should the 'servers' file "win" over the auth 
cache in terms of priority?  Seems to me that the auth cache -- which 
typically is populated with known-good credentials, should be given 
precedence over the 'servers' file.  That would allow me to put "cmpilato" 
in the [global] section of my servers file and yet not have that contradict 
creds in the auth cache which use a different username.

In fact, I guess I kinda expected to see code something like this:

    creds = check-auth-cache
    if (! creds)
       creds = check-servers-file
    if (! creds)
       creds = apr_getusername()

With such an ordering, the servers file effectively becomes just a way to 
override the username assumption.

But maybe this discussion was already played out previously and I missed it?

-- 
C. Michael Pilato <cm...@collab.net>
CollabNet   <>   www.collab.net   <>   Distributed Development On Demand

Re: [PATCH] Fix issue #2324

Posted by Senthil Kumaran S <se...@collab.net>.
-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

Hi,

Senthil Kumaran S wrote:
> I am attaching an updated patch which applies cleanly on r32089 of trunk.

I am attaching a new updated patch which applies cleanly on r32366 of trunk.

[[[
Fix issue #2324.

Disable username assumption.

* subversion/include/svn_config.h
  (SVN_CONFIG_OPTION_USERNAME): New config option.

* subversion/libsvn_subr/config_file.c
  (svn_config_ensure): Add text about the new config file parameter 'username'.

* subversion/libsvn_subr/simple_providers.c
  (simple_first_creds_helper): Fetch username from servers file if it is
   defined.
  (prompt_for_simple_creds): If we didnt get a password in
   simple_first_creds_helper then we need to get the username again here.

Patch by: stylesen
]]]

Thank You.
- --
Senthil Kumaran S
http://www.stylesen.org/
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.6 (GNU/Linux)

iD8DBQFIl/St9o1G+2zNQDgRAqaMAKCKswqALIWMqXz9F4T9S0un9Og+cQCeIooQ
ktEDFZlfuqRlNgk0fuf4ZLg=
=u/13
-----END PGP SIGNATURE-----

Re: [PATCH] Fix issue #2324

Posted by Senthil Kumaran S <se...@collab.net>.
-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

Senthil Kumaran S wrote:
> I am attaching an updated patch along with this email as per Dave's
> suggestion for adding the username in the servers file.

I am attaching an updated patch which applies cleanly on r32089 of trunk.

[[[
Fix issue #2324.

Disable username assumption.

* subversion/include/svn_config.h
   (): Add SVN_CONFIG_OPTION_USERNAME.

* subversion/libsvn_subr/config_file.c
   (svn_config_ensure): Add text about the new config file parameter 'username'.

* subversion/libsvn_subr/simple_providers.c
   (simple_first_creds_helper): Fetch username from servers file if it is
    defined.
   (prompt_for_simple_creds): If we didnt get a password in
    simple_first_creds_helper then we need to get the username again here.

Patch by: stylesen
]]]

Thank You.
- --
Senthil Kumaran S
http://www.stylesen.org/
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.6 (GNU/Linux)

iD8DBQFIe0Hq9o1G+2zNQDgRAoMLAJ90xC/kK+D3Q4I0vh5fci2qBLJ0ewCgvO82
VcdJpKGQlL3Kdj+m/z2VQXU=
=qzcB
-----END PGP SIGNATURE-----

Re: [PATCH] Fix issue #2324

Posted by Senthil Kumaran S <se...@collab.net>.
Hi,

David Glasser wrote:
>>  <snip>
>>  [groups]
>>  group1 = *.collab.net
>>  othergroup = repository.blarggitywhoomph.com
>>
>>  ### Information for the first group:
>>  [group1]
>>  http-proxy-host = proxy1.some-domain-name.com
>>  http-proxy-port = 80
>>  http-proxy-username = blah
>>  http-proxy-password = doubleblah
>>  http-timeout = 60
>>  http-auth-types = basic;digest;negotiate
>>  neon-debug-mask = 130
>>  username = stylesen
>>
>>  ### Information for the second group:
>>  [othergroup]
>>  http-proxy-host = proxy2.some-domain-name.com
>>  http-proxy-port = 9000
>>  username = harry
>>
>>  [global]
>>  http-proxy-exceptions = *.exception.com, www.internal-site.org
>>  http-proxy-host = defaultproxy.whatever.com
>>  http-proxy-port = 7000
>>  http-proxy-username = defaultusername
>>  http-proxy-password = defaultpassword
>>  username = sally
>>  </snip>
> 
> Yup, that's the sort of thing I'm thinking about.

I am attaching an updated patch along with this email as per Dave's suggestion 
for adding the username in the servers file.

[[[
Fix issue #2324.

Disable username assumption.

* subversion/include/svn_config.h
   (): Add SVN_CONFIG_OPTION_USERNAME.

* subversion/libsvn_subr/config_file.c
   (svn_config_ensure): Add text about the new config file parameter 'username'.

* subversion/libsvn_subr/simple_providers.c
   (simple_first_creds_helper): Fetch username from servers file if it is
    defined.
   (prompt_for_simple_creds): If we didnt get a password in
    simple_first_creds_helper then we need to get the username again here.

Patch by: stylesen
]]]

Thank You.
-- 
Senthil Kumaran S
http://www.stylesen.org/

Re: [PATCH] Fix issue #2324

Posted by David Glasser <gl...@davidglasser.net>.
On Tue, Apr 1, 2008 at 1:07 AM, Senthil Kumaran S <se...@collab.net> wrote:
> Hi Dave,
>
>
>  David Glasser wrote:
>  > 2008/3/25 Senthil Kumaran S <se...@collab.net>:
>  >>
>
> >>  I am attaching a patch along with this email which will fix issue #2324. This
>  >>  adds a new parameter 'username' to the subversion config file.
>  >
>
> > I think this would go better in ~/.subversion/servers.
>
>  Can the username parameter be something like the following in
>  ~/.subversion/servers?
>
>  <snip>
>  [groups]
>  group1 = *.collab.net
>  othergroup = repository.blarggitywhoomph.com
>
>  ### Information for the first group:
>  [group1]
>  http-proxy-host = proxy1.some-domain-name.com
>  http-proxy-port = 80
>  http-proxy-username = blah
>  http-proxy-password = doubleblah
>  http-timeout = 60
>  http-auth-types = basic;digest;negotiate
>  neon-debug-mask = 130
>  username = stylesen
>
>  ### Information for the second group:
>  [othergroup]
>  http-proxy-host = proxy2.some-domain-name.com
>  http-proxy-port = 9000
>  username = harry
>
>  [global]
>  http-proxy-exceptions = *.exception.com, www.internal-site.org
>  http-proxy-host = defaultproxy.whatever.com
>  http-proxy-port = 7000
>  http-proxy-username = defaultusername
>  http-proxy-password = defaultpassword
>  username = sally
>  </snip>

Yup, that's the sort of thing I'm thinking about.

(This makes me think that this also would be a reasonable place to put
an "always-authenticate" option, which can avoid this issue (both on
DAV and svnserve) where anonymous access is allowed but some files are
unreadable, and svn doesn't ask the user to authenticate until too
late...)

--dave


-- 
David Glasser | glasser@davidglasser.net | http://www.davidglasser.net/

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org