Acosix / alfresco-keycloak

Alfresco addon to provide Keycloak-related extensions / customisations for Repository and Share
Apache License 2.0
36 stars 20 forks source link

[BUG] Skipping processKeycloakAuthenticationAndActions as login page was explicitly requested #29

Open howkymike opened 2 years ago

howkymike commented 2 years ago

Hello @AFaust

Bug description

I noticed that when I'm logged out and I access a shared page (i.e. https://example.com/share/s/ABCD) and click "Login":

  1. Forced SSO does not work (i have force-keycloak-sso enabled) - I'm redirected to standard Alfresco's login page
  2. After clicking "Sign via SSO" I'm redirected again to the login page over and over again

Proposed Solution:

In the KeycloakAuthenticationFilter.checkForSkipCondition() function there is a check if page is explicitly requesting a login page. If yes, whole Keycloak authentication is omitted. I propose to delete this check.

Additional note

I have a feeling that also other checks in this function might have to be deleted - there are other situations when this happens as well.

AFaust commented 2 years ago

Ah, I usually don't deal with Shared pages, so have not included that in my tests. If the login button there redirects to an explicit login page request, it would quite hard to distinguish from an explicit login page request which is kept excluded from forced SSO handling for a reason. It may require a customisation to the shared page logic to use a slightly altered login URL redirect, either to simply inject a URL query parameter that can be checked to determine that SSO handling should still apply.

As with the checks in checkForSkipCondition: I'll happily go over them again, but due to the quite weird manners in which Alfresco Share handles authentication, all of them had a very specific reason for being included.

howkymike commented 2 years ago

That's exactly what i did - in addition to modyfing checkForSkipCondition() method, I also customalized shared page to use different URL (always document-details page) But the question is, do we really need login page on explicit request, especially if force-sso flag is enabled? And what about that login loop, that is certanly not desired (it happens only with the explicit login page request).

AFaust commented 2 years ago

Having login page on explicit request is identical to how Alfresco handled this for all other SSO mechanisms, and this allows a local administrator to bypass SSO authentication in very specific cases (i.e. SSO is broken somewhere). The login loop should certainly be fixed.