mbi / django-simple-captcha

Django Simple Captcha is an extremely simple, yet highly customizable Django application to add captcha images to any Django form.
http://django-simple-captcha.readthedocs.io/en/latest/
MIT License
1.38k stars 322 forks source link

django-simple-captcha on last wizard form #6

Open ghost opened 12 years ago

ghost commented 12 years ago

I tried to use django-simple-captcha on last wizard form. When I choose correct value, captcha stay impassively to "invalid".

By browsing web, i found an old patch here (comment 6) : http://code.google.com/p/django-simple-captcha/issues/detail?id=4 What do you think about?

Regards

Alir3z4 commented 11 years ago

+1 on this. Is there any clean solution for this? I haven't tried that patch on google code, it's little bit old and I don't think it would work anymore.

I couldn't get captcha field working on WizardForm, neither last step or others.

delehef commented 10 years ago

Confirmed here. Isn't there any possibility to use a captcha on a form wizard ? For instance, could not simple captcha accept to be validated twice ? (it's ugly, but, well...)

mbi commented 10 years ago

AFAIK Django doesn't tell us whether the form validation's clean() method is called from the context of a form wizard or a plain form, so we don't really know if we should "delete" a captcha after it has been validated the first time.

We can't really leave a validated captcha around because an attacker could solve a captcha once and reuse the same field over and over again, completely defeating the purpose of having a captcha in the first place.

Maybe a solution would be to have a counter on the CaptchaStore which we decrement each time a captcha is validated, and we delete the captcha when the counter reaches zero. By default captchas would be created with a counter of 1, but if we know that a particular captcha field will be used in a form wizard, we could instantiate the field with a counter of 2, so that it could be validated twice.

delehef commented 10 years ago

As I'm using it for a small personal website for now, I just disabled the deletion of the captcha, which is, as you said, quite ugly.

Anyway, thanks for your answer!

allenling commented 10 years ago

May be, we can just have something modified in our own revalidation_failure method, after all, captcha error occur in this method.

When validating captcha data in post, it is find, the captcha data will be loaded in session or cookie, it depends on which backend you use. But, when call render_done, the data of every form would be revalidated. Then, captcha failure.

As we know, FormWizard will load data validated successfully in backend. I think, we could fetch if captcha data be in our backend, true, well that means we can ignore this captcha error, or false, that may be means something real wrong happen.

It looks like: jkmw _5 9383tbq 7kr9

lechup commented 9 years ago

My solution going @mbi suggestions and using memory caching. It works only for captcha in last step of wizard.

# -*- coding: utf-8 -*-
from django.core.exceptions import ValidationError
from django.utils.translation import ugettext_lazy

from captcha.fields import CaptchaField
from captcha.conf import settings
from captcha.models import CaptchaStore, get_safe_now

class CaptchaWizardField(CaptchaField):
    def __init__(self, *args, **kwargs):
        self.validity_count = kwargs.pop('validity_count', 2)
        self.validity_cache = {}
        super(CaptchaWizardField, self).__init__(*args, **kwargs)

    def clean(self, value):
        super(CaptchaField, self).clean(value)
        CaptchaStore.remove_expired()

        response, value[1] = (value[1] or '').strip().lower(), ''
        hashkey = value[0]

        if settings.CAPTCHA_TEST_MODE and response.lower() == 'passed':
            # automatically pass the test
            try:
                # try to delete the captcha based on its hash
                CaptchaStore.objects.get(hashkey=hashkey).delete()
            except CaptchaStore.DoesNotExist:
                # ignore errors
                pass
        elif not self.required and not response:
            pass
        else:
            # let enable validity_count times
            # of clean() method
            if hashkey in self.validity_cache and self.validity_cache[hashkey] > 0:
                self.validity_cache[hashkey] -= 1
                return value
            try:
                captcha = CaptchaStore.objects.get(response=response, hashkey=hashkey, expiration__gt=get_safe_now())
                self.validity_cache[hashkey] = self.validity_count - 1
                captcha.delete()
            except CaptchaStore.DoesNotExist:
                raise ValidationError(getattr(self, 'error_messages', {}).get('invalid',
                                                                              ugettext_lazy('Invalid CAPTCHA')))
        return value

And then:

class WizardFormLast(forms.Form):
    captcha = CaptchaWizardField()

Hope it helps someone, I was fighting with it 4 hours ...

lechup commented 9 years ago

@mbi do You think we could merge it somehow to master?

I see two solutions:

If any is an option please answer here - I'll try to prepare a PR.

galeo commented 8 years ago

Hi, @lechup I've seen your solution code above using separate CaptchaWizardField, I have 2 questions:

  1. Are the validity_count and validity_cache attributes shared in all CaptchaWizardField instances?
  2. Should we clean the item with value is 0 in validity_cache?

The answer to question 1 seems yes by checking code in __deepcopy__ method of django.forms.fields.Field.

@lechup @mbi, do you have plans about this issue or what progress is being made?

barseghyanartur commented 8 years ago

Any progress on this?