Contrib.social

Consider Backing out User Destination Manipulation (3101732)

Open on Drupal.org →
Created on 4 August 2022, over 2 years ago
Updated 27 September 2023, over 1 year ago

Problem/Motivation

After reviewing the change log for 8.x-1.4, I really do not understand why such a use-case-specific feature was added to this module to address 📌 After logout due to inactivity , login redirection is not right Fixed . This makes auto-logout take control of the destination page only in the very specific case that the user happened to be viewing the profile page for another user, but:

  • What if the site happens to be a public community site and it is perfectly acceptable to be viewing another user's profile as an anonymous user or some other user?
  • What if a site has other destinations that should be rewritten upon a change in user context? Those cases are not handled by what was added in 3101732.

This change should not have been accepted.

It seems to be that this type of request would have been better handled with these two business rules:

  1. If the destination that was set on the auto-logout is for a page that the user does not have access to, redirect to the homepage. This covers the case of a social site that makes user profiles private and wants to make the user's profile the default landing page without even requiring a special case in the event subscriber.
  2. If the site needs special logic for where the user goes after log-in, encourage them to use an Event Subscriber instead of hacking this module. Automated Logout doesn't need to put this specialized business logic in its event subscriber -- a site that needs the logic to rewrite redirects could just as easily have coded that in its own event subscriber. Said differently, there's already a hook/injection point for this business logic as evidenced by what was coded up for this, so I do not understand why this logic exists in this module.

The logic above covers at least the 80% case, if not 100% of cases, without forcing this destination rewrite onto sites that do not have the same business rules as the OP of 3101732.

Proposed resolution

Remaining tasks

User interface changes

API changes

Data model changes

✨ Feature request
Status

Active

Version

1.0

Component

Code

Created by

🇺🇸United States GuyPaddock

Live updates comments and jobs are added and updated live.
Sign in to follow issues

Comments & Activities

Not all content is available!

It's likely this issue predates Contrib.social: some issue and comment data are missing.

  • Comment over 1 year ago →
    admirlju

    I think this whole thing should just be removed. I think privacy access to user profiles is out of the scope of what this module is supposed to do. Even the redirecting to the homepage if you do not have access to the profile, should not be handled by this module.

    I'll add that as an alternative proposed solution and people can discuss what would be the more useful approach.

  • Comment over 1 year ago →
    admirlju
  • Comment 7 months ago →
    🇺🇸United States azinck

    I just want to add my vote that the change added in 📌 After logout due to inactivity , login redirection is not right Fixed was not appropriate for the scope of this module, and reflective of a very narrowly-considered use-case. It should be removed. There are a million other situations just like this where if a different user logs in they could end up on pages that they don't have access to. That's just intrinsic in the way this works. It's impossible and inappropriate to account for all of it.

    I don't even think being able to set some sort of general redirect destination when the user doesn't have access to the destination page should be handled by this module. The `destination` functionality is part of Drupal core and can be set and used by a variety of modules, not just this module. There's no reason this module should modify its behavior. If some other module wants to take on that task then fine, but I think it's clearly outside the scope of this module.

  • Comment 4 months ago →
    🇺🇸United States japerry KVUO

    I think there is some validity to this request, but with subsequent modifications to that code, its not a simple revert anymore. Perhaps this issue can be morphed into make that feature optional?

contrib.social Blog FAQ Discussions
Production build 0.71.5 2024