Skip to content
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

Don't allow deleting the project through the API with just a token #1206

Open
zorun opened this issue Jul 29, 2023 · 4 comments
Open

Don't allow deleting the project through the API with just a token #1206

zorun opened this issue Jul 29, 2023 · 4 comments

Comments

@zorun
Copy link
Collaborator

zorun commented Jul 29, 2023

See #1204 and https://ihatemoney.readthedocs.io/en/latest/security.html#giving-access-to-a-project for context

If you only have an auth token (and not the private code), there's one thing you can do through the API that you cannot do through the web interface: deleting the project. This is annoying, because we would like tokens to have less privilege than the private code (since they are used in invite links)

I see two ways to improve it:

  • use separate tokens for invite links and API access
  • make sure the private code is required to delete the project through the API

For the second solution, we cannot reliably ask the private code in the body of a DELETE request. We could switch to a POST request instead.

@almet
Copy link
Member

almet commented Aug 7, 2023

Yeah, this is not the intended behavior, you're right.

If you have the project code, you can use it to authenticate using basic auth and issue a DELETE on the project. That seem to be what we want here.

We should just disable the delete endpoint when the clients authenticate using tokens.

@zorun
Copy link
Collaborator Author

zorun commented Aug 7, 2023

That looks like a layering violation: I wouldn't expect my authentication details to affect what calls I can and cannot do.

But that may actually be possible, and that would not be the first weird thing about our API ;) I'm just not sure it's a good idea.

@almet
Copy link
Member

almet commented Aug 7, 2023

I'm not sure to understand why this would be bad ? (not sure what is a layering violation).

To me, we have different ways to authenticate on the API :

  • Basic Auth for authentication with the project code. It means you have "admin" rights ;
  • Bearer Token, if you have a token. It means you have user rights.

Am I missing something ?

@almet
Copy link
Member

almet commented Apr 28, 2024

Here is the summary of the discussion with @zorun on the matter:

  • change the API to ask for the private code when deleting
  • keep a compatibility layer where the old endpoint is still working (check if the user used a token to authenticate, and refuse in this case)
  • on a future API version bump, delete the old endpoint.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants