-
Notifications
You must be signed in to change notification settings - Fork 10.5k
Address API review feedback for what was IApiEndpointMetadata #63283
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
Changes from 4 commits
Commits
Show all changes
5 commits
Select commit
Hold shift + click to select a range
7daf30e
Address API review feedback for what was IApiEndpointMetadata
halter73 66629db
Apply suggestion from @Copilot
halter73 3941d9a
Update src/Security/Authentication/test/CookieTests.cs
halter73 a126411
Finish fixing local test function typo
halter73 ff36a61
Apply suggestion from @halter73
halter73 File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
20 changes: 0 additions & 20 deletions
20
src/Http/Http.Abstractions/src/Metadata/ApiEndpointMetadata.cs
This file was deleted.
Oops, something went wrong.
20 changes: 20 additions & 0 deletions
20
src/Http/Http.Abstractions/src/Metadata/DisableCookieRedirectMetadata.cs
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,20 @@ | ||
| // Licensed to the .NET Foundation under one or more agreements. | ||
| // The .NET Foundation licenses this file to you under the MIT license. | ||
|
|
||
| namespace Microsoft.AspNetCore.Http.Metadata; | ||
|
|
||
| /// <summary> | ||
| /// Metadata that indicates the endpoint should disable cookie-based authentication redirects. | ||
| /// When present, authentication handlers should prefer returning status codes over browser redirects. | ||
| /// </summary> | ||
| internal sealed class DisableCookieRedirectMetadata : IDisableCookieRedirectMetadata | ||
| { | ||
| /// <summary> | ||
| /// Singleton instance of <see cref="DisableCookieRedirectMetadata"/>. | ||
| /// </summary> | ||
| public static readonly DisableCookieRedirectMetadata Instance = new(); | ||
|
|
||
| private DisableCookieRedirectMetadata() | ||
| { | ||
| } | ||
| } |
13 changes: 13 additions & 0 deletions
13
src/Http/Http.Abstractions/src/Metadata/IAllowCookieRedirectMetadata.cs
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,13 @@ | ||
| // Licensed to the .NET Foundation under one or more agreements. | ||
| // The .NET Foundation licenses this file to you under the MIT license. | ||
|
|
||
| namespace Microsoft.AspNetCore.Http.Metadata; | ||
|
|
||
| /// <summary> | ||
| /// Metadata that indicates the endpoint should allow cookie-based authentication redirects. | ||
| /// This is normally the default behavior, but it exists to override <see cref="IDisableCookieRedirectMetadata"/> no matter the order. | ||
| /// When present, the cookie authentication handler will prefer browser login or access denied redirects over 401 and 403 status codes. | ||
| /// </summary> | ||
| public interface IAllowCookieRedirectMetadata | ||
| { | ||
| } |
12 changes: 0 additions & 12 deletions
12
src/Http/Http.Abstractions/src/Metadata/IApiEndpointMetadata.cs
This file was deleted.
Oops, something went wrong.
17 changes: 17 additions & 0 deletions
17
src/Http/Http.Abstractions/src/Metadata/IDisableCookieRedirectMetadata.cs
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,17 @@ | ||
| // Licensed to the .NET Foundation under one or more agreements. | ||
| // The .NET Foundation licenses this file to you under the MIT license. | ||
|
|
||
| namespace Microsoft.AspNetCore.Http.Metadata; | ||
|
|
||
| /// <summary> | ||
| /// Metadata that indicates the endpoint should disable cookie-based authentication redirects | ||
| /// typically because it is intended for API clients rather than direct browser navigation. | ||
| /// | ||
| /// <see cref="IAllowCookieRedirectMetadata"/> overrides this no matter the order. | ||
| /// | ||
| /// When present and not overridden, the cookie authentication handler will prefer using | ||
| /// 401 and 403 status codes over redirecting to the login or access denied paths. | ||
| /// </summary> | ||
| public interface IDisableCookieRedirectMetadata | ||
| { | ||
| } |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
16 changes: 16 additions & 0 deletions
16
src/Http/Http.Extensions/src/AllowCookieRedirectAttribute.cs
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,16 @@ | ||
| // Licensed to the .NET Foundation under one or more agreements. | ||
| // The .NET Foundation licenses this file to you under the MIT license. | ||
|
|
||
| using Microsoft.AspNetCore.Http.Metadata; | ||
|
|
||
| namespace Microsoft.AspNetCore.Http; | ||
|
|
||
| /// <summary> | ||
| /// Specifies that cookie-based authentication redirects are allowed for an endpoint. | ||
| /// This is normally the default behavior, but it exists to override <see cref="IDisableCookieRedirectMetadata"/> no matter the order. | ||
| /// When present, the cookie authentication handler will prefer browser login or access denied redirects over 401 and 403 status codes. | ||
| /// </summary> | ||
| [AttributeUsage(AttributeTargets.Method | AttributeTargets.Class)] | ||
| public sealed class AllowCookieRedirectAttribute : Attribute, IAllowCookieRedirectMetadata | ||
| { | ||
| } |
43 changes: 43 additions & 0 deletions
43
src/Http/Http.Extensions/src/CookieRedirectEndpointConventionBuilderExtensions.cs
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,43 @@ | ||
| // Licensed to the .NET Foundation under one or more agreements. | ||
| // The .NET Foundation licenses this file to you under the MIT license. | ||
|
|
||
| using Microsoft.AspNetCore.Http; | ||
| using Microsoft.AspNetCore.Http.Metadata; | ||
|
|
||
| namespace Microsoft.AspNetCore.Builder; | ||
|
|
||
| /// <summary> | ||
| /// Cookie redirect extension methods for <see cref="IEndpointConventionBuilder"/>. | ||
| /// </summary> | ||
| public static class CookieRedirectEndpointConventionBuilderExtensions | ||
| { | ||
| private static readonly AllowCookieRedirectAttribute _allowCookieRedirectAttribute = new(); | ||
|
|
||
| /// <summary> | ||
| /// Specifies that cookie-based authentication redirects are disabled for an endpoint using <see cref="IDisableCookieRedirectMetadata"/>. | ||
| /// When present and not overridden by <see cref="AllowCookieRedirect"/> or <see cref="IAllowCookieRedirectMetadata"/>, | ||
| /// the cookie authentication handler will prefer using 401 and 403 status codes over redirecting to the login or access denied paths. | ||
| /// </summary> | ||
| /// <typeparam name="TBuilder">The type of endpoint convention builder.</typeparam> | ||
| /// <param name="builder">The endpoint convention builder.</param> | ||
| /// <returns>The original convention builder parameter.</returns> | ||
| public static TBuilder DisableCookieRedirect<TBuilder>(this TBuilder builder) where TBuilder : IEndpointConventionBuilder | ||
| { | ||
| builder.Add(b => b.Metadata.Add(DisableCookieRedirectMetadata.Instance)); | ||
| return builder; | ||
| } | ||
|
|
||
| /// <summary> | ||
| /// Specifies that cookie-based authentication redirects are allowed for an endpoint using <see cref="IAllowCookieRedirectMetadata"/>. | ||
| /// This is normally the default behavior, but it exists to override <see cref="IDisableCookieRedirectMetadata"/> no matter the order. | ||
| /// When present, the cookie authentication handler will prefer browser login or access denied redirects over 401 and 403 status codes. | ||
| /// </summary> | ||
| /// <typeparam name="TBuilder">The type of endpoint convention builder.</typeparam> | ||
| /// <param name="builder">The endpoint convention builder.</param> | ||
| /// <returns>The original convention builder parameter.</returns> | ||
| public static TBuilder AllowCookieRedirect<TBuilder>(this TBuilder builder) where TBuilder : IEndpointConventionBuilder | ||
| { | ||
| builder.Add(b => b.Metadata.Add(_allowCookieRedirectAttribute)); | ||
| return builder; | ||
| } | ||
| } |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1 +1,6 @@ | ||
| #nullable enable | ||
| Microsoft.AspNetCore.Builder.CookieRedirectEndpointConventionBuilderExtensions | ||
| Microsoft.AspNetCore.Http.AllowCookieRedirectAttribute | ||
| Microsoft.AspNetCore.Http.AllowCookieRedirectAttribute.AllowCookieRedirectAttribute() -> void | ||
| static Microsoft.AspNetCore.Builder.CookieRedirectEndpointConventionBuilderExtensions.AllowCookieRedirect<TBuilder>(this TBuilder builder) -> TBuilder | ||
| static Microsoft.AspNetCore.Builder.CookieRedirectEndpointConventionBuilderExtensions.DisableCookieRedirect<TBuilder>(this TBuilder builder) -> TBuilder |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
102 changes: 102 additions & 0 deletions
102
src/Http/Http.Extensions/test/CookieRedirectEndpointConventionBuilderExtensionsTests.cs
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,102 @@ | ||
| // Licensed to the .NET Foundation under one or more agreements. | ||
| // The .NET Foundation licenses this file to you under the MIT license. | ||
|
|
||
| using Microsoft.AspNetCore.Builder; | ||
| using Microsoft.AspNetCore.Http.Metadata; | ||
|
|
||
| namespace Microsoft.AspNetCore.Http.Extensions.Tests; | ||
|
|
||
| public class CookieRedirectEndpointConventionBuilderExtensionsTests | ||
| { | ||
| [Fact] | ||
| public void DisableCookieRedirect_AddsMetadata() | ||
| { | ||
| // Arrange | ||
| var builder = new TestEndpointConventionBuilder(); | ||
|
|
||
| // Act | ||
| builder.DisableCookieRedirect(); | ||
|
|
||
| // Assert | ||
| Assert.IsAssignableFrom<IDisableCookieRedirectMetadata>(Assert.Single(builder.Metadata)); | ||
| } | ||
|
|
||
| [Fact] | ||
| public void AllowCookieRedirect_AddsMetadata() | ||
| { | ||
| // Arrange | ||
| var builder = new TestEndpointConventionBuilder(); | ||
|
|
||
| // Act | ||
| builder.AllowCookieRedirect(); | ||
|
|
||
| // Assert | ||
| Assert.IsAssignableFrom<IAllowCookieRedirectMetadata>(Assert.Single(builder.Metadata)); | ||
| } | ||
|
|
||
| [Fact] | ||
| public void DisableCookieRedirect_ReturnsBuilder() | ||
| { | ||
| // Arrange | ||
| var builder = new TestEndpointConventionBuilder(); | ||
|
|
||
| // Act | ||
| var result = builder.DisableCookieRedirect(); | ||
|
|
||
| // Assert | ||
| Assert.Same(builder, result); | ||
| } | ||
|
|
||
| [Fact] | ||
| public void AllowCookieRedirect_ReturnsBuilder() | ||
| { | ||
| // Arrange | ||
| var builder = new TestEndpointConventionBuilder(); | ||
|
|
||
| // Act | ||
| var result = builder.AllowCookieRedirect(); | ||
|
|
||
| // Assert | ||
| Assert.Same(builder, result); | ||
| } | ||
|
|
||
| [Fact] | ||
| public void DisableCookieRedirect_WithMultipleCalls_AddsSingleMetadata() | ||
| { | ||
| // Arrange | ||
| var builder = new TestEndpointConventionBuilder(); | ||
|
|
||
| // Act | ||
| builder.DisableCookieRedirect(); | ||
| builder.DisableCookieRedirect(); | ||
|
|
||
| // Assert | ||
| Assert.Equal(2, builder.Metadata.Count); | ||
| Assert.All(builder.Metadata, metadata => Assert.IsAssignableFrom<IDisableCookieRedirectMetadata>(metadata)); | ||
| } | ||
|
|
||
| [Fact] | ||
| public void AllowCookieRedirect_WithMultipleCalls_AddsSingleMetadata() | ||
| { | ||
| // Arrange | ||
| var builder = new TestEndpointConventionBuilder(); | ||
|
|
||
| // Act | ||
| builder.AllowCookieRedirect(); | ||
| builder.AllowCookieRedirect(); | ||
|
|
||
| // Assert | ||
| Assert.Equal(2, builder.Metadata.Count); | ||
| Assert.All(builder.Metadata, metadata => Assert.IsAssignableFrom<IAllowCookieRedirectMetadata>(metadata)); | ||
| } | ||
halter73 marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||
|
|
||
| private sealed class TestEndpointConventionBuilder : EndpointBuilder, IEndpointConventionBuilder | ||
| { | ||
| public void Add(Action<EndpointBuilder> convention) | ||
| { | ||
| convention(this); | ||
| } | ||
|
|
||
| public override Endpoint Build() => throw new NotImplementedException(); | ||
| } | ||
| } | ||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Uh oh!
There was an error while loading. Please reload this page.