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

Prefix while enqueuing #135

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

ashishtilara
Copy link

to add one more level of saperation, (e.g. If you have a class between application and resque, the class can handle this, so a prefix can be userid or something in background, so one user do not access another user's queue status items)

to add one more level of saperation, (e.g. If you have a class between application and resque, the class can handle this, so a prefix can be userid or something in background, so one user do not access another user's queue status items)
@danhunsaker
Copy link
Contributor

First, apologies for incorrectly stating that this change would affect interoperability with Resque Web - job status tracking isn't a feature of Ruby Resque, near as I can tell.

Now, this separation is only for job status (and return value, if PR #134 gets merged) - other users will be able to see other jobs if they check the queue directly. This probably isn't a significant problem, but I can see scenarios (by virtue of actually facing many myself) where the request payload contains sensitive data but the return value does not, making this type of security incomplete.

That said, I suspect that most software using PHP-Resque searches for jobs by status rather than by queue-diving, so it's still a reasonable measure to take. As I understand it, the purpose of this proposed change is to provide isolation between users of a single pool of PHP-Resque workers, so you don't have to spin up a new worker set for each new user. If that's not the case, the existing global prefix would suffice, as the entire PHP-Resque environment is then isolated from any other. Of course, even that is only marginal security since anyone with a Redis client and network access to the server can pull every key with ease, but that's beyond the scope of this project.

Ultimately? I don't really see any problems with this PR being merged in. Existing code would be unaffected since the empty prefix is the default. The only tricky bit is likely to be presenting the need for this modification well enough to get it merged. It doesn't break anything, but the utility is a bit on the questionable side, all things considered.

Let's see what happens!

@ashishtilara
Copy link
Author

Thank you for your comments @danhunsaker, really appreciate your help on this. 👍

@arturo-c
Copy link

looks good 👍

danhunsaker added a commit to resque/php-resque that referenced this pull request Dec 11, 2018
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

Successfully merging this pull request may close these issues.

3 participants