Skip to content

Conversation

@danxuliu
Copy link
Member

#549 might be fixed by this, although I am not sure if the problem can happen too when using SAML 2.0 or only when authenticating via environment variable (this pull request only fixes it when authenticating via environment variable).

handleApacheAuth() can throw a LoginException when trying to authenticate as a disabled user (and it has done it since Nextcloud 23.0.0, 22.2.1, 21.0.6 and 20.0.14). This needs to be explicitly handled to redirect to an error page, as otherwise the login page will try to be loaded which, in turn, will try to authenticate again and cause an endless loop.

How to test

  • Setup authentication via environment variable
  • In a private browser window, login with a user (to ensure that it will appear in Nextcloud user list)
  • Close the private window
  • Disable the user that just logged in
  • In another private browser window, login again as the disabled user

Result with this pull request

An error page with Account disabled is shown

Result without this pull request

Infinite loop of redirections

"handleApacheAuth()" can throw a LoginException when trying to
authenticate as a disabled user. This needs to be explicitly handled to
redirect to an error page, as otherwise the login page will try to be
loaded which, in turn, will try to authenticate again and cause an
endless loop.

Signed-off-by: Daniel Calviño Sánchez <[email protected]>
* SPDX-License-Identifier: AGPL-3.0-or-later
*/

use OC\User\LoginException;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think there should be a fitting Exception or interface in the OCP space, no?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

sheesh 😨

@blizzz blizzz merged commit f59e79c into master Dec 3, 2024
48 checks passed
@blizzz blizzz deleted the handle-loginexception-when-authenticating-with-apache branch December 3, 2024 19:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants