Ember-simple-auth: Add the ability to make all routes use AuthenticatedRouteMixin by default (except the auth route)

[mdentremont]

It would be nice if there was an option to automatically make _all_ routes require authentication (except for the authentication route).

This would prevent issues where a dev forgets to add the Authentication mixin. One could then use the UnauthenticationRouteMixin to explicitly not require authentication on certain routes.

[marcoow]

Would you have sth. in mind how that might look? Maybe you can come up with a PR?

[mdentremont]

I'll preface by saying the following is a nasty hack, but it demonstrates one way of implementing the desired functionality:

import Ember from 'ember';

import Configuration from 'ember-simple-auth/configuration';
import UnauthenticatedRouteMixin from 'ember-simple-auth/mixins/unauthenticated-route-mixin';

export default Ember.Route.reopen({
  session: Ember.inject.service('session'),

  beforeModel(transition) {
    if (Configuration.requireAuthenticateByDefault) {
      if (this.routeName != 'application' && (!UnauthenticatedRouteMixin.detect(this)) &&
          !this.get('session.isAuthenticated')) {
        Ember.assert(
            'The route configured as Configuration.authenticationRoute cannot implement the AuthenticatedRouteMixin mixin as that leads to an infinite transitioning loop!',
            this.get('routeName') !== Configuration.authenticationRoute);

        transition.abort();
        this.set('session.attemptedTransition', transition);
        this.transitionTo(Configuration.authenticationRoute);
        return;
      }
    }
    return this._super(...arguments);
  }
};

[mdentremont]

We might be able to add the following code to the ApplicationRouteMixin in order to add the desired functionality:

  beforeModel(transition) {
    if (!config['simple-auth'].authenticatedRoutes || this.get('session.isAuthenticated')) {
      return this._super(...arguments);
    }

    if (transition.targetName === Configuration.authenticationRoute) {
      Ember.assert(
          'The route configured as Configuration.authenticationRoute must not implement the AuthenticatedRouteMixin mixin, as that leads to an infinite transitioning loop!',
          !AuthenticatedRouteMixin.detect(this));

      return this._super(...arguments);
    }

    for (let handlerInfo of transition.handlerInfos) {
      if (handlerInfo.name === 'application') {
        // Skip the application route
        continue;
      }

      if (!UnauthenticatedRouteMixin.detect(handlerInfo.handler)) {
        // Allow transition if one of the routes is an unauthenticated route
        this._redirectToAuthenticationRoute(transition);
        return;
      }
    }

    // Only "unauthenticated routes" in the stack, proceed
    return this._super(...arguments);
  },

  _redirectToAuthenticationRoute(transition) {
    transition.abort();
    this.set('session.attemptedTransition', transition);
    this.transitionTo(Configuration.authenticationRoute);
  }

[marcoow]

I think the best solution is probably to just hook into the router DSL as torii does it. As you have to define all routes in app/router.js anyway, it's no extra effort to decide whether the route should authenticated or not there. I'm thinking sth. like this:

Router.map(function() {
  this.unauthenticatedRoute('login'); // automatically mixes in the UnauthenticatedRouteMixin
  this.authenticatedRoute('protected'); // automatically mixes in the AuthenticatedRouteMixin
  this.route('about'); // the regular routing method, Ember Simple Auth doesn't change that
});

[ryanlabouve]

Sweet! I'll get a PR for this by end of week.

[mtangoo]

lovely!

[mdentremont]

@marcoow: I really like the propose solution!

One thing that does concern me is what happens to auto-generated routers, which are typically not defined in the router.js. An example of these would be the loading routes.

I suppose one solution would be to explicitly define them in the router.js?

[marcoow]

Hm, the loading routes would be sub routes of the authenticated routes and thus would be authenticated automatically I think as the parent route's beforeModel should be executed before they display. Would be sth. that should be verified of course and we should probably have a test for that.

[mdentremont]

@marcoow Unfortunately the behaviour I'm seeing is that loading routes (when hit directly via the URL in the inspector) do not trigger their parent route's beforeModel 😿

[marcoow]

Hm, ok. We can probably handle that in the router DSL method though.

[ryanlabouve]

Finishing up tests on my first pass implementation. Will add some for this use case @mdentremont and push up.

[mike-north]

If you have a route hierarchy

  / \
 B   C

Route A's beforeModel hook should be called (and be resolved) before route A's loading substate is entered (A.loading). The parent loading state (loading) should be in place while any promises from route A's promise-aware hooks are unresolved.

Unfortunately the behaviour I'm seeing is that loading routes (when hit directly via the URL in the inspector) do not trigger their parent route's beforeModel 😿

There are plenty of things that users could cause trouble with when using dev tools :) Is there any more realistic situation you can identify where users, while not authenticated, can access a child route of an auth-protected parent route?

[mdentremont]

The issue I'm worried about is when hitting the loading route directly (via the URL which is visible in the Ember inspector). It definitely isn't the biggest issue in the world though.

[marcoow]

Sure, should still behave consistently though.

[mdentremont]

@marcoow Looks like the loading routes might not be an issue, I think I was just missing a route and was potentially relying on an index route being used.

[mdentremont]

@marcoow One last edge case: It seems like there is a "routename_loading" route which is generated for each of the root routes (at least in our app), and I can't seem to get those to use the "AuthenticatedRouteMixin" in any way.

[mike-north]

@mdentremont this is working as intended imo. The *_loading route should rely on the authentication check in the beforeModel hook from the parent route -- it shouldn't need to do any authentication check on its own.

[marcoow]

closing this as it will be no longer needed once we have #971