chore: Update swagger summaries, add scopes, global description #1929
Conversation
📝 WalkthroughWalkthroughThis pull request updates API documentation by removing placeholder references such as “link TBD” from endpoint descriptions. It also adjusts JWT bearer authentication scopes for both EndUser and ServiceOwner endpoints, adds a new security requirements operation processor, and refines error messages in UUID validators. Additionally, a global API description constant is introduced and XML documentation in the API client interface is cleaned up. These modifications enhance documentation clarity and security configuration without affecting endpoint functionality. Changes
Possibly related PRs
Suggested reviewers
✨ Finishing Touches
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
Documentation and Community
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (5)
src/Digdir.Domain.Dialogporten.WebApi/Common/Json/SecurityRequirementsOperationProcessor.cs (1)
13-13: Consider making path handling more robust.The hardcoded path comparison might be fragile if the route structure changes. Consider:
- Moving
ServiceOwnerSearchPathto a shared constants file.- Using route templates or path patterns for more flexible matching.
Example improvement:
- private const string ServiceOwnerSearchPath = "/api/v1/serviceowner/dialogs"; + // Move to Constants.cs + public static class ApiPaths + { + public const string ServiceOwnerSearch = "/api/v1/serviceowner/dialogs"; + } private static bool IsServiceOwnerSearchEndpoint(OperationProcessorContext context) - => context.OperationDescription.Path == ServiceOwnerSearchPath; + => context.OperationDescription.Path.StartsWith(ApiPaths.ServiceOwnerSearch);Also applies to: 36-38
src/Digdir.Domain.Dialogporten.WebApi/Endpoints/V1/ServiceOwner/Dialogs/Create/CreateDialogEndpointSummary.cs (1)
14-18: LGTM! Consider enhancing the description.The removal of the placeholder link improves documentation clarity. The link to validation rules is valuable for developers.
Consider adding a brief explanation of the configuration parameters to make the description more informative:
- The dialog is created with the given configuration. + The dialog is created with the given configuration, including parameters such as service identifier, content, and metadata.src/Digdir.Domain.Dialogporten.WebApi/Common/Constants.cs (1)
13-13: Improve HTML formatting in GlobalDescription.While the content is comprehensive and valuable, the HTML formatting could be enhanced for better readability.
Consider this improved HTML structure:
- internal const string GlobalDescription = "Dialogporten API description for both enduser and serviceowner users, as well as open metadata information for public key material.<br><br>All operations* described within this document require authentication and authorization. Read more at <a href=\"https://docs.altinn.studio/dialogporten/user-guides/authenticating/\">https://docs.altinn.studio/dialogporten/user-guides/authenticating/</a><br><br><strong>All GET operations* and POST operations may return or contain, respectively, personal identifiable information (national identity numbers and names).</strong><br><br>For more information about this product, see <a href=\"https://docs.altinn.studio/dialogporten\">https://docs.altinn.studio/dialogporten</a><br><br><em>* Except the metadata APIs"; + internal const string GlobalDescription = """ + <div class="api-description"> + <p>Dialogporten API description for both enduser and serviceowner users, as well as open metadata information for public key material.</p> + + <p>All operations* described within this document require authentication and authorization. Read more at + <a href="https://docs.altinn.studio/dialogporten/user-guides/authenticating/">https://docs.altinn.studio/dialogporten/user-guides/authenticating/</a> + </p> + + <p><strong>All GET operations* and POST operations may return or contain, respectively, personal identifiable information (national identity numbers and names).</strong></p> + + <p>For more information about this product, see + <a href="https://docs.altinn.studio/dialogporten">https://docs.altinn.studio/dialogporten</a> + </p> + + <p><em>* Except the metadata APIs</em></p> + </div> + """;docs/schema/V1/swagger.verified.json (2)
6404-6404: Verify the consistency of tag casing.The tag "serviceowner" is inconsistently cased compared to other occurrences which use "Serviceowner". This should be standardized for consistency.
Apply this diff to fix the casing:
- "serviceowner" + "Serviceowner"
348-349: Remove placeholder documentation reference.The description contains a placeholder "link TBD" which should be replaced with an actual link or removed.
Apply this diff to fix the description:
- "description": "Media type of the content, this can also indicate that the content is embeddable.\nFor a list of supported media types, see (link TBD).", + "description": "Media type of the content, this can also indicate that the content is embeddable.",
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (27)
docs/schema/V1/swagger.verified.json(49 hunks)src/Digdir.Domain.Dialogporten.Application/Common/Extensions/FluentValidation/FluentValidationUuiDv7Extensions.cs(2 hunks)src/Digdir.Domain.Dialogporten.WebApi/Common/Constants.cs(1 hunks)src/Digdir.Domain.Dialogporten.WebApi/Common/Json/SecurityRequirementsOperationProcessor.cs(1 hunks)src/Digdir.Domain.Dialogporten.WebApi/Endpoints/V1/EndUser/DialogActivities/Get/GetDialogActivityEndpointSummary.cs(1 hunks)src/Digdir.Domain.Dialogporten.WebApi/Endpoints/V1/EndUser/DialogSeenLogs/Get/GetDialogSeenLogEndpointSummary.cs(1 hunks)src/Digdir.Domain.Dialogporten.WebApi/Endpoints/V1/EndUser/DialogSeenLogs/Search/SearchDialogSeenLogEndpointSummary.cs(1 hunks)src/Digdir.Domain.Dialogporten.WebApi/Endpoints/V1/EndUser/DialogTransmissions/Get/GetDialogTransmissionEndpointSummary.cs(1 hunks)src/Digdir.Domain.Dialogporten.WebApi/Endpoints/V1/EndUser/Dialogs/Get/GetDialogEndpointSummary.cs(1 hunks)src/Digdir.Domain.Dialogporten.WebApi/Endpoints/V1/EndUser/Dialogs/Search/SearchDialogEndpointSummary.cs(1 hunks)src/Digdir.Domain.Dialogporten.WebApi/Endpoints/V1/EndUser/Parties/Get/GetPartiesEndpointSummary.cs(1 hunks)src/Digdir.Domain.Dialogporten.WebApi/Endpoints/V1/ServiceOwner/DialogActivities/Create/CreateDialogActivityEndpointSummary.cs(1 hunks)src/Digdir.Domain.Dialogporten.WebApi/Endpoints/V1/ServiceOwner/DialogActivities/Get/GetDialogActivitySwaggerConfig.cs(1 hunks)src/Digdir.Domain.Dialogporten.WebApi/Endpoints/V1/ServiceOwner/DialogSeenLogs/Get/GetDialogSeenLogEndpointSummary.cs(1 hunks)src/Digdir.Domain.Dialogporten.WebApi/Endpoints/V1/ServiceOwner/DialogSeenLogs/Search/SearchDialogSeenLogEndpointSummary.cs(1 hunks)src/Digdir.Domain.Dialogporten.WebApi/Endpoints/V1/ServiceOwner/DialogTransmissions/Create/CreateDialogTransmissionEndpointSummary.cs(1 hunks)src/Digdir.Domain.Dialogporten.WebApi/Endpoints/V1/ServiceOwner/DialogTransmissions/Get/GetDialogTransmissionEndpointSummary.cs(1 hunks)src/Digdir.Domain.Dialogporten.WebApi/Endpoints/V1/ServiceOwner/Dialogs/Create/CreateDialogEndpointSummary.cs(1 hunks)src/Digdir.Domain.Dialogporten.WebApi/Endpoints/V1/ServiceOwner/Dialogs/Delete/DeleteDialogEndpointSummary.cs(1 hunks)src/Digdir.Domain.Dialogporten.WebApi/Endpoints/V1/ServiceOwner/Dialogs/Get/GetDialogEndpointSummary.cs(1 hunks)src/Digdir.Domain.Dialogporten.WebApi/Endpoints/V1/ServiceOwner/Dialogs/Purge/PurgeDialogEndpointSummary.cs(1 hunks)src/Digdir.Domain.Dialogporten.WebApi/Endpoints/V1/ServiceOwner/Dialogs/Restore/RestoreDialogEndpointSummary.cs(1 hunks)src/Digdir.Domain.Dialogporten.WebApi/Endpoints/V1/ServiceOwner/Dialogs/Search/ListDialogEndpointSummary.cs(1 hunks)src/Digdir.Domain.Dialogporten.WebApi/Endpoints/V1/ServiceOwner/Dialogs/Update/UpdateDialogEndpointSummary.cs(1 hunks)src/Digdir.Domain.Dialogporten.WebApi/Endpoints/V1/ServiceOwner/ServiceOwnerGroup.cs(1 hunks)src/Digdir.Domain.Dialogporten.WebApi/Program.cs(2 hunks)src/Digdir.Library.Dialogporten.WebApiClient/Features/V1/RefitterInterface.cs(14 hunks)
✅ Files skipped from review due to trivial changes (19)
- src/Digdir.Domain.Dialogporten.WebApi/Endpoints/V1/ServiceOwner/DialogSeenLogs/Get/GetDialogSeenLogEndpointSummary.cs
- src/Digdir.Domain.Dialogporten.WebApi/Endpoints/V1/ServiceOwner/DialogActivities/Create/CreateDialogActivityEndpointSummary.cs
- src/Digdir.Domain.Dialogporten.WebApi/Endpoints/V1/EndUser/Dialogs/Get/GetDialogEndpointSummary.cs
- src/Digdir.Domain.Dialogporten.WebApi/Endpoints/V1/EndUser/DialogSeenLogs/Search/SearchDialogSeenLogEndpointSummary.cs
- src/Digdir.Domain.Dialogporten.WebApi/Endpoints/V1/EndUser/Parties/Get/GetPartiesEndpointSummary.cs
- src/Digdir.Domain.Dialogporten.WebApi/Endpoints/V1/ServiceOwner/DialogSeenLogs/Search/SearchDialogSeenLogEndpointSummary.cs
- src/Digdir.Domain.Dialogporten.WebApi/Endpoints/V1/ServiceOwner/Dialogs/Delete/DeleteDialogEndpointSummary.cs
- src/Digdir.Domain.Dialogporten.WebApi/Endpoints/V1/ServiceOwner/Dialogs/Search/ListDialogEndpointSummary.cs
- src/Digdir.Domain.Dialogporten.WebApi/Endpoints/V1/EndUser/DialogActivities/Get/GetDialogActivityEndpointSummary.cs
- src/Digdir.Domain.Dialogporten.WebApi/Endpoints/V1/ServiceOwner/Dialogs/Restore/RestoreDialogEndpointSummary.cs
- src/Digdir.Domain.Dialogporten.WebApi/Endpoints/V1/ServiceOwner/Dialogs/Update/UpdateDialogEndpointSummary.cs
- src/Digdir.Domain.Dialogporten.WebApi/Endpoints/V1/ServiceOwner/Dialogs/Purge/PurgeDialogEndpointSummary.cs
- src/Digdir.Domain.Dialogporten.WebApi/Endpoints/V1/EndUser/DialogSeenLogs/Get/GetDialogSeenLogEndpointSummary.cs
- src/Digdir.Domain.Dialogporten.WebApi/Endpoints/V1/ServiceOwner/DialogTransmissions/Create/CreateDialogTransmissionEndpointSummary.cs
- src/Digdir.Domain.Dialogporten.WebApi/Endpoints/V1/EndUser/DialogTransmissions/Get/GetDialogTransmissionEndpointSummary.cs
- src/Digdir.Domain.Dialogporten.Application/Common/Extensions/FluentValidation/FluentValidationUuiDv7Extensions.cs
- src/Digdir.Domain.Dialogporten.WebApi/Endpoints/V1/EndUser/Dialogs/Search/SearchDialogEndpointSummary.cs
- src/Digdir.Library.Dialogporten.WebApiClient/Features/V1/RefitterInterface.cs
- src/Digdir.Domain.Dialogporten.WebApi/Endpoints/V1/ServiceOwner/ServiceOwnerGroup.cs
🔇 Additional comments (10)
src/Digdir.Domain.Dialogporten.WebApi/Endpoints/V1/ServiceOwner/Dialogs/Get/GetDialogEndpointSummary.cs (1)
13-17: LGTM! Clear and concise description.The description is now more focused and provides all necessary information without referencing non-existent documentation.
src/Digdir.Domain.Dialogporten.WebApi/Endpoints/V1/ServiceOwner/DialogActivities/Get/GetDialogActivitySwaggerConfig.cs (1)
13-15: LGTM! Clear and concise description.The description is now more focused and provides all necessary information without referencing non-existent documentation.
src/Digdir.Domain.Dialogporten.WebApi/Endpoints/V1/ServiceOwner/DialogTransmissions/Get/GetDialogTransmissionEndpointSummary.cs (1)
13-15: LGTM! Clear and concise description.The description is now more focused and provides all necessary information without referencing non-existent documentation.
src/Digdir.Domain.Dialogporten.WebApi/Common/Json/SecurityRequirementsOperationProcessor.cs (1)
10-34: LGTM! Well-structured security requirements processor.The implementation correctly processes JWT bearer authentication scopes based on operation tags, with proper handling of missing security requirements and preservation of existing scopes.
src/Digdir.Domain.Dialogporten.WebApi/Program.cs (1)
139-140:❓ Verification inconclusive
LGTM! Verify security requirements configuration.
The addition of global description and security requirements processor enhances the API documentation.
Please verify that the security requirements are correctly applied by checking the generated Swagger documentation. Run this script to examine the security definitions:
Also applies to: 154-156
🏁 Script executed:
#!/bin/bash # Description: Check security definitions in Swagger documentation # Find and examine swagger.json files fd -e json -x grep -l "security" {} \; | while read -r file; do echo "Examining security definitions in $file:" jq '.security, .components.securitySchemes' "$file" doneLength of output: 486
LGTM! The global API description and the definition of the
JWTBearerAuthsecurity scheme undercomponents.securitySchemesare correctly added. However, the generated Swagger documentation shows anullvalue for the top-levelsecurityfield. This might be intentional if the security requirements are applied per operation rather than globally, but it’s worth verifying that secured endpoints are configured as intended.
- Verify that each endpoint requiring authentication has its security requirement applied.
- Confirm that the use of
SecurityRequirementsOperationProcessorresults in the expected per-operation security settings.docs/schema/V1/swagger.verified.json (5)
4848-4851: API description has been updated with comprehensive security and PII information.The API description now includes important information about authentication requirements and potential handling of personal identifiable information (PII). This is a good security practice to ensure API consumers are aware of these requirements.
5135-5140: Security scopes have been properly defined for different API operations.The security requirements have been clearly defined with appropriate scopes:
- End-user endpoints require
digdir:dialogportenscope- Service owner endpoints require either:
digdir:dialogporten.serviceproviderfor write operations- Both
digdir:dialogporten.serviceprovideranddigdir:dialogporten.serviceprovider.searchfor search operationsThis is a good security practice that follows the principle of least privilege.
Also applies to: 5761-5765, 6037-6043
6051-6051: Documentation link to validator source code is a good practice.The inclusion of a direct link to the validator source code in the description is helpful for API consumers to understand the validation rules.
4901-4901: Clear documentation of date parameter requirements.The description explicitly states the required format for date parameters with examples, which is helpful for API consumers.
7364-7368:❓ Verification inconclusive
Verify the server URL configuration.
The server URL is set to localhost which may not be appropriate for production use.
🏁 Script executed:
#!/bin/bash # Check if there are any environment-specific server configurations in the codebase rg -A 5 "servers.*url.*localhost"Length of output: 35
Server URL uses a localhost address – please verify its appropriateness for production.
The configuration in
docs/schema/V1/swagger.verified.json(lines 7364-7368) shows:"servers": [ { "url": "https://localhost:7214/" } ]While the previous multi-line regex did not yield output (likely due to pattern spanning multiple lines), an explicit search for "localhost" should confirm this setting is in place. Please double-check that this configuration isn’t inadvertently left for a production environment. If this URL is intended only for development, consider applying environment-specific overrides to avoid any unintended deployment issues.
🏁 Script executed:
[run_scripts] #!/bin/bash # Explicitly check for "localhost" in the swagger configuration file to verify the server URL setting rg "localhost" docs/schema/V1/swagger.verified.jsonLength of output: 212
Action Required: Confirm Environment-Specific Server URL
In the file
docs/schema/V1/swagger.verified.json(lines 7364-7368), the server URL is set to:"servers": [ { "url": "https://localhost:7214/" } ]
- Verify if local usage is intentional: Confirm that this URL is meant only for development/testing.
- Review for production deployment: If this configuration might propagate to production, consider updating the URL to a publicly accessible server address or employing environment-specific configuration (e.g., environment variables or configuration profiles).
src/Digdir.Domain.Dialogporten.WebApi/Endpoints/V1/ServiceOwner/ServiceOwnerGroup.cs
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (3)
src/Digdir.Domain.Dialogporten.WebApi/Common/Json/SecurityRequirementsOperationProcessor.cs (3)
10-14: Consider making the ServiceOwnerSearchPath more configurable.While the implementation is correct, hardcoding the API path as a constant could make the code less maintainable if the endpoint path changes or if multiple search endpoints need similar processing.
Consider:
- Moving the path to configuration
- Using a more flexible path matching mechanism
- Or creating a dedicated attribute to mark search endpoints
- private const string ServiceOwnerSearchPath = "/api/v1/serviceowner/dialogs"; + private readonly string _serviceOwnerSearchPath; + + public SecurityRequirementsOperationProcessor(IConfiguration configuration) + { + _serviceOwnerSearchPath = configuration.GetValue<string>("Api:ServiceOwnerSearchPath") + ?? "/api/v1/serviceowner/dialogs"; + }
15-38: Add XML documentation and improve error handling.The implementation is clean but could benefit from better documentation and error handling:
- Missing XML documentation for the interface implementation
- No logging for unexpected cases
- Silent handling of missing security requirements
Consider adding:
+ /// <summary> + /// Processes the operation to add appropriate security requirements based on the operation's tags. + /// </summary> + /// <param name="context">The operation processor context.</param> + /// <returns>True if processing should continue, false otherwise.</returns> public bool Process(OperationProcessorContext context) { + ArgumentNullException.ThrowIfNull(context); + var securityRequirement = context.OperationDescription.Operation.Security?.FirstOrDefault(); if (securityRequirement == null || !securityRequirement.TryGetValue(JwtBearerAuth, out var value)) { + _logger.LogWarning("No security requirement found for operation {OperationId}", + context.OperationDescription.Operation.OperationId); return true; }
40-42: Add XML documentation for the helper method.The helper method is well-implemented but lacks documentation.
Add XML documentation:
+ /// <summary> + /// Determines if the operation is a service owner search endpoint. + /// </summary> + /// <param name="description">The OpenAPI operation description.</param> + /// <returns>True if the operation is a GET request to the service owner search endpoint.</returns> private static bool IsServiceOwnerSearchEndpoint(OpenApiOperationDescription description) => description is { Path: ServiceOwnerSearchPath, Method: OpenApiOperationMethod.Get };
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
docs/schema/V1/swagger.verified.json(49 hunks)src/Digdir.Domain.Dialogporten.WebApi/Common/Json/SecurityRequirementsOperationProcessor.cs(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- docs/schema/V1/swagger.verified.json
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: build / build-and-test
🔇 Additional comments (2)
src/Digdir.Domain.Dialogporten.WebApi/Common/Json/SecurityRequirementsOperationProcessor.cs (2)
1-8: LGTM! Well-organized imports and namespace.The imports are comprehensive and properly organized, covering all required dependencies for OpenAPI processing and authorization handling.
1-43:❓ Verification inconclusive
Well-implemented security requirements processor!
The implementation successfully achieves the PR objectives by:
- Adding proper scope documentation to API operations
- Correctly distinguishing between service owner and end user endpoints
- Handling special case for search endpoints with additional scopes
This enhances the OpenAPI documentation with accurate security requirements.
Let's verify the implementation is properly registered:
🏁 Script executed:
#!/bin/bash # Check if the processor is registered in Program.cs rg -A 5 "AddOpenApiDocument|ConfigureOpenApiDocument" | rg "SecurityRequirementsOperationProcessor"Length of output: 99
Security Requirements Processor is well-implemented – please verify its registration manually
The implementation adds the correct security scopes for both service owner and end user endpoints and handles the special case for search endpoints as intended. However, our automated check for processor registration (via the OpenAPI configuration in Program.cs or similar) did not produce any output. Since this check was inconclusive, please manually verify that the
SecurityRequirementsOperationProcessoris properly registered in your application's OpenAPI setup (e.g., inProgram.csor an equivalent configuration file).
🤖 I have created a release *beep* *boop* --- ## [1.55.2](v1.55.1...v1.55.2) (2025-02-22) ### Miscellaneous Chores * Add docblocks to parties DTO ([#1934](#1934)) ([c9e10db](c9e10db)) * Return claimsprinciple on dialogtoken validate ([#1933](#1933)) ([2814365](2814365)) * Update swagger summaries, add scopes, global description ([#1929](#1929)) ([d91eacb](d91eacb)) --- This PR was generated with [Release Please](https://github.com/googleapis/release-please). See [documentation](https://github.com/googleapis/release-please#release-please).
This makes adjustments and improves on documentation in the generated OAS. It adds a IOperationProcessor to document scope requirements to all operations (populating the "security" property in the OAS).
Related issue: #1841