-
Notifications
You must be signed in to change notification settings - Fork 550
Fix LinkedIn provider with v2 endpoints to get name, email and profile pictures #298
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
Conversation
src/AspNet.Security.OAuth.LinkedIn/LinkedInAuthenticationConstants.cs
Outdated
Show resolved
Hide resolved
src/AspNet.Security.OAuth.LinkedIn/LinkedInAuthenticationConstants.cs
Outdated
Show resolved
Hide resolved
src/AspNet.Security.OAuth.LinkedIn/LinkedInAuthenticationConstants.cs
Outdated
Show resolved
Hide resolved
src/AspNet.Security.OAuth.LinkedIn/LinkedInAuthenticationConstants.cs
Outdated
Show resolved
Hide resolved
src/AspNet.Security.OAuth.LinkedIn/LinkedInAuthenticationDefaults.cs
Outdated
Show resolved
Hide resolved
src/AspNet.Security.OAuth.LinkedIn/LinkedInAuthenticationOptions.cs
Outdated
Show resolved
Hide resolved
src/AspNet.Security.OAuth.LinkedIn/LinkedInAuthenticationOptions.cs
Outdated
Show resolved
Hide resolved
src/AspNet.Security.OAuth.LinkedIn/LinkedInAuthenticationOptions.cs
Outdated
Show resolved
Hide resolved
src/AspNet.Security.OAuth.LinkedIn/LinkedInAuthenticationOptions.cs
Outdated
Show resolved
Hide resolved
test/AspNet.Security.OAuth.Providers.Tests/LinkedIn/bundle.json
Outdated
Show resolved
Hide resolved
|
Thanks for the contribution @asiffermann - as this includes code breaking-changes, this is probably not going to be released as part of any 2.x release (if we do one) as it would need to be 3.0 for SemVer, and not shipping ASP.NET Core 3.0 support in the 3.0 release would be confusing. |
|
Other than my comment regarding localisation, this looks good! 😎 |
|
I was answering to your comment regarding localization, I'd be glad to read your thoughts about it 😉 About the release, I understand it will be difficult for you to release it independently, but it is now a critical issue for this library, as LinkedIn apps are now being forced to use the new API (the v1 endpoint now returns HTTP error code 410). So it should be considered as a bug fix rather than a breaking change enhancement 😉 |
|
I understand, although in the short-term you could use the version with your fixes (once this is merged) from the MyGet feed from a pre-release version of the package. Maybe @PinpointTownes has some opinions on how to manage this? |
…lized value should be taken
src/AspNet.Security.OAuth.LinkedIn/LinkedInAuthenticationOptions.cs
Outdated
Show resolved
Hide resolved
src/AspNet.Security.OAuth.LinkedIn/LinkedInAuthenticationOptions.cs
Outdated
Show resolved
Hide resolved
src/AspNet.Security.OAuth.LinkedIn/LinkedInAuthenticationOptions.cs
Outdated
Show resolved
Hide resolved
src/AspNet.Security.OAuth.LinkedIn/LinkedInAuthenticationOptions.cs
Outdated
Show resolved
Hide resolved
src/AspNet.Security.OAuth.LinkedIn/LinkedInAuthenticationOptions.cs
Outdated
Show resolved
Hide resolved
src/AspNet.Security.OAuth.LinkedIn/LinkedInAuthenticationOptions.cs
Outdated
Show resolved
Hide resolved
src/AspNet.Security.OAuth.LinkedIn/LinkedInAuthenticationOptions.cs
Outdated
Show resolved
Hide resolved
src/AspNet.Security.OAuth.LinkedIn/LinkedInAuthenticationOptions.cs
Outdated
Show resolved
Hide resolved
test/AspNet.Security.OAuth.Providers.Tests/LinkedIn/LinkedInTests.cs
Outdated
Show resolved
Hide resolved
….OAuth.Providers into dev
src/AspNet.Security.OAuth.LinkedIn/LinkedInAuthenticationOptions.cs
Outdated
Show resolved
Hide resolved
Yeas please, any simple instructions in how to get this fix? (in the case it won't be released soon) |
|
The quickest way to consume this is to download the package from the AppVeyor CI Build and check into source control and configure a local from-disk NuGet feed and reference the prerelease version from there. For instance I’m using this approach for 3.0 support for the Amazon provider here: martincostello/alexa-london-travel-site#255 If this is merged but retained for a future 3.0 release in September (i.e. we don’t push to NuGet.org as SemVer breaking and want to reserve the 3.0 version number for simplicity), an alternative is that you could consume it from the MyGet feed in the README. |
|
As an alternative, which would allow this to be a minor release, you could add back all the deleted members and instead mark them as |
|
In fact, I think that's the best approach. Restore the deleted constants, and mark them as something like This could then be released as a 2.1 version and remain SemVer compatible. Users who've actually consumed the constants whose values have changed in their own code would just need to recompile against a later version to pick up the value changes. |
|
Thanks for your work on this @asiffermann 🎈 I'll merge it later on today. |
|
Thanks to you for your review! 😉 |
As mentioned in #277, Linkedin has enforced users to migrate to API version 2.0 by May 1.
The migration FAQ describes the main differences:
This pull request implements all those changes. Feel free to ask me any changes you'd like!