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

Expose methods to allow running cronjobs against session's expiry time #148

Closed
adoublef opened this issue Jul 31, 2023 · 6 comments · Fixed by #222
Closed

Expose methods to allow running cronjobs against session's expiry time #148

adoublef opened this issue Jul 31, 2023 · 6 comments · Fixed by #222

Comments

@adoublef
Copy link
Contributor

adoublef commented Jul 31, 2023

I have enjoyed using this library for handling sessions. However I have noticed that at some point there will be sessions that don't get deleted from the the store and so overtime could grow and will need a way to delete them.

As I understand the Kv store API currently doesn't have and true TTL capabilities natively but I think it should be possible to still expose a way to search/filter sessions by expiry time.

The official docs state this:

- A range selector selects all keys that are lexicographically between the given start and end keys (including the start, and excluding the end). For example, the selector ["users", "a"], ["users", "n"] will select all keys that start with the prefix ["users"] and have a second key part that is lexicographically between a and n, such as ["users", "alice"], ["users", "bob"], and ["users", "mike"], but not ["users", "noa"] or ["users", "zoe"].

Based on this I think if a key included a timestamp (could be [PREFIX, timestamp]) then could expose a function like getExpiredSessions that may look like this:

const db = await Deno.openKv();
const entries = db.list({ start: [KV_PREFIX, 0], end: [KV_PREFIX, unix_timestamp] })
for await (const entry of entries) {
  entry.key; // [KV_PREFIX, expiry_as_unix]
  entry.value; // { sessionId, expiryAsUnix, ...rest }
  entry.versionstamp; // "00000000000000010000"
}

I'm unsure on the deleteExpiredSessions method but I think it would need to use transactions for consistency.

Be interested to open the dialogue to see other opinions on the matter.

I saw this bit of code and felt it may be a good starting point

@adoublef adoublef changed the title Possible to run a cronjob with the existing API to allow deletion of expired sessions Expose mehtods to allow run a cronjobs against sessions expiry time Jul 31, 2023
@adoublef adoublef changed the title Expose mehtods to allow run a cronjobs against sessions expiry time Expose methods to allow run a cronjobs against sessions expiry time Jul 31, 2023
@adoublef adoublef changed the title Expose methods to allow run a cronjobs against sessions expiry time Expose methods to allow running cronjobs against session's expiry time Jul 31, 2023
@iuioiua
Copy link
Contributor

iuioiua commented Jul 31, 2023

Yeah, I've also been thinking about this issue. Taking advantage of kv.list(), I think, is a great approach! Let me do some tinkering.

@adoublef
Copy link
Contributor Author

adoublef commented Aug 1, 2023

Yh amazing, I will keep a look out for that. another take I have is maybe hold an array of sessionIds that were issued in a day  as the value. then I think you could open up to using both list or get 

const db = await Deno.openKv();
const entries = db.list({ start: [KV_PREFIX, "01-01-2020"], end: [KV_PREFIX, "04-01-2022"] }) // array of arrays
for await (const entry of entries) {
  entry.key; // [KV_PREFIX, expiry_as_unix]
  entry.value; // [sessionId1, sessionId2, sessionIdN]
  entry.versionstamp; // "00000000000000010000"
}

const entry = db.get([KV_PREFIX, "01-01-2020"]) 
entry.key; // [KV_PREFIX, expiry_as_unix]
entry.value; // [sessionId1, sessionId2, sessionIdN]
entry.versionstamp; // "00000000000000010000"

@iuioiua
Copy link
Contributor

iuioiua commented Aug 27, 2023

So it seems that to strike the best balance between KV-space efficiency and user convenience, the most we can do is set OAuth session entries to expire in KV automatically and not site sessions. Keeping site sessions indefinitely allows us to use the refresh token to refresh the access token without requiring user interaction.

Check out #170. I'll merge that PR once KV entry expiry is supported on Deno Deploy.

@adoublef
Copy link
Contributor Author

Ah I see, that looks good to me. Also reading through the PR, was nice to learn the recommended expiry time stated by RFC6749.

And I found the PR referencing the support for expiry so will keep a look for that too. Thank you

@iuioiua
Copy link
Contributor

iuioiua commented Sep 24, 2023

FYI, take a look at #224. It removes all database storage and retrieval of tokens. That means this module will automatically keep the database completely clean once implemented alongside OAuth session expiry.

@adoublef
Copy link
Contributor Author

Oh I have just updated my deno project using this module so good timing. I will lookout for the breaking change. ty

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment