Issues when updating a user's email using /user/current endpoint

[ericholguin]

I'm submitting a ...

• bug report
• new feature / enhancement request
• improvement request (usability, performance, tech debt, etc.)
• other

Traffic Control components affected ...

• CDN in a Box
• Documentation
• Grove
• Traffic Control Client
• Traffic Monitor
• Traffic Ops
• Traffic Ops ORT
• Traffic Portal
• Traffic Router
• Traffic Stats
• Traffic Vault
• unknown

Current behavior:

When making a PUT request to /user/current with the following request body:

{
    "user": {
        "email": ""
    }
}

Will return:

HTTP/1.1 200 OK
Content-Type: application/json

{
    "alerts": [
        {
            "text": "User profile was successfully updated",
            "level": "success"
        }
    ],
    "response": {
        .
        .
        "email": "",
        .
    }
}

And when making a PUT request to /user/current/ with the following request body:

{
    "user": {
        "email": null
    }
}

Will return

HTTP/1.1 500 Internal Server Error
Content-Type: application/json

{
    "alerts": [
        {
            "text": "Internal Server Error",
            "level": "error"
        }
    ]
}

Expected / new behavior:

Both requests should return:

HTTP/1.1 400 Bad Request
Content-Type: application/json

{
    "alerts": [
        {
            "text": "Couldn't parse request: email: cannot be null or empty string",
            "level": "error"
        }
    ]
}

Minimal reproduction of the problem with instructions:

[mitchell852]

technically, according to the tm_user table in the database, email is not required (NOT NULL=false) so blank or null would be fine assuming that null actually worked..

[ericholguin]

When creating a user, the email field is required. Both Traffic Portal and Traffic Ops API do not allow you to create a user with a blank email field. Traffic Portal also does not allow you to update a user's email to a blank value. I would expect the API to behave the same way.

[mitchell852]

@ZugNZwang - yeah, you're right imo. if you can't create a user (POST /users) w/o an email, you shouldn't be able to update and remove the email. Does PUT /users/{id} allow you to remove the email as well?

[ericholguin]

PUT /users/{id} doesn't allow you to have a blank email. Returns:

{
    "alerts": [
        {
            "text": "'email' cannot be blank",
            "level": "error"
        }
    ]
}

[ocket8888]

If it's true that it's not meant to be possible to have a user with no (valid) email address, then there ought to be a database constraint for that as well.

[mitchell852]

If it's true that it's not meant to be possible to have a user with no (valid) email address, then there ought to be a database constraint for that as well.

Well I know we have many users defined with no email address so adding a db constraint would require some data clean up. Honestly, I'd just start with making PUT /user/current require an email address.

[ocket8888]

Well what I'm asking firstly is: is it true that the intent is that no user can exist without an an email?

[mitchell852]

hmm. well you have to provide an email with:

POST /users
PUT /users/id

so from an API perspective, it's basically required so it makes sense to me to fix the glitch in:

PUT /user/current

[ocket8888]

Oh. I know CiaB's "admin" user has no email, so I thought that wasn't a requirement for user creation. But on further inspection, that user is directly inserted into the database (and it has to be because you need to be authenticated to use the API).

[ocket8888]

Closed by #4346