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

With SAML enabled, explicit logout takes you to the native login page, instead of re-authing through SAML #19098

Closed
john-thomas-dotcms opened this issue Aug 14, 2020 · 10 comments
Assignees
Labels

Comments

@john-thomas-dotcms
Copy link
Contributor

Describe the bug

When SAML is enabled, if a user explicitly logs out of the dotCMS back-end, they are taken to the /dotAdmin/#/public/login URL, instead of the plain /dotAdmin URL. This page does not redirect to the SAML authorization.

This means that any time a user explicitly logs out of dotCMS, they can not log back in (or get reauthenticated) using SAML unless they explicitly type in the /dotAdmin URL.

To Reproduce

Steps to reproduce the behavior:

  1. Log into the back-end using SAML.
  2. Log out from the back-end user menu, at the top-right of the screen.
    Result: You are redirected to the /dotAdmin/#/public/login page, which displays the native login screen instead of authorizing through SAML.

Expected behavior

I'm not sure why a logout redirects to the longer /dotAdmin/#/public/login URL to begin with, but I see at least 2 possible solutions:

  1. Change dotCMS to login to the standard /dotAdmin path after a logout (instead of /dotAdmin/#/public/login.
  2. Update SAML to recognize and perform authorization when the /dotAdmin/#/public/login path is hit (as well as the standard /dotAdmin path).

Desktop (please complete the following information):

  • OS: Win 10 Pro
  • Browser: Chrome
  • Version: dotCMS 5.3.6.1

Additional context

This appears to be a hold-over from the behavior of the SAML plugin. I think it's most properly classified as a long-time bug in the plugin that we've now brought into the core.

Since this has existed in the plugin for a long time, the priority might be closer to an enhancement request than a bug. But with SAML now in core, more customers are likely to use SAML and run into this problem, so I think we should give it the priority of a regular bug (not an old bug).

@wezell
Copy link
Contributor

wezell commented Aug 14, 2020

Users need to bounce somewhere. Maybe they should go to / ?

@john-thomas-dotcms
Copy link
Contributor Author

Yeah, we could have a discussion about where a logged-out user should go - but I think going back to the login page is fine. Which means they could just bounce to /dotAdmin - so if SAML is enabled it will take over, but if it's not they'll still hit the native login (so no real behavior change if SAML is not enabled, just a different URL).

@john-thomas-dotcms
Copy link
Contributor Author

The only reason this is an issue is because logout adds /#/public/login to the end of the URL it bounces the user to. So if we remove that from the bounce URL, or change SAML to intercept that, this issue will go away.

jdotcms added a commit that referenced this issue Sep 30, 2020
jdotcms added a commit that referenced this issue Oct 6, 2020
jdotcms added a commit that referenced this issue Oct 6, 2020
fmontes pushed a commit that referenced this issue Oct 9, 2020
…in page, instead of re-authing through SAML

* #19098 adding a new interceptor to handle the logout process

* #19098 upgrade saml version

* label

* #19098 changes for logout

* #19098 changes for handling logout

* #19098 adding new changes to show the logout page

* add styles

* #19098 adding fixes for the logout dotCMS/SAML

Co-authored-by: Humberto Morera <humberto.morera@dotcms.com>
@fmontes fmontes added this to the Lunik Current milestone Oct 9, 2020
@wezell
Copy link
Contributor

wezell commented Oct 9, 2020

the logout screen in master- background not stretching, font issues, no button

Screen Shot 2020-10-09 at 5 25 41 PM

@hmoreras
Copy link
Contributor

hmoreras commented Oct 14, 2020

after testing the styling issue, looks like a cache issue.

@erickgonzalez
Copy link
Contributor

PR:#19366

fmontes pushed a commit that referenced this issue Oct 14, 2020
* include css in jsp

* label updated
@erickgonzalez
Copy link
Contributor

Logout Page shows without issues.
Screen Shot 2020-10-14 at 4 25 17 PM

@bryanboza
Copy link
Member

Fixed, tested on release-20.10.1 // Postgres // FF

@wezell wezell closed this as completed Nov 3, 2020
dsilvam added a commit that referenced this issue Nov 4, 2020
* Update dotcmsReleaseVersion and coreWebReleasion version

* update release version

* #18505 JSONTool does not return sub arrays

* #18505 now the JSONTool uses the Jackson to map the string json as a single Maps and Lists

* #18505 now the JSONTool uses the Jackson to map the string json as a single Maps and Lists

* #19364 Unable to edit category permissions as limited user even you have full rights

* #18314 Make Query Tool Use fetch() to fill response

* #19098 SAML update logout page.  (#19450)

* include css in jsp

* label updated

* Updating sql files (#19478)

* Updating sql files to remove contraints

* Updating sql files to remove contraints

* #18690 Allow Push publish just for enterprise license in the receiver (#19492)

* #18690 Allow Push publish just for enterprise license in the receiver

* testing

* Fixing test

* Issue 19500 sql injection containers (#19501)

* #18605 pauses and then unpauses based on a cache invalidation

* #18605 adding ttl to the cache put in the logger

* #18605 less logging

* #19500 sanitize sql

* #19500 fixes potential sql vunerabilities

* #19500 writing tests

* #19500 tests

* we should not need TLS set to true

* #19500 removing unneeded files

* #19338 dont lowercase (#19506)

* #19338 dont lowercase

* #19338 integration test

* #19338 missing test resource

* #19509 use proper db columm in query (#19510)

* #19509 use proper db columm in query

* #19509 use proper property from contentlet

* #19509 fix integration test

* #19509 fix integration test

* #19471 Use proper value when discarding conflicts (#19519)

* #18780 fixes job when new hostname starts with  original hostname (#19522)

* #19509 Fixing bug when use comma in host's name (#19528)

* #19509 Fixing bug when use comma in host's name

* Fixing test

* update core-web version

* merge with master

* Update .gitmodules

* Update gradle.properties

Co-authored-by: Jonathan <jonathan.sanchez@dotcms.com>
Co-authored-by: erickgonzalez <erick.gonzalez@dotcms.com>
Co-authored-by: hmoreras <31667212+hmoreras@users.noreply.github.com>
Co-authored-by: Freddy Rodriguez <freddy0309@gmail.com>
Co-authored-by: Will Ezell <will@dotcms.com>
@jcastro-dotcms jcastro-dotcms added the LTS : Next Ticket that will be added to LTS label Jan 20, 2021
jcastro-dotcms added a commit that referenced this issue Feb 7, 2021
…anges were already included as part of GIT-19773 : SAML needs to transform user ID sent by IdP into a valid dotCMS user ID
@jcastro-dotcms jcastro-dotcms added LTS: Excluded Ticket that has been excluded from at least one LTS LTS: Released labels Feb 7, 2021
@jcastro-dotcms
Copy link
Contributor

The official code changes have been included in the dotCMS 5.3.8.4 LTS release only. The 5.2.8.4 LTS release is several commits behind, which made the code back-port not possible.

@john-thomas-dotcms john-thomas-dotcms added Release : 5.3.8.4 Included in LTS patch release 5.3.8.4 and removed LTS : Next Ticket that will be added to LTS LTS: Released labels Feb 10, 2021
@Zakinator123
Copy link

Was this included in 5.3.8.5? In our 5.3.8.5 installation with SAML not enabled, logging out of the admin does not lead to that logout page. We're using an OAuth2 OSGi plugin to provide an SSO experience for the admin login, so preferably logging out shouldn't lead to the dotCMS login page.

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

No branches or pull requests

9 participants