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): TikTok provider #2720

Closed
wants to merge 3 commits into from
Closed

feat(providers): TikTok provider #2720

wants to merge 3 commits into from

Conversation

denkristoffer
Copy link

@denkristoffer denkristoffer commented Sep 10, 2021

Reasoning 💡

Adds TikTok provider. Their implementation is a bit funky, so there's a bit of customisation.

Checklist 🧢

  • Documentation
  • Tests
  • Ready to be merged

@github-actions github-actions bot added core Refers to `@auth/core` providers labels Sep 10, 2021
@balazsorban44 balazsorban44 self-requested a review September 10, 2021 17:03
@denkristoffer denkristoffer changed the base branch from next to beta October 12, 2021 11:24
@balazsorban44 balazsorban44 changed the title feat: TikTok provider feat(providers): TikTok provider Oct 27, 2021
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.

Thank you! I added some suggestions.

Do note that we plan to release v4 to stable very soon, so it would be nice to get this merged before that.

Could you show some kind of proof of login that shows the returned name, email, picture, or any of those that are actually returned?

We could use that, later on, to refer to, when people are having issues.

Thank you!

src/providers/tiktok.ts Outdated Show resolved Hide resolved
src/providers/tiktok.ts Outdated Show resolved Hide resolved
src/providers/tiktok.ts Outdated Show resolved Hide resolved
src/providers/tiktok.ts Outdated Show resolved Hide resolved
src/providers/tiktok.ts Outdated Show resolved Hide resolved
@balazsorban44
Copy link
Member

balazsorban44 commented Nov 25, 2021

Gentle bump @denkristoffer. We will release v4 next week, if this doesn't address my comments, we cannot merge it and therefore will be closed.

@balazsorban44
Copy link
Member

An accompanying docs page should also be added at https://github.com/nextauthjs/docs/tree/main/docs/providers

@github-actions github-actions bot removed providers core Refers to `@auth/core` labels Nov 26, 2021
@denkristoffer
Copy link
Author

Thank you for the bump. I didn't mean to close this, I will try to get to this in time for v4. Rebasing on the beta branch.

@denkristoffer denkristoffer reopened this Nov 26, 2021
@github-actions github-actions bot added core Refers to `@auth/core` providers labels Nov 26, 2021
@denkristoffer
Copy link
Author

denkristoffer commented Nov 26, 2021

@balazsorban44

Could you show some kind of proof of login that shows the returned name, email, picture, or any of those that are actually returned?

We could use that, later on, to refer to, when people are having issues.

What are you looking for here? A demo website?

@balazsorban44
Copy link
Member

Basically just a screenshot of the returned data by the profile callback. Either the JSON from a console.log or an image from the site. Example: #2848 (comment)

Comment on lines 54 to 56
client[custom.http_options] = (_url, options) => {
return { ...options, followRedirect: true }
}
Copy link
Member

Choose a reason for hiding this comment

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

I think I'll update the httpOptions to expose followRedirect as well, so we hopefully can get rid of the openid-client custom import.

@balazsorban44
Copy link
Member

If you merge beta into your branch now, #3287 exposes the httpOptions, so you can get rid of the above-mentioned import.

@balazsorban44
Copy link
Member

I've tried to register an App, it had some caveats, that should be mentioned in the docs.

Would be appreciated if you opened a PR regarding this #2720 (comment)

First of all, it seems like you cannot use http. even for local development. Second, my app registration has to go through a seemingly manual review process.

@denkristoffer
Copy link
Author

denkristoffer commented Nov 27, 2021

Great! Docs PR is under way 🙂 TikTok seems to manually approve apps and it can take a few days.

@balazsorban44
Copy link
Member

In that case, could you paste in the logged in result just so I (and future everyone) knows that this provider should work?

@denkristoffer
Copy link
Author

I don't currently have a site using this provider, but I'll try to run it with the demo website later today.

@balazsorban44
Copy link
Member

I don't expect an actual site. do try with the demo app. otherwise how did you even create the PR? 😀

@denkristoffer
Copy link
Author

I don't expect an actual site. do try with the demo app. otherwise how did you even create the PR? 😀

I created this for a personal project, but eventually decided against using TikTok for auth since their oauth implementation honestly seemed to change quickly.

I couldn't get the demosite working with the provider yesterday because of the oauth client complaining about an "Invalid URL". I will debug and try again tonight, hopefully.

@balazsorban44
Copy link
Member

Unfortunately, if we cannot show proof of login, I will have to close this PR. I'll check back at the end of my day, and if works then, we can merge. Otherwise, I'll close this for now. We can revisit the idea at a later time after v4 has landed in stable.

Thank you for your contribution nonetheless.

@Oxicode
Copy link

Oxicode commented Aug 9, 2022

anything news??

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

Successfully merging this pull request may close these issues.

4 participants