Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

How to subclass AuthenticationTokenView and use it in a subclassed LoginView #720

Open
isparks-tg opened this issue Apr 11, 2024 · 2 comments

Comments

@isparks-tg
Copy link

I have a subclassed LoginView which defines a set of forms:

class UserLogin(LoginView):
    form_list = (
        (LoginView.AUTH_STEP, MyAuthenticationForm),
        (LoginView.TOKEN_STEP, MyAuthenticationTokenForm),
        (LoginView.BACKUP_STEP, MyBackupTokenForm),
    )

To my surprise the TOKEN_STEP form was being switched to AuthenticationTokenForm, I finally tracked it down to code in the LoginView class

    def get_form(self, step=None, **kwargs):
        """
        Returns the form for the step
        """
        if (step or self.steps.current) == self.TOKEN_STEP:
            # Set form class dynamically depending on user device.
            method = registry.method_from_device(self.get_device())

            self.form_list[self.TOKEN_STEP] = method.get_token_form_class() #<-- returns AuthenticationTokenForm from registry but in other setups would be other forms

It is not clear what the best approach is here. I overrode get_form() in my LoginView subclass to bypass this:

    def get_form(self, step=None, **kwargs):
        """
        Returns the form for the step
        """

        # This is a copy of code from core.py but it does not override the self.TOKEN_STEP which LoginView does

        form = super(LoginView, self).get_form(step=step, **kwargs)
        if self.show_timeout_error:
            form.cleaned_data = getattr(form, 'cleaned_data', {})
            form.add_error(None, ValidationError(_('Your session has timed out. Please login again.')))
        return form

but possibly I should be doing something different/better maybe with registry?

It feels like a gap in the docs regarding what you do if you want to specify a fixed class in the token step and don't want it dynamically switching on you.

Expected Behavior

For my use case either it should respect the token step form defined in forms_list or block setting that value because it will be ignored, i.e.

    form_list = (
        (LoginView.AUTH_STEP, forms.MyAuthenticationForm),
        (LoginView.TOKEN_STEP, forms.MyAuthenticationTokenForm), <-- This should be a runtime error, don't do that?
        (LoginView.BACKUP_STEP, forms.MyBackupTokenForm),
    )

Current Behavior

Quietly switches classes on you behind your back without warning you that your TOKEN_STEP value is going to be overridden from the registry which is very confusing.

@moggers87
Copy link
Collaborator

That is surprising and a little bit confusing too! I'm almost tempted to call this a bug as we populate form_list and then just go and ignore it - I feel like we should do something different to make it clearer that we're going to call the registry for that particular form.

@aarighi
Copy link

aarighi commented May 2, 2024

I have the same issues, my token auth form is getting replaced and it took me a lot to figure out why,

I hope this can be fixed ASAP

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants