search

UMDLARS / dtanm

A framework to teach adversarial thinking to software developers without requiring any special security knowledge
3 stars 2 forks source link

Allow Admins to delete attacks #131

Closed jnowaczek closed 2 years ago

jnowaczek commented 2 years ago

Ref Issue #114

This PR adds a new Admin interface page, the Attacks page. It's basically the same as the user-facing Attacks page but it provides the ability to delete attacks:

image

After confirming the deletion request, all results for a given attack are deleted from the database along with the attack entry. A toast notification reports the number of results deleted:

image image

I wasn't sure (and don't have an easy way to test) if deleting pending scoring tasks would be necessary. I'm guessing it is, I bet a pending scoring task would create a new result for an attack that doesn't exist and break things... I briefly looked into deleting the task from Redis which shouldn't be too hard, otherwise maybe forcing a rescore would avoid this problem? I'm open to suggestions on how to proceed!

derpferd commented 2 years ago

Even if you managed to remove the tasks in queue for Redis a worker could have already been started. So you would need to check if the attack still exists right before you commit the result (preferably in the same transaction). Of course removing the existing tasks in the queue for the attack would be a good optimization. (might be easier to ignore them on the worker side?)

jnowaczek commented 2 years ago

I went with ignoring them on the worker side, I figured the only way to do this without introducing a race condition was to catch the error when the result was being inserted with an invalid foreign key.

One side effect of this is other database errors that share the IntegrityError class might get handled by the try-except I added, but I don't think that's a big problem? The previous behavior in that case would be to crash the worker, but now it will log the exception and discard the problematic result.