Tenancy Error Message when Delivery Service Already Exists outside of a user's tenancy

[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:

Delivery Service with xmlId: test2 exists, belonging to a user with tenancy: -root
Login as another user with tenancy: --unassigned
POST request to https://{{TO_BASE_URL}}/api/{{api_version}}/deliveryservices with xmlId: test2 in the body.
Returns:

HTTP/1.1 403 Forbidden
Content-Type: application/json

{
    "alerts": [
        {
            "text": "not authorized on this tenant",
            "level": "error"
        }
    ]
}

Expected / new behavior:

Should Return

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

{
    "alerts": [
        {
            "text": "deliveryservice xml_id 'test2' already exists.",
            "level": "error"
        }
    ]
}

Minimal reproduction of the problem with instructions:

Anything else:

[mitchell852]

This is an interesting one. The first error is confusing to the user because in this case they were working inside their tenancy but got a "not authorized on this tenant" error when in fact the real error was that they picked an xml_id that is already in use (outside of their tenancy).

I like the 2nd error better as it is more accurate.....but it exposes the fact that test2 exists...

not sure of the right solution. @ocket8888 thoughts?

also, pretty sure this is not a regression...and probably not even a bug.

[ocket8888]

No that's definitely a regression:

HTTP/1.1 400 Bad Request
Connection: keep-alive
Access-Control-Allow-Credentials: true
Access-Control-Allow-Headers: Origin, X-Requested-With, Content-Type, Accept
Set-Cookie: mojolicious=...; expires=Tue, 03 Dec 2019 23:25:47 GMT; path=/; HttpOnly
Whole-Content-SHA512: YuD9RBlIjNiqZZvJLeAZ1/pbrO0mucAkW2X2vE1akE+zUFjdUpDondWUskyZj/wLqJ1+roxJJgnWV4tjhydptg==
Cache-Control: no-cache, no-store, max-age=0, must-revalidate
Date: Tue, 03 Dec 2019 19:25:47 GMT
Access-Control-Allow-Origin: *
Content-Encoding: gzip
Vary: Accept-Encoding
Access-Control-Allow-Methods: POST,GET,OPTIONS,PUT,DELETE
Content-Length: 100
Content-Type: application/json
Server: Mojolicious (Perl)

{ "alerts": [
	{
		"level": "error",
		"text": "A deliveryservice with xmlId test already exists."
	}
]}

[ocket8888]

Actually, never mind, you're right. I was on the wrong tenant when I did the request that got me the above response.

If we're returning that they have insufficient permissions to modify a DS, then we've already disclosed the DS's existence.

[mitchell852]

If we're returning that they have insufficient permissions to modify a DS, then we've already disclosed the DS's existence.

So you're saying that either message discloses the existence of the DS, right?

"text": "not authorized on this tenant",

or

"text": "deliveryservice xml_id 'test2' already exists.",

in that case, i think the better error message is the 2nd one. would you agree @ocket8888 ? if so i would classify this as an improvement and not a bug, right?

[ocket8888]

Yes, the second one is a much better UX.