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(provider): Add instagram #817

Closed
wants to merge 10 commits into from
Closed

feat(provider): Add instagram #817

wants to merge 10 commits into from

Conversation

PolMrt
Copy link

@PolMrt PolMrt commented Nov 1, 2020

Add Instagram as a provider.

Note that I've add node-fetch npm package because the Instagram api only returns some informations such as username, id, account_type. I've add a request to Instagram public api (https://www.instagram.com/instagram/?__a=1) which, strangely, returns more informations such as full name and profile pictures (this works for public and private accounts).

@vercel
Copy link

vercel bot commented Nov 1, 2020

This pull request is being automatically deployed with Vercel (learn more).
To see the status of your deployment, click below or on the icon next to each commit.

🔍 Inspect: https://vercel.com/nextauthjs/next-auth/o3fipdopn
✅ Preview: https://next-auth-git-main.nextauthjs.vercel.app

@vercel vercel bot temporarily deployed to Preview November 1, 2020 09:41 Inactive
@vercel vercel bot temporarily deployed to Preview November 1, 2020 09:45 Inactive
@vercel vercel bot temporarily deployed to Preview November 1, 2020 09:47 Inactive
@ndom91
Copy link
Member

ndom91 commented Dec 6, 2020

@PolMrt next.js ships with an isomorphic version of fetch since 9.4+.

Take a look at this example for usage:

https://github.com/nextauthjs/next-auth/blob/canary/src/client/index.js

It's wrapped up in a custom function _fetchData located towards the bottom of the file. Can you please use that instead of adding a new dependency? Otherwise it looks good, thanks! 🎉

@balazsorban44 balazsorban44 changed the title [providers] Add instagram feat(provider): Add instagram Dec 6, 2020
@ndom91 ndom91 changed the base branch from main to canary December 6, 2020 18:40
@PolMrt
Copy link
Author

PolMrt commented Dec 6, 2020

Thanks I'll update that. But I tried to use this "public API" for another project and it seems to not be usable as an API, sometimes it will redirects you to a login page if you haven't logged to Insta with your ip in a while... So I don't think this is the best approach.

Would it be okay if we only stuck with the username ? Or should we still use this unstable API and return no email, picture, name (as the Instagram API don't return that kind of information for non-business account) if there's a redirection ?

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 added some suggestions to eliminate node-fetch. Could you please check out if this still works for you locally?

@PolMrt
Copy link
Author

PolMrt commented Dec 6, 2020

Thanks for your help @balazsorban44. Everything seem to be fine, gonna try this tomorrow morning !

Co-authored-by: Balázs Orbán <[email protected]>
@vercel vercel bot temporarily deployed to Preview December 7, 2020 09:43 Inactive
PolMrt and others added 3 commits December 7, 2020 10:43
@vercel vercel bot temporarily deployed to Preview December 7, 2020 09:44 Inactive
image: data.graphql.user.profile_pic_url_hd
}
} catch(error) {
logger.error("PROFILE_FETCH_ERROR", error)
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
logger.error("PROFILE_FETCH_ERROR", error)
logger.error("PROFILE_FETCH_ERROR", error)
return {
id: profile.id,
name: profile.username
}

Maybe return the profile id and username in case of an error also?

@balazsorban44 balazsorban44 added the enhancement New feature or request label Dec 7, 2020
@balazsorban44
Copy link
Member

@PolMrt I added another suggestion. Also, could you make sure that there are no linting errors, and could you report back if you have tested these changes locally successfully? Thank you!

@vercel vercel bot temporarily deployed to Preview December 7, 2020 14:56 Inactive
@PolMrt
Copy link
Author

PolMrt commented Dec 11, 2020

I don't think we're in the good path right now. Let me explain :

My initial idea (using Instagram public api) was working because I tried it on localhost. Next's server side and client side were using the same IP. So it was always working because of it. But when it will be used in production, this would never work, not even once since the IP of the server has never been used in Instagram.

I think we should just return user id and username, nothing else. Implementing a solution that might works some time but not most of the time will be very confusing for developers.

We should say in the doc that only the username is returned and maybe add a second Instagram provider for business account using the API reserved to that kind of account (which returns a lot more of informations).

Please tell me if that sounds good to you and I'll update the PR.

@balazsorban44
Copy link
Member

Sorry for the delay. So I found this documentation: https://developers.facebook.com/docs/instagram-basic-display-api/reference/user

if I understand it correctly, you simply don't have the option to get the user's email?! 😕 Very strange from Instagram.

I'm not sure how to proceed, cc @iaincollins

@balazsorban44 balazsorban44 self-assigned this Jan 17, 2021
@Ericrobert22

This comment has been minimized.

@github-actions
Copy link

github-actions bot commented Mar 5, 2021

🎉 This issue has been resolved in version 3.8.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

@github-actions
Copy link

🎉 This issue has been resolved in version 4.0.0-next.5 🎉

The release is available on:

Your semantic-release bot 📦🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants