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

Authenticated staff users should not see the honeypot. #6

Open
kezabelle opened this issue Apr 23, 2013 · 5 comments
Open

Authenticated staff users should not see the honeypot. #6

kezabelle opened this issue Apr 23, 2013 · 5 comments

Comments

@kezabelle
Copy link

Rationale:

  • It's pointless; they're signed in, they're trusted, their inability to remember the correct URL is their only crime
  • Valid admin users can get 'stuck' trying to sign in, even though they are.
  • It exposes the real admin URL, but only in a way that is broken, see next point.
  • It displays the "Welcome, X ... Change Password / Log out" text, and this cannot be worked around because that text is not contained within a {% block %} in the admin/base.html within the standard contrib module.

Recommended solution:

redirect_conditions = [
    request.user.is_authenticated(),
    request.user.is_active,
    request.user.is_staff,
]
if all(redirect_conditions):
    return redirect(somewhere_else)

Problems:

I cannot think of a way to redirect to the correct admin area, or rather, I can, but only by assuming that the AdminSite being used mounts itself using the admin: namespace, which isn't ideal, though probably is acceptably correct.

@dmpayton
Copy link
Owner

Hi @kezabelle, thanks for the report!

To address the first two points of your rational, has this actually been a problem for you? I generally don't even mention admin_honeypot to non-developer admins, I just tell them to go to /secret-lair/ and they remember it in their own way (browser bookmark, sticky note on their monitor, tattoo on their arm... ;)

I don't agree that authenticated users should be redirected to the actual admin. Say an admin user account is compromised by some means and the malicious -- but authenticated -- user goes to /admin/. You propose to forward them to the real admin, which allows them to wreak all kinds of havoc.

I would much rather solve the issues addressed in your third and fourth points. Having the admin_honeypot show a b0rked login when you're already authenticated is something I've wanted to address for a while but, as you already mentioned, that text isn't contained within a block that we can override. It may be that we need to file a ticket on Django and see how receptive the core devs are to the idea of wrapping it in a template block, or if they have any alternative recommendations.

Thoughts?

@kezabelle
Copy link
Author

I'll confess I hadn't thought about the compromised accounts part of it, though I think by the time a user has elevated themselves to staff and or superuser, things are a bit screwed anyway; but giving them the keys to the castle is admittedly a bad idea.

The correct fix is, then:

  • Open a ticket against Django, and propose wrapping the whole <div> in {% block usertools %} or something. I can't see an argument against it, so it'd hopefully go through for >~1.6

In the mean time, I can think of the following solutions:

  • Investigate how the login form renders when is_popup is set into the context as True, as this would preclude the whole header from being displayed.
  • Alternatively, add some javascript to remove the user-tools HTML node from the DOM.

(Addendum: It is actually a problem for me, but only because I have years of muscle memory telling me to type one thing, rather than the other)

@dmpayton
Copy link
Owner

So, I've had some time to think -- 7 years(!) in fact.

Instead of displaying a notice in the template (à la #16), I'm leaning towards simply redirecting the user to the admin if they're an authenticated staff-level user.

Thoughts and feedback welcome.

@kezabelle
Copy link
Author

So, I've had some time to think -- 7 years(!) in fact.

I've been there, for sure ;)

Fair warning, I can barely remember the problem I was describing about the bleeding of real information from admin/base.html into the honeypot (and I helpfully haven't included an image for either of us to reference. I hate past-me) but it looks like over the years blocks for {% block usertools %}, {% block welcome-msg %} and {% block userlinks %} now exist, along with {% if has_permission %}

Between those changes, it might be possible to address the ones you were interested in (points 3 & 4) without compromising on:

Say an admin user account is compromised by some means and the malicious -- but authenticated -- user goes to /admin/. You propose to forward them to the real admin, which allows them to wreak all kinds of havoc.

whilst also allowing for showing a debug message as per #16?

@Carewen
Copy link

Carewen commented Jul 1, 2022

So, I've had some time to think -- 7 years(!) in fact.

Instead of displaying a notice in the template (à la #16), I'm leaning towards simply redirecting the user to the admin if they're an authenticated staff-level user.

Thoughts and feedback welcome.

Also catching up. The approach I take may not directly address the broader issues, but it's a usability fix that is proving to be quite successful. I resolve the issue of logged in staff users not even needing to be informed of the hidden URL by having a link for it that only displays to staff. Within a staff-only dropdown navbar I simply include the link for them:

<a href="{% url "admin:index" %}">Django Admin</a>

The assumption here is that if you've logged in on a staff account you should not need to enter the url to get to the admin portal. It should be a link that is presented to you automatically. There is no need to inform staff of the hidden url. Certainly, they can still get it from the browser's address bar, but that's less obvious and no attention has been drawn to the existence of a honeypot in the first place.

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

No branches or pull requests

3 participants