search

loculus-project / loculus

An open-source software package to power microbial genomic databases
https://loculus.org
GNU Affero General Public License v3.0
37 stars 2 forks source link

Remove authentication requirements from `/debug/delete-all-sequence-data` endpoint #2101

Open corneliusroemer opened 5 months ago

corneliusroemer commented 5 months ago

The need for authentication makes the /debug/delete-all-sequence-data endpoint a pain to use.

If you're debugging, it's not necessary to ensure authentication, as it's just you testing. If you want to avoid others deleting your stuff, just don't run in debug mode.

I haven't used the endpoint purely because there's not a simple button/endpoint I can call without requiring jumping through hoops to get the JWT etc.

I suggest we remove authn/authz requirements for debug endpoints.

chaoran-chen commented 5 months ago

The reason why we have it is that I think that we should have multiple layers of protection against a configuration error, especially given the speed in which we make changes, merge PRs and change configs. If we remove the authentication requirement, I think that we should introduce something else to prevent the endpoint being accessible by mistake. For example something additional that prevents us from running the app in debug mode?

corneliusroemer commented 5 months ago

For me, it's hard to imagine misuse:

  1. To enable, you have to set "--loculus.debug-mode=true"
  2. In debug mode, we show a red banner
  3. The worst case is that one has to recover from a database snapshot

Do you have a concrete scenario where real damage would be done?

Maybe we call it "debug-mode" but "self-destruct-mode"? I guess you're worried about people enabling it without understanding the implications? We could make the argument more explicit, something like enable-everyone-can-delete-everything=True? If you set that in production it's your own fault :D

We could also make the required arg yes-i-know-what-i-am-doing instead of true, so one really needs to know what one is doing:

backendExtraArgs:
  - "--loculus.debug-mode.enable-everyone-can-delete-everything=true-yes-i-know-what-i-am-doing"

There's still a red banner (unless there's a bug), and also, one still needs to actually hit the endpoint. Plus one should have backups.

For comparison: PostgreSQL has a setting fsync=off

Never turn off unless your data is entirely disposable. Setting fsync=off is the equivalent of saying “I don't care about my data, I can recreate the database from scratch if I have to.” If synch activity is a performance concern, see synchronous_commit. https://postgresqlco.nf/doc/en/param/fsync/

chaoran-chen commented 5 months ago

Hmm, okay, this sounds convincing. As long as we emphasize the danger of the debug mode clearly (in future docs and maybe in variable naming), I am fine with removing the authentication.