oras-project / oras-py

ORAS Python SDK
https://oras-project.github.io/oras-py/
Apache License 2.0
38 stars 31 forks source link

Passing an initialized auth backend? #157

Open xarses opened 2 days ago

xarses commented 2 days ago

I'm working on adding in a custom auth backend for me to use with AWS Elastic Container Registry (ECR).

Currently the process for adding a new backend appears to be to create a parameterless descendent of oras.auth.base.AuthBackend and add it the mapping of auth backends.

Seems fine. However ECR credentials are specific to each registry, so ideally we'd prepare the auth client by constructing it with the account already known. However this pattern would require that the registry could be passed an instantiated auth instance. I could also skim the account number out of the request in authenticate_request, but that feels a little backwards.

vsoch commented 2 days ago

Can you walk me through the desired interaction you'd like the user to take, and we can think about it?

xarses commented 2 days ago

I'm not sure it matters too much. My current flow is to look at the registry URI and split the desired parts out of the URL and construct the registry client and return it, I cache the resulting registry to avoid requesting new credentials when processing images (I work with a lot of images). In this case, I've implemented the Registry being able to accept a constructed auth_backend instance

class ECRAuth(oras.auth.BasicAuth):
    def __init__(self, account: str):
        self.account = account
        super().__init__()
        self.update_auth()
...

def __init_ecr_registry(registry: str):
    account, _, _, region, *_ = registry.split(".")
    reg = Registry(auth_backend=ECRAuth(account))
    return reg

Writing this out, it would be nearly indifferent between this method, and siphoning the account out of a request requiring auth. The only differences I can think of is that in this flow, I construct the initial auth and designate the account to the instance. This allows for the auth flow with ECR via tha boto3 client to be performed at instantiation which tends to improve understanding what is going on when this leg of the auth fails.

It further locks the instance to the given account and would prevent the instance from accidentally switching between registries for which the auth was currently valid (if locked it would raise an auth failure) If we silently took the account from the request uri, then it would be possible for the client to attempt to switch the auth to another registry (if it was coded to refresh on a 401, which I wanted to support in this client). Such a call to boto3 might be at additional risk of failing if the caller doesn't have access to that registry (I currently control this when I determine which Registry I want to use from my cache of constructed registries)

vsoch commented 1 day ago

Could that not be accomplished with a registry init function that allows for kwargs that are passed to the auth backend? Or for example have some parameter that can hold "registry" as you have it above (or a similar name) that is passed to the backend to parse, or just ignored.

xarses commented 1 day ago

If I'm following, you're proposing something like

class Registry:
    def __init__(self, ..., auth_backend, auth_kwargs):
        ...
        self.auth = oras.auth.get_auth_backend(
            auth_backend, self.session, **auth_kwargs
        )
vsoch commented 1 day ago

yes! That would definitely work - that's even better than what I had in mind to have auth_kwargs as separate from others. I would default to None and then do:

class Registry:
    def __init__(self, ..., auth_backend, auth_kwargs=None):
        ...
        auth_kwargs = auth_kwargs or {}
        self.auth = oras.auth.get_auth_backend(
            auth_backend, self.session, **auth_kwargs
        )
xarses commented 1 day ago

Sounds good, this method would still require that the extension adds its self to the oras.auth.auth_backends map. Are you open to allowing auth_backend being passed as a subclass of oras.auth.base.AuthBackend or a str and having oras.auth.get_auth_backend(...) handle the two?

vsoch commented 1 day ago

Do you mean passing the auth backend directly instead of calling get_auth_backend? Yes that would be OK - likely we can have: