Field Error in Reauthentication Form

[Daksh2000]

Fixes: https://github.com/pypi/warehouse/issues/16460

For proper error in password field while reauthenticating, I made some changes in reauthenticate function, adding a renderer and returning HTTP status code 303 only in case if all form fields validation are successful , otherwise re render the form with appropriate error.

[Daksh2000]

Issue: #16460 (comment)

For proper error in password field while reauthenticating, I made some changes in reauthenticate function, adding a renderer and returning HTTP status code 303 only in case if all form fields validation are successful , otherwise re render the form with appropriate error.

Hi @di , Please share your views.

[Daksh2000]

Issue: #16460 (comment) For proper error in password field while reauthenticating, I made some changes in reauthenticate function, adding a renderer and returning HTTP status code 303 only in case if all form fields validation are successful , otherwise re render the form with appropriate error.

Hi @di , Please share your views.

Hi Team, could someone review this change

[Daksh2000]

Issue: #16460 (comment) For proper error in password field while reauthenticating, I made some changes in reauthenticate function, adding a renderer and returning HTTP status code 303 only in case if all form fields validation are successful , otherwise re render the form with appropriate error.

Hi @di , Please share your views.

Hi Team, could someone review this change

Hi @di , looking forward for updates on this PR

[di]

This mostly works but has the effect of changing the URL to /account/reauthenticate/ if the password is wrong, since in that case it renders the current view rather than redirecting.

As a result, visiting /account/reauthenticate/ directly always asks the user to re-authenticate (since the request is not a GET) and it never redirects elsewhere, even if the password is correct (because next_route is the same as the reauthentication route).

I think we might need to find another way to do this that doesn't require the view to render the page. Some options:

• We could flash any errors instead: this wouldn't be great because we don't do this elsewhere
• We could store any errors on the redirect_to route to make them accessible to the rendering page (like we do with next_route_matchdict and next_route_query)

[Daksh2000]

This mostly works but has the effect of changing the URL to /account/reauthenticate/ if the password is wrong, since in that case it renders the current view rather than redirecting.

As a result, visiting /account/reauthenticate/ directly always asks the user to re-authenticate (since the request is not a GET) and it never redirects elsewhere, even if the password is correct (because next_route is the same as the reauthentication route).

I think we might need to find another way to do this that doesn't require the view to render the page. Some options:

• We could flash any errors instead: this wouldn't be great because we don't do this elsewhere
• We could store any errors on the redirect_to route to make them accessible to the rendering page (like we do with next_route_matchdict and next_route_query)

Hi @di , Thank you for the insights and hints, I almost missed this case where user could visit the reauthenticate url directly and get stuck in a loop, I tried to pass a query param "errors" which has a "password" key and its corresponding error message, and also made some changes to "reauth_view" accordingly to incorporate for any query param,

Also , the views which have "require-reauth" set to True were facing this issue, so I made some changes in the wrapper function and they seem to work Also made changes to few test cases accordingly, Please share your views.

[Daksh2000]

This mostly works but has the effect of changing the URL to /account/reauthenticate/ if the password is wrong, since in that case it renders the current view rather than redirecting. As a result, visiting /account/reauthenticate/ directly always asks the user to re-authenticate (since the request is not a GET) and it never redirects elsewhere, even if the password is correct (because next_route is the same as the reauthentication route). I think we might need to find another way to do this that doesn't require the view to render the page. Some options:

• We could flash any errors instead: this wouldn't be great because we don't do this elsewhere
• We could store any errors on the redirect_to route to make them accessible to the rendering page (like we do with next_route_matchdict and next_route_query)

Hi @di , Thank you for the insights and hints, I almost missed this case where user could visit the reauthenticate url directly and get stuck in a loop, I tried to pass a query param "errors" which has a "password" key and its corresponding error message, and also made some changes to "reauth_view" accordingly to incorporate for any query param,

Also , the views which have "require-reauth" set to True were facing this issue, so I made some changes in the wrapper function and they seem to work Also made changes to few test cases accordingly, Please share your views.

Hi @di , Also added some test cases to have more branch coverage.