Including routes from shards

[matthewmcgarvey]

The way lucky routes work, routes are essentially "self-registered". By declaring a route in a file and requiring the file, the route gets loaded. That's fine for routes made within the app, but consider routes that are defined in shards.

Say I created an admin dashboard shard that creates routes to allow you to manage all the things in your app. If I create those routes the normal way, those routes will be included as soon as the shard is required and will be publicly available. Ideally, they would be behind an admin login but with the way things work today, it's up to the shard to implement that functionality or to monkey-patch it in.

You can see this with breeze right now, as soon as breeze is required the endpoints exist and it is up to the provided shard configuration to remove the routes if the user doesn't want them on in certain conditions. https://github.com/luckyframework/breeze/blob/5a98f72cd0d5282b6aeb71d1e02fe70ae7c06456/src/breeze/actions/breeze_action.cr#L8

Is this the way we want to keep going or do we want to consider alternatives?

[matthewmcgarvey]

shield comes to mind since it has a bunch of modules that you have to include in your own routes to get the functionality instead of defining the routes.

Example: https://github.com/GrottoPress/shield/blob/c0263ca0a7efdee40a9e0c47094c07fc398c591e/src/shield/actions/current_user/new.cr#L1

[jwoertink]

I actually don't mind how it's currently done. To me it makes sense that as soon as you require the shard, you have the routes. No need for extra configurations. With Breeze, we basically want each new extension to manage it's own additional routes. You include the extension, then you get the added routes. I think the current way also gives us the added type-checking at compile-time.

[matthewmcgarvey]

The problem comes up in my mind when considering admin-related shards. Are they to implement their own auth mechanism, or do they rely on authentic? What about shield? That's where I'm thinking that being able to "mount" shard routes would be better since you could wrap it in your own auth

[jwoertink]

I think if you're making an admin shard, then it's also your choice if you want to handle it or not. The shard could say "We give you the routes, but no security, that's up to you", or the shard could say "We have baked in auth using Shield". The main issue there would be more with the models than the routes. If I'm using User already, and the admin shard wants to add a User models with special admin columns, then it becomes tricky.

[robacarp]

The way rails handles this with mountable engines has worked really well for me over the years. This gives the module maintainer the ability to manage their own routes, which decreases friction around updating dashboard-like modules. It also provides an end-user (end-developer, I guess) managed route namespace so collisions are avoided.

[stephendolan]

Agreed, I love Rails' implementation of this because it puts both authentication and authorization in the hands of the app creator, since that will be different among users of a given shard.

Obviously having one routes.rb file makes this a bit easier on their end, but my brain wants to write something like this:

class MyBreezeRoutes < Breeze::Routes
  get Breeze::IndexPath do
    authorize

    yield
  end

  get Breeze::SettingsPath do
    authorize

    yield
  end

  private def authorize
    raise UnauthorizedAccessError unless accessing_user_is_admin?
  end

  private def accessing_user_is_admin?
    user = current_user

    user && user.is_admin?
  end
end

I'm sure that the macro-based nature of our routes makes that a bit more difficult to actually accomplish, but I agree that allowing maintainers to structure the routes, but ignore auth completely, is really valuable.

The reason I think doing multiple routes per files is valuable is so that these are a bit easier to manage and don't bloat your app files with a bunch of stuff that people may never touch. This single file could be added with something like lucky breeze.init.