A setting can now be specified to restrict access to authenticated users. V2

[pranav377]

A setting can now be specified to restrict access to authenticated users.

[jkeifer]

Hey @pranav377, thanks for the contributions.

Before I dig into a review, I just want to note that if you have new changes to an existing PR it typically is better to simply update your branch rather than opening a new PR. That will keep the history of all comments pertaining to your changes together in one place. Even if you need to rebase, just do a force push to your branch and it will still update the current PR.

Anyway, I have looked through the changes and I have a few thoughts.

One, I very much prefer users create their own subclasses for easily-modifiable changes like this, rather than adding settings to this project. For example, if you wanted to change the property of the ChunkedUpload model, it would make more sense to subclass the AbstractChunkedUpload model and make your changes to your model. That same approach I believe can easily be used for the view in this case, like this:

class RestrictedUploadView(ChunkedUploadView):
    permission_classes = [IsAuthenticated]

Two, the ChunkedUploadView doesn't appear to gain a whole lot from adding the permission_classes when DRF_CHUNKED_UPLOAD_USER_RESTRICTED is True (the default). In that case, a request to list uploads will return an empty list, a get for a specific upload pk will return a 404 due to the existing user restrictions, and an attempt to PUT/POST an upload will result in a 400 with the message "Upload requires user authentication but user cannot be determined".

In light of this existing behavior, do you have any specific reason(s) you need the permission classes set to IsAuthenticated?

Third, the .gitignore change is undesirable; personal config like that does not belong in the repo. You can put your venvs outside project directories to avoid this (and keep them uniquely named, which you probably want), or you can name them all the same and add them to your global gitignore config.

[jkeifer]

@pranav377 Did you have any comment regarding why setting permission classes to IsAuthenticated is specifically required for you application? I am certainly open to further discussion here if you have a need, I just want to make sure the changes are well-considered.

[jkeifer]

I'm going to close this PR due to the lack of response, but if new information is provided I am more than willing to consider reopening it.