Oh, reading this again it seems I was wrong.
How would it guess the associations?
Oh, reading this again it seems I was wrong.
How would it guess the associations?
Indeed
The way that many apps deal with this pain is using shortcut urls like /issues/5 that simply get the indicated record, look up the parent records, and then figure out permissions based on the "implicit" hierarchy.
I think it make a lot of sense. I'd +1 on this.
I think since we already know what portion of the path is called, we can do something simple like:
if record.respond_to? :project path_portion[1] = record.prefix end
(that's psudocode btw, the actual impl will be more complex. Just to get you the idea.)
- Prem
One reason that the code to generate all those URLs seems not to be DRY might be that any URL that can be programmatically deduced from the model at the end of the chain is itself non-DRY. i.e. /users/1/projects/2/issues/3 adds no more information than /issues/3 would in that case. Obviously in some cases you want nested routes to provide a more human-readable URL, but there are real security pitfalls to using nested routes if you don’t unpack the URLs correctly and validate the associations in your receiving controller.
I wonder whether a bit of vinegar might actually be a good thing in this case? If you really want deeply nested routes, you have to do a little more leg work, because the simpler and safer way is to avoid nesting. Thoughts?
-john
John,
I feel like there definitely would be some security risk but I can’t think of a real, solid example. Can you lay one out for me?
As for actually implementing this, my train of thought is to use ActiveRecord::Reflections to keep checking what a model belongs_to and then calling that association.
- Michael Boutros
As for actually implementing this, my train of thought is to use ActiveRecord::Reflections to keep checking what a model belongs_to and then calling that association.
There is no sane way to guess which association to use. “Issue” from your example may as well belong to category, author or other issue (and many other things). Also, such implementation would be rather slow, complicated and would depend on ActiveRecord’s API, while we try to only depend on ActiveModel.
I believe that the implementation should look like the example provided by Prem. In such scenario developer would be responsible to set the prefix for given record, which is good from various reasons:
-
we won’t break any existing APIs
-
it should be pretty fast considering that it will just add respond_to? call for people not using this feature
I have mixed feelings about that, as I don’t use nested resources heavily, but I can see that it could make it easier to manage nestings in some of the apps.
I feel like there definitely would be some security risk but I can’t think of a real, solid example. Can you lay one out for me?
Hi Michael, here’s one example. Let’s say that you must be the creator of a project to add issues to it (perhaps contrived in this case, but many access control use cases fit this kind of pattern). This would be a vulnerable implementation of an issue create action because an attacker could set a different user_id in the url:
def create
@user = User.find(params[:user_id])
raise “alert!!” unless @user == current_user
@project = Project.find(params[:project_id])
@issue = @project.issues.new(params[:issue])
if @issue.save
head :no_content
else
head :unprocessable_entity
end
end
It’s easy to dismiss as an obvious mistake, but I’ve seen it done too many times to want to encourage deeply nested routes by automating the URL generation. If your controller is forced find the user from the project in order to perform access control, then the odds of making a mistake are much lower:
dev create
@project = Project.find(params[:project_id])
raise “alert!!” unless @project.user == current_user
@issue = @project.issues.new(params[:issue])
if @issue.save
head :no_content
else
head :unprocessable_entity
end
end
-john
Curious - is there a reason to prefer this code to the alternative:
@project = current_user.projects.find(params[:project_id])
which turns attempts to access other's projects into 404s?
--Matt Jones
Hi Matt,
Nope, that would be the Correct way to do it, I was demonstrating a simplistic buggy implementation that I’ve seen people new to Rails use in the past. I should reiterate that it’s not impossible to write secure code that uses deeply nested routes (I’ve used them myself when appropriate), it just increases your attack surface by giving you more user input to validate, and is among the reasons that deeply nested routes aren’t the best way to go for many use cases.
-john
The only thing I really have to add to this conversation is that your routes are not necessarily mapped one to one to your models. You might have a nested url that is actually a different association but there's also an association matching the url - how do you tell it not to use the automatically discovered one? Tying urls and models together means giving up a certain degree of flexibility.
Also the `resources` and `resource` definitions in routes.rb are just macros to reduce the amount of typing needed to define RESTful url patterns. This is perfectly illustrated by the form_for/singleton resource issue (#1769).
Andrew White
