ca98am79 / connect-dynamodb

DynamoDB session store for Connect
http://ca98am79.github.com/connect-dynamodb/
MIT License
144 stars 66 forks source link

Session data incorrect #44

Closed superallumette closed 4 years ago

superallumette commented 7 years ago

Hello,

I have tried to implement your solution and have managed to store sessions in dynamodb and retrieve them when my client make requests. However, I have ran into an issue with some sessions that only contains an id, an expiration number but no other field (sess and type). When my client make a request with this id, I get a "Cannot read property 'S' of undefined" error in connect-dynamodb.js line 146 var sess = result.Item.sess.S.toString();

It makes sense, because there is no valid sess object.

I can try to detail my configuration more if needed, but do you please have any idea about what may cause this error ? Thank you very much, best regards

rafaelrpinto commented 7 years ago

If you send the dependencies listed on your pom and how you configured the session middleware I can try to help you.

superallumette commented 7 years ago

Thanks, here are the relative lines in my package.json : "dependencies": { "babel-plugin-transform-object-rest-spread": "^6.23.0", "body-parser": "^1.17.1", "connect": "^3.6.2", "connect-dynamodb": "^1.0.11", "dynamoose": "^0.7.0", "express": "^4.15.2", "express-session": "^1.15.3", "express-validator": "^3.2.0", "uuid": "^3.0.1" }

And my implementation : `import { config } from "./config"; import session from "express-session";

var DynamoDBStore = require("connect-dynamodb")({ session: session }); var options = { AWSConfigJSON: { accessKeyId: config.accessKeyId, secretAccessKey: config.secretAccessKey, region: config.region }, }; export default function sessionManagementConfig(app) { app.use( session({ store: new DynamoDBStore(options), secret: "secret", saveUninitialized: true, resave: false, cookie: { path: "/", httpOnly: true, secure: false, domain: config.domain }, name: "id" }) ); }`

It seems that our problem appeared with a new version of the package because, since we downgraded to the version we were using before it seems that we are not experiencing the problem now.

As I said, sometimes we end up with incorrect object in the table and subsequent requests referring to these sessions crash in connect-dynamodb.

Hmachalani commented 7 years ago

+1 we're running into the same issue as we upgraded to 1.0.12. We're downgrading back to 1.0.11 to see if it fixes the issue.

lef-apago commented 7 years ago

I ran into the same issue, with the session table being left with entries that looked like: {"expires":{"N":"1503608283"},"id":{"S":"sess:awkd99aLsGDn3Kmrv7JFSaMzPTL3rzfH"}}

My solution was to check for the existence of 'sess' before referencing sess.S and treating it as an expired session, which worked in my application. The tables entries did not come from a proper logout sequence which removes all session information, but appeared to happen when users left a session running without using the application for a period of time.

U-4-E-A commented 5 years ago

I ran into the same issue, with the session table being left with entries that looked like: {"expires":{"N":"1503608283"},"id":{"S":"sess:awkd99aLsGDn3Kmrv7JFSaMzPTL3rzfH"}}

My solution was to check for the existence of 'sess' before referencing sess.S and treating it as an expired session, which worked in my application. The tables entries did not come from a proper logout sequence which removes all session information, but appeared to happen when users left a session running without using the application for a period of time.

Bit of an old thread but I ran into the same problem using 2.0.3. How do you check for the existence of sess before referencing sess.S? Are you doing this in your application or are you editing the connect-dynamodb.js itself? If it is the latter, I would assume you are creating a copy of the file outside of node_modules rather than editing the source file directly?

TomiTakussaari commented 4 years ago

I ran into this too.

Issue seems to be that somehow store.touch() is called sometime after store.destroy(), using same sessionId.

And this results in touch storing new value value to dynamodb, without "sess" key, because it uses updateItem, which is basically "update or insert".

Fix could be that touch is implemented like this:

      const params = {
        TableName: this.table,
        Key: {
          [this.hashKey]: {
            S: key,
          },
        },
        ConditionExpression: 'attribute_exists(sess)', // prevent inserting new data
        UpdateExpression: 'set expires = :e',
        ExpressionAttributeValues:{
          ':e': {
            N:  JSON.stringify(expires),
          },
        },
        ReturnValues:'UPDATED_NEW',
      };

      this.awsClient.updateItem(params, callback);

So it does not create new entries, but only updates existing sessions.

I am not 100% sure why touch() is called after destroy(), it could be something wrong in my code too..

mherger commented 4 years ago

Just adding my voice that I'm seeing this too, with v2.0.3.

Any workaround? Wrapping the session handler middleware into another middleware under my control to catch the error?