Fix bugs for error response in the form_post and error view #1702
Conversation
This file contains 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
Bug When an error is rendered with the `form_post.html.erb` view, an empty form is posted instead of the correct error and error_description. Root cause The form_post view used the body of `@authorize_response`. However, the `redirect_or_render` could be called with `authorization.deny` or `pre_auth.error_response`. In this case, `@authorize_response` would only contain an empty body. Fix Instead of using `@authorize_response`, we pass in a local variable `auth` to the `form_post.html.erb`, which corresponding to the auth object passed into the redirect_or_render function. Tests are backfilled to the authorizations controller spec. During the test phase, I discovered error.html.erb always renders an incorrect error description. It turns out that `respond_to(:error_respon se)` always return false in the view. I changed it to use `local_assigns` as the correct condition.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Bug
When the
form_post.html.erbview attempts to render an error, itincorrectly posts an empty form rather than displaying the appropriate
error and error_description.
Root cause
The form_post view used the body of
@authorize_response. Thisissue arose when the
redirect_or_rendermethod was invoked witheither
authorization.denyorpre_auth.error_response, resulting in@authorize_responsecontaining only an empty body, failing todisplay the error details as intended.
Fix
Instead of using
@authorize_response, we now introduce a localvariable
auth, which represents the authorization object passed tothe
redirect_or_renderfunction. This ensures that the correct errorinformation is provided to the view. Additionally, I have backfilled
tests in the authorizations controller spec to verify the fix.
Additional Findings During Testing
During the test phase, I discovered
error.html.erbalways renders anincorrect error description. It turns out that
respond_to(:error_response)always return false in the view. I changedit to use
local_assignsas the correct condition.