Skip to content

React to MapIdentityApi HTTP endpoints API review feedback #50067

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

Merged
merged 3 commits into from
Aug 14, 2023

Conversation

halter73
Copy link
Member

Fixes #50009

@halter73 halter73 requested a review from BrennanConroy August 14, 2023 22:10
@halter73 halter73 requested a review from Tratcher as a code owner August 14, 2023 22:10
@ghost ghost added the area-identity Includes: Identity and providers label Aug 14, 2023
@@ -90,12 +90,13 @@ public static IEndpointConventionBuilder MapIdentityApi<TUser>(this IEndpointRou
});

routeGroup.MapPost("/login", async Task<Results<Ok<AccessTokenResponse>, EmptyHttpResult, ProblemHttpResult>>
([FromBody] LoginRequest login, [FromQuery] bool? cookieMode, [FromQuery] bool? persistCookies, [FromServices] IServiceProvider sp) =>
([FromBody] LoginRequest login, [FromQuery] bool? useCookies, [FromQuery] bool? useSessionCookies, [FromServices] IServiceProvider sp) =>
Copy link
Member

@BrennanConroy BrennanConroy Aug 14, 2023

Choose a reason for hiding this comment

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

Suggested change
([FromBody] LoginRequest login, [FromQuery] bool? useCookies, [FromQuery] bool? useSessionCookies, [FromServices] IServiceProvider sp) =>
([FromBody] LoginRequest login, [FromQuery] bool? usePersistentCookies, [FromQuery] bool? useSessionCookies, [FromServices] IServiceProvider sp) =>

Was session deliberate?

Copy link
Member Author

@halter73 halter73 Aug 14, 2023

Choose a reason for hiding this comment

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

Was session deliberate?

It was. I'm going to send an email to update the API proposal. The reason is the default behavior prior to this change with just cookieMode=true was to presist cookies. persistCookies=false opted out of that, so persistCookies defaulted to true.

It was a horrible API, but making the simpler useCookies=true persist cookies by default still makes sense in my opinion.

@halter73 halter73 merged commit 1d2c90a into main Aug 14, 2023
@halter73 halter73 deleted the halter73/50009 branch August 14, 2023 23:51
@ghost ghost added this to the 8.0-rc1 milestone Aug 14, 2023
Copy link

@Carl-Binneman Carl-Binneman Jun 19, 2024

Choose a reason for hiding this comment

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

Is there any way to bring back snake_case?
Allowing to set casing would be ideal

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-identity Includes: Identity and providers
Projects
None yet
Development

Successfully merging this pull request may close these issues.

MapIdentityApi HTTP endpoints
3 participants