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.

Sign up for GitHub

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

Jump to bottom

Update OpenIddict.Core 4.2.0 #13467

Merged
merged 2 commits into from
Mar 25, 2023
Merged

Update OpenIddict.Core 4.2.0 #13467

merged 2 commits into from
Mar 25, 2023

Conversation

hishamco
Copy link
Member

No description provided.

@hishamco hishamco requested a review from kevinchalet March 25, 2023 19:31
@hishamco hishamco requested a review from agriffard as a code owner March 25, 2023 19:31
@hishamco hishamco removed the request for review from agriffard March 25, 2023 19:32
kevinchalet
kevinchalet reviewed Mar 25, 2023
View reviewed changes
Copy link
Member

@kevinchalet kevinchalet left a comment

Choose a reason for hiding this comment

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

Thanks!

src/OrchardCore.Build/Dependencies.props Outdated
@@ -47,7 +47,7 @@
<PackageManagement Include="NLog.Web.AspNetCore" Version="5.2.2" />
<PackageManagement Include="NodaTime" Version="3.1.8" />
<PackageManagement Include="OpenIddict.AspNetCore" Version="4.1.0" />
Copy link
Member

Choose a reason for hiding this comment

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

You'll also want to bump this package to 4.2.0 😃

Copy link
Member Author

@hishamco hishamco Mar 25, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

Oops

BTW seems you added 26 new providers on 4.2.0

Copy link
Member

Choose a reason for hiding this comment

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

BTW seems you added 26 new providers on 4.2.0

Yeah. That said, Orchard's OpenID module is not based on the new OpenIddict client (it uses MSFT's OIDC client), so it's not something the OC users will benefit from 😃

If there was demand from the community, replacing the MSFT OIDC client by OpenIddict is definitely something we could consider (but it would be a breaking change, so not something to take lightly).

Copy link
Member Author

Choose a reason for hiding this comment

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

If you don't mind I might need your help to build a new OpenId module based on OpenIddict on OCC

Copy link
Member

Choose a reason for hiding this comment

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

Sure, what do you have in mind?

Copy link
Member Author

Choose a reason for hiding this comment

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

I might open an issue in OCC repo describing what's my intend in more details

@hishamco hishamco requested a review from kevinchalet March 25, 2023 19:57
@hishamco hishamco merged commit 5eca92d into main Mar 25, 2023
@hishamco hishamco deleted the hishamco/openiddict branch March 25, 2023 20:08
@hishamco hishamco mentioned this pull request Mar 28, 2023
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@kevinchalet kevinchalet kevinchalet approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@hishamco @kevinchalet