RFC: state machine guard function

[pechorin]

Hi everyone. I develop site with roles system. Sometimes user can access some states and sometimes not.

The easiest way to handle this constrains is:

// $currentUser -> service representing current user
$rootScope.$on('$stateChangeStart', function(event, toState) {
  if (toState.name == 'private_page' && $currentUser.canAccess(toState.name) == false) {
  event.preventDefault;
  alert("go away, please");
}
})

This is okay solution, but not in cases where we have many states and many different constraints. This produce many event handlers and personally i think what guard constraints is a "state machine" responsibility ;) Also i don't like idea to define "guard functions" in separate place. I think this constraints should be defined in-place like this:

$stateProvider.state('private_page', function() {
  parent: ..,
  url: ..,
  templateUrl: ...,
  controller: ...,
  guard: function ($currentUser) {
    if ($currentUser.canAccess('private_page') == false) {
      alert('go away please');
      return false
    } 
  }
})

I patch some internals and implementation looks pretty easy:

      // prevent event if guard returns false
      if (isFunction( to.self.guard )) {
        var guardResult = $injector.invoke(to.self.guard);

        if (guardResult == false) return TransitionPrevented;
      }

      // Broadcast start event and cancel the transition if requested
      var evt = $rootScope.$broadcast('$stateChangeStart', to.self, toParams, from.self, fromParams);

So if community like this idea i will provide patch and tests. The main motivation for me: "State machine should handle guard conditions by self".

[timkindberg]

I really like this! It's a great alternative option for guarding states.

However I do think you will be writing more code this way. Now instead of one $stateChangeStart handler you have to write a guard function for every state.

[laurelnaiad]

Perhaps allow it to be inherited down the state tree by default...

[nateabele]

There was already some talk about the ability to abort transitions in onEnter or onExit, which makes more sense than implementing yet-another-method.

[josh-hobson]

I am also really interested in this.

The code above looks great, however I would like to add in the possibility of a redirect. This would be useful if say you wanted the blocked user to be sent to a specific page, or to stop them being sent to your .otherwise() route if they we're navigating directly to this state.

$stateProvider.state('public_page', function() {
  ...
})
$stateProvider.state('private_page', function() {
  ...
  rule: function ($currentUser) {
    if ($currentUser.canAccess('private_page') == false) {
      if($currentUser.isReal == true) {
        var newState = {
          to: 'public_page',
          params: {}
        };
        alert('Taking you where you belong');
        return newState;
      else {
        alert('Go away please');
        return false;
      }
    } 
  }
})

if (isFunction( to.self.rule)) {
  var ruleResult = $injector.invoke(to.self.rule);
  if (ruleResult.to!= undefined) {
    return $state.transitionTo(ruleResult.to, redirect.params);
  } 
  else if (ruleResult == false) return TransitionPrevented;
}

I agree it should be possible to inherit this down the state tree, would this be easy to implement?

[nateabele]

Again, though: why not allow onEnter and onExit to abort transitions, rather than adding yet-another-method?

[josh-hobson]

The main reason I can think of is to avoid loading up resource intensive states only to find you don't need them.

Having looked through the code a bit more and taking in the potential for doing inherited rules, I think it would be possible to add it into the following:

var resolved = $q.when(locals);
for (var l=keep; l<toPath.length; l++, state=toPath[l]) {
  resolved = checkStateRule(state, resolved);
  locals = toLocals[l] = inherit(locals);
  resolved = resolveState(state, toParams, state===to, resolved, locals);
}

function checkStateRule(state, inherited) {
   // add rule checking to the promise chain
   // this may actually work better simply placed in the resolveState function
}

I feel other areas may need to be patched to fit, but this would allow the parent to resolve before the child's rule is checked. In a complex application this could easily catch a rule before a complex resolve is executed, hence increasing speed.

Another option would be to allow resolve functions to abort transitions and do redirects. This is where I went first to get this to work, but my redirect in the resolve was reverted when it got back to carrying out the initial transition.

[nateabele]

The main reason I can think of is to avoid loading up resource intensive states only to find you don't need them.

For as many instances where that's the case, there are likely just as many where one would need said resources in order to make that decision.

Furthermore:

app.config(function($stateProvider) {
  $stateProvider.state('privatePage', {
    data: {
      rule: function(user) {
        // ...
      }
  });
});

app.run(function($rootScope, $state, $currentUser) {
  $rootScope.$on('$stateChangeStart', function(e, to) {
    if (!angular.isFunction(to.data.rule)) return;
    var result = to.data.rule($currentUser);

    if (result && result.to) {
      e.preventDefault();
      $state.transitionTo(to, result.params);
    }
  });
});

...and, done.

No internal changes required, which means you have complete control over the implementation, which avoids future RFCs requesting additional configurability for whatever theoretical solution we might implement, which means I have more time and energy to spend implementing things this module isn't currently capable of, rather than merely making things that are already possible 10-lines-of-user-code easier.

[josh-hobson]

Nate,

I understand what you are saying and I completely agree that you shouldn't have to spend your time making things 10-lines-of-user-code easier. I am very grateful to you for supporting this library and all the work you have put into it. I am also sorry that this has turned into what essentially looks like a Stack Overflow problem.

However, I had already tried your solution above and it did not fix my problem. Here is a plunkr showing exactly the problem I am having: http://plnkr.co/edit/IwTn4q?p=preview

As you can see, if someone shortcuts the middle state (normally done through url routing), the app allows you to view the banned page. You can't then revisit this page since the resolve has updated the dataStore (this would normally be a data request, so the client wouldn't know the outcome until the resolve had completed). If there was some kind of broadcast telling me each resolve down the tree had finished, then that could potentially be used, but this seems awkward.

I also tried using $stateChangeSuccess to catch the change. Although this does fix the problem it has a visible flicker which could end up being longer if more was actually going on.

I am happy to try to work out a fix myself. I'm very new to the magical world of open-source but I am very keen to pull my own weight and thought this might be a good way to start out.

Let me know what you think? If there is an obvious way around the problem then I'm happy to use it. If not then we could discuss exactly what a theoretical solution should do and I can try my best to implement it :)

[nateabele]

I understand what you are saying and I completely agree that you shouldn't have to spend your time making things 10-lines-of-user-code easier. I am very grateful to you for supporting this library and all the work you have put into it. I am also sorry that this has turned into what essentially looks like a Stack Overflow problem.

No worries. The best solutions are produced by rigorous discussion and as a result, @timkindberg has a new code example for the FAQ.

However, I had already tried your solution above and it did not fix my problem.

Yeah, it was kinda off-the-cuff. Probably something wrong with the syntax. I'll take a look later tonight or tomorrow.

[pechorin]

@nateabele Sorry, this is not a best solution. The main problem is what i should define rules in separate place (file) and i don't like this separation. Please provide ability to abort transaction in onEnter callback. It will be fantastic to have api for this:

...
...
onEnter: function(event, $state) { event.preventDefault(); $state.go('previous_page') }

But this abort should be triggered before controller runs. This is main desire :)

