git/list[1] front-page[2] threads[3] people[4] search[5] about
 

Re: [PATCH 2/2] http: prevent redirect from dropping credentials during reauth

From
QBQuentin Bouget <ypsah@devyard.org>
Date
Feb 5, 2024, 03:06 UTC
Message-ID
<CYWTAIFR6W5W.1GH1O6KKP0U5U@devyard.org>
In-Reply-To
<xmqq8r3zonat.fsf@gitster.g>
On Sun Feb 4, 2024 at 11:51 PM CET, Junio C Hamano wrote:
Show 12 quoted lines
> Quentin Bouget <ypsah@devyard.org> writes:
>
> > During a re-authentication (second attempt at authenticating with a
> > remote, e.g. after a failed GSSAPI attempt), git allows the remote to
> > provide credential overrides in the redirect URL and unconditionnaly
> > drops the current HTTP credentials in favors of those, even when there
> > aren't any.
> >
> > This commit makes it so HTTP credentials are only overridden when the
> > redirect URL actually contains credentials itself.
>
> "This commit makes it so" -> "Make it so"
Will change.
Show 9 quoted lines
> > +			char *username = NULL, *password = NULL;
> > +
> > +			if (http_auth.username)
> > +				username = xstrdup(http_auth.username);
> > +			if (http_auth.password)
> > +				password = xstrdup(http_auth.password);
>
> Not a huge deal, but we have xstrdup_or_null() helper function
> exactly for a use case like this.
Thanks, will change.
Show 15 quoted lines
> >  			credential_from_url(&http_auth, options->base_url->buf);
> > +
> > +			if (http_auth.username)
> > +				free(username);
> > +			else if (username)
> > +				http_auth.username = username;
> > +
> > +			if (http_auth.password)
> > +				free(password);
> > +			else if (password)
> > +				http_auth.password = password;
>
> This is an interesting change.  I wonder what breaks if we
> completely ignored such credential materials forced by the remote
> via a redirect?

Me too. Maybe the original author would know. Is it OK to Cc them in this case?

> >  			url = options->effective_url->buf;
> >  		}
> >  	}

Thanks, Quentin

Previous: Junio C HamanoNext: rsbecker@nexbridge.com
Message 12 of 15 in “Fix gitlab's token-based authentication w/ kerberos”
  1. 0/2 Fix gitlab's token-based authentication w/ kerberosQuentin Bouget, Feb 4, 2024
  2. 1/2 http: only reject basic auth credentials once they have been triedQuentin Bouget, Feb 4, 2024
  3. Junio C HamanoFeb 4, 2024
  4. Quentin BougetFeb 5, 2024
  5. Patrick SteinhardtFeb 5, 2024
  6. 2/2 http: prevent redirect from dropping credentials during reauthQuentin Bouget, Feb 4, 2024
  7. brian m. carlsonFeb 4, 2024
  8. Quentin BougetFeb 5, 2024
  9. brian m. carlsonFeb 5, 2024
  10. rsbecker@nexbridge.comFeb 5, 2024
  11. Junio C HamanoFeb 4, 2024
  12. Quentin BougetFeb 5, 2024
  13. rsbecker@nexbridge.comFeb 4, 2024
  14. Quentin BougetFeb 5, 2024
  15. Robert CoupFeb 5, 2024

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.