czue / celery-progress

Drop in, configurable, dependency-free progress bars for your Django/Celery applications.
MIT License
463 stars 90 forks source link

get_progress endpoint allows retrieval of arbitrary tasks #83

Open hansegucker opened 3 years ago

hansegucker commented 3 years ago

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

OmarWKH commented 3 years ago

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 commented 3 years ago

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 commented 3 years ago

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 commented 3 years ago

@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:

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 commented 2 years ago

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 commented 2 years ago

Very welcome!

On Fri, Oct 8, 2021, 20:21 Tim Nyborg @.***> wrote:

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.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/czue/celery-progress/issues/83#issuecomment-939032879, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAAQH6ZFS5U4LL6ELKM6DJ3UF4ZCDANCNFSM435LQPFA . Triage notifications on the go with GitHub Mobile for iOS https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675 or Android https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub.

timnyborg commented 2 years ago

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