RF CredentialManager to have a formal backend class

[mih]

And have this class perform the interaction with the current git-config+keyring storage combo.

This would make it easier to see what operations are needed exactly and how alternative implementations could be realized.

[mih]

Going through the current implementation, it seems as if a backend would need to support four operations. The following has a sketch of an API. The precise names of the operations are not fixed, but the semantics should become clear.

Such a backend could not be used by the current implementation of CredentialManager, but the required changes to do so seem to result in better structured code.

class CredentialBackend:
    def get_credential(self, identifier: str) -> Dict:
        """Retrieve a particular credential

        The returned credentials contains all known properties and a secret
        (if available).

        Parameters
        ----------
        identifier: str
          The identifier of the credential to be retrieved.

        Returns
        -------
        dict
          A mapping of credential properties to their values. The property key
          of a credential's secret is 'secret'.
        """
        raise NotImplementedError

    def set_credential(self, identifier: str, properties: Dict):
        """Set a particular credential.

        Parameters
        ----------
        identifier: str
          The identifier of the credential to be deleted.
        properties: dict
          Any number of credential property key/value pairs. Values of
          ``None`` indicate removal of a stored property from a credential.
        """
        raise NotImplementedError

    def delete_credential(self, identifier: str) -> bool:
        """Delete a particular credential

        Parameters
        ----------
        identifier: str
          The identifier of the credential to be deleted.

        Returns
        -------
        bool
          True if a credential was removed, and False if not (because
          no respective credential was found).
        """

        raise NotImplementedError

    def list_credentials(self) -> Generator[str]:
        """Yields the identifiers of all known credentials"""
        raise NotImplementedError

Derived from such a base class, there would be GitConfigKeyringBackend which implements all modern pieces of the current backend logic. On top of that there would be GitConfigKeyringWithLegacySupportBackend, which has the compat code with the old credential system -- so that the entire class could simply be dropped at some point.

[mih]

So this is definitely possible. It is also definitely useful, because in trying to implement it, many awkward logic pieces in the current implementation become apparent. However, I think I do not have the resource right now to actually pull it off.

I am posting my interim conclusions here, to get an easier start the next time I try:

• we should have a Credential abstraction, no just passing around dicts. I am posting a draft below. It has a few notes how exactly this differs conceptually from the Credential class in datalad-core.
• we should have support for multiple backends, already now. it is possible to implement the current git-config+keyring backend without the legacy credential support for datalad-core. And it is possible to implement that support on top of this backend in a derived class -- such that discontinuing this legacy support is nothing but a backend switch.
• a backend could be implemented as a Python container class with few added features. A draft API is below.

# this must implement enough of the dict API to work as a drop-in
# replacement.
# this is specifically not like the Credential class in datalad-core.
# this one does UI interactions, and interfaces operations with
# a secret store. here we only aim at providing a container for
# a credential.
# we make it a mapping, such that **credential works
class Credential(Mapping):
    #_secret_field = must be a str
    #_mandatory_props = must be a container like a tuple
    def __init__(self, *args, **kwargs):
        # _props is the true information store
        # most of what this class does is filter information access to
        # this dict
        self._props = dict(*args, **kwargs)
        self._props['type'] = self.type_label

    def __getitem__(self, key: str) -> str:
        return self._props[key]

    def __setitem__(self, key: str, value: str | None):
        if key == 'type' and self._props.get(key) != value:
            raise KeyError(
                'Must not change credential type, create new instance instead')
        self._props[key] = value

    def __delitem__(self, key: str):
        del self._props[key]

    def __iter__(self) -> Generator[str, None, None]:
        yield from self._props

    def __contains__(self, key: str) -> bool:
        return key in self._props

    def __repr__(self) -> str:
        return f'{self.__class__.__name__}({self._props.__repr__()})'

    def __len__(self) -> int:
        return len(self._props)

    def get(self, key, default=None):
        return self._props.get(key, default)

    @property
    def secret(self) -> str:
        return self._props.get(self._secret_field)

    @secret.setter
    def secret(self, val: str):
        self._props[self._secret_field] = val

    @property
    def secret_fieldname(self):
        return self._secret_field

    @property
    def properties(self) -> MappingProxyType:
        # read-only view
        return MappingProxyType(
            {k: v for k, v in self._props.items() if k != self._secret_field}
        )

    @property
    def missing_properties(self) -> set:
        # which properties are explicitly set to None
        missing = set(
            k for k, v in self._props.items() if v is None
        )
        # which props are declared mandatory and have no value or do not
        # exist at all
        missing.update(
            k for k in self._mandatory_props if not self._props.get(k)
        )
        return missing

    @property
    def type_label(self):
        return self.__class__.__name__.lower()


class Secret(Credential):
    # standard credential, a secret plus any credentials
    _secret_field = 'secret'
    _mandatory_props = tuple()


class UserPassword(Credential):
    # standard credential, a secret plus any credentials
    _secret_field = 'password'
    _mandatory_props = ('user', 'password')

    @property
    def type_label(self):
        # name choice has historic reasons, this is what datalad-core
        # did from its infancy
        return 'user_password'


class Token(Credential):
    _secret_field = 'token'
    _mandatory_props = tuple()

class CredentialBackend:
    """Interface to be implemented for any CredentialManager backend"""
    def __getitem__(self, identifier: str) -> Credential:
        """Retrieve a particular credential

        The returned credentials contains all known properties and a secret
        (if available).

        Parameters
        ----------
        identifier: str
          The identifier of the credential to be retrieved.

        Returns
        -------
        dict
          A mapping of credential properties to their values. The property key
          of a credential's secret is 'secret'.
        """
        raise NotImplementedError

    def __setitem__(self, identifier: str, value: Credential):
        """Set a particular credential.

        Parameters
        ----------
        identifier: str
          The identifier of the credential to be deleted.
        value: Credential
          The credential to set.
        """
        raise NotImplementedError

    def __delitem__(self, identifier: str):
        """Delete a particular credential

        Parameters
        ----------
        identifier: str
          The identifier of the credential to be deleted.
        """
        raise NotImplementedError

    def __iter__(self) -> Generator[str, None, None]:
        """Yields the identifiers of all known credentials"""
        raise NotImplementedError

    def __contains__(self, identifier: str) -> bool:
        raise NotImplementedError

    def get_credential_type(self, type_id: str | None = None) -> Type[Credential]:
        # the only recognized standard credentials is comprised (at minimum)
        # of a 'secret' only.
        # particular backends may support additional/different credentials.
        # the reason why backends get a say in this at all is that they
        # need to ensure that the "secret" (in whatever disguise it is
        # presented) actually is put into a secret store -- which may
        # be different from the place all other credential properties
        # are stored.
        # The credential manager is asking the backend on what information
        # it needs to know, and how they should be labeled, and feeds
        # the collected credential info in this format back to the backend.
        return Secret

[mih]

A suitable half-step could be to switch the return types of credential manager to the Credential type drafted above. This has a good chance to avoid any breakage in the future.

[mih]

Closing in favor of #687