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
VK provider fix #3709
VK provider fix #3709
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you. I actually created a VK app on my own and added a few suggestions to this PR.
Looks like there is also a merge conflict, please fix it as well!
Hi! I ran into the same problem today, also a bug 'invalid client (client ID not defined)'. |
Co-authored-by: Balázs Orbán <info@balazsorban.com>
@alexTayanovsky is attempting to deploy a commit to the NextAuth Team on Vercel. A member of the Team first needs to authorize it. |
Co-authored-by: Balázs Orbán <info@balazsorban.com>
This pull request is being automatically deployed with Vercel (learn more). 🔍 Inspect: https://vercel.com/nextauthjs/next-auth/CvFFgyBrEYVCMCZ4CjX3SbwT97Tz |
@balazsorban44 Thank you very much for assistance! I've addressed your review comments & suggestions |
Seems like Mailru provider is also broken
|
@sergeyshaykhullin Can you open a separate issue for |
So theres two errors here according to your output:
(See Models) Unless you changed something in your prisma schema, those are the default values / field names. |
@ndom91 Yes, i changed using naming conventions, it works with google provider https://next-auth.js.org/adapters/prisma#naming-conventions I am using This is my schema.prisma datasource db {
provider = "postgresql"
url = env("DATABASE_URL")
}
generator client {
provider = "prisma-client-js"
}
model Account {
id String @id @default(uuid())
userId String @map("user_id")
type String
provider String
providerAccountId String @map("provider_account_id")
refresh_token String? @db.Text
access_token String? @db.Text
expires_at Int?
token_type String?
scope String?
id_token String? @db.Text
session_state String?
oauth_token_secret String?
oauth_token String?
user User @relation(fields: [userId], references: [id], onDelete: Cascade)
@@unique([provider, providerAccountId])
@@map("accounts")
}
model Session {
id String @id @default(uuid())
sessionToken String @unique @map("session_token")
userId String @map("user_id")
expires DateTime
user User @relation(fields: [userId], references: [id], onDelete: Cascade)
@@map("sessions")
}
model User {
id String @id @default(uuid())
name String?
email String? @unique
emailVerified DateTime? @map("email_verified")
image String?
accounts Account[]
sessions Session[]
@@map("users")
}
model VerificationToken {
identifier String
token String @unique
expires DateTime
@@unique([identifier, token])
@@map("verification_tokens")
} And vk provider registration VkProvider({
clientId: process.env.VK_CLIENT_ID,
clientSecret: process.env.VK_CLIENT_SECRET
}), |
Okay gotcha, makes sense. So |
return { | ||
id: profile.id, | ||
name: [profile.first_name, profile.last_name].filter(Boolean).join(" "), | ||
email: null, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@ndom91 Maybe email: null is trying to save accounts.email column?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No so I meant that the email
field is not on the Account
table at all, but the User
table (or users
in your case).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
But it in terms of why its failing on that linkAccount
call, i'm still not sure..
In the /src/core/lib/callback-handler.ts
, for example, it's just passing the whole account
object into linkAccount
function. This account
, however, should only have these 4 fields according to the type definiton, and not email
. See lines 156
and 206
..
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@ThangHuuVu @balazsorban44 so this is confusing me a bit, its looking like its trying to pass an Account
, as well as some of the fields on User
to the linkAccuont()
method. Which should only take an account..
Is this the fault of this adapter? Or should we be deleting the email
key off the data before tryign to pass it into linkAccount()
as an argument?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@ndom91 So if I understand correctly, the problem is that we're passing the account
object which can contain all kinds of properties (it's extending from Record<string, unknown>
and Partial<TokenSet>
as seen here) into the adapter function linkAccount
which expects only the object of type Account
.
In this case, I think Prisma
is working as expected. The proper solution IMO is that we should use the object with the desired type Account
in the linkAccount
implementation. Does that make sense?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah that makes sense. So basically drop email
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let us drop anything but the expected properties... so I worry that another unexpected property could break the adapter again when used with another provider. WDYT?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah that makes a lot of sense 👍
Any updates here? VK provider still not working properly |
I waited for this merge for tvo years. |
I also got |
@alexTayanovsky can you give me |
@alexTayanovsky there is a merge conflict, if you could please address that, and allow maintainers to edit, we could get this through the finish line! Otherwise I'm going to close this PR, so someone else can open a new one including the fixes here 🙏 💚 |
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
@balazsorban44 done ✅ |
@@ -4,7 +4,7 @@ import GitHubProvider from "next-auth/providers/github" | |||
import Auth0Provider from "next-auth/providers/auth0" | |||
import KeycloakProvider from "next-auth/providers/keycloak" | |||
import TwitterProvider, { | |||
TwitterLegacy as TwitterLegacyProvider, | |||
// TwitterLegacy as TwitterLegacyProvider, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why is this commented out?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this is an unused import that caused eslint warning, I guess this line can be removed instead
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks!
So, is it fixed now? |
Release this pls |
The error still appears. In addition to VK, my project uses authorization with Google, Yandex and Mail.ru and they work fine. |
Is there any workaround? |
@Almasx I managed to fix the issue in a pull request here (#7070), unfortunately it's been rejected. If you need Vk to work and want to implement my solution, you can run this command in your project directory:
(if you're using ubuntu, you may need to wrap the link in a single quotation mark) Alternatively, you can open your
Then run If this still doesn't work and you only need it for your local environment, you can open token: `https://oauth.vk.com/access_token?v=${apiVersion}`, to this: token: {
url: `https://oauth.vk.com/access_token?v=${apiVersion}`,
async request({ client, params, checks, provider }) {
const response = await client.oauthCallback(
provider.callbackUrl,
params,
checks,
{ exchangeBody: { client_id: options.clientId } }
)
return {
tokens: {
access_token: response.access_token,
expires_at: response.expires_at
}
}
},
}, |
Reasoning 💡
VK provider is broken (see #3587, #2524 (comment)), so this PR aims to fix it, and adds Typescript support.
Checklist 🧢
Affected issues 🎟