fix(core): provider do_request to maintain verify in all request, basic headers

[tarilabs]

Hi 👋

This proposed PR fixes what I believe are 2 edit: *3 problems:

1. atm do_request do not maintain verify=self._tls_verify for all the requests's calls (it does maintain it only on the first one)
2. The "One retry if 403 denied (need new token?)" is actually done anyway, and this causes 4xx problem as the third request is always invoked if the second one got invoked, causing 4xx in some scenarios
3. Finally, we need to maintain the headers from the original (first) request in the subsequent ones when augmented with the auth using basic auth

I'm looking forward to your feedback and if I missed anything?

How was this tested?

locally, I have a pytest as:

@pytest.mark.with_auth(True)
def test_directory_push_pull_selfsigned_auth(tmp_path, registry, credentials, target_dir):
    """
    Test push and pull for directory using a self-signed cert registry and basic auth
    """
    client = oras.client.OrasClient(hostname=registry, tls_verify=False, auth_backend="basic")

    # Test upload of a directory
    upload_dir = os.path.join(here, "upload_data")
    res = client.push(files=[upload_dir], target=target_dir)
    assert res.status_code == 201
    files = client.pull(target=target_dir, outdir=tmp_path)

    assert len(files) == 1
    assert os.path.basename(files[0]) == "upload_data"
    assert str(tmp_path) in files[0]
    assert os.path.exists(files[0])
    assert "artifact.txt" in os.listdir(files[0])

I'm using a Zot configured with:

• self signed certs

openssl req -newkey rsa:4096 -nodes -sha256 -keyout test/data/server.key -x509 -days 365 -out test/data/server.cert

• auth

htpasswd -bBn dinosaur oras_pass >> test/data/htpasswd

configuration of Zot server:

{
  "distSpecVersion": "1.1.0",
  "storage": {
    "rootDirectory": "/tmp/zot"
  },
  "http": {
    "address": "0.0.0.0",
    "port": "8443",
    "tls": {
      "cert": "test/data/server.cert",
      "key": "test/data/server.key"
    },
    "auth": {
      "htpasswd": {
        "path": "test/data/htpasswd"
      }
    },
    "accessControl": {
      "repositories": {
        "**": {
          "policies": [{
            "users": ["dinosaur"],
            "actions": ["read", "create", "update", "delete"]
          }],
          "defaultPolicy": ["read"],
          "anonymousPolicy": []
        }
      }
    }
  },
  "log": {
    "level": "debug"
  },
  "extensions": {
    "search": {
      "cve": {
        "updateInterval": "2h"
      }
    },
    "ui": {
      "enable": true
    }
  }
}

BEFORE

