-
Notifications
You must be signed in to change notification settings - Fork 541
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
Support for the new LinkedIn API version format #834
Support for the new LinkedIn API version format #834
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for your contribution.
This PR needs work before we can accept it due to the breaking nature of the changes to the library's public API surface and other changes to the test code that were not necessary.
test/AspNet.Security.OAuth.Providers.Tests/Infrastructure/ApplicationFactory.cs
Outdated
Show resolved
Hide resolved
test/AspNet.Security.OAuth.Providers.Tests/LinkedIn/LinkedInTests.cs
Outdated
Show resolved
Hide resolved
…wareolatomiwa/AspNet.Security.OAuth.Providers into dev_linkedin_updated_scopes
…wareolatomiwa/AspNet.Security.OAuth.Providers into dev_linkedin_updated_scopes
src/AspNet.Security.OAuth.LinkedIn/LinkedInAuthenticationConstants.cs
Outdated
Show resolved
Hide resolved
src/AspNet.Security.OAuth.LinkedIn/LinkedInAuthenticationHandler.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/LinkedInAuthenticationOptions.cs
Outdated
Show resolved
Hide resolved
…ants.cs Co-authored-by: Martin Costello <martin@martincostello.com>
…wareolatomiwa/AspNet.Security.OAuth.Providers into dev_linkedin_updated_scopes
@@ -6,6 +6,7 @@ | |||
|
|||
using System.Security.Claims; | |||
using System.Text.Json; | |||
using Microsoft.Extensions.Options; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Doesn't look like this is needed for anything?
Can you also confirm that this all works as expected with the real live LinkedIn API? |
Given it's technically a behavior breaking change - that requires subscribing to the new OIDC-based service - it would probably make more sense to bump the minor version (i.e 8.1 instead of 8.0.1). We'll also need to mention that clearly in the release notes. |
@martincostello I confirm it works as expected with the real live LinkedIn API |
Hi @martincostello |
@kevinchalet So do you want to re-version everything as 8.1.0 for releasing this? |
Yeah, that would be better. In the same vein, we should probably bump the minor version instead of the patch version when introducing new features/providers: only bumping the patch version is something we had to do when ASP.NET Core had minor versions (as we wanted to have matching major+minor versions), but it's no longer necessary now that .NET never increases its minor version. |
OK - once I merge the open PRs, I'll do a follow up to update |
Thanks for your contribution @softwareolatomiwa - these changes are now available in version 8.1.0. |
New Apps created on LinkedIn returns with Error 403 on getting user details using the /me endpoint. The endpoint has been changed and scope also has been changed to include openid, profile and email
ALso the format of user data returned has changed.
https://www.linkedin.com/developers/news/featured-updates/openid-connect-authentication