Proposal to add option for Method Level Authorization header attribute#897
Conversation
|
Important Review skippedDraft detected. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing touches🧪 Generate unit tests (beta)
Tip Issue Planner is now in beta. Read the docs and try it out! Share your feedback on Discord. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Roflincopter
force-pushed
the
add-option-to-generate-method-level-authorization-header-attribute
branch
from
59881ac to
7dbf6c0
Compare
|
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #897 +/- ##
==========================================
- Coverage 92.95% 92.67% -0.28%
==========================================
Files 23 23
Lines 2031 1407 -624
==========================================
- Hits 1888 1304 -584
- Misses 48 56 +8
+ Partials 95 47 -48
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Pull request overview
This pull request enhances authentication header generation by replacing a boolean setting with an enum-based approach and adding support for bearer token authentication. The changes enable generating authentication headers either as method-level attributes (using Refit's AuthorizationHeaderValueGetter) or as method parameters, addressing a previously unexposed feature and extending it to support modern bearer token authentication patterns.
Changes:
- Introduces
AuthenticationHeaderStyleenum with three modes: None, Parameter, and Method - Exposes authentication header generation to the CLI tool via
--generate-authentication-headeroption - Adds bearer token support for both Parameter and Method styles
- Updates all existing tests to use the new enum-based API
Reviewed changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated 7 comments.
Show a summary per file
| File | Description |
|---|---|
| src/Refitter.Core/Settings/AuthenticationHeaderStyle.cs | New enum defining three authentication header generation modes |
| src/Refitter.Core/Settings/RefitGeneratorSettings.cs | Replaces boolean property with enum-typed property |
| src/Refitter/Settings.cs | Adds CLI option for authentication header generation |
| src/Refitter/GenerateCommand.cs | Maps CLI setting to core generator setting |
| src/Refitter.Core/RefitInterfaceGenerator.cs | Implements Method-style header generation |
| src/Refitter.Core/ParameterExtractor.cs | Adds bearer token support to Parameter-style generation |
| src/Refitter.Tests/SwaggerPetstoreTests.cs | Updates tests to use new enum API |
| src/Refitter.Tests/SwaggerPetstoreApizrTests.cs | Updates Apizr tests to use new enum API |
| if (securityScheme is { Type: OpenApiSecuritySchemeType.ApiKey, In: OpenApiSecurityApiKeyLocation.Header } | ||
| && !operationModel.Parameters.Any(p => p is { Kind: OpenApiParameterKind.Header, IsHeader: true } && p.Name == securityScheme.Name)) | ||
| { | ||
| headers.Add($"\"{securityScheme.Name}\""); | ||
| } | ||
| else if (securityScheme is { Type: OpenApiSecuritySchemeType.Http, Scheme: "bearer" }) | ||
| { |
There was a problem hiding this comment.
The logic for handling ApiKey headers in Method style appears problematic. At line 295, the code adds only the header name without a value (e.g., "auth_key"), which would generate [Headers("auth_key")]. However, Refit's Headers attribute expects the format "HeaderName: value" for static headers. Since ApiKey headers require dynamic values (the actual API key), they cannot be meaningfully represented as static method-level headers. Consider either removing this branch for ApiKey headers in Method style, or documenting that Method style only supports HTTP Bearer authentication and not ApiKey authentication.
| if (securityScheme is { Type: OpenApiSecuritySchemeType.ApiKey, In: OpenApiSecurityApiKeyLocation.Header } | |
| && !operationModel.Parameters.Any(p => p is { Kind: OpenApiParameterKind.Header, IsHeader: true } && p.Name == securityScheme.Name)) | |
| { | |
| headers.Add($"\"{securityScheme.Name}\""); | |
| } | |
| else if (securityScheme is { Type: OpenApiSecuritySchemeType.Http, Scheme: "bearer" }) | |
| { | |
| if (securityScheme is { Type: OpenApiSecuritySchemeType.Http, Scheme: "bearer" }) | |
| { |
| [Test] | ||
| [Arguments(SampleOpenSpecifications.SwaggerPetstoreJsonV3WithUnsafeAuthenticationHeaders, "SwaggerPetstoreWithUnsafeAuthenticationHeaders.json")] | ||
| [Arguments(SampleOpenSpecifications.SwaggerPetstoreYamlV3WithUnsafeAuthenticationHeaders, "SwaggerPetstoreWithUnsafeAuthenticationHeaders.yaml")] | ||
| [Arguments(SampleOpenSpecifications.SwaggerPetstoreJsonV2WithUnsafeAuthenticationHeaders, "SwaggerPetstoreWithUnsafeAuthenticationHeaders.json")] | ||
| [Arguments(SampleOpenSpecifications.SwaggerPetstoreYamlV2WithUnsafeAuthenticationHeaders, "SwaggerPetstoreWithUnsafeAuthenticationHeaders.yaml")] | ||
| public async Task Can_Generate_Code_With_Unsafe_AuthenticationHeaders(SampleOpenSpecifications version, string filename) | ||
| { | ||
| var settings = new RefitGeneratorSettings(); | ||
| settings.GenerateAuthenticationHeader = true; | ||
| settings.AuthenticationHeaderStyle = AuthenticationHeaderStyle.Parameter; | ||
| var generatedCode = await GenerateCode(version, filename, settings); | ||
| generatedCode.Should().Contain("[Header(\"auth.key\")] string auth_key"); | ||
| } | ||
|
|
||
| [Test] | ||
| [Arguments(SampleOpenSpecifications.SwaggerPetstoreJsonV3WithAuthenticationHeaders, "SwaggerPetstoreWithAuthenticationHeaders.json")] | ||
| [Arguments(SampleOpenSpecifications.SwaggerPetstoreYamlV3WithAuthenticationHeaders, "SwaggerPetstoreWithAuthenticationHeaders.yaml")] | ||
| [Arguments(SampleOpenSpecifications.SwaggerPetstoreJsonV2WithAuthenticationHeaders, "SwaggerPetstoreWithAuthenticationHeaders.json")] | ||
| [Arguments(SampleOpenSpecifications.SwaggerPetstoreYamlV2WithAuthenticationHeaders, "SwaggerPetstoreWithAuthenticationHeaders.yaml")] | ||
| public async Task Can_Generate_Code_With_AuthenticationHeaders(SampleOpenSpecifications version, string filename) | ||
| { | ||
| var settings = new RefitGeneratorSettings(); | ||
| settings.GenerateAuthenticationHeader = true; | ||
| settings.AuthenticationHeaderStyle = AuthenticationHeaderStyle.Parameter; | ||
| var generatedCode = await GenerateCode(version, filename, settings); | ||
| generatedCode.Should().Contain("[Header(\"auth_key\")] string auth_key"); | ||
| } | ||
|
|
||
| [Test] | ||
| [Arguments(SampleOpenSpecifications.SwaggerPetstoreJsonV3WithAuthenticationHeaders, "SwaggerPetstoreWithAuthenticationHeaders.json")] | ||
| [Arguments(SampleOpenSpecifications.SwaggerPetstoreYamlV3WithAuthenticationHeaders, "SwaggerPetstoreWithAuthenticationHeaders.yaml")] | ||
| [Arguments(SampleOpenSpecifications.SwaggerPetstoreJsonV2WithAuthenticationHeaders, "SwaggerPetstoreWithAuthenticationHeaders.json")] | ||
| [Arguments(SampleOpenSpecifications.SwaggerPetstoreYamlV2WithAuthenticationHeaders, "SwaggerPetstoreWithAuthenticationHeaders.yaml")] | ||
| public async Task Can_Generate_Code_With_AuthenticationHeaders_Without_OperationHeaders(SampleOpenSpecifications version, string filename) | ||
| { | ||
| var settings = new RefitGeneratorSettings(); | ||
| settings.GenerateOperationHeaders = false; | ||
| settings.GenerateAuthenticationHeader = true; | ||
| settings.AuthenticationHeaderStyle = AuthenticationHeaderStyle.Parameter; | ||
| var generatedCode = await GenerateCode(version, filename, settings); | ||
| generatedCode.Should().Contain("[Header(\"auth_key\")] string auth_key"); | ||
| } | ||
|
|
||
| [Test] | ||
| [Arguments(SampleOpenSpecifications.SwaggerPetstoreJsonV3, "SwaggerPetstore.json")] | ||
| [Arguments(SampleOpenSpecifications.SwaggerPetstoreYamlV3, "SwaggerPetstore.yaml")] | ||
| [Arguments(SampleOpenSpecifications.SwaggerPetstoreJsonV2, "SwaggerPetstore.json")] | ||
| [Arguments(SampleOpenSpecifications.SwaggerPetstoreYamlV2, "SwaggerPetstore.yaml")] | ||
| public async Task Can_Generate_Code_Without_AuthenticationHeaders(SampleOpenSpecifications version, string filename) | ||
| { | ||
| var settings = new RefitGeneratorSettings(); | ||
| settings.GenerateAuthenticationHeader = false; | ||
| settings.AuthenticationHeaderStyle = AuthenticationHeaderStyle.None; | ||
| var generatedCode = await GenerateCode(version, filename, settings); | ||
| generatedCode.Should().NotContain("[Header(\"auth_key\")] string? auth_key"); | ||
| } |
There was a problem hiding this comment.
There are no tests for the new AuthenticationHeaderStyle.Method behavior. The PR introduces a new way to generate authentication headers, but all existing tests only cover AuthenticationHeaderStyle.Parameter and AuthenticationHeaderStyle.None. Add tests that verify Method style generates the correct [Headers("Authorization: Bearer")] attribute for bearer token authentication, and ensure the generated code compiles and behaves correctly. Consider testing with both the existing Swagger Petstore specs and a new spec that includes bearer token security schemes.
| [Arguments(SampleOpenSpecifications.SwaggerPetstoreJsonV2WithUnsafeAuthenticationHeaders, "SwaggerPetstoreWithUnsafeAuthenticationHeaders.json")] | ||
| [Arguments(SampleOpenSpecifications.SwaggerPetstoreYamlV2WithUnsafeAuthenticationHeaders, "SwaggerPetstoreWithUnsafeAuthenticationHeaders.yaml")] | ||
| public async Task Can_Generate_Code_With_Unsafe_AuthenticationHeaders(SampleOpenSpecifications version, string filename) | ||
| { | ||
| var settings = new ApizrGeneratorSettings { GenerateAuthenticationHeader = true }; | ||
| var settings = new ApizrGeneratorSettings { AuthenticationHeaderStyle = AuthenticationHeaderStyle.Parameter }; | ||
| var generatedCode = await GenerateCode(version, filename, settings); | ||
| generatedCode.Should().Contain("[Header(\"auth.key\")] string auth_key"); | ||
| } | ||
|
|
||
| [Test] | ||
| [Arguments(SampleOpenSpecifications.SwaggerPetstoreJsonV3WithAuthenticationHeaders, "SwaggerPetstoreWithAuthenticationHeaders.json")] | ||
| [Arguments(SampleOpenSpecifications.SwaggerPetstoreYamlV3WithAuthenticationHeaders, "SwaggerPetstoreWithAuthenticationHeaders.yaml")] | ||
| [Arguments(SampleOpenSpecifications.SwaggerPetstoreJsonV2WithAuthenticationHeaders, "SwaggerPetstoreWithAuthenticationHeaders.json")] | ||
| [Arguments(SampleOpenSpecifications.SwaggerPetstoreYamlV2WithAuthenticationHeaders, "SwaggerPetstoreWithAuthenticationHeaders.yaml")] | ||
| public async Task Can_Generate_Code_With_AuthenticationHeaders(SampleOpenSpecifications version, string filename) | ||
| { | ||
| var settings = new ApizrGeneratorSettings { GenerateAuthenticationHeader = true }; | ||
| var settings = new ApizrGeneratorSettings { AuthenticationHeaderStyle = AuthenticationHeaderStyle.Parameter }; | ||
| var generatedCode = await GenerateCode(version, filename, settings); | ||
| generatedCode.Should().Contain("[Header(\"auth_key\")] string auth_key"); | ||
| } | ||
|
|
||
| [Test] | ||
| [Arguments(SampleOpenSpecifications.SwaggerPetstoreJsonV3WithAuthenticationHeaders, "SwaggerPetstoreWithAuthenticationHeaders.json")] | ||
| [Arguments(SampleOpenSpecifications.SwaggerPetstoreYamlV3WithAuthenticationHeaders, "SwaggerPetstoreWithAuthenticationHeaders.yaml")] | ||
| [Arguments(SampleOpenSpecifications.SwaggerPetstoreJsonV2WithAuthenticationHeaders, "SwaggerPetstoreWithAuthenticationHeaders.json")] | ||
| [Arguments(SampleOpenSpecifications.SwaggerPetstoreYamlV2WithAuthenticationHeaders, "SwaggerPetstoreWithAuthenticationHeaders.yaml")] | ||
| public async Task Can_Generate_Code_With_AuthenticationHeaders_Without_OperationHeaders(SampleOpenSpecifications version, string filename) | ||
| { | ||
| var settings = new ApizrGeneratorSettings { GenerateOperationHeaders = false, GenerateAuthenticationHeader = true }; | ||
| var settings = new ApizrGeneratorSettings { GenerateOperationHeaders = false, AuthenticationHeaderStyle = AuthenticationHeaderStyle.Parameter }; | ||
| var generatedCode = await GenerateCode(version, filename, settings); | ||
| generatedCode.Should().Contain("[Header(\"auth_key\")] string auth_key"); | ||
| } | ||
|
|
||
| [Test] | ||
| [Arguments(SampleOpenSpecifications.SwaggerPetstoreJsonV3, "SwaggerPetstore.json")] | ||
| [Arguments(SampleOpenSpecifications.SwaggerPetstoreYamlV3, "SwaggerPetstore.yaml")] | ||
| [Arguments(SampleOpenSpecifications.SwaggerPetstoreJsonV2, "SwaggerPetstore.json")] | ||
| [Arguments(SampleOpenSpecifications.SwaggerPetstoreYamlV2, "SwaggerPetstore.yaml")] | ||
| public async Task Can_Generate_Code_Without_AuthenticationHeaders(SampleOpenSpecifications version, string filename) | ||
| { | ||
| var settings = new ApizrGeneratorSettings { GenerateAuthenticationHeader = false }; | ||
| var settings = new ApizrGeneratorSettings { AuthenticationHeaderStyle = AuthenticationHeaderStyle.None }; | ||
| var generatedCode = await GenerateCode(version, filename, settings); | ||
| generatedCode.Should().NotContain("[Header(\"auth_key\")] string? auth_key"); | ||
| } |
There was a problem hiding this comment.
Similar to the tests in SwaggerPetstoreTests.cs, there are no tests for the new AuthenticationHeaderStyle.Method behavior. Add tests to verify that the Method style generates the correct [Headers("Authorization: Bearer")] attribute when using Apizr settings.
| [DefaultValue(null)] | ||
| public string? CustomTemplateDirectory { get; set; } = null; | ||
|
|
||
| [Description("Generate Authorization header method 'Method' or 'Parameter'. Default value is None")] |
There was a problem hiding this comment.
The new CLI option --generate-authentication-header should be documented in the README.md file. Add a description explaining the three modes (None, Parameter, Method), when to use each, and provide examples showing the generated output for both Parameter and Method styles. The Parameter style should explain that it adds method parameters for authentication, while Method style should explain that it uses Refit's AuthorizationHeaderValueGetter delegate for bearer tokens.
| [Description("Generate Authorization header method 'Method' or 'Parameter'. Default value is None")] | |
| [Description("Controls generation of Authorization header support. Options: None (no authentication code is generated), Parameter (adds method parameters such as string accessToken that are used to build the Authorization: Bearer header for each call), and Method (generates a Refit AuthorizationHeaderValueGetter delegate that returns a bearer token used for the Authorization header). Example Parameter style output: Task<User> GetUserAsync(string id, string accessToken, CancellationToken cancellationToken = default). Example Method style output: Task<User> GetUserAsync(string id, CancellationToken cancellationToken = default) with a generated AuthorizationHeaderValueGetter that supplies the bearer token. Default value is None.")] |
| /// The Authentication header style to use. | ||
| /// </summary> | ||
| [Description("The Authentication header style to use")] | ||
| public enum AuthenticationHeaderStyle { |
There was a problem hiding this comment.
The enum should not have a space after the opening brace. C# convention is to place the opening brace on the same line without a trailing space. This should be public enum AuthenticationHeaderStyle { changed to public enum AuthenticationHeaderStyle.
| public enum AuthenticationHeaderStyle { | |
| public enum AuthenticationHeaderStyle | |
| { |
| @@ -378,7 +378,7 @@ payloads with (yet) unknown types are offered by newer versions of an API | |||
| /// Gets or sets a value indicating whether to generate Security Schema Authentication headers. | |||
| /// </summary> | |||
| [Description("Generate Security Schema Authentication headers")] | |||
There was a problem hiding this comment.
The AuthenticationHeaderStyle property should have a JsonConverter attribute to ensure consistent serialization when saving to .refitter files. Other enum properties in this file like CollectionFormat (line 388) use [JsonConverter(typeof(JsonStringEnumConverter))]. Add this attribute above the property declaration to maintain consistency.
| [Description("Generate Security Schema Authentication headers")] | |
| [Description("Generate Security Schema Authentication headers")] | |
| [JsonConverter(typeof(JsonStringEnumConverter))] |
|
|
||
| } |
There was a problem hiding this comment.
There is trailing whitespace after the closing brace on this line. Remove the trailing space for consistency with the codebase style.
|
@Roflincopter thanks for taking the time to investigate and implement this I remember we had this discussion in the past regarding authorization headers, and if my memory serves me right, then I think back then we discussed that the best approach to handle authorization was through a HttpClient delegating handler hooked up to the HttpClientFactory configured for the HttpClient used by Refit. That's at least how I always did authorization. The actual authentication is handled by some secure token service which in the end gives me an OAuth access token and refresh token, then I'll have some tooling that handles adding the access token to the authorization header as a bearer token. The same delegating handler (or chain of delegating handlers) would handle checking the validity of said access token, and would also be responsible of renewing the access token using the refresh token The example you you're suggesting seems like the Refit client will be used directly for making authentication or login calls then use the response of that as the authorization header. It's not something I would ever use but since you took the time to build this I guess its something you have a specific use case for. I don't mind getting this in but I'll to test various scenarios for this first. It's important that this doesn't break the default behavior as Refitter has gained popularity over the years and it's user base has grown massively I'll review this thoroughly over the weekend |
|
Yeah we had. My use case admittedly is very small, It's just that Refit has support for this style of Bearer token authorization, Admittedly the AI points out that it would only work for the Authorization: Bearer. The strategy of delegating handlers is a valid one, but I'm concerned with how I can discern what functions need the authorization header and which do not. The openApi spec knows. but when the interface no longer knows I need to re-encode some of the openapi specification in my library code. So i'm fine if you reject this as being too niche. |
2 participants
name: Pull request
title: ''
labels: enhancement
assignees: christianhelle
Description:
A previous PR introduced an optional parameter for authentication headers. As far as I could tell this functionality was never exposed to the commandline tool. and never added support for Bearer tokens.
I want to use this opportunity to both add this to the cli tool and either break compat or write a backwards compatible conversion. To move away from the boolean type in favor of an enum. And add to this enum a way to generate the authenticatoin in the [Headers] attribute for a method. This alleviates the burden of adding the authentication header contents as a parameter on each request and in case of the bearer token authentication it can be configured on refit client creation like this
Issues
Example OpenAPI Specifications:
Example output
with
dotnet run --framework net10.0 -- --generate-authentication-header Method ~/projects/refitter/test/example.yamlaka new behaviourExample output with
dotnet run --framework net10.0 -- --generate-authentication-header Parameter ~/projects/refitter/test/example.yamlaka old behaviour