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

feat: createLinkedInOAuthConfig() #287

Open
wants to merge 14 commits into
base: main
Choose a base branch
from
Open

feat: createLinkedInOAuthConfig() #287

wants to merge 14 commits into from

Conversation

iuioiua
Copy link
Contributor

@iuioiua iuioiua commented Dec 24, 2023

Not yet tested.

Closes #282
Closes #285

Comment on lines 33 to 48
return {
clientId,
clientSecret,
authorizationEndpointUri: "https://www.linkedin.com/oauth/v2/authorization",
tokenUri: "https://www.linkedin.com/oauth/v2/accessToken",
redirectUri: config.redirectUri,
defaults: {
requestOptions: {
body: {
client_id: clientId,
client_secret: clientSecret,
},
},
scope: config.scope,
},
};
Copy link
Member

Choose a reason for hiding this comment

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

I couldn't get access toke with this settings. https://www.linkedin.com/oauth/v2/accessToken seems returning invalid_client error.

Error: Client authentication failed
    at AuthorizationCodeGrant.getTokenResponseError (https://deno.land/x/oauth2_client@v1.0.2/src/grant_base.ts:156:14)
    at eventLoopTick (ext:core/01_core.js:182:7)
    at async AuthorizationCodeGrant.parseTokenResponse (https://deno.land/x/oauth2_client@v1.0.2/src/grant_base.ts:72:13)
    at async handleCallback (file:///Users/kt3k/denoland/deno_kv_oauth/lib/handle_callback.ts:69:18)
    at async handler (file:///Users/kt3k/denoland/deno_kv_oauth/server.ts:22:28)
    at async ext:deno_http/00_serve.js:460:18 {
  error: "invalid_client",
  errorDescription: "Client authentication failed",
  errorUri: undefined,
  state: undefined
}

@kt3k
Copy link
Member

kt3k commented Jan 19, 2024

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
Copy link
Contributor Author

iuioiua commented Jan 22, 2024

Could you please show me your test source code?

@kt3k
Copy link
Member

kt3k commented Jan 22, 2024

// 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
Copy link
Contributor Author

iuioiua commented Jan 22, 2024

I just found cmd-johnson/deno-oauth2-client#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: nextauthjs/next-auth#8831

@breuerfelix
Copy link

breuerfelix commented Apr 25, 2024

@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

Signed-off-by: Asher Gomez <ashersaupingomez@gmail.com>
@iuioiua
Copy link
Contributor Author

iuioiua commented Apr 26, 2024

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
Copy link

breuerfelix commented Apr 26, 2024

@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

Copy link

codecov bot commented Apr 29, 2024

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 ⚠️
Additional details and impacted files
@@            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     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@iuioiua
Copy link
Contributor Author

iuioiua commented Apr 29, 2024

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
Copy link

  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
Copy link
Contributor Author

iuioiua commented Apr 30, 2024

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.

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.

suggestion: add LinkedIn provider "client_id" is missing when using a custom OAuth config for LinkedIn
4 participants