OpenCTI-Platform / opencti

Open Cyber Threat Intelligence Platform
https://opencti.io
Other
6.32k stars 932 forks source link

SSO Broken in 6.3.6 - "User already exists" #8742

Open MaxwellDPS opened 2 days ago

MaxwellDPS commented 2 days ago

Description

Following a re-index and upgrade to 6.3.6 no users can not auth via pre-existing SSO

Environment

  1. OS (where OpenCTI server runs): CentOS Stream 9
  2. OpenCTI version: 6.3.6
  3. OpenCTI client: Frontent
  4. Other environment details: Clustered k8s

Reproducible Steps

Steps to create the smallest reproducible scenario:

  1. Attempt SSO via pre-existing SAML config

Expected Output

Successful login session

Actual Output

Redirects to the login page

Additional information

{
    "category": "APP",
    "errors": [
        {
            "attributes": {
                "genre": "BUSINESS",
                "http_status": 400,
                "user_id": "<REMOVED UUID>"
            },
            "message": "User already exists",
            "name": "FUNCTIONAL_ERROR",
            "stack": "GraphQLError: User already exists\n    at error (/opt/opencti/build/src/config/errors.js:7:10)\n    at FunctionalError (/opt/opencti/build/src/config/errors.js:94:50)\n    at addUser (/opt/opencti/build/src/domain/user.js:530:11)\n    at processTicksAndRejections (node:internal/process/task_queues:95:5)"
        }
    ],
    "level": "error",
    "message": "User already exists",
    "provider": "saml",
    "source": "backend",
    "timestamp": "2024-10-22T16:02:28.131Z",
    "version": "6.3.6"
}
richard-julien commented 2 days ago

Thats strange ... The only strange thing I can see in the code is a loading with the email in lowercase versus a loading without case manipulation. I will try to reproduce.

richard-julien commented 2 days ago

After some testing, there is no problem, the query is not case sensitive. The code responsible for this is quite simple.

First we load the user to check if it exists. So in your case, the function return an empty value

  const user = await elLoadBy(context, SYSTEM_USER, 'user_email', email, ENTITY_TYPE_USER);
  if (!user) {
    // If user doesn't exist, create it. Providers are trusted
    const newUser = { name, firstname, lastname, user_email: email.toLowerCase(), external: true };
    return addUser(context, SYSTEM_USER, newUser).then(() => {
      // After user creation, reapply login to manage roles and groups
      return loginFromProvider(userInfo, opts);
    });
  }

Because the return is empty, we try to create the user. As you can see the addUser function try to load the user first. Except the lowercasing, its exactly the same call. In your case this time the load function return a result and so the addUser function reject the execution.

export const addUser = async (context, user, newUser) => {
  const userEmail = newUser.user_email.toLowerCase();
  const existingUser = await elLoadBy(context, SYSTEM_USER, 'user_email', userEmail, ENTITY_TYPE_USER);
  if (existingUser) {
    throw FunctionalError('User already exists', { user_id: existingUser.internal_id });
  }
  ...
}

Maybe something with the mappings and the keyword definition