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

Unhandled null reference in UserAuthenticationResource #3739

Closed
2 tasks done
alessandrocavalli opened this issue Sep 11, 2023 · 0 comments
Closed
2 tasks done

Unhandled null reference in UserAuthenticationResource #3739

alessandrocavalli opened this issue Sep 11, 2023 · 0 comments
Assignees
Labels
group:support All requests that are linked to a customer request. DRI: Tassilo scope:webapp Changes to all the webapps. type:bug Issues that describe a user-facing bug in the project. version:7.22.3 version:7.23.0

Comments

@alessandrocavalli
Copy link

alessandrocavalli commented Sep 11, 2023

Environment (Required on creation)

Camunda 7.19.0+ & 7.18.6+

Description (Required on creation; please attach any relevant screenshots, stacktraces, log files, etc. to the ticket)

When using an external authentication system, in case the user is no longer existent, the UserAuthenticationResource will throw a NullPointerException.

org.camunda.commons.logging.BaseLogger.logWarn ENGINE-REST-HTTP500 java.lang.NullPointerException at org.camunda.bpm.webapp.impl.security.auth.UserAuthenticationResource.doLogin(UserAuthenticationResource.java:107) at

Steps to reproduce (Required on creation)

  1. Create a user in an external authentication system like LDAP.
  2. Configure the engine to use that external authentication system.
  3. Log in with the user.
  4. Remove the user from the external system.
  5. After the auth cache time-to-live is exceeded, try to interact with the web apps.

Observed Behavior (Required on creation)

An NPE is thrown and the user is logged out.

Expected behavior (Required on creation)

The user is logged out and Camunda returns a different error (It could be "Unauthorized")

Root Cause (Required on prioritization)

The user is fetched from the identity provider and used without a null check. If it is null, we run into an NPE.

UserAuthentication authentication = AuthenticationUtil.createAuthentication(processEngine, username);

Solution Ideas

Add a null check to the authentication code and return unauthorized if it is.

...
if (!isPasswordValid) {
      if(isWebappsAuthenticationLoggingEnabled(processEngine)) {
        LOGGER.infoWebappFailedLogin(username, "bad credentials");
      }
      return unauthorized();
    }

    UserAuthentication authentication = AuthenticationUtil.createAuthentication(processEngine, username);

    if (authentication == null) { 
       return unauthorized();
   }
...

Hints

Links

Breakdown

Pull Requests

Preview Give feedback
  1. bot:backport:7.22 ci:webapp-integration
    joaquinfelici

Dev2QA handover

  • Does this ticket need a QA test and the testing goals are not clear from the description? Add a Dev2QA handover comment
@alessandrocavalli alessandrocavalli added the type:bug Issues that describe a user-facing bug in the project. label Sep 11, 2023
@tmetzke tmetzke added group:support All requests that are linked to a customer request. DRI: Tassilo scope:webapp Changes to all the webapps. labels Sep 11, 2023
@ThorbenLindhauer ThorbenLindhauer removed their assignment Sep 12, 2023
@joaquinfelici joaquinfelici self-assigned this Jan 29, 2025
joaquinfelici added a commit that referenced this issue Jan 31, 2025
* fix(webapp): Add test to reproduce current behavior

Related to #3739

* fix(webapp): Handle null authentication as unauthorized

Related to #3739
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
group:support All requests that are linked to a customer request. DRI: Tassilo scope:webapp Changes to all the webapps. type:bug Issues that describe a user-facing bug in the project. version:7.22.3 version:7.23.0
Projects
None yet
Development

No branches or pull requests

7 participants