.netrc should not be honored if username and password is given to the client

[tobixen]

Ref #205, it was found that the python requests library will honor a .netrc file. While it's a feature that such a file is honored when username and password is omitted, it's a bug that the file is honored when a (different) username and password is given to the library.

[googol42]

I did some more research and you can disable netrc by setting trust_env to False, at least this is suggested here psf/requests#2773 and here psf/requests#919. Usage see here: https://stackoverflow.com/a/28521696 But it disables not only netrc, see https://requests.readthedocs.io/en/latest/api/?highlight=trust_env#requests.Session.trust_env

Regardings proxys: One can already allow to specify the proxy to use. So that trust_env=False disables this is not that bad (one can work around this), but this not good. There might be more side effects we are not aware right now.

I looked at the requests library and might have found a workaround:

But first, lets look at the code.... get_netrc_auth from https://github.com/psf/requests/blob/main/requests/utils.py#L194 is accessing the netrc file. This function is only called from prepare_request from https://github.com/psf/requests/blob/main/requests/sessions.py#L457

The if we are interested look like this (in prepare_request at this location https://github.com/psf/requests/blob/main/requests/sessions.py#L480): if self.trust_env and not auth and not self.auth and since we (probably) don't want to change trust_env (because of possible side effects) either auth or self.auth must be filled. self.auth in the Session class currently has one write (in __init__ => not critical) and two reads in prepare_request. The first read is the if condition above. The second read is in this line (https://github.com/psf/requests/blob/main/requests/sessions.py#L494). When we look at merge_setting from https://github.com/psf/requests/blob/main/requests/sessions.py#L61 we see, that it merges to dicts and it returns the first parameter (dict) which is the auth parameter passed to the request when the second is not a Mapping-Type. The second parameter is the self.auth variable.

from requests import sessions

class SessionIgnoringNetrc(sessions.Session):
    def prepare_request(self, request):
        # set this to anything except None and a dict (so the if block is skipped)
        # that is not a mapping type (so the value we set is not merged with those
        # from the request). 
        self.auth = ("foo", "bar")
        prepared_request = super(SessionIgnoringNetrc, self).prepare_request(request)
        self.auth = None # might not be needed
        return prepared_request

When you use the following code at the first request where we don't use any authentication and also want to ignore netrc, you could use this code:

with SessionIgnoringNetrc() as session:
    response = session.request(method="get", url="some url")

I know, it is hacky... This solution is interception at two points. First the self.auth variable and the overriding of prepare_request. When prepare_request would be renamed upstream this will cause errors (hence won't silently cause regression). When self.auth is removed/renamed this would not be noticed, but a check (for existence) could be added. Anyways, both are not marked internal (via _).

[tobixen]

I'm frequently getting pull requests on various parameters people want passed from the DAVClient to the requests library, I think trust_env could be added to that list. That's of course not solving this issue (though it will allow for an easy work-around for a small subclass of people struck by this issue - though most users probably wouldn't know what hit them ... and that's just the direct users of the library, most of the users are probably indirect users, using the library through HomeAssistant caldav plugin, the calendar-cli tool or others. (I've noticed a big influx of bug reports coming in through HA users lately), but I think it's a good idea nevertheless.

I think there are many possible solutions to this problem, here are some ideas:

Push the problem to the requests library

As said, I believe this complexity belongs in the requests library and not in the caldav library. It would be nice to do some research - how is this solved by other users of the requests library? I find it hard to believe that this problem is unique to the caldav library. Best thing would probably be to communicate directly and informally with the main contributors/maintainers of the requests library and hear what they think. If it's difficult to get in touch with them, raising an issue at their side could possibly be a way to get in touch with the relevant people. I thought of raising an issue earlier that it should be possible to pass username and password to the library without explicitly telling what kind of auth to use - but then it would be a "feature request" and not a "bug report", and feature requests are not accepted.

There is a drawback - there exists many weird caldav servers out there, and the current code is like optimized to work on as many servers as possible. If the whole logic is to be moved to the requests library, it's a risk that it will cause regression issues for some users.

Your SessionIgnoringNetrc solution

I think the requests library is a very mature library, so the risk that self.auth or self.request should disappear overnight is very low - I think it can be dismissed as an acceptable risk (at least if we fix some test code that will catch this). The biggest drawback is that it's additional complexity added to the library and hence a risk of breaking things.

Explicit check that username and password is utilized

It's possible to do a check - if username and password is provided but still not utilized when doing the current-user-principal, then some extra logic could be applied. For instance, your SessionIgnoringNetrc solution, or simply turning trust_env to False. The biggest drawback is that this introduces extra complexity and hence a risk of breaking things.

Reverting to the old behaviour - try all kind of auth methods

I really don't want to go this route, and as experienced this also caused problems for some (very few) users.

Simply ignoring the whole issue and ensure it's documented

... possibly combined with raising an issue at the requests library issue tracker.

To encounter this issue, it's needed to have a .netrc file, the .netrc has to cover the calendar server, and the username/password provided in the netrc has to deviate from what is given to the caldav server. How likely is it that other people are affected by this issue?

[tobixen]

Seems like there is a version 3 of the requests library coming at some point in the future.

While I will accept a pull request on the SessionIgnoringNetrc-solution, I think my favored approach is to wait for requests3 and consider from there what is the best solution.