Good example of state machine implementation: https://github.com/pluginaweek/state_machine

Is it reasonable for you?

[nateabele]

Good example of state machine implementation: https://github.com/pluginaweek/state_machine

@pechorin No, not quite. That's a good example of a classical FSM. That's a bad example of a hierarchical state tree of the kind ui.router uses. Totally different use cases and totally different implementations.

But this abort should be triggered before controller runs. This is main desire :)

Yup, that's the idea.

[pechorin]

My approach to handle state constraints:

https://gist.github.com/pechorin/7911719

the main idea is forget about angular-events and define constraints in dsl-like manner:

app.run(function($stateConstraints) {

  $stateConstraints.protect({
    state : 'myState',
    guard : function($currentUser, $state) { // <- any service
      // guard should return FALSE if transition to state should be cancelled
      if ($currentUser.isSignedOut()) { 
        $state.go('other')
        return false
      }
    };
    onFail: function() {
      alert("Signed users only");
    }
  });

})

[pechorin]

Okay, if you think that this kind feature is unnecessary and we should cancel state events inside controllers then i think i should publish this code as lib :)

[nateabele]

Yeah, I would definitely recommend creating a library for it. There are lots of different valid approaches to access control (of which this is one), and different people will want to do different things.

[josh-hobson]

Hey Nate, did you manage to have a have a look at my Plunkr? http://plnkr.co/edit/IwTn4q?p=preview

Unfortunately I think it means I can't use pechorin's method. I'm still tempted to write something custom but let me know if you still think using the onEnter function is the way to go.

[nateabele]

@josh-hobson I took a quick look over it, yeah, but didn't have time to think through the full implications of each possible approach.

To be clear, though, the onEnter/onExit thing is still just a proposal, and hasn't been implemented yet.

[pechorin]

@josh-hobson i fix your code: http://plnkr.co/edit/Iw7SnmcQllAIaNjVKdvc?p=preview now it works :)

[pechorin]

@nateabele can we help? :D

[nateabele]

@pechorin Yeah, you could work on getting the callers of onEnter and onExit (state.js, ~380-400) to check if the return value === false, and return TransitionAborted. Then write some tests to ensure the proper behavior, and ensure that nothing is left in an inconsistent state.

Let me know if you're interested in working on this, and if you need any help along the way, especially with the tests.

[timkindberg]

@timkindberg has a new code example for the FAQ.

...and added. I do so much.

[timkindberg]

Only thing that is (potentially) lame is that onEnter and onExit run after all resolves for the new states are resolved... good if you need the resolve, but bad if you don't, then its a performance problem.

[nateabele]

@timkindberg Yeah, that's the idea behind the solution I presented above. If you need something simple where rules can be attached to states, use that. If you need something more complex that requires resolves, use onEnter/onExit.

[timkindberg]

@nateabele ok yes I see. I'd wondered if these guys were thinking onEnter/onExit would be able to do it all somehow... but it won't.

[TheSharpieOne]

Just going to throw this out there since this is an RFC....

It would be nice to be able to defer the transition until after an asyc function can return judgement as whether or not the user is permitted. Say I need to validate something server-side, I need to make that call and when ever it gets done I can decide transition or not.

One should be able to return a promise and have the transition wait until it is resolved or rejected or something like that.

Also, it should be easy to override the toState in case one would wish to send them to some 401 looking state (Or some other specific state based on some logic)

[timkindberg]

@TheSharpieOne https://github.com/angular-ui/ui-router/wiki/Frequently-Asked-Questions#how-to-create-rules-to-prevent-access-to-a-state

[TheSharpieOne]

Thanks @timkindberg I guess for asyc calls, one could immediately event.preventDefault() then after the call is returned, in the callback do $state.transitionTo(newTo, result.params) This example should probably be added to the FAQs in that section linked, One would think it would be a common scenario.

Also, does $state.transitionTo trigger $stateChangeStart event? It seems like it would just keep looping if it did. I mean, it would also cancel the current state change and create a new one triggering it again and again. I made a quick little fiddle, and I get RangeError: Maximum call stack size exceeded

[timkindberg]

Hmmm, I guess you can't do async checks in $stateChangeStart... you can do them in $locationChangeSuccess though. https://github.com/angular-ui/ui-router/wiki/Quick-Reference#urlroutersync

[timkindberg]

@TheSharpieOne back to your original point, yes I now see, I agree that $stateChangeStart needs to be able to do async calls and then resume the transition, much like $urlRouter.sync(), maybe event.halt() and event.resume()?

[laurelnaiad]

If you use the preventDefault technique then presumably you're supposed to satisfy some condition before you call transitionTo so the next time stateChangeStart fires you wouldn't run your preventDefaultstuff again right?

[mfield]

@TheSharpieOne is this what you mean by returning a promise and waiting for it to be resolved or rejected? http://fiddle.jshell.net/L9jxf/1/

[TheSharpieOne]

@mfield Yes, just like that or at least very similar. That is a very interesting place to put it. It fits the needs for my scenario and works in the current solution, though it would be better if it was somewhere more abstracted.

[timkindberg]

@stu-salsbury yes that's what I was thinking too, but I think I came to the conclusion that we would need some additional methods other than $state.go to use within the $stateChangeStart handler, because calling it will result in a loop like @TheSharpieOne points out.

So I had some ideas:

• event.pause() to temporarily pause the transition allowing you to perform async calls.
• event.resume() to optionally resume the transition after the async/guarding logic is complete.
• event.halt() may also be needed to completely stop the transition, same as preventDefault(), just an alias.

[timkindberg]

Wait hold on...

@TheSharpieOne Can't you just set the notify option of $state.go() to false within the $stateChangeStart handler in order to prevent the infinite loop? Maybe that's the solution right there...

[laurelnaiad]

I still don't get why the condition that caused the initial "side route" to be taken wouldn't be cleared up before you re-enter $on.('$stateChangeStart'... but @timkindberg's idea to set notify to false sounds like a nice way to emphatically say that there shouldn't be any $stateChangeStart-based conditions on the transition.

[TheSharpieOne]

Does setting notify to false prevent the the view from being updated? This fiddle uses the notify false when the state change should "resume" and it works great. It will not go through an infinite loop. But the view also never gets updated... If you remove the 15-31 code block, it will show the view. While the code is there, you can follow it and see that it is hitting the transition with notify false and then never updates the view.

[timkindberg]

@TheSharpieOne Oh crap, yeah, the view updates itself by listening to the state event. Damn... so I'm thinking we have some options: A) We change it so that we broadcast the view related event even when notify is false. Not sure if this ruins some use case where you'd want to not update the view. B) We implement my idea from my comment above

@stu-salsbury I think you may be wrong, here's a breakdown of the steps that happen:

1. $stateChangeStart called
2. e.preventDefault() called right away, transition stopped for good
3. async logic performed, results in one of following:
   • a. Stay on current state (aka not allowed to access 'to' state)
   • b. Proceed to 'to' state (aka allowed to access it), return to step 1
   • c. Redirect to other state, return to step 1

So if you are technically allowed to access the 'to' state you'll continuously be "allowed" to go to it, but never get there, because the state will constantly be preventDefault'ed. Am I missing something?

[dlukez]

@timkindberg @TheSharpieOne It looks like the view may no longer update via the state event on this branch: https://github.com/angular-ui/ui-router/tree/viewpath. Perhaps the issue being discussed will be solved more easily on top of the refactoring @nateabele has been doing?

[timkindberg]

@dlukez interesting. That would mean notify false would then work.

[laurelnaiad]

@timkindberg -- I was under the assumption that something in the general state of the application (e.g. some value in a service property such as myDeflector.dontDeflect == true) would indicate that preventDefault shouldn't get in the way, once you've satisfied whatever async stuff you needed to do.

[timkindberg]

@stu-salsbury ah ok, I think I see now.

[laurelnaiad]

Your notify: false idea is better anyway!

[timkindberg]

Did we even decide anything here??

[jnapier]

@timkindberg -- Being able to support async calls in $stateChangeStart would be a great feature for me. Did this request ever go anywhere?

I currently use a state filtering system to guard states that I ported over from ng-route. Filters can be chained together and the whole filtering system works with promises. Currently I have the filters run as a resolve. This works great because resolve works with promises. However, this gets complicated once child states define additional resolves that get data since all resolves execute at the same time. You can wrap those additional resolves with the filters but then they execute twice which I would like to avoid. If $stateChangeStart dealt with promises then this would be much easier to deal with. I think your ideas above would work well and provide the capability to handle a great many scenarios to prevent, delay, or continue state transition. These ideas were

• event.pause() to temporarily pause the transition allowing you to perform async calls.
• event.resume() to optionally resume the transition after the async/guarding logic is complete.
• event.halt() may also be needed to completely stop the transition, same as preventDefault(), just an alias.

I think event.halt() should take a rejection reason though so that custom rejection data could be passed along to $stateChangeError.

Here is a sample of what could potentially be done if $stateChangeStart event supported your proposal and adjustments to halt. http://plnkr.co/edit/Uje7kbvZfRX37yTyeEV6

This plunker doesnt work but its an idea of what could be. This sample would block the state transition until a current user was logged in and a permission could be checked.

[nateabele]

So far as I am aware, attaching custom methods to the Event object is not possible in Angular's current event system.

[laurelnaiad]

It seems like @jnapier might be implying that those functions on the event object could be created and supported outside of Angular's purview and with no help from it? You could presumably attach them to a parameter instead, since that is what you have control over.

[jnapier]

@nateabele - I was just following @timkindberg's lead. @stu-salsbury is correct, that event object doesn't necessarily need to be the one that indicates state transition control flow . An additional argument could be used to indicate control flow. Ui-router just needs to know what to do on pause, resume, and hault. All that would have to happen in $state.transitionTo

[timkindberg]

Hey I just come up with the big ideas. I don't think about their feasibility! Technically we could extend the event object though it may not follow convention.

[nateabele]

Technically we could extend the event object though it may not follow convention.

@timkindberg There is no reliable way to extend an event object. When you dispatch an event, the event object is factoried inside Angular core, and the dispatching code has no chance to touch it. The event is then routed to the first configured event listener. Hence, the only way to manipulate the event object is through a listener, and there is no way to ensure that a listener applied in ui-router itself would supersede a user's. Hence, no way to ensure that a hypothetical event object method exists before it is called.

[dmackerman]

I seem to be getting Maximum call stack size exceeded when I try and do a very simple catch inside of $stateChangeStart.

.controller('AppCtrl', function ($scope, $state, $kinvey) {
  $scope.$on('$stateChangeStart', function(event, toState, toParams, fromState, fromParams) {
    if ($kinvey.getActiveUser() === null) {
      $state.transitionTo('login');
      event.preventDefault();
    }
  });

Is there any workaround for this? AppCtrl is my main controller that I bootstrap like:<html ng-controller="AppCtrl">

If I put a log inside the conditional, it gets called nearly 1000 times?

[TheSharpieOne]

@dmackerman When you transition to login, it is probably triggering the $stateChangeState event and then checking if $kinvey.getActiveUser() is null and then tranisitioning to login again and then checking if $kinvey.getActiveUser() is null and then tranisitioning to login again and then checking if $kinvey.getActiveUser() is null and then tranisitioning to login again and ... x1000 and then boom.

You could set notify to false in the options that are passed... it would prevent it from triggering the $stateChangeState event when you whisk the user away to login.