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: linkedin provider #238

Merged
merged 13 commits into from
Jan 4, 2022
Merged

feat: linkedin provider #238

merged 13 commits into from
Jan 4, 2022

Conversation

riderx
Copy link
Contributor

@riderx riderx commented Oct 17, 2021

What kind of change does this PR introduce?

feature, it add Linkedin Provider

What is the current behavior?

No provider for linkedin before

What is the new behavior?

WIP: Linkedin provider.
Ps: i know nothing of golang and don't like it much, but i really need this provider for socialselling.ai ,so i did try to start the work

So i did try to do it.

Few things are missing :

  • tests, i didn't write or run the code i just made the biding from my js implementation
  • i did try to handle catch all in type for name localization but not sure if it's handle well

Help is more than welcome :)

Additional context

Add any other context or screenshots.

@kangmingtay
Copy link
Member

Hi @riderx, thanks for contributing, could you please fix the linting and formatting errors? If you're using vscode, you can install the go package extension which helps to format the code. Please add the necessary tests too, thanks!

@riderx
Copy link
Contributor Author

riderx commented Oct 18, 2021

@kangmingtay i can fix the lint error, but i don't have any clue on how do testing, i know nothing about Golang, i will need help on this

@riderx
Copy link
Contributor Author

riderx commented Oct 18, 2021

@kangmingtay i fixed lint issue and found a maybe solution for casting.
I did a copy of Apple test and Create Linkedin one, again i need help on this one.

@riderx
Copy link
Contributor Author

riderx commented Oct 22, 2021

@kangmingtay @awalias @anindha @gozeloglu @FlorianBouron your help is welcome :)

@gozeloglu
Copy link
Contributor

@kangmingtay @awalias @anindha @gozeloglu @FlorianBouron your help is welcome :)

What's the problem? What kind of help is needed?

@riderx
Copy link
Contributor Author

riderx commented Oct 25, 2021

@gozeloglu test in env, since i have no clue how to test it. and review the code if you see errors. I'm a super begginner in go

@riderx
Copy link
Contributor Author

riderx commented Oct 25, 2021

event the failling of the test i don't understand why : https://github.com/supabase/gotrue/runs/3972582714?check_suite_focus=true

HarryET added a commit to WalletConnect/gotrue that referenced this pull request Nov 14, 2021
Takes code from supabase#238 and finished implementation, still needs improvements in provider impl file
@HarryET
Copy link
Contributor

HarryET commented Nov 14, 2021

I added a working implementation to this in #269

@dthyresson
Copy link

Hi @riderx.

Thanks for adding these envars.

In order for them to be recognized, could you also add the provider and config for LinkedIn as well?

Not sure if they need to be: LINKEDIN or LINKED_IN to match a LinkedIn or LinkedIn case.

Thanks!

See: https://github.com/supabase/gotrue/blob/1dddc248fb820e638f0d54b9d51c2803295b5c1d/api/external.go#L365

and

See: https://github.com/supabase/gotrue/blob/1dddc248fb820e638f0d54b9d51c2803295b5c1d/conf/configuration.go#L82

@HarryET
Copy link
Contributor

HarryET commented Nov 14, 2021

Hi @riderx.

Thanks for adding these envars.

In order for them to be recognized, could you also add the provider and config for LinkedIn as well?

Not sure if they need to be: LINKEDIN or LINKED_IN to match a LinkedIn or LinkedIn case.

Thanks!

See:

https://github.com/supabase/gotrue/blob/1dddc248fb820e638f0d54b9d51c2803295b5c1d/api/external.go#L365

and

See:

https://github.com/supabase/gotrue/blob/1dddc248fb820e638f0d54b9d51c2803295b5c1d/conf/configuration.go#L82

Hi this isn't being implemented in this PR. It was added to my other PR #269

@HarryET
Copy link
Contributor

HarryET commented Nov 14, 2021

@riderx might be worth closing this now that its been implemented in #269

@dthyresson
Copy link

Thanks @HarryET - I hadn't seen your PR #269.

I'll run a few tests later and check with @kangmingtay but agree that your PR means that this can be closed.

@HarryET
Copy link
Contributor

HarryET commented Nov 14, 2021

Thanks @HarryET - I hadn't seen your PR #269.

I'll run a few tests later and check with @kangmingtay but agree that your PR means that this can be closed.

Awesome. I still need to get my GoTrue Local Environment sorted, but from the looks of things should be working. Thanks @dthyresson

@dthyresson
Copy link

I still need to get my GoTrue Local Environment sorted,

I setup a local GoTrue last week. Was a little bumpy, but I intend to update the docs and contributing readme to make it smoother for new contributors.

I'll be sure to cc you on the PR so you can test it out and try it out.

@HarryET
Copy link
Contributor

HarryET commented Nov 14, 2021

I still need to get my GoTrue Local Environment sorted,

I setup a local GoTrue last week. Was a little bumpy, but I intend to update the docs and contributing readme to make it smoother for new contributors.

I'll be sure to cc you on the PR so you can test it out and try it out.

Awesome. I've really enjoyed contributing to GoTrue so far just need a better local setup to tackle issues other than providers. I don't know if you saw but I have an RFC for 2FA just need to pad it out a bit more but though about trying to get a PR rolling if that was something you were interested in?

@dthyresson
Copy link

I have an RFC for 2FA

I did not, but a fan of 2FA and I have seen requests for this feature. I'll be sure to watch it!

@riderx
Copy link
Contributor Author

riderx commented Nov 14, 2021

@dthyresson thanks for the doc improvement as a newbe in golang, i wasn't able to run gotrue locally, all my code was done without been able to test anything, it was very furstrating.
Thanks a lot for the help guys

@riderx riderx closed this Nov 14, 2021
@riderx riderx reopened this Nov 19, 2021
@riderx
Copy link
Contributor Author

riderx commented Nov 19, 2021

as @dthyresson say in #269 it's better to have specific Pr, so I reopen this one with the fix of @HarryET, thanks a lot for the work

@riderx
Copy link
Contributor Author

riderx commented Nov 26, 2021

@HarryET took 1 hour with me to make Gotrue work.
Thanks for that.
@dthyresson please take this into consideration, to help the project we need smooth install process.

Now scripts fail, we had to fix so many things.

I have Gotrue work, does the redirect to Linkedin, and then redirect back to my test app, but I'm stuck again since I run only Gotrue, Supabase refuse:

{error: "invalid_grant", error_description: "Invalid Refresh Token"}
error: "invalid_grant"
error_description: "Invalid Refresh Token"

Can we have a docker-compose who run everything in one time ?
to help test :)

Here is my test code :

import { GoTrueClient } from '@supabase/gotrue-js'
import { supabase } from '../services/supabase'

const route = useRoute()

const GOTRUE_URL = 'http://localhost:9999'

const auth = new GoTrueClient({ url: GOTRUE_URL })

const code = String(route.query?.code)
console.log('code', code)
if (!code) {
  auth.signIn({
    provider: 'linkedin',
  }).then((user) => {
    console.log(user)
  })
}
else {
  supabase.auth.signIn({
    refreshToken: code,
  }).then((user) => {
    console.log('user', user)
  })

@riderx riderx changed the title feat: linkedin provider WIP feat: linkedin provider Dec 6, 2021
@riderx
Copy link
Contributor Author

riderx commented Dec 16, 2021

@dthyresson do you need more things to review the PR ?

@kangmingtay kangmingtay merged commit 786efee into supabase:master Jan 4, 2022
@github-actions
Copy link
Contributor

github-actions bot commented Jan 4, 2022

🎉 This PR is included in version 2.3.0 🎉

The release is available on GitHub release

Your semantic-release bot 📦🚀

@kangmingtay
Copy link
Member

hi @riderx, thanks for laying the groundwork for the linkedin provider! apologies for the delay, i know it's been put off for quite awhile but i've made some changes to fix this issue with the PR and also added some tests!

@riderx
Copy link
Contributor Author

riderx commented Jan 4, 2022

@kangmingtay thanks a lot <3
I understand you cannot handle everything in same time, it's normal.
I would love a bit more communication about what you handle, otherwise happy to be a part of it :)

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

Successfully merging this pull request may close these issues.

None yet

5 participants