feat(auth): add URL-dispatched session auth for GitHub and GitLab

[tiran]

Pull Request Description

What

Add SessionAuth class that dispatches authentication by (scheme, hostname). Lazy callbacks resolve credentials from netrc or environment variables on first request and cache the result. create_session() returns the session and auth handler so callers can register additional hosts.

Move authentication documentation to docs/how-tos/authentication.md and update http-retry.md to reference the new guide.

Why

see #1168

• PR follows CONTRIBUTING.md guidelines

[coderabbitai]

Caution

Review failed

An error occurred during the review process. Please try again later.

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[tiran]

The new code generalized authentication for GitHub and GitLab. We can drop a bunch of custom code from downstream.

[ryanpetrello]

The caching/lazy mechanism makes sense to me as I read it, but I'm wondering if the complexity is worth it in terms of what we're saving/avoiding here. I had to step through this and think about it to make sure I understood - for example - the cache invalidation.

Without the caching, we're basically re-reading a .netrc on every request. Is that it?

If so, would it be simpler to just make that file read a singleton and change our callback signature so that we pass in the parsed netrc as a parameter (psuedo-code ahead)

class SessionAuth(requests.auth.AuthBase):

    def __init__(self) -> None:
        self._netrc = get_netrc_auth()

    def get(self, url: str) -> dict[str, str]:
        ...
        callback = self._callbacks.get(key)
        return callback(*key, self._netrc) if callback else {}

[ryanpetrello]

I should also say, @tiran I don't feel particularly strongly about #1173 (review)

If you're happy with the implementation as-is, I'm happy to give a thumbs up 👍 (though my approval is non-voting on merge).

This in particular:

We can drop a bunch of custom code from downstream.

...makes me inclined to say "ship it".

[rd4398]

Looks overall good. I have left a couple of comments

[tiran]

The caching/lazy mechanism makes sense to me as I read it, but I'm wondering if the complexity is worth it in terms of what we're saving/avoiding here. I had to step through this and think about it to make sure I understood - for example - the cache invalidation.

Without the caching, we're basically re-reading a .netrc on every request. Is that it?

Kind of. The two callbacks use requests' utils to locate, load, and parse a netrc file, then find a matching entry for an URL. In the future, callbacks may do other things that do not involve a netrc file.

[rd4398]

The caching/lazy mechanism makes sense to me as I read it, but I'm wondering if the complexity is worth it in terms of what we're saving/avoiding here. I had to step through this and think about it to make sure I understood - for example - the cache invalidation.

Without the caching, we're basically re-reading a .netrc on every request. Is that it?

If so, would it be simpler to just make that file read a singleton and change our callback signature so that we pass in the parsed netrc as a parameter (psuedo-code ahead)

class SessionAuth(requests.auth.AuthBase):

    def __init__(self) -> None:
        self._netrc = get_netrc_auth()

    def get(self, url: str) -> dict[str, str]:
        ...
        callback = self._callbacks.get(key)
        return callback(*key, self._netrc) if callback else {}

+1 to what @ryanpetrello commented.

Alternate suggestion:

drop ` _cache`  entirely and call the callback on every request. If profiling later shows netrc parsing is consuming resources, we could add caching then.

[tiran]

What profiling? We don't do performance profiling regularly.

[rd4398]

What profiling? We don't do performance profiling regularly.

Yeah, I missed that part.

[rd4398]

LGTM!