Bug #52566 for CGI-Application-Plugin-ProtectCSRF: CAP::ProtectCSRF bug

Hi Akira, I tried using CAP::ProtectCSRF in a recent project but I ran into a problem. When a form is initially displayed, postrun callback _add_csrf_id() is magically called to add the token (as a hidden field) to the HTML form, based on the "PublishCSRFID" attribute being present: sub create : Runmode :PublishCSRFID { my ($self, $errs) = @_; my $t = $self->load_tmpl('request_ form.html'); ... $t->output } # <------ _add_csrf_id() gets called sub create_check : Runmode :ProtectCSRF { my $self = shift; my $dfv_results = $self->check_rm('create', Request::dfv_create_rules) || return $self->check_rm_error_page; # <----- _add_csrf_id() does not get called ... } However, when there is a problem with the form, check_rm_error_page() calls the create() runmode and the form is redisplayed to the user with errors, but the postrun callback for create() is not called, so the HTML form does not have the hidden field with the token. If this new form is submitted, it fails with a CSRF error, since there is no CSRF token in the submitted form. I suppose this is a ProtectCSRF bug, since I don't think I'm doing anything odd here. On the CGI-App mailing list I discussed a possible solution: The postrun callback for ProtectCSRF runmodes could be enhanced to detect that ValidateRM was used, and that there were errors on the form. Something like this: if(defined $self->{'__DFV_RESULT'}) { # hmm, using CAP::ValidateRM my $r = $self->dfv_results; if($r->has_missing or $r->has_invalid) { # the form has errors, assume we need to generate an new CSRF token: # ... code here to generate a new token and add it to the HTML ... This is a bit chummy -- a module shouldn't be looking at another module's "private" data -- but there is no method in CAP::ValidateRM that indicates if check_rm() or validate_rm() was called. I'm not sure if a new token should be generated, or the existing one should be (re)used. At a minimum, the hidden field with the token needs to be added to the HTML again. For CA apps that don't use ValidateRM, you could consider adding a new method to CAP::ProtectCSRF that would set some kind of flag so that the postrun callback (for ProtectCSRF) would know to generate a token and add it to the HTML. The app developer would have to remember to call it though -- it wouldn't happen behind the scenes like above. Something like this: sub create_check : Runmode :ProtectCSRF { my $self = shift; # ... code here that check the form without using my ValidateRM ... if(there are problems with the form) { $self->add_csrf_id_to_html(); # and maybe $self->generate_csrf_token(); } # ... code to re-generate the original form with errors ... } <--- the postrun callback/hook would add the HTML hidden field, only if the flag was set by add_csrf_id_to_html() Another enhancement I thought of for your module: instead of supporting only one token, you could support multiple tokens using attribute handler data, e.g.: sub create :Runmode :PublishCSRF(create_token) { ... } sub create_check :Runmode :ProtectCSRF(create_token) { ... clear_csrf_id('create_token'); } sub edit :Runmode :PublishCSRF(edit_token) { ... } sub edit_check :Runmode :ProtectCSRF(edit_token) { ... clear_csrf_id('edit_token'); } -- Mark