KeyringBackend set_password() keyword argument renamed by wrapper

[jdhalbert]

Describe the bug
Calling set_password() on a KeyringBackend with the keyword argument service results in a TypeError, but calling it with positional arguments works fine. service is (seemingly accidentally) renamed system here: https://github.com/jaraco/keyring/blame/24f94795c9e610d51d76caae5d139227e334a8cf/keyring/backend.py#L58

To Reproduce
Example in Windows 11:

k = keyring.get_keyring()
k.set_password(service='test_service', username='test_username', password='test_password')
# TypeError: WinVaultKeyring.set_password() missing 1 required positional argument: 'system'

k.set_password('test_service', 'test_username', 'test_password')
# OK

Expected behavior
set_password() should work with the keyword argument service instead of system.

Environment

• OS:
  Tested with Windows but probably affects all plaforms.

$ pip list | grep keyring
keyring                 25.6.0
$ keyring --list-backends
keyring.backends.fail.Keyring (priority: 0)
keyring.backends.chainer.ChainerBackend (priority: -1)
keyring.backends.Windows.WinVaultKeyring (priority: 5)

Additional context
When using the KeyringBackend, the keyword is service but when using keyring the keyword is service_name. It may be more intuitive to have consistency here as well, but that is beside the point of this issue that the keyword system not used anywhere else.

[ThomasBachSap]

+1
This is a showstopper regression as using keyword arguments worked before. After upgrading to the current version, the custom backend I used did not work anymore because of the reasons stated in the original post.

One must remove all usage of keyword arguments, otherwise it does not run. As we have functions with 3 strings, using keyword arguments is a matter of sanity. Who will otherwise remember the right order of these 3 strings or can check it during a review. Just adapting instead of removing the keyword arguments does not work because as stated above, the names are incoherent. Sometimes system, sometimes service, sometimes service_name. Also, python complains to not provide an keyword argument for self if we try to use keyword arguments. That is because how the wrapper is implemented.

I dont know if it is a good idea at all to hook in this validate_username function with a wrapper. If I implement a backend and I am fine with empty usernames, so let it be like that.

Fixing the wrapper as it is seems not to be that easy. Like if we do instead of the original:

@functools.wraps(orig)
def wrapper(self, system, username, *args, **kwargs):
    self._validate_username(username)
    return orig(self, system, username, *args, **kwargs)

a:

@functools.wraps(orig)
def wrapper(*args, **kwargs):
    for key, value in kwargs.items():
        if key == "username":
            self._validate_username(value)
    return orig(*args, **kwargs)

Then it does not work for non-keyword arguments. Which means we would need to make the username argument a mandatory keyword argument.