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

added trakt provider #3771

Merged
merged 6 commits into from
Feb 3, 2022
Merged

added trakt provider #3771

merged 6 commits into from
Feb 3, 2022

Conversation

blekmus
Copy link
Contributor

@blekmus blekmus commented Jan 31, 2022

Added Trakt provider

Reasoning 💡

I'm making an app that uses the trakt api. So figured adding it as a provider would be helpful.

Checklist 🧢

  • Documentation
  • Tests
  • Ready to be merged

Proof

2022_01_31-12_40_06

  • Trakt does not provide the user's emails
  • The profile image link cannot be hotlinked as seen in the image above. Trakt returns an access forbidden error

This is the first time I'm making a pull request so I hope there aren't any loose ends.

@github-actions github-actions bot added core Refers to `@auth/core` providers labels Jan 31, 2022
@balazsorban44
Copy link
Member

Thank you! Unfortunately following the steps you outlined in the accompanying docs PR, I am presented with the following error when trying to log in:

image

About the image property, I find it very confusing, what's the point of their API if you cannot display the image anyway? 🤔😕

@blekmus
Copy link
Contributor Author

blekmus commented Feb 3, 2022

@balazsorban44 sorry for that and thanks for pointing it out. It seems I had an incorrect url all this time. I updated the docs and this PR with a fix.

About the image. The ever weirder thing is if you have their default profile picture on. It works perfectly fine. But if you somehow change it. It all of a sudden stops working and you've got a forbidden error in your hands.

@balazsorban44
Copy link
Member

I understand. Indeed, weird. Could you fix the merge conflict, please? 🙏

@balazsorban44
Copy link
Member

I see two other URLs with api.. Are those correct then? BTW, I might have fooled myself, cause I set up the application on the staging API, I'll test it with a prod API now, see if the OAuth error goes away. 👍

@balazsorban44
Copy link
Member

Yep, setting up the client app in production worked!

@blekmus
Copy link
Contributor Author

blekmus commented Feb 3, 2022

I see two other URLs with api.. Are those correct then? BTW, I might have fooled myself, cause I set up the application on the staging API, I'll test it with a prod API now, see if the OAuth error goes away. +1

Yup the others are correct. It was only that one single url that's different.

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.

Added a few comments.

Managed to simplify the request call for userinfo. could you please update it?

src/providers/trakt.ts Show resolved Hide resolved
src/providers/trakt.ts Outdated Show resolved Hide resolved
src/providers/trakt.ts Outdated Show resolved Hide resolved
blekmus and others added 3 commits February 3, 2022 19:47
Co-authored-by: Balázs Orbán <info@balazsorban.com>
Co-authored-by: Balázs Orbán <info@balazsorban.com>
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!

@codecov-commenter
Copy link

Codecov Report

Merging #3771 (29c1cce) into main (c9e16fb) will decrease coverage by 0.07%.
The diff coverage is 0.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##             main    #3771      +/-   ##
==========================================
- Coverage   13.01%   12.94%   -0.08%     
==========================================
  Files          92       93       +1     
  Lines        1460     1468       +8     
  Branches      393      390       -3     
==========================================
  Hits          190      190              
- Misses       1256     1267      +11     
+ Partials       14       11       -3     
Impacted Files Coverage Δ
src/providers/trakt.ts 0.00% <0.00%> (ø)
src/core/index.ts 0.00% <0.00%> (ø)
src/core/pages/error.tsx 0.00% <0.00%> (ø)
src/core/lib/oauth/client-legacy.ts 0.00% <0.00%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update c9e16fb...29c1cce. Read the comment docs.

@balazsorban44 balazsorban44 merged commit 844c9b1 into nextauthjs:main Feb 3, 2022
@blekmus blekmus deleted the trakt-provider branch February 3, 2022 15:53
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.

None yet

4 participants