Opened 5 years ago

Closed 4 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 5 years ago.
The required patch.

Download all attachments as: .zip

Change History (10)

comment:1 Changed 5 years ago by mhlakhani

  • Needs documentation unset
  • Needs tests unset
  • Owner changed from nobody to mhlakhani
  • Patch needs improvement unset
  • Status changed from new to assigned

comment:2 Changed 5 years ago by mhlakhani

  • Has patch set

I added the patch. This should work.

comment:3 Changed 5 years ago by mhlakhani

  • Triage Stage changed from Unreviewed to Accepted

comment:4 Changed 5 years ago by lasko

  • 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 5 years ago by SmileyChris

  • Patch needs improvement set
  • Version 1.2 deleted

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 5 years ago by mhlakhani

The required patch.

comment:6 Changed 5 years ago by mhlakhani

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

comment:7 Changed 5 years ago by SmileyChris

  • Patch needs improvement unset

Great, now just docs and tests to go :)

comment:8 Changed 4 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 4 years ago by floguy

  • Resolution set to fixed
  • Status changed from assigned to closed

Fixed in [13723]

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