Skip to content
This repository has been archived by the owner on Jul 17, 2022. It is now read-only.

fix(dynamo): Ensure expires attribute is stored as a UNIX timestamp [Take 2] #367

Merged
merged 2 commits into from
Jan 10, 2022

Conversation

GiacoCorsiglia
Copy link
Contributor

Repeat of #366, targeted at main branch

Reasoning 💡

Hi! This is a small change compared to the number of words and tests I wrote!

This change applies to the DynamoDB adapter.

DynamoDB supports a "TTL" feature that automatically purges old items from the database if they are marked with a special attribute. The existing adapter docs suggest enabling this feature and setting it to use the expires attribute. This is a reasonable choice, because Next Auth already uses the expires property on Sessions and VerificationTokens for basically the same purpose.

However, as clarified in the AWS docs, the special TTL attribute must be stored as a UNIX timestamp in seconds. I have fixed a bug in the DynamoDB adapter, which resulted in the expires attribute being stored as an ISO string, meaning that the TTL feature would not work for Next Auth Sessions and VerificationTokens. Following this PR, the TTL feature should work, because now expires is stored as a numeric timestamp.

I added some unit tests for the code that converts objects to and from the format saved in DynamoDB. Most of the tests were passing before this PR, but I thought it was worth adding them anyway. All the tests pass on my machine with these changes.

The code before I edited it appeared to convert expires between a number and a string. However, so far as I can tell, the expires property on AdapterSession and VerificationToken is a Date, so I think that code path was just being skipped entirely. I can confirm that, in my database, the expires attribute is in fact being stored as an ISO string. Paging @mathisobadia in case they know what the code was originally trying to do.

Thanks :)

Checklist 🧢

  • [ ] Documentation
  • Tests
  • Ready to be merged

Affected issues 🎟

This is related to #252, but that issue was closed (possibly incorrectly? I don't know if this worked in a previous version!).

Intentionally includes two failing tests having to do with the `expires` attribute.
@github-actions github-actions bot added dynamodb @next-auth/dynamodb-adapter related tests Changes to testing labels Jan 9, 2022
@balazsorban44 balazsorban44 merged commit c262110 into nextauthjs:main Jan 10, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
dynamodb @next-auth/dynamodb-adapter related tests Changes to testing
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants