Refactor MultiDomainBasicAuth

[jfly]

(This PR is some refactoring that I want to land before creating a PR to implement #12750. If the additional context is useful, you can see my upcoming changes over in jfly/pip@jfly:issue-12750-pre-work-refactor...jfly:pip:issue-12750-configurable-keyring-subprocess-location)

I was starting to work on #12750, but
got hung up on some strange OOP patterns going on here. In particular, I
found it odd that there are 2 different ways we set some knobs
(prompting and keyring_provider) and for instances of
MultiDomainBasicAuth:

1. In tests, we do it by passing these knobs as constructor
   parameters to MultiDomainBasicAuth.
2. In the real cli codepath, we do it by mutating session.auth (an
   instance of MultiDomainBasicAuth) after it's constructed.

I believe that 2) is the reason for the careful cache management logic
you see me tearing out in the PR. Basically: when a
MultiDomainBasicAuth is constructed, it's possible that the keyring
provider will change after the fact, so we can't do any useful caching
in our constructor.

In this PR, I reworked all of this to behave like more normal OOP: I've
tightened up the API of MultiDomainBasicAuth so that these knobs are
only passed in as constructor parameters, and the other instance
attributes are now "private" (underscore-prefixed) values that you're
not supposed to read or write to (except for some tests that look at
these attributes to verify things are working).

This change allowed me to get rid of all the careful cache management
code we had, and sets me up to implement
#12750 as I'm going to be adding yet
another knob (keyring_executable). This new pattern will allow me to
validate that keyring_executable is compatible with keyring_provider
in MultiDomainAuthSettings's constructor (it doesn't make any sense to
specify a path to an executable if you're using the import provider, for
example). With the previous code
pattern, there was just no good place to validate that.

You'll notice I did end up touching a couple of tests:

• test_collector.py: Logging is slightly different now, as we now
  immediately go fetch the keyring provider, rather than lazily. I could
  rework this to preserve the old behavior, but I don't think it's an
  important difference, it only affects tests that are trying to assert
  on every single log message.
• test_network_auth.py: Nothing significant here. Just removing the
  now-unnecessary reset_keyring logic, and updating .password to
  ._password to reflect the new api.

[jfly]

@Darsstar @uranusjr I'd appreciate your thoughts on this PR!

[Darsstar]

LGTM

[jfly]

@ichard26, what would it take to get this PR merged?

[ichard26]

@jfly the honest truth is that no one on the pip team is working on pip's keyring support. We lack the expertise or experience to write, and more importantly—maintain—a keyring implementation. Yes, the keyring functionality is probably in a state of disrepair, and we would like to see improvements, but it's a daunting task no one has taken up yet.

For context, we generally lack the visibility into corporate environments and what their HTTP authentication needs may look like. pip's keyring support was accepted under the guise of being a simple solution, but as the many open keyring issues make clear, it is clearly insufficient. Any potential solution runs the risk of being significantly more complex, so complex, the pip team would have no hope to maintain the logic.

At the end of the day, we have very limited maintainer resources, and this isn't a priority. That's hardly an excuse, but it's still true.

[ichard26]

What would be helpful for now would be someone stepping back to investigate what our users, and especially our corporate users, need for managing HTTP authentication. How pip's keyring support meets and does not meet those meets. Whether it can be retrofitted and improved to meet those needs in a maintainable way, or rather a new solution is needed. There doesn't need to be any code involved. The project management and planning side would be enough to get started on a proper path forward IMO.

It's possible someone has done this work (even partially) for us in some issue, but I haven't taken a look at the related issues in detail.

Of course, none of this is a commitment that this would magically mean pip will see better keyring functionality.

[jfly]

That's totally fair. I appreciate the update.

It's possible someone has done this work (even partially) for us in some issue

This PR came from someone in a corporate environment ;) I think the refactor here should only simplify things.

I'll hold off on sending in PRs for new features (such as #12750, which I had planned to implement once this PR lands).

[jabbera]

Unsure if this is the appropriate place for feedback but I'm an enterprise user. For my users I need the same concept as described in #12750. Essentially for any installation of pip to use a well-known keyring binary regardless of the PATH or any other virtual environment setting.

Above is a corrected version what I wrote below:

Unsure if this is the appropriate place for feedback but I'm an enterprise user. For my users I need the same concept as described in https://github.com//issues/12750. Essentially for any installation of keyring to use a well-known binary regardless of the PATH or any other virtual environment setting.

[Darsstar]

Essentially for any installation of keyring to use a well-known binary regardless of the PATH or any other virtual environment setting.

If you mean it exactly as you wrote it, go read the keyring documentation on how to configure it. You can create a config file and have it load a backend installed from a location of your choosing.

I suspect however you intended it more along these lines:

Essentially for any installation of pip to use a well-known keyring binary regardless of the PATH or any other virtual environment setting.

[jabbera]

I suspect however you intended it more along these lines:

Essentially for any installation of pip to use a well-known keyring binary regardless of the PATH or any other virtual environment setting.

Indeed! Thanks for the clarification, edited my original comment with the incorrect statement left for reference.