Skip to content

Conversation

@fdemir
Copy link

@fdemir fdemir commented Apr 13, 2024

I was trying to use the google provider and noticed that kv_oauth was old. I've upgraded it and make it easy to add new providers etc. Feel free to give feedback.

@iuioiua
Copy link
Contributor

iuioiua commented Apr 14, 2024

Thank you for the PR, @fdemir. I have to consider how we simulate a signed-in user for the tests. Please allow me some time to devise a solution to unblock this PR.

@ycmjason
Copy link
Contributor

ycmjason commented Sep 1, 2024

@iuioiua hey i have upgraded my own project to the latest kv_oauth and have successfully simulated user sign in. Here's how I've done it.

async function fakeSignIn(user: User): Promise<void> {
  await createUser(user);
  await kv.set(["site_sessions", user.sessionId], true);
}

kv_oauth can't pick up the data from ["site_sessions", user.sessionId] because each instances of sqlite opened using :memory: have data seperated. After looking into how sqlite work as guided from the deno doc, we just have to change the database filename from :memory: to file::memory:?cache=shared.

so my test script become:

    "test": "DENO_KV_PATH=\"file::memory:?cache=shared\" deno test -A --parallel --unstable-kv",

Voila, this works like a charm.

Hope this helps!

Amazing project btw, saved me so much time. ❤️

@iuioiua
Copy link
Contributor

iuioiua commented Sep 9, 2024

Hm... That seems quite hacky. Good find, nevertheless. Let me get back to you on what we do here.

@ycmjason
Copy link
Contributor

ycmjason commented Sep 9, 2024

That seems quite hacky

Why do you find it hacky? What are the major issues you are having while trying to simulate user sign in?

@iuioiua
Copy link
Contributor

iuioiua commented Sep 10, 2024

I find it hacky because it uses a barely-known KV path. I don't currently recall what the blocker was here. I'll look into this again once things calm down with std and the upcoming Deno 2 release.

signIn,
signOut,
} from "kv_oauth/mod.ts";
import { createGitHubOAuthConfig, createHelpers } from "jsr:@deno/kv-oauth";
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
import { createGitHubOAuthConfig, createHelpers } from "jsr:@deno/kv-oauth";
import { createGitHubOAuthConfig, createHelpers } from "@deno/kv-oauth";

Ditto for other imports. Please run deno add jsr:@deno/kv-oauth to add the import to the deno.json file.

{
path: "/signin",
handler: async (req) => await signIn(req, createGitHubOAuthConfig()),
handler: async (req) => await signIn(req),
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
handler: async (req) => await signIn(req),
handler: (req) => signIn(req),

Nit

{
path: "/signout",
handler: signOut,
handler: async (req) => await signOut(req),
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
handler: async (req) => await signOut(req),
handler: (req) => signOut(req),

Nit

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants