salesagility / SuiteCRM

SuiteCRM - Open source CRM for the world
https://www.suitecrm.com
GNU Affero General Public License v3.0
4.49k stars 2.09k forks source link

Invalid JSON response when creating user via V8 API #9002

Open peterkappelt opened 3 years ago

peterkappelt commented 3 years ago

Hey everyone, I hope y'all enjoyed your holidays. I want to create SuiteCRM users via the V8 api, but the response is invalid json and contains some generic validation error message.

I'm not sure whether my request is actually malformed or the error message just shouldn't appear.

Issue

Create a new User by calling: POST https://mysuitecrm.com/Api/V8/module

{
  "data": {
    "type": "Users",
    "attributes": {
      "user_name": "mail@example.org",
      "first_name": "John",
      "last_name": "Doe",
      "external_auth_only": 1,
      "status": "Active",
      "email1": "mail@example.org",
      "email_addresses": "mail@example.org",
      "email_addresses_primary": "mail@example.org"
    }
  }
}

The user will be created, but the response looks like this:

{
    "data": {
        "type": "User",
        "id": "1acd4bbf-7f8c-4e6f-aa92-5fea6e799b74",
        "attributes": {...}
    }
}Sie müssen einen Benutzernamen und eine E-Mail Adresse angeben.

The response includes the error messages string in the end. It translate to "You must specify a username and an e-mail address".

Expected Behavior

There shouldn't be any clear-text error messages appended to the JSON

Actual Behavior

There is a generic error message string appended to the JSON response

Possible Fix

Not sure

Steps to Reproduce

See above. Create User via API

Context

It's a blocker for me, since the API response can't be parsed properly by my user creation script.

Your Environment

pgorod commented 3 years ago

I'm just guessing, from a quick look, but I'd say...

User->save() calls User->saveFormPreferences, which has this line:

require_once 'modules/Users/GeneratePassword.php';

And inside that file there are several echo statements, namely

echo  $mod_strings['LBL_PROVIDE_USERNAME_AND_EMAIL'];

GeneratePassword.php is one of those files that should be completely refactored, no reason not to have that code in a proper class and separate processing from outputting screen information...

peterkappelt commented 3 years ago

I was able to fix this error by setting the user_hash attribute in the request, the error message now won't appear anymore.

Thank you for the hint. For me, this issue is now low priority since I was able to find a workaround.

Mac-Rae commented 3 years ago

I could be missing something, however, this doesn't look like an issue to me.

The system will always require a user_hash to be set, even if external auth is required. Therefore this only really comes down to an error message which could do with being slightly improved, no?

peterkappelt commented 3 years ago

Well, yes and no.

The improvement of the error message and the input validation is definitely one side of the problem. The error message is a bit misleading, since it literally says "You must specify an username and a mail address" - so I had no clue it was about the missing password. Also, it should definitely be JSON-formatted. I think this should be sufficient for fixing this issue. I could do it myself, though @pgorod suggested to do a complete refactoring of GeneratePassword.php - I don't have resources to do that.

On my SuiteCRM instance, I observed that the system set user_hash to NULL if a user was created via LDAP. I expected the same thing to be the case when creating users with the API for SAML auth. How is it supposed to work?

pgorod commented 3 years ago

Removing the echo from a function that is called on a Bean save is a bug that need to be fixed. It can break the system in many unexpected ways.

The minimum fix, without any big rewrites, is removing the echo commands and ensuring that errors are showing correctly when called from the UI, and that when calling from the API the return is valid JSON.