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

Implement delete security #371

Open
1 task
fyliu opened this issue Aug 22, 2024 · 3 comments
Open
1 task

Implement delete security #371

fyliu opened this issue Aug 22, 2024 · 3 comments
Labels
complexity: medium dependency Issue has dependencies draft This issue is not fully-written p-feature: API private endpoint p-feature: API public endpoint question Further information is requested ready for dev lead role: back end s: CTJ stakeholder: Civic Tech Jobs s: hackforla.org stakeholder: hackforla.org website s: kb stakeholder: knowledgebase s: PD team stakeholder: People Depot Team s: VRMS stakeholder: VRMS size: 1pt Can be done in 4-6 hours

Comments

@fyliu
Copy link
Member

fyliu commented Aug 22, 2024

Dependency

Overview

We need to disable the DELETE method for all models. This was decided by @bonniewolfe in the 2024/08/15 meeting. The idea is that we do not actually delete data in most cases, but we'll work out the actual "delete" implementation later. For now, we just disable the endpoints

Action Items

Resources/Instructions

@fyliu fyliu added complexity: medium p-feature: API public endpoint p-feature: API private endpoint question Further information is requested role: back end s: CTJ stakeholder: Civic Tech Jobs s: hackforla.org stakeholder: hackforla.org website s: kb stakeholder: knowledgebase s: PD team stakeholder: People Depot Team s: VRMS stakeholder: VRMS size: 1pt Can be done in 4-6 hours labels Aug 22, 2024
@fyliu fyliu added this to the v0.01 - initial setup milestone Aug 22, 2024
@fyliu fyliu moved this to New Issue Review in P: PD: Project Board Aug 22, 2024
@fyliu fyliu moved this from New Issue Review to Questions/Review in P: PD: Project Board Aug 22, 2024
@fyliu
Copy link
Member Author

fyliu commented Aug 22, 2024

@bonniewolfe How do we want to implement this?

  1. not have the DELETE endpoints so clients can't call them
  2. show DELETE but return an error
  3. show DELETE and return success but do nothing for now

I'm leaning towards the last option as it's closest to what we eventually want to do.


Note that we can have a custom modelview class to disable this for every model for now, and then switch to the standard ModelViewSet and implement "delete" actions separately for each model.

# declare all model viewsets to use a temporary custom viewset which disables delete
class UserViewSet(NoDeleteModelViewSet):
    ...

# switch to the standard ModelViewSet and add a custom delete method in each model viewset
class UserViewSet(rest_framework.viewsets.ModelViewSet):
    ...

    def delete(self, request, *args, **kwargs):
        ...

@shmonks shmonks changed the title Disable DELETE method for all models in API Disable DELETE method for all models except admins in API Aug 30, 2024
@shmonks shmonks changed the title Disable DELETE method for all models except admins in API Disable DELETE method for all users except admins in API Aug 30, 2024
@shmonks
Copy link
Member

shmonks commented Aug 30, 2024

Fang's question at the top no longer applies

@fyliu
Copy link
Member Author

fyliu commented Aug 30, 2024

This issue needs rewriting due to change in scope.

@dmartin4820 dmartin4820 added the draft This issue is not fully-written label Sep 6, 2024
@dmartin4820 dmartin4820 moved this from 🆕New Issue Review to 🧊Ice Box in P: PD: Project Board Sep 6, 2024
@ethanstrominger ethanstrominger changed the title Disable DELETE method for all users except admins in API Implement delete security Sep 6, 2024
@shmonks shmonks added the dependency Issue has dependencies label Sep 20, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
complexity: medium dependency Issue has dependencies draft This issue is not fully-written p-feature: API private endpoint p-feature: API public endpoint question Further information is requested ready for dev lead role: back end s: CTJ stakeholder: Civic Tech Jobs s: hackforla.org stakeholder: hackforla.org website s: kb stakeholder: knowledgebase s: PD team stakeholder: People Depot Team s: VRMS stakeholder: VRMS size: 1pt Can be done in 4-6 hours
Projects
Status: 🧊Ice Box
Development

No branches or pull requests

3 participants