Play nicely with LoginToboggan when giving 403 errors

An update: In the LoginToboggan config page (at admin/config/system/logintoboggan), if you set the "Present login form on access denied (403)" to "enabled", what it is really doing it setting changing the settings on the "Site Information" page (at admin/config/system/site-information), so that the "Default 403 (access denied) page" is set to "toboggan/denied". It has overwritten the "customerror/403", which was there before.

So maybe what is needed is a contrib-addon for Login Toboggan -- a way to that it could handle the initial 403, but then defer to the "Custom error alternate for authenticated" module (or any other module) if the user is already logged in.

Thoughts? Should I be posting this to Login Toboggan's issue queue?

Thanks for the prompt reply, @gisle. I dug a bit deeper, and I have a solution! Basically, it involves overriding Login Toboggan's theme function, for 403 messages, theme_lt_access_denied().

But first, it requires a small refactoring of the Custom Error module. Basically, we need to have the option to be able to call the customerror_page() function directly and pass our error-code in to it, rather than having to pull the error-code out of arg(1). See my attached patch, which accomplishes this.

Then, you just need this:

function MYTHEME_lt_access_denied() {
  return customerror_page(403);
}

If the patch from #3 is already applied to CustomError, then here is how you can override the theme function in a custom module:

/**
 * Implements hook_theme_registry_alter()
 */
function MYMODULE_theme_registry_alter(&$theme_registry) {
  if (module_exists('logintoboggan')) {
    $theme_registry['lt_access_denied']['theme path'] = drupal_get_path('module', 'MYMODULE');
    $theme_registry['lt_access_denied']['function'] = '_MYMODULE__theme_lt_access_denied';
  }
}

/*
 * Theme function for Login Toboggan's 'lt_access_denied'.
 */
function _MYMODULE__theme_lt_access_denied() {
  return customerror_page(403);
}

And of course, that lends itself to the possibility of this being bundled right in the CustomError module:

/**
 * Implements hook_theme_registry_alter()
 */
function customerror_theme_registry_alter(&$theme_registry) {
  if (module_exists('logintoboggan')) {
    $theme_registry['lt_access_denied']['theme path'] = drupal_get_path('module', 'customerror');
    $theme_registry['lt_access_denied']['function'] = '_customerror__theme_lt_access_denied';
  }
}

/*
 * Theme function for Login Toboggan's 'lt_access_denied'.
 */
function _customerror__theme_lt_access_denied() {
  return customerror_page(403);
}

It could also be broken out into another contrib-module that could be bundled with CustomError: Something like "CustomError-LoginToboggan integration".

Switching this back to a code feature-request, as I'd like to get my patch from #3 incorporated, so that my solutions in #3 and #4 will work. I think that the patch is actually an improvement to the code (slightly better style).

Thanks, Gisle! Very glad to help.

A couple more comments on this:

* LoginToboggan[1]:
When this module is enabled, CustoMerror will override
LoginToboggan's theme function for access denied pages so that you
can use CustomError and Custom error alternate for authenticated
to handle the error.

This is not entirely accurate. It should be something more like this:

* LoginToboggan[1]:
Our "Custom Error Alternate for Authenticated" module normally takes over handling the system "Default 403 (access denied) page". But if LoginToboggan is enabled, and if its "Present login form on access denied (403)" config option is enabled, then LoginToboggan will take control of the 403 messages, instead of Custom Error Alternate for Authenticated.

Also, the patch in #3 actually has nothing to do with Login Toboggan -- it simply allows other modules or theme functions to be able to call customerror_page() directly, should they want to. It should be ok to release, as-is.

The change I suggested in the bottom half of #4 is the one that might benefit from some Login Toboggan eyes.

Sorry for the confusion! I read too quickly, and thought you had only committed my patch in #3, but now I see that you also included the changes that I had suggested from #4.

Now your change to the README makes a lot more sense. But I'd still alter it a little, partly based on a little more that I've understood about how this module and LoginToboggan both handle 403 errors (and how they can conflict). I'd put something like this (feel free to edit/reword it):

If LoginToboggan is enabled, in can enhance CustomError's handling of access-denied messages, but you have to be careful to set them up to work together correctly.

These two modules both attempt to take over handling of system 403 ("access denied") messages, and can conflict. CustomError does it by asking you to go to "admin/config/system/site-information" and manually setting the "Default 403 (access denied) page" to "customerror/403", whereas LoginToboggan sets that same field to "toboggan/denied" automatically (overwriting any other value that was there), when you enable its "Present login form on access denied (403)".

If you are using CustomError with LoginToboggan, you should allow LoginToboggan to perform this take-over (in other words, DON'T set the "Default 403 (access denied) page" to "customerror/403"). This way, if someone attempts to access a page that they don't have access to, LoginToboggan will first give them a chance to log in if they haven't yet. If they *still* don't have access to the page, CustomError then takes over from LoginToboggan (by overriding one of its theme functions), displaying its customisable messages for access-denied errors.