Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Roles.userIsInRole returns false in onBeforeAction #61

Closed
aldeed opened this issue Sep 30, 2014 · 18 comments
Closed

Roles.userIsInRole returns false in onBeforeAction #61

aldeed opened this issue Sep 30, 2014 · 18 comments

Comments

@aldeed
Copy link

aldeed commented Sep 30, 2014

Updated to latest iron:router and latest alanning:roles and Meteor 0.9.3, and now Roles.userIsInRole does not seem to work within a route's onBeforeAction.

this.route('admin', {
    onBeforeAction: function (pause) {
      if (!Roles.userIsInRole(Meteor.userId(), 'admin')) {
        console.log(Roles.userIsInRole(Meteor.userId(), 'admin'), Meteor.userId());
        this.redirect("home");
      }
    }
  });

I can see from the console.log that the user ID is correct, but userIsInRole is returning false. However, after it redirects, I can enter Roles.userIsInRole(Meteor.userId(), 'admin') in the browser console and it prints true.

So it seems like the roles are not being correctly determined until after onBeforeAction runs. Possibly I need to waitOn the subscription to the roles field, but I don't think I had to do that before.

@aldeed
Copy link
Author

aldeed commented Sep 30, 2014

Actually Meteor.user() is undefined, too

@aldeed
Copy link
Author

aldeed commented Sep 30, 2014

So based on that, it seems like an iron:router regression. I've submitted an issue there, but I'll keep this one open for now, too.

@alanning
Copy link
Contributor

Thanks Eric. One thing that is sorely lacking from the roles repo is an example that uses "iron:router". If you would like to contribute a simple example app, that would definitely be a big help. It would also give us a base to test with when trying out new releases.

On Sep 30, 2014, at 8:01 AM, Eric Dobbertin notifications@github.com wrote:

So based on that, it seems like an iron:router regression. I've submitted an issue there, but I'll keep this one open for now, too.


Reply to this email directly or view it on GitHub.

@alanning
Copy link
Contributor

alanning commented Oct 9, 2014

@aldeed, naming the subscription is fine with me if that will solve the issue. Won't help the underlying cause of no way to wait on "null" published collections of course but that's for MDG. :-)

Feel like making the change or want me to do it?

@aldeed
Copy link
Author

aldeed commented Oct 9, 2014

I wonder if Meteor.subscribe(null) returns a handle for all null subs? I haven't tried it yet.

It will be a little while before I'd have time to submit a PR, so feel free to make the change, or I will get around to it eventually.

@jhuenges
Copy link

Any updates?

@alanning
Copy link
Contributor

@jhuenges, just needs some love (ie. precious, precious time). This is still good-to-go if you'd like to submit a PR.

@jhuenges
Copy link

I ll take a look at it next week

@rhyslbw
Copy link

rhyslbw commented Jan 26, 2015

This appears to be resolved now

@jhuenges
Copy link

@rhyslbw what versions of the roles package and iron router are you using?

@rhyslbw
Copy link

rhyslbw commented Jan 27, 2015

@jhuenges
Meteor 1.0.3.1
iron:router 1.0.7
alanning:roles 1.2.13

I haven't actually tested this in isolation, and there is probably a long enough delay in my case to hide the problem.

@jhuenges
Copy link

Thanks, I ll check it out!

@leebenson
Copy link
Contributor

Try changing the doc's example server-side publication to:

Meteor.publish('roles', function (){
    return Meteor.roles.find({});
});

And then client-side, in Iron Router's waitOn()...

return Meteor.subscribe('roles');

That should wait for the full role list, so Roles.userIsInRole works correctly in onBeforeAction()

@aldeed
Copy link
Author

aldeed commented Mar 9, 2015

@leebenson, it still won't be correct unless this subscription is ready. So that publication should be named and the client sub handle should be exported. Yes, we could publish the roles field in a separate pub/sub, but that seems inefficient if the roles pkg is already publishing it.

@leebenson
Copy link
Contributor

@aldeed I see, didn't realise this was already explicitly published. For sure it needs naming to play nicely with packages that need to wait for subscriptions. Simple oneline fix. @alanning ?

@leebenson
Copy link
Contributor

Created pull request #88

@juliomac
Copy link

The pub/sub solution from @leebenson also worked for me. Thanks for it!

As for an up-to-date example for usage with iron-router, that would definitely be a good think. Meteor is evolving so quickly that it is like shooting on a moving target. None of the examples here or on StackHolder worked for me. I tested with this.pause(), render() or redirect() and finally got one working with Router.go(). Hope this helps someone else:

Router.route('/Client/Admin',
  {
    name: 'Admin',
    template: 'Admin',
    layoutTemplate:"layoutWithoutNav" ,
    onBeforeAction: function() {
      if (!(Meteor.userId())) {
        console.log("It is not logged-in");
        Router.go('atSignIn');
      }
      else if (!(Roles.userIsInRole(Meteor.userId(),['Admin']))){
          alert("need to be an Admin");
          console.log("Not an Admin");
          Router.go('home');
        }
          else this.next();
    }

@alanning
Copy link
Contributor

Thanks @juliomac. I did add both an iron-router and flow-router example recently but they are pretty simple so I don't think they run into this problem. Happy to accept updates that make the examples more realistic.

I'll be consolidating PRs and should be able to release a new version in around a month that will include Lee's PR. I'm also working on the design of roles 2.0 so if you have any requests, please open a new issue so we can discuss.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

6 participants