feat: custom form post writer #58
Conversation
This implements a custom form post response writer interface into the configurator.
WalkthroughThe recent updates bring a new approach to handling form post responses in the application. A significant shift replaces the Changes
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 as PR comments)
Additionally, you can add CodeRabbit Configration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Review Status
Actionable comments generated: 3
Configuration used: CodeRabbit UI
Files selected for processing (8)
- README.md (1 hunks)
- authorize_helper.go (1 hunks)
- authorize_helper_test.go (1 hunks)
- config.go (1 hunks)
- config_default.go (3 hunks)
- fosite.go (1 hunks)
- integration/authorize_form_post_test.go (1 hunks)
- response_handler.go (2 hunks)
Check Runs (3)
test completed (1)
format completed (1)
generate completed (5)
Additional comments: 10
fosite.go (1)
- 158-158: The addition of
FormPostResponseProviderto theConfiguratorinterface aligns well with the PR's objectives. Ensure that the implementation of this interface is consistent and integrates smoothly with existing functionalities across the application.response_handler.go (2)
- 114-114: The modification to use
FormPostResponseProviderfor writing form post responses inhandleWriteAuthorizeResponseis a significant improvement. Consider adding documentation or comments explaining the rationale behind this change for future maintainability.- 225-225: The addition of
FormPostResponseProvidertoResponseModeHandlerConfiguratoris crucial for the new form post response handling mechanism. Ensure that all implementations ofResponseModeHandlerConfiguratornow properly implement this interface.README.md (1)
- 72-72: The addition of "Custom Form Post Response Writer" to the list of features is a valuable update. Consider providing more context or a brief description of this feature to help readers understand its significance and usage.
authorize_helper.go (2)
- 192-192: The introduction of the
FormPostResponseWritertype is a significant enhancement. Consider adding documentation or comments explaining its purpose and usage to ensure clarity and maintainability.- 194-194: The
DefaultFormPostResponseWriterfunction provides a valuable default implementation. Ensure that it is accompanied by comprehensive documentation or comments detailing its behavior and intended use.config.go (1)
- 283-288: The introduction of the
FormPostResponseProviderinterface with theGetFormPostResponseWritermethod is a commendable addition. It aligns with the PR's goal of enhancing form post response handling by offering a customizable approach. This change promotes flexibility and adaptability in managing form post responses, which is beneficial for the application's overall architecture.config_default.go (3)
- 149-149: The addition of the
FormPostResponseWriterfield to theConfigstruct introduces a new way to handle form post responses, aligning with the PR's objectives. Ensure that the default value of this field is properly handled to avoid nil pointer dereferences.- 278-284: The implementation of
GetFormPostResponseWritermethod correctly checks for a nilFormPostResponseWriterand assigns theDefaultFormPostResponseWriterif necessary. This ensures safe usage of theFormPostResponseWriterthroughout the application.- 632-632: The interface
_FormPostResponseProvideris correctly implemented by theConfigstruct, ensuring that theConfigcan be used wherever aFormPostResponseProvideris required. This change supports the PR's goal of enhancing form post response handling.
| @@ -254,17 +254,15 @@ func (m *DecoratedFormPostResponse) ResponseModes() oauth2.ResponseModeTypes { | |||
| func (m *DecoratedFormPostResponse) WriteAuthorizeResponse(ctx context.Context, rw http.ResponseWriter, ar oauth2.AuthorizeRequester, resp oauth2.AuthorizeResponder) { | |||
| rw.Header().Add(consts.HeaderContentType, consts.ContentTypeTextHTML) | |||
| resp.AddParameter("custom_param", "foo") | |||
| oauth2.WriteAuthorizeFormPostResponse(ar.GetRedirectURI().String(), resp.GetParameters(), oauth2.GetPostFormHTMLTemplate(ctx, | |||
| new(oauth2.Config)), rw) | |||
| oauth2.DefaultFormPostResponseWriter(rw, oauth2.GetPostFormHTMLTemplate(ctx, new(oauth2.Config)), ar.GetRedirectURI().String(), resp.GetParameters()) | |||
There was a problem hiding this comment.
The modification to use oauth2.DefaultFormPostResponseWriter in the WriteAuthorizeResponse method of DecoratedFormPostResponse changes the way form post responses are handled. Verify that the new parameter order and function logic are correctly implemented to maintain the integrity of the integration tests.
| } | ||
|
|
||
| func (m *DecoratedFormPostResponse) WriteAuthorizeError(ctx context.Context, rw http.ResponseWriter, ar oauth2.AuthorizeRequester, err error) { | ||
| rfc := oauth2.ErrorToRFC6749Error(err) | ||
| errors := rfc.ToValues() | ||
| errors.Set(consts.FormParameterState, ar.GetState()) | ||
| errors.Add("custom_err_param", "bar") | ||
| oauth2.WriteAuthorizeFormPostResponse(ar.GetRedirectURI().String(), errors, oauth2.GetPostFormHTMLTemplate(ctx, | ||
| new(oauth2.Config)), rw) | ||
| oauth2.DefaultFormPostResponseWriter(rw, oauth2.GetPostFormHTMLTemplate(ctx, new(oauth2.Config)), ar.GetRedirectURI().String(), errors) |
There was a problem hiding this comment.
Similarly, the change to use oauth2.DefaultFormPostResponseWriter in the WriteAuthorizeError method must be carefully reviewed to ensure that error handling in form post responses remains effective and aligns with the expected behavior in integration tests.
This implements a custom form post response writer interface into the configurator.
Summary by CodeRabbit
Custom Form Post Response Writerfor enhanced handling of form post responses in authorization flows.FormPostResponseWriter.FormPostResponseProviderto standardize the retrieval of form post response writers.