on main, gives error (SSLError(SSLCertVerificationError ...):

../../Library/Caches/pypoetry/virtualenvs/testcontainers-ABJcKhkA-py3.12/lib/python3.12/site-packages/urllib3/util/retry.py:515: MaxRetryError

During handling of the above exception, another exception occurred:

tmp_path = PosixPath('/private/var/folders/n0/3c71vqs570b8l21fmnvb5k640000gn/T/pytest-of-mmortari/pytest-89/test_directory_push_pull_selfs0'), registry = 'localhost:8443'
credentials = TestCredentials(with_auth=True, user='dinosaur', password='oras_pass'), target_dir = 'localhost:8443/dinosaur/directory:v1'

    @pytest.mark.with_auth(True)
    def test_directory_push_pull_selfsigned_auth(tmp_path, registry, credentials, target_dir):
        """
        Test push and pull for directory using a self-signed cert registry and basic auth
        """
        client = oras.client.OrasClient(hostname=registry, tls_verify=False, auth_backend="basic")
    
        # Test upload of a directory
        upload_dir = os.path.join(here, "upload_data")
>       res = client.push(files=[upload_dir], target=target_dir)

oras/tests/test_oras.py:150: 
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ 
oras/provider.py:762: in push
    response = self.upload_blob(blob, container, layer)
oras/provider.py:275: in upload_blob
    response = self.put_upload(blob, container, layer)
oras/provider.py:518: in put_upload
    r = self.do_request(upload_url, "POST", headers=headers)
oras/decorator.py:60: in __call__
    return self.func(cls, *args, **kwargs)
oras/provider.py:946: in do_request
    response = self.session.request(
../../Library/Caches/pypoetry/virtualenvs/testcontainers-ABJcKhkA-py3.12/lib/python3.12/site-packages/requests/sessions.py:589: in request
    resp = self.send(prep, **send_kwargs)
../../Library/Caches/pypoetry/virtualenvs/testcontainers-ABJcKhkA-py3.12/lib/python3.12/site-packages/requests/sessions.py:703: in send
    r = adapter.send(request, **kwargs)
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ 

self = <requests.adapters.HTTPAdapter object at 0x106c187a0>, request = <PreparedRequest [POST]>, stream = False, timeout = Timeout(connect=None, read=None, total=None)
verify = True, cert = None, proxies = OrderedDict()

    def send(
        self, request, stream=False, timeout=None, verify=True, cert=None, proxies=None
    ):
        """Sends PreparedRequest object. Returns Response object.
    
        :param request: The :class:`PreparedRequest <PreparedRequest>` being sent.
        :param stream: (optional) Whether to stream the request content.
        :param timeout: (optional) How long to wait for the server to send
            data before giving up, as a float, or a :ref:`(connect timeout,
            read timeout) <timeouts>` tuple.
        :type timeout: float or tuple or urllib3 Timeout object
        :param verify: (optional) Either a boolean, in which case it controls whether
            we verify the server's TLS certificate, or a string, in which case it
            must be a path to a CA bundle to use
        :param cert: (optional) Any user-provided SSL certificate to be trusted.
        :param proxies: (optional) The proxies dictionary to apply to the request.
        :rtype: requests.Response
        """
    
        try:
            conn = self.get_connection_with_tls_context(
                request, verify, proxies=proxies, cert=cert
            )
        except LocationValueError as e:
            raise InvalidURL(e, request=request)
    
        self.cert_verify(conn, request.url, verify, cert)
        url = self.request_url(request, proxies)
        self.add_headers(
            request,
            stream=stream,
            timeout=timeout,
            verify=verify,
            cert=cert,
            proxies=proxies,
        )
    
        chunked = not (request.body is None or "Content-Length" in request.headers)
    
        if isinstance(timeout, tuple):
            try:
                connect, read = timeout
                timeout = TimeoutSauce(connect=connect, read=read)
            except ValueError:
                raise ValueError(
                    f"Invalid timeout {timeout}. Pass a (connect, read) timeout tuple, "
                    f"or a single float to set both timeouts to the same value."
                )
        elif isinstance(timeout, TimeoutSauce):
            pass
        else:
            timeout = TimeoutSauce(connect=timeout, read=timeout)
    
        try:
            resp = conn.urlopen(
                method=request.method,
                url=url,
                body=request.body,
                headers=request.headers,
                redirect=False,
                assert_same_host=False,
                preload_content=False,
                decode_content=False,
                retries=self.max_retries,
                timeout=timeout,
                chunked=chunked,
            )
    
        except (ProtocolError, OSError) as err:
            raise ConnectionError(err, request=request)
    
        except MaxRetryError as e:
            if isinstance(e.reason, ConnectTimeoutError):
                # TODO: Remove this in 3.0.0: see #2811
                if not isinstance(e.reason, NewConnectionError):
                    raise ConnectTimeout(e, request=request)
    
            if isinstance(e.reason, ResponseError):
                raise RetryError(e, request=request)
    
            if isinstance(e.reason, _ProxyError):
                raise ProxyError(e, request=request)
    
            if isinstance(e.reason, _SSLError):
                # This branch is for urllib3 v1.22 and later.
>               raise SSLError(e, request=request)
E               requests.exceptions.SSLError: HTTPSConnectionPool(host='localhost', port=8443): Max retries exceeded with url: /v2/dinosaur/directory/blobs/uploads/ (Caused by SSLError(SSLCertVerificationError(1, '[SSL: CERTIFICATE_VERIFY_FAILED] certificate verify failed: self-signed certificate (_ssl.c:1000)')))

../../Library/Caches/pypoetry/virtualenvs/testcontainers-ABJcKhkA-py3.12/lib/python3.12/site-packages/requests/adapters.py:620: SSLError
============================================================================= short test summary info ==============================================================================
FAILED oras/tests/test_oras.py::test_directory_push_pull_selfsigned_auth - requests.exceptions.SSLError: HTTPSConnectionPool(host='localhost', port=8443): Max retries exceeded with url: /v2/dinosaur/directory/blobs/uploads/ (Caused by SSLError(SSLCer...
======================================================================== 1 failed, 19 deselected in 10.46s =========================================================================

AFTER

with this PR:

% env | grep ORAS_
ORAS_HOST=localhost
ORAS_PORT=8443
ORAS_USER=dinosaur
ORAS_PASS=oras_pass
ORAS_AUTH=true

% pytest -k test_directory_push_pull_selfsigned -s
=============================================================================== test session starts ================================================================================
platform darwin -- Python 3.12.3, pytest-7.4.3, pluggy-1.4.0
rootdir: /Users/mmortari/git/oras-py
configfile: pyproject.toml
plugins: asyncio-0.23.5, anyio-4.3.0, cov-4.1.0
asyncio: mode=Mode.STRICT
collected 20 items / 19 deselected / 1 selected                                                                                                                                    

oras/tests/test_oras.py Successfully pushed localhost:8443/dinosaur/directory:v1
.

================================================================================= warnings summary =================================================================================
oras/tests/test_oras.py::test_directory_push_pull_selfsigned_auth
  /opt/homebrew/Cellar/python@3.12/3.12.3/Frameworks/Python.framework/Versions/3.12/lib/python3.12/tarfile.py:2221: DeprecationWarning: Python 3.14 will, by default, filter extracted tar archives and reject files or modify their metadata. Use the filter argument to control this behavior.
    warnings.warn(

-- Docs: https://docs.pytest.org/en/stable/how-to/capture-warnings.html
=================================================================== 1 passed, 19 deselected, 1 warning in 0.28s ====================================================================

I didn't find a way to provide integration testing based on the specific use case here:

• self signed server cert
• AND basic auth

but kindly let me know if I missed anything

[vsoch]

These changes look good! Is there any reason not to add a test for it here as well (as opposed to just locally)?

[tarilabs]

These changes look good! Is there any reason not to add a test for it here as well (as opposed to just locally)?

The test in the original comment requires a OCI registry with self signed certs (and basic auth), and it's not clear to me how to replicate the local setup (I happened to use Zot) I've described in my original comment. Let me know your feedback, thanks!

[vsoch]

We have tests with auth: https://github.com/oras-project/oras-py/blob/main/.github/workflows/auth-tests.yaml

What would be the difference between the setup there and the test you have with the self signed certs? Or why would it fail with the latter but not the current tests (and others using auth)?

[tarilabs]

To replicate the original failure, now fixed with this PR, the second call (the one augmented with the auth header) would have failed the cert validation with the "SSLError(SSLCertVerificationError" error since it didn't maintain the verify=self._tls_verify.

I can include the test from my original comment, it would not replicate 100% the original scenario. Wdyt?

[vsoch]

I can include the test from my original comment, it would not replicate 100% the original scenario. Wdyt?

Yes, that would be great! And then the final tweak is to bump the version in oras/version.py and add a corresponding note to CHANGELOG.md. Thank you!

[tarilabs]

@vsoch pardon me quick q: I can understand changelog, but its not clear to me about version bump: I can see a 2.0 was marked, but I can't find Tag, Release or Pypi of it ? 🤔

[tarilabs]

@vsoch I let you decide:

• the original test is in fc3207d which however fails (Oras Auth Tests / test-auth (pull_request) Failing after 2m
  Details) due to the GHA registry server not-using-self-signed-certs
• I need to supply also c70d728 to adapt the test to the GHA test infrastructure

[vsoch]

@tarilabs it's not clear to me why a self signed certificate would lead to the wrong version number (or rather, not having one). Did you try updating urllib3 first?

[tarilabs]

Did you try updating urllib3 first?

the failure in Details is GHA for fc3207d

my understanding is that in the case of this repo GHA job, there is no self-signed-cert/SSL, so I need to say insecure=True

in "my" case of server with:

• self signed cert: I need verify=self._tls_verify in all the requests
• otherwise at the second call, without verify=self._tls_verify, it would have the basic auth header, but would fail SSL cert validation

[vsoch]

So the with auth workflow is adding an http password but not SSL. Why not add the same self signed cert to the setup? If you think "with auth" is a different use case entirely we arguably should have a third workflow.

[tarilabs]

we arguably should have a third workflow

cool, thanks for the feedback; so the next step would be:

• add self-signed certs to existing GHA for "with auth": if the already-existing tests do not break, I could stop there
• otherwise, I'd add a third GHA workflow, specifically for the scenario of this added test/use-case (which is: self-signed certs and basic auth)

have I summarized it correctly?

[tarilabs]

@vsoch I've also aligned the previously existing auth test to make use of the self-signed certs server workflow 🤗

let me know if overall this PR now matches your expectations :)

[vsoch]

Looks like this is also 0.2.0 (your addition is correct!) but let's indent the line below it to be a sub-bullet, and remove the second mention of 0.2.0.

[tarilabs]

Will do! 👍

[tarilabs]

done with f848752 🚀

[vsoch]

This is great! We now have a proper test for ssl.

[vsoch]

Thank you for this work - it's a great contribution on many levels!