AdminSiteOTPRequiredMixin modified

[aseem-hegshetye]

When admin user is staff user and is active, but doesnt have OTP setup, AdminSiteOTPRequiredMixin will redirect user to OTP setup page after login.

[moggers87]

Tests are required and imports need to be ordered (see failing Travis build)

[aseem-hegshetye]

When I test this class alone, all tests pass.
When I run entire test suit, 2 tests in this class fail

1. test_otp_admin_without_otp
2. test_otp_admin_without_otp_named_url

When running entire test suite, self.client.get('/otp_admin/', follow=True) returns login page url /account/login/?next=%2Fotp_admin%2F, when its supposed to return a OTP setup url /account/two_factor/setup/.
I was unable to find why this happens.
Any help is appreciated

[aseem-hegshetye]

When I just test this class: AdminSiteOTPRequiredMixin().login() func executes.
When I run entire test suite - AdminSiteOTPRequiredMixin().login() func never executes.

[moggers87]

I've not had a chance to look at this yet and I'm not overly familiar with the admin code.

I would imagine that our tests aren't quite as isolated from eachother as we'd actually want. Have you tried running just the test_admin module and then the class those two tests belong to? That might help narrow down where the problem is.

[aseem-hegshetye]

I kept AdminSiteTest and OTPAdminSiteTest in test_admin.py and tried to test make test TARGET=tests.test_admin
They work fine as long as there is no get request made in AdminSiteTest class.
If I make that get request in AdminSiteTest.test_default_admin I get error in OTPAdminSiteTest class.

class AdminSiteTest(UserMixin, TestCase):

    def setUp(self):
        super().setUp()
        self.user = self.create_superuser()
        self.login_user()
        
    def test_default_admin(self):
        with self.settings(ROOT_URLCONF='tests.urls_admin'):
            # if I comment the line below, test_admin.py tests successfully.
            response = self.client.get('/admin/')


class OTPAdminSiteTest(UserMixin, TestCase):
    def setUp(self):
        super().setUp()
        self.user = self.create_superuser()
        self.login_user()

    def test_otp_admin_without_otp(self):
        with self.settings(ROOT_URLCONF='tests.urls_otp_admin'):
            response2 = self.client.get('/otp_admin/', follow=True)
            redirect_to = reverse('two_factor:setup')
            self.assertRedirects(response2, redirect_to)

Error:
Response redirected to '/admin/', expected '/account/two_factor/setup/'

So in conclusion, error in OTPAdminSiteTest class depends on which test was run previously. If I were to substitute AdminSiteTest with AdminPatchTest in the above code, my error will change to following:
Response redirected to '/account/login/?next=/otp_admin/', expected '/account/two_factor/setup/

[moggers87]

Why not self.has_permission(request)?

[aseem-hegshetye]

self.has_permission() func checks if user (is_staff, is_active and is_verified) .
AdminSite().has_permission() checks if user (is_staff, is_active ). Then based on if user is not verified (OTP not setup) we want to redirect to OTP setup page.

[moggers87]

super() should be used instead then. This is a mixin and we don't know what other checks users might have/want so there might be other classes involved, not just AdminSite.

[aseem-hegshetye]

Can anyone help please?

[Jenselme]

@aseem-hegshetye @moggers87 any updates on this? Can I help? I'd like this feature.