denoland / deno_kv_oauth

High-level OAuth 2.0 powered by Deno KV.
https://jsr.io/@deno/kv-oauth
MIT License
261 stars 27 forks source link

feat: `createLinkedInOAuthConfig()` #287

Open iuioiua opened 11 months ago

iuioiua commented 11 months ago

Not yet tested.

Closes #282 Closes #285

kt3k commented 10 months ago

I was able to show permission consent screen (with the scope updated to profile email) like the below:

Screenshot 2024-01-19 at 13 08 09

But callback handler doesn't generate access token successfully. I currently have no idea what's going wrong

iuioiua commented 10 months ago

Could you please show me your test source code?

kt3k commented 10 months ago
// server.ts
import {
  createLinkedInOAuthConfig,
  getSessionId,
  handleCallback,
  signIn,
  signOut,
} from "./mod.ts";

const oauthConfig = createLinkedInOAuthConfig({
  redirectUri: "http://localhost:8000/oauth/callback",
  // scope: "r_liteprofile r_emailaddress",
  scope: "profile email",
});

async function handler(request: Request) {
  const { pathname } = new URL(request.url);
  switch (pathname) {
    case "/oauth/signin":
      return await signIn(request, oauthConfig);
    case "/oauth/callback": {
      const { response } = await handleCallback(request, oauthConfig);
      return response;
    }
    case "/oauth/signout":
      return await signOut(request);
    case "/protected-route":
      return await getSessionId(request) === undefined
        ? new Response("Unauthorized", { status: 401 })
        : new Response("You are allowed");
    default:
      return new Response(null, { status: 404 });
  }
}

Deno.serve(handler);

Also I have the below settings in linkedin developers site:

Screenshot 2024-01-22 at 15 45 25
iuioiua commented 10 months ago

I just found https://github.com/cmd-johnson/deno-oauth2-client/issues/37, which points out that LinkedIn doesn't follow the OAuth spec correctly and doesn't include the token_type field when requesting the access token. I can't think of a workaround, so I'll ask upstream what can be done. CC @kevingorski

Edit: Nevermind. We're getting a Client authentication failed error. It appears to be yet another bug on LinkedIn's side: https://github.com/nextauthjs/next-auth/issues/8831

breuerfelix commented 7 months ago

@iuioiua @kt3k i figured out the bug since i am working on this too. when fetching the accesstoken the lib sends code_verifier in the body and linked in does not like that. if you omit this value (or set it to an empty string), it works. The problem is, there is no way to override this value. setting defaults.requestOptions.body.code_verifier: "" does not work either because this setting gets overriden in the final this.buildRequest method.

We could implement a bool that allows to disable to set the code_verifier in the request body.

setting:

  const tokens = await new OAuth2Client(oauthConfig)
    .code.getToken(request.url, {...oauthSession, codeVerifier: ""});

works like a charm

any plans on how to implement this nicely into this lib? i can do the work

iuioiua commented 7 months ago

Thanks for the insight, @breuerfelix. I've adjusted this to make it work, but I'm not yet convinced it's the right way to go, as it seems hacky. Are you able to test now?

breuerfelix commented 7 months ago

@iuioiua thanks for the fast reply! yesterday i googled long time how to consume a module from github and it seems like... it doesn't work ...

any idea how i can easily use your fork for testing?

yes it seems hacky. another idea would be to add a new key called overrideOptions so its clear that this behaves a little different than the default options

codecov[bot] commented 6 months ago

Codecov Report

Attention: Patch coverage is 89.65517% with 3 lines in your changes are missing coverage. Please review.

Project coverage is 98.60%. Comparing base (57605e7) to head (5780817).

Files Patch % Lines
lib/create_helpers.ts 50.00% 2 Missing and 1 partial :warning:
Additional details and impacted files ```diff @@ Coverage Diff @@ ## main #287 +/- ## ========================================== - Coverage 99.15% 98.60% -0.55% ========================================== Files 22 23 +1 Lines 475 503 +28 Branches 24 25 +1 ========================================== + Hits 471 496 +25 - Misses 2 4 +2 - Partials 2 3 +1 ```

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

iuioiua commented 6 months ago

any idea how i can easily use your fork for testing?

  1. git clone https://github.com/denoland/deno_kv_oauth.git
  2. cd deno_kv_oauth
  3. git checkout linkedin

yes it seems hacky. another idea would be to add a new key called overrideOptions so its clear that this behaves a little different than the default options

I'd prefer not to do that either. @cmd-johnson, is there a way we override the codeVerifier returned from OAuth2Client.code.getToken() by a setting within OAuth2ClientConfig?

breuerfelix commented 6 months ago
  1. git clone https://github.com/denoland/deno_kv_oauth.git
  2. cd deno_kv_oauth
  3. git checkout linkedin

Well i mean how can i use your branch as a deno package in my application instead of the upstream package? Can i do some override of the package in deno.json?

I'd prefer not to do that either. @cmd-johnson, is there a way we override the codeVerifier returned from OAuth2Client.code.getToken() by a setting within OAuth2ClientConfig?

The problem is that, using the codeVerifier (which is the state i guess) is a good thing to do since it prevents man in the middle attacks. We just shouldn't pass it in the last step (for linkedIn). Sad that linkedIn behaves a little different than all the other providers. I hate that.

iuioiua commented 6 months ago

Well i mean how can i use your branch as a deno package in my application instead of the upstream package? Can i do some override of the package in deno.json?

Yes. Replace Deno KV OAuth's import specifier with https://raw.githubusercontent.com/denoland/deno_kv_oauth/linkedin/mod.ts.