Opened 6 years ago

Closed 6 years ago

#13714 closed (fixed)

Login Required decorator should support LOGIN_URL

Reported by: m.hasnain.lakhani@… Owned by: mhlakhani
Component: contrib.auth Version:
Severity: Keywords:
Cc: Triage Stage: Accepted
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: UI/UX:

Description

In django.contrib.auth.decorators, both the user_passes_test and permission_required decorators allow the caller to specify a custom LOGIN_URL to redirect visitors to. However, the login_required decorator does not allow the caller to specify a custom login URL. This feature would be helpful in my opinion and not hard to add.

Attachments (1)

login_required_login_url.diff (841 bytes) - added by mhlakhani 6 years ago.
The required patch.

Download all attachments as: .zip

Change History (10)

comment:1 Changed 6 years ago by mhlakhani

Owner: changed from nobody to mhlakhani
Status: newassigned

comment:2 Changed 6 years ago by mhlakhani

Has patch: set

I added the patch. This should work.

comment:3 Changed 6 years ago by mhlakhani

Triage Stage: UnreviewedAccepted

comment:4 Changed 6 years ago by Brandon M Height

Needs documentation: set
Needs tests: set

I agree that this is easy to implement and see not reason why this could not be implemented on trunk.

Currently however you can already do this under the implementation of providing login_required with "function=your_own_decorator" under which you can implement your own version of the lambda statement.

I would however be +0 for implementing this as it is a nicety but there are ways to do this without modification to the django source.

comment:5 Changed 6 years ago by Chris Beaven

Patch needs improvement: set
Version: 1.2

You'd also want to make the new argument the last, as some people will be using positional arguments to call the decorator.

But yeah, it seems useful enough to add (and no backwards compatibility issues after the argument is moved to the end).

Changed 6 years ago by mhlakhani

The required patch.

comment:6 Changed 6 years ago by mhlakhani

I've updated the patch, making the new argument the last as required.

comment:7 Changed 6 years ago by Chris Beaven

Patch needs improvement: unset

Great, now just docs and tests to go :)

comment:8 Changed 6 years ago by floguy

Needs documentation: unset
Needs tests: unset

I've updated the patch to include docs and tests for this. It's all isolated in a branch here: http://github.com/ericflo/django/compare/ticket13714

comment:9 Changed 6 years ago by floguy

Resolution: fixed
Status: assignedclosed

Fixed in [13723]

Note: See TracTickets for help on using tickets.
Back to Top