Filter states for which login is required during state change

[igorauad]

A state parameter was added for the routes that require user authentication. Now, everytime a statechange occurs, the destination state is checked and user is redirected to signin page if necessary (without flickering). Note the state parameter is added within data, so that nested states can inherent its value.

This would conflict with #487 . However, I am anticipating the PR, so that I can receive some feedback.

[rhutchison]

@igorauad I don't see them as conflicting, but supplementing eachother. I think this is a good PR, but I think it should be taken further. I am looking at client-side roles/security, not just authentication required, but I believe this is a good step.

[lirantal]

if it looks good and makes sense, let's push it in, and further work can be done on 0.5.0.

Except for a couple big PRs like the Admin Mode and User Refactor I don't intend on adding major PRs/changes to 0.4.0 because we need it out of the door as soon as we can.

[igorauad]

@lirantal @rhutchison do you think here we should check the role of Authentication.user? It would make easy for future expansion towards role-based access control. I mean, in the stateProvider configuration, instead of data: { requiresLogin: true }, we could have for example data: { allowedRoles: ['user'] }. Should I do that?

[trainerbill]

This is very nice and exactly what I was talking about in the AdminModule PR. Do we have to have it nested in data? why not just do a roles:[] on the state and check indexOf. Or have

authentication: {
  login:true,
  roles: ['user', 'admin]
}

[igorauad]

Thanks @trainerbill . The reason why it is nested in data is because of inheritance on nested states (see ui-router documentation). Furthermore, I guess login: true can be implied by roles not being anything other than ['guest']. WDYT?

[trainerbill]

good to know. I pretty much forgot about the guest role... That is way better. We have an admin module going in here: #676

We want to be able to restrict it to only those with the admin role. So I would toss something in like if roles.length > 0 then check to make sure that the Authentication.user has all of the roles that were passed into the state. Also if it is a "role based restriction" i would not send them to authentication but keep them on the same state.

[igorauad]

@trainerbill, @rhutchison I'm aware of #676, this is why I thought such transition would be useful. I just made an initial step. I haven't completed, because I want to hear what you think.

[mleanos]

+1 I prefer the whitelist strategy as well.

[igorauad]

Ok, @mleanos @trainerbill I've updated

[trainerbill]

I don't think this will work. You will need something like this:

https://github.com/trainerbill/mean/tree/requireLogin

I can PR if you want. Was not able to find your fork to compare and send you a PR. Otherwise you need to refactor the app.js like I have. It checks all the roles in the state to the users roles.

[trainerbill]

This should work as is because if roles are not on the state it is allowed and assumed guest. So you shouldn't even need to check for guest. Only restrict if data.roles is present.

[rhutchison]

@igorauad can you please work with @trainerbill to finalize this PR. You are very close. Please join gitter chat if you want to discuss