If a user is logged in, going to /saml/login should not display a "access denied" page.

I wasn't sure until now whether this was covered by any other issue, but it's not.

I (still relative D8 beginner) just traced what happens in the similar /user/login case: actually the HTTP kernel throws an AccessDeniedHttpException, which then dispatches a 'kernel.exception' event. \Drupal\user\EventSubscriber\AccessDeniedSubscriber reacts on that event by setting a redirect response, to /user.

OK, great. We can do the same thing. (An event subscriber has the added advantage that we can just implement the exact same behavior... but if people don't like this, they can alter that behavior by e.g.
* always displaying a "you are already logged in" message
* forcing logout and redirecting to the IDP
* ...
)

CommentFileSizeAuthor
#6 samlauth-2901757-6.patch1.32 KBakalam
#5 samlauth-saml-login-should-not-yield-access-denied-2901757-5.patch2.9 KBcruno

Comments

roderik created an issue. See original summary.

  • roderik committed 049991a on 8.x-2.x 
    Issue #2901757 by roderik: /saml/login should not yield "access denied"
roderik’s picture

roderik
Primary language Dutch
Location Amsterdam,NL / Budapest,HU
commented
Status: Active » Fixed
Issue tags: +Drupalaton 2017

Done. Same also done for /saml/login.

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.

cruno’s picture

cruno commented
StatusFileSize
new samlauth-saml-login-should-not-yield-access-denied-2901757-5.patch2.9 KB

Since this issue still exists in alpha1 and alpha2 release has not yet been set, I'm uploading a patch that includes the Event Subscriber and service entry from the commit posted in #2.

This patch does not include the code standard change also in that commit.

This is being uploaded here so that others facing this issue may apply the patch with Composer if needed before alpha2 is released.

akalam’s picture

akalam commented
StatusFileSize
new samlauth-2901757-6.patch1.32 KB

There are some issues on redirection, for example on Acquia Cloud Site Factory when more than one domain is configured, you get the following error message:

"Redirects to external URLs are not allowed by default, use \Drupal\Core\Routing\TrustedRedirectResponse for it."

I'm not 100% sure if it is the best way to fix it, but attaching a new patch (against 8.x-2.x branch) trying to solve it.

akalam’s picture

akalam commented

Can we reopen this issue?

akalam’s picture

akalam commented

Please IGNORE my previous patch. If you expect issues in multi-lingual sites, check this issue:
https://www.drupal.org/project/samlauth/issues/2848809

There are patches for 2.x and 3.x branches.