feature: add sqlite storage #21
Conversation
|
I think an improvement here would be to make the table name configurable, or make it something much less likely to have a collision |
🦋 Changeset detectedLatest commit: bd9ce7b The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
|
made some changes but i think TTL is not respected? |
|
Added one liner to cleanup expired rows on get/scan. Do we need something similar for memory storage as well to cleanup expired values? |
|
sorry i should have been more explicit i don't think we should run a cleanup on every call - i meant when we do a similarly with scan, if we come across an expired value we should delete it it's not necessary so i'll merge even without that just as long as the adapter is not returning expired values |
|
btw i just pushed a basic set of tests, maybe can try them with the sql adapter |
| const TABLE_NAME = input?.tableName ?? "__openauth__kv_storage"; | ||
|
|
||
| db.exec( | ||
| `CREATE TABLE IF NOT EXISTS ${TABLE_NAME} (key TEXT PRIMARY KEY, value TEXT, expiry INTEGER)` |
There was a problem hiding this comment.
expiry is a separate column instead of being part of the value, as it's relatively easier to query.
| let storage = SQLiteStorage(); | ||
|
|
||
| beforeEach(async () => { | ||
| storage = SQLiteStorage(); |
There was a problem hiding this comment.
This is pretty much the same as memory storage test
| async set(key: string[], value: any, ttl?: number) { | ||
| const expiry = ttl ? Date.now() + ttl * 1000 : undefined; |
There was a problem hiding this comment.
This is already set in https://github.com/openauthjs/openauth/blob/master/packages/openauth/src/storage/storage.ts#L33
| async set(key: string[], value: any, ttl?: number) { | |
| const expiry = ttl ? Date.now() + ttl * 1000 : undefined; | |
| async set(key: string[], value: any, expiry?: Date) { |
|
Be careful with deleting a token in the request that validates it. Especially if a db is involved. This would allow very easy timing attacks (given now a Select + Delete is involved) in knowing whether a token existed or not. In case of "expired token" it's risks are less, but if in the future additional checks like "ip-address" are added this could allow for timing attacks. It's generally better to either simulate the deletion process for valid tokens, do the deletion after sending the response, or in a cronjob running behind the scenes. |
Adds sqlite storage option using libsql. better-sqlite3 doesn't play well with bun.
Fixes #11