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(providers): Add Clerk OAuth Provider #11349

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

Conversation

rogervila
Copy link

@rogervila rogervila commented Jul 8, 2024

☕️ Reasoning

Implement Clerk as an OAuth 2 Provider based on their docs.

🧢 Checklist

  • Documentation
  • Tests (manual tests)
  • Ready to be merged

🎫 Affected issues

Fixes #9316

📌 Resources

Copy link

vercel bot commented Jul 8, 2024

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
auth-docs ✅ Ready (Inspect) Visit Preview 💬 Add feedback Aug 9, 2024 2:58pm
2 Skipped Deployments
Name Status Preview Comments Updated (UTC)
next-auth-docs ⬜️ Ignored (Inspect) Visit Preview Aug 9, 2024 2:58pm
proxy ⬜️ Ignored (Inspect) Visit Preview Aug 9, 2024 2:58pm

Copy link

vercel bot commented Jul 8, 2024

@rogervila is attempting to deploy a commit to the authjs Team on Vercel.

A member of the Team first needs to authorize it.

@rogervila rogervila changed the title Clerk OAuth Provider feat(providers): Add Clerk OAuth Provider Jul 8, 2024
@rogervila rogervila marked this pull request as ready for review July 8, 2024 15:29
@rogervila
Copy link
Author

Dear reviewers (@ThangHuuVu, @k-taro56, @ubbe-xyz, @ndom91),

I understand there are many PRs to review and I do not want to put extra pressure. Nonetheless, I would like to ask for authorization on the Vercel - proxy CI step so I can see if the PR is green.

Thank you!

@mjlangan
Copy link

This would be useful for auth work my team is planning!

*/
export default function Clerk(
config: OAuthUserConfig<ClerkProfile> & {
baseUrl: string
Copy link
Member

Choose a reason for hiding this comment

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

No need for baseUrl or wellKnown, if the user passes issuer, we will construct the correct discovery URL.

Anything related to it should be dropped.

Copy link
Member

@balazsorban44 balazsorban44 left a comment

Choose a reason for hiding this comment

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

I am not sure which is correct, but either Clerk is OIDC or OAuth compliant.

https://clerk.com/docs/advanced-usage/clerk-idp says OAuth2, but since you set the well-known endpoint, I am not sure.

Can you clarify?

Comment on lines +95 to +117
type: "oauth",
wellKnown: `${baseUrl}/.well-known/openid-configuration`,
authorization: {
url: `${baseUrl}/oauth/authorize`,
params: { scope: "email profile" },
},
token: `${baseUrl}/oauth/token`,
userinfo: {
url: `${baseUrl}/oauth/userinfo`,
async request({ tokens, provider }) {
const profile = await fetch(provider.userinfo?.url as URL, {
headers: {
Authorization: `Bearer ${tokens.access_token}`,
"User-Agent": "authjs",
},
}).then(async (res) => await res.json())

return profile
},
},
profile(profile) {
return profile
},
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
type: "oauth",
wellKnown: `${baseUrl}/.well-known/openid-configuration`,
authorization: {
url: `${baseUrl}/oauth/authorize`,
params: { scope: "email profile" },
},
token: `${baseUrl}/oauth/token`,
userinfo: {
url: `${baseUrl}/oauth/userinfo`,
async request({ tokens, provider }) {
const profile = await fetch(provider.userinfo?.url as URL, {
headers: {
Authorization: `Bearer ${tokens.access_token}`,
"User-Agent": "authjs",
},
}).then(async (res) => await res.json())
return profile
},
},
profile(profile) {
return profile
},
type: "oidc",

When a provider is OIDC compliant, none of these need to be set

Comment on lines +50 to +54
* Clerk({
* clientId: CLERK_CLIENT_ID,
* clientSecret: CLERK_CLIENT_SECRET,
* baseUrl: CLERK_BASE_URL
* }),
Copy link
Member

Choose a reason for hiding this comment

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

Should be

Suggested change
* Clerk({
* clientId: CLERK_CLIENT_ID,
* clientSecret: CLERK_CLIENT_SECRET,
* baseUrl: CLERK_BASE_URL
* }),
* Clerk,

This should be enough, if the user sets AUTH_CLERK_ID, AUTH_CLERK_SECRET and AUTH_CLERK_ISSUER

*
* @module providers/clerk
*/
import type { OAuthConfig, OAuthUserConfig } from "./index.js"
Copy link
Member

Choose a reason for hiding this comment

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

Should be updated to OIDCConfig and OIDCUserConfig

*
* ### Resources
*
* - [Clerk - Use Clerk as an OAuth 2 Provider](https://clerk.com/docs/advanced-usage/clerk-idp)
Copy link
Member

Choose a reason for hiding this comment

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
core Refers to `@auth/core` examples providers
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants