Page MenuHomePhabricator

add a "delete on success" option.
Closed, ResolvedPublic

Description

Author: infinity0x

Description:
basic (and incomplete) implementation of requested feature

Please add a "delete on success" option to the FancyCaptcha component, to avoid using the same captcha twice. This makes it simpler for system admins to set up (e.g.) a cron job that re-generates captchas when necessary, because obsolete ones are automatically removed.

An incomplete implementation is supplied; it only works if $wgCaptchaDirectoryLevels == 0.


Version: unspecified
Severity: enhancement

attachment wgCaptchaDeleteOnSuccess.diff ignored as obsolete

Details

Reference
bz24730

Event Timeline

bzimport raised the priority of this task from to Medium.Nov 21 2014, 11:05 PM
bzimport set Reference to bz24730.

The problem is, it sometimes needs to reread them:
"in at least some circumstances, Konqueror tries to reload the image even if you haven't navigated away from the page"

What you can do is to delete from a cron the used files based on the atime.

infinity0x wrote:

hmm... has anyone submitted a bug to konqueror about this? that is more correctly a bug on their side.

also, currently there is no simple way to detect from outside mediawiki whether the access was a success or a failure. there is no point deleting captchas that have not yet been solved.

infinity0x wrote:

actually, wait look my patch is fine - it only deletes the captcha after it has been *solved*, ie. after the konquerer user has viewed it, and typed in the correct answer.

On second thought, removing after it has being shown as you were doing looks ok. Although it would be better to do it by overriding clearCaptcha(), not passCaptcha().

hmm... has anyone submitted a bug to konqueror about this? that is more
correctly a bug on their side.

I don't know. Probably not. It was just a comment in the code.

I think the whole content of the conditional could be replaced by:

$filename = $this->imagePath( $info['salt'], $info['hash'] );
if ( is_file( $filename ) ) {
unlink( $filename );
}

infinity0x wrote:

oh cool, i guess that solves the $wgC..DirectoryLevel > 0 issue :) (I didn't look too closely at the rest of the code, I just mashed that up in a few minutes.)

another thought is $wgC..DeleteOnSolve maybe a better config name than $wgC..DeleteOnSuccess

soxred93 wrote:

Adding patch and need-review keywords

infinity0x wrote:

full implementation

new patch incorporates suggested changes, except overriding clearCaptcha rather than passCaptcha. (not sure that is even a good idea, because it's called on both a fail and a pass, whereas this patch only affects the pass case.)

Attached:

s/need-review/reviewed/
Patch has been applied.