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

get_progress endpoint allows retrieval of arbitrary tasks #83

Open
hansegucker opened this issue Apr 30, 2021 · 7 comments
Open

get_progress endpoint allows retrieval of arbitrary tasks #83

hansegucker opened this issue Apr 30, 2021 · 7 comments

Comments

@hansegucker
Copy link

Any user, independent whether they are logged in or permitted, can get data about any task using the get_progress endpoint.

@OmarWKH
Copy link
Collaborator

OmarWKH commented May 2, 2021

To do this I wrapped the get_progress view in my own view that requires authorization. My view would take a task id, does authorization, then forward the id to celery_progress.views.get_progress and return its result.

The authorization step depends on how your project is set up. Just protect your view like any other.

And in urls.py, I hooked up my own view instead of celery_progress.urls.

@czue
Copy link
Owner

czue commented May 2, 2021

Agreed with the above. Though i do wonder if there's a way to improve the situation out-of-the-box via a few settings or something. certainly at least adding a setting to require auth would be trivial. I think limiting it to the specific user would be possible, but a bit more work.

At a minimum I think this behavior should be more clearly documented so that it doesn't trip people up.

@hansegucker
Copy link
Author

First of all, thanks for your fast responses 👍🏻.

@OmarWKH Do you limit the task fetching to specific users or do you allow it just for all logged-in users? If yes, it would be nice to know how you save the information which task belongs to which user.

@czue I think some kind of information like the one provided by @OmarWKH would be already fine.

@OmarWKH
Copy link
Collaborator

OmarWKH commented May 2, 2021

@hansegucker Specific users.

I had a Task model that stored users and tasks in the database. I made sure to delete the row and forget the task when I'm done with it.

I wrote the code a while ago so I don't want to give details that I could be misinterpreting. Also my solution was a bit different to fit my use case. So here are some notes that might not be applicable:

  • I used transaction.on_commit in the view to start the task.

  • I pre-generated the task id so I can store it before the task starts.

  • I used on_success and on_failure in the task to call a function that updates a model I had. That function also schedules a task that 1) deletes the Task row and 2) forgets the original task after a while.

This never went to production so I don't know if this is the best way to do it.

#50 has a relevant discussion.

@timnyborg
Copy link
Contributor

Wrapping the endpoint is easy enough that I'd suggest adding some documentation with a couple examples (fbv, cbv)

For example:

from celery_progress.views import get_progress

class GetProgress(LoginRequiredMixin, generic.View):
    """A wrapper around celery-progress' get_progress view, to require login"""

    def get(self, request, task_id: str, *args, **kwargs):
        return get_progress(request, task_id=task_id)

Happy to contribute if it'd be welcome.

@czue
Copy link
Owner

czue commented Oct 8, 2021 via email

@timnyborg
Copy link
Contributor

I've added some examples in #90, so at least the behaviour would be documented

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

No branches or pull requests

4 participants