Skip to content

Delete/remove client method (flask client) #583

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
lcdunne opened this issue Sep 20, 2023 · 2 comments · May be fixed by #585
Open

Delete/remove client method (flask client) #583

lcdunne opened this issue Sep 20, 2023 · 2 comments · May be fixed by #585
Labels
client Concerns a client implementation feature request good first issue

Comments

@lcdunne
Copy link

lcdunne commented Sep 20, 2023

I can only speak for the Flask client as that is what I'm using. It would be great if the oauth.register(...) method had an oauth.remove(client_name).

Is your feature request related to a problem? Please describe.
I wanted to write a test for something and it involved registering an oauth2 (flask) client for testing purposes. After the tests complete, I want a clean way to remove that testing client from the oauth object, but I cannot see a way to do this.

Without it, running the test poses the following issues. (1) If I create the client in my test with a fixed name (e.g. 'Test Client'), it will return the first client from the oauth registry for all subsequent tests, instead of creating a new client each time. But if I create the client with a unique name for each test (e.g. 'Test Client 1', 'Test Cleint 2', ...), the oauth registry will grow for as many clients are registered until the server restarts. Obviously the latter is the expected behaviour, but a .remove_client method seems missing.

Describe the solution you'd like
Just like with oauth.register(...), maybe something like oauth.remove_client(client_name) would be nice. For example, in the https://github.com/lepture/authlib/blob/master/authlib/integrations/base_client/registry.py, we could add a new method:

def remove(self, client_name):
    if client_name not in self._registry:
        raise KeyError(f"Client {client_name} not found in registry.")

    if client_name not in self._clients:
        raise KeyError(f"Client {client_name} not found in registry.")

    del self._registry[client_name]
    del self._clients[client_name]

But I am not sure if this is sufficient or if it overlooks other issues.

Describe alternatives you've considered
As far as I can tell, we might be able to do this by simply editing the internal oauth._clients and oauth._registry dictionaries. For example del oauth._clients[my_test_client_name]; del oauth._registry[my_test_client_name] but I don't know if this fully accomplishes the goal. It definitely doesn't feel nice.

@lepture
Copy link
Owner

lepture commented Sep 20, 2023

I think it is a good feature, feel free to send a pull request. The method should be added in every clients.

@lcdunne
Copy link
Author

lcdunne commented Sep 20, 2023

Great, I'll take a look.

The method should be added in every clients

Could you please explain a bit more what you mean by this? The BaseOAuth class looks like it is inherited by the clients, so I assumed it would be enough to just add the method there.

@azmeuk azmeuk added good first issue feature request client Concerns a client implementation labels Feb 20, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
client Concerns a client implementation feature request good first issue
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants