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

FEATURE: Allow users to sign in using LinkedIn OpenID Connect #26281

Merged
merged 4 commits into from Apr 19, 2024

Conversation

Drenmi
Copy link
Contributor

@Drenmi Drenmi commented Mar 21, 2024

What is this change?

LinkedIn has grandfathered its old OAuth2 provider. This can only be used by existing apps. New apps have to use the new OIDC provider.

This PR adds a linkedin_oidc provider to core. This will exist alongside the discourse-linkedin-auth plugin, which will be kept for those still using the deprecated provider.

I chose to inline the authenticator for now, because the omniauth-linkedin-oidc gem only supports omniauth >= 2. Once we're done with the OmniAuth upgrade we can choose to keep this inline or switch to the gem.

Does it even work?

Running on my local, authenticating against my own LinkedIn developer app, and using ngrok to expose the callback URL to the internet.

linkedin-signup

@github-actions github-actions bot added the i18n PRs which update English locale files or i18n related code label Mar 21, 2024
@Drenmi Drenmi force-pushed the feature/linkedin-oidc-logins branch 5 times, most recently from 2f973ea to 05d045b Compare March 25, 2024 03:54
@Drenmi Drenmi force-pushed the feature/linkedin-oidc-logins branch 3 times, most recently from ce878c7 to 236238b Compare April 19, 2024 04:36
@Drenmi Drenmi force-pushed the feature/linkedin-oidc-logins branch from 236238b to c628899 Compare April 19, 2024 05:15
@Drenmi Drenmi marked this pull request as ready for review April 19, 2024 05:27
@discoursebot
Copy link

This pull request has been mentioned on Discourse Meta. There might be relevant details there:

https://meta.discourse.org/t/sign-in-with-linkedin-method-deprecated-by-linkedin/288164/22

Copy link
Contributor

@martin-brennan martin-brennan left a comment

Choose a reason for hiding this comment

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

Looks good to me, but yeah might be worth David having a look too because he is very familiar with these auth flows. Also this part:

This will exist alongside the discourse-linkedin-auth plugin, which will be kept for those still using the deprecated provider.

IIRC we still want to get rid of this plugin and move that code into core as another provider right? Could call it linkedin_legacy_authenticator or something.

config/locales/server.en.yml Show resolved Hide resolved
@Drenmi
Copy link
Contributor Author

Drenmi commented Apr 19, 2024

IIRC we still want to get rid of this plugin and move that code into core as another provider right?

I don't think I've seen this mentioned, but given no new customers can use the legacy provider, maybe it's not worth it. Especially since they would need to update their existing setups.

@davidtaylorhq
Copy link
Member

IIRC we still want to get rid of this plugin and move that code into core as another provider right?

I don't think I've seen this mentioned, but given no new customers can use the legacy provider, maybe it's not worth it. Especially since they would need to update their existing setups.

Yeah I agree - makes sense to keep the old implementation in the plugin. As soon as this core change is merged, we can introduce an admin warning in the plugin like

⚠️ discourse-linkedin-auth is deprecated. Switch to new provider using instructions on meta

Then we can archive the plugin repo and mark it EOL on Meta.

@Drenmi Drenmi force-pushed the feature/linkedin-oidc-logins branch from b382aa5 to b0786c9 Compare April 19, 2024 10:05
@Drenmi Drenmi merged commit 9e31135 into main Apr 19, 2024
16 checks passed
@Drenmi Drenmi deleted the feature/linkedin-oidc-logins branch April 19, 2024 10:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
i18n PRs which update English locale files or i18n related code
4 participants