Custom override loading - How to handle errors?

[Splines]

Hi there, I've got a question regarding custom override loading, which is described here in the great documentation. An example is listed there:

class BooksController < ApplicationController
  before_action :find_published_book, only: :show
  load_and_authorize_resource

  private

  def find_published_book
    @book = Book.released.find(params[:id])
  end
end

Now imagine we have an update method. And let the ability depend on the parameter retrieved in the find_... method, e.g. imagine that it depends on the book whether the user has rights to update it or not.

Then what is the preferred way to handle the case where the find_published_book method cannot find the book? We should not give an error message like "Book with requested id does not exist", right? Since otherwise, malicious actors could infer from trial and error via calling the endpoint many times, that some IDs exist and others not (only a problem if you don't use UUIDs).

How are we supposed to handle such an error gracefully anyways? I haven't found a section for this in the docs so far and would appreciate some help from the community which is more experienced in using CanCanCan. Thank you ;)

Tip

TL;DR:

• have an update method in a controller
• have an ability that depends on the resource to update to determine whether user has rights or not
• What's a "good way" to handle the case where the resource to update cannot be found and thus the ability cannot be evaluated?

Further background information

To avoid XY problems, here's some more background information. At MaMpf, we currently introduce a voucher system. For your information: MaMpf is an e-learning platform for mathematics. It has the notion of lectures (like lectures at university). Teachers can assign tutors for their lectures (tutors are people that will help other students and correct their homework). Teachers shouldn't be able to choose from all users of our entire database when they want to assign who will be a tutor. Therefore, we want to introduce the system of vouchers. Teachers can generate a secure hash and users can redeem this code in their profile to become tutors of the lecture.

We have an ability like this:

can [:create, :invalidate], Voucher do |voucher|
  user.can_update_personell?(voucher.lecture)
end

And as you can see, it depends on the lecture that the voucher belongs to, whether the user is allowed to create or invalidate it. E.g. the user is allowed to do so if they are a teacher (then can_update_personell? is true). For the invalidate controller action, we use a custom before_action find_voucher:

def find_voucher
  @voucher = Voucher.find_by(id: params[:id])
  return if @voucher

  handle_voucher_not_found
end

And the question is basically, what a sensible implementation of handle_voucher_not_found should do. I hope this wasn't too confusing 😅

[walterdavis]

404 is a perfectly cromulent response to any content that doesn't (from the perspective of the visitor) exist. I mean what else would you expect? Now if you are using numerical ids in your URLs, and they correspond with an auto-incrementing primary key, then, as Adam Savage likes to say, "There's your problem right there!" The RDBMS primary key is of interest and importance to your database engine, tangentially important to your application, and of no possible interest to your site visitors. Pick something else to use in public to identify each page in your site. Make it something human-meaningful that you can change if you want to, but please, don't change URLs without a strategy for keeping them corrected and up to date. The point of using CanCanCan (in conjunction with a role scheme or similar) is that you can have different privilege levels for different user types. An admin or editor won't see a 404 on a non-published page, because they need to get at it to either preview it or to manage it (maybe to set it to "published" it for the first time). But a regular user or anonymous user won't see anything but a 404, because that's the literal truth -- it doesn't exist from their point of view. Walter

…

On Sep 5, 2024, at 6:41 PM, Splines ***@***.***> wrote: Hi there, I've got a question regarding custom override loading, which is described here in the great documentation. An example is listed there: class BooksController < ApplicationController before_action :find_published_book, only: :show load_and_authorize_resource private def find_published_book @book = Book.released.find(params[:id]) end end Now imagine we have an update method. And let the ability depend on the parameter retrieved in the find_... method, e.g. imagine that it depends on the book whether the user has rights to update it or not. Then what is the preferred way to handle the case where the find_published_book cannot find the book? We should not give an error message like "Book with requested id does not exist", right? Since otherwise, malicious actors could infer from trial and error via calling the endpoint many times, that some IDs exist and others not (only a problem if you don't use UUIDs). How are we supposed to handle such an error gracefully anyways? I haven't found a section for this in the docs so far and would appreciate some help from the community which is more experienced in using CanCanCan. Thank you ;) — Reply to this email directly, view it on GitHub, or unsubscribe. You are receiving this because you are subscribed to this thread.

[Splines]

Thanks Walter, a 404 makes a lot of sense then as we're using secure UUIDs anyways here 👍

Pick something else to use in public to identify each page in your site. Make it something human-meaningful that you can change if you want to [...].

What do you mean by "Pick something else" than a numerical id in your URL? Not every website is as simple as /about. We have some URLs like /lectures/42 or /media/45654/play and there's no way around including some kind of numerical reference in the URL itself for these cases. It wouldn't make sense to use something "human readable" instead like /media/title-of-media/play since the title does not need to be unique. But of course, a UUID would be a fitter fit here.

[walterdavis]

I use FriendlyID for a lot of my sites. That offers me the ability to use something more meaningful instead of a numerical ID without stretching my brain too hard. To use your example, in a CMS, there might be a title, or a date for a lecture, or some other unique and meaningful description that you could work with. /lectures/life-the-universe-and-everything ...gives the search engine spiders something to chew on, and all you had to do to get it there is title that lecture Life, the Universe, and Everything. If you happen to forget that there already was one named that, there are many different fallback options you can choose from, or just use the default: life-the-universe-and-everything-[a SHA hash here] Two benefits of this system right off the bat, besides the spider food angle: 1. It completely eliminates any chance of someone successfully iterating numerical IDs for fun and profit. 2. If you decide to change the title later, the history module will store the old slug[s] while your model's record will store the new one, and if someone uses the old one, it will still show them the correct content. Plus, it just looks nicer to people who have their browsers set to show the entire URL. Anyone? Just me? Harumph. Get off my lawn! Walter

…

On Sep 6, 2024, at 5:09 PM, Splines ***@***.***> wrote: Thanks Walter, a 404 makes a lot of sense then as we're using secure UUIDs anyways here 👍 Pick something else to use in public to identify each page in your site. Make it something human-meaningful that you can change if you want to [...]. What do you mean by "Pick something else" than a numerical id in your URL? Not every website is as simple as /about. We have some URLs like /lectures/42 or /media/45654/play and there's no way around including some kind of numerical reference in the URL itself for these cases. It wouldn't make sense to use something "human readable" instead like /media/title-of-media/play since the title does not need to be unique. But of course, a UUID would be a fitter fit here. — Reply to this email directly, view it on GitHub, or unsubscribe. You are receiving this because you commented.