lindleycb / meteor-stale-session

Stale session and session timeout handling for meteorjs
MIT License
40 stars 20 forks source link

Doesn't work if my publications are referencing this.userId #7

Closed wesyah234 closed 9 years ago

wesyah234 commented 9 years ago

This package was working great for me until I added this:

  if (!this.userId) {
    console.log("subscribing without being logged in, from: " + this.connection.clientAddress + " denied!");
    return;
  }

to the top of my Meteor.publish function.

Once I added these lines (as a security check so that you coudn't subscribe to publications when logged out)... it seems that when I get to a stale session state, the websocket is severed, but my app still functions, but it can't get to the server any more... I suppose I'll have to put this code into a demo to explain better... but hoping you might know off hand what the issue is.

In the Network tab of the inspector in Chrome, I see a second websocket show up once the session "times out" ie. becomes stale. But the app doesn't show that I'm logged out.

But, without the check for this.userId in the publish, it does work fine, and the user is shown the login screen and they are "logged out" of the app automatically.

AdamBrodzinski commented 9 years ago

Ultimately I believe this is a Meteor core issue, but at any rate it's an issue when using this package.

Can you post all of your publication? What happens when you return this.ready() if there is no user?

Are you publishing data from Meteor.users? I had this problem and replaced a userInfo publication into a null publication (each user gets their own data automatically), however I haven't tested using this.userId for another collection publication.

Working publication (fails if null is replaced with 'userInfo')

Meteor.publish(null, function() {
  if (this.userId) {
    return Meteor.users.find({_id: this.userId}, {fields: {info: 1}});
  }
});   

There's also a reference to a similar problem in issue #4 . Also, unrelated but theres a fork link in that issue that adds client-side callbacks (hoping to make a PR this weekend). It could somewhat solve the issue as the clientside calls Meteor.logout() as the session expires, and the server-side logic basically takes care of users that do not have the browser open.

wesyah234 commented 9 years ago

Sorry I had to get back to work to test this!... I changed all my publications to look like this, per your suggestion to return this.ready() instead of just returning nothing, and that has been the solution!! Here is an example of what my publish methods look like now:

Meteor.publish("chartViewHistory", function (chartId) {
  if (!this.userId) {
    console.log("chartViewHistory subscribing without being logged in, from: " + this.connection.clientAddress + " denied!");
    return this.ready();
  }
  console.log("getting chart view history for chartId " + chartId + " from ip: " + this.connection.clientAddress);
  var cvh = ChartViewHistory.find({"CHART_ID": chartId});
  console.log("found " + cvh.fetch().length + " history recs");
  return cvh;
});

Also, BTW, this is how I get my app to show the login screen automatically when the user becomes logged out on the client:

<template name="layout">  
  {{#if loggedInUser}}
  {{> navbar}}
  {{> yield}}
  {{else}}
  {{> customLogin}}
  {{/if}}  
</template>

and the loggedInUser helper:

Template.layout.helpers({
  loggedInUser: function () {
    var userId = Meteor.userId();
    if (!userId) {
      console.log("no userid, so route to the root");
      Router.go('/');
    }
    return userId;
  }
});

I was looking through issue #4 and saw people were discussing how to show the login screen automatically when you timed out... anyway, thanks for that suggestion, now I have exactly what I need for a timeout solution!

AdamBrodzinski commented 9 years ago

No prob! glad it worked :beers: