No Error when child tenant accesses parent tenant's delivery-services

[ericholguin]

I'm submitting a ...

• bug report

Traffic Control components affected ...

• Traffic Ops
• Traffic Portal

Current behavior:

A GET request to /deliveryservices/{{ID}} (where ID identifies a Delivery Service assigned to the parent tenant of the currently logged-in user's tenant) returns status code 200 OK, and an empty response array.

In Traffic Portal the following Delivery Service endpoint pages have issues:

• /#!/delivery-services/{{ID}}?type=HTTP
• /#!/delivery-services/{{ID}}/servers?type=HTTP
• /#!/delivery-services/{{ID}}/required-server-capabilities?type=HTTP

Expected / new behavior:

The above GET request should return 403 Forbidden.

Traffic Portal Page should show the same error.

[ocket8888]

We're trying to favor query parameters over path parameters these days, so the API is designed with an eye toward that. The handler for /foos?id={{ID}} is in nearly all cases the same handler as /foos/{{ID}}. The semantics of foos?id={{ID}} are "return the 'foo' objects, filtered where the id is this value".

It's also currently our policy to try to hide the existence of a Delivery Service a user doesn't have tenant-level permissions to access from that user. When you put those two things together, you see that the result of filtering Delivery Services by an ID over which the current user has no tenant permissions is an empty array.

tl;dr, this isn't a bug. It would be an improvement for path-based, parameterized URIs to return 404 Not Found when no corresponding object can be returned, but I wouldn't hold your breath if I was you. We'll probably move away from path-based parameterization before that happens.

[mitchell852]

I'm pretty sure GET /deliveryservices/4 where id=4 is not in your tenancy has always resulted in a 403 forbidden

Or any tenantable resource for that matter. ie.

GET /deliveryservices/4 <-- 403 GET /deliveryservices/4/servers <-- 403 GET /users/34 <-- 403 GET /users/34/deliveryservices <-- 403 GET /tenants/44 <-- 403 GET /origins/88 <-- 403

yes, that approach "leaks" info to the user in the sense that now the user knows that ds=4 exists but that's just the way it has always been in 1.x.

[ocket8888]

For as long as the DS endpoints have been using the CRUDer that's definitely not true. Because both /deliveryservices and /deliveryservices/{{ID}} use api.ReadHandler with the same object. And I think that change happened at least before I started working on TO.

[lbathina]

this is a bug not an improvement. FYI something that doesn't work as expected is called bug and something we think it would be nice to have is improvement. In this case, it is expected to work in a certain way by rules of tenancy. Also the return should be 403 not 404. refer to our conversation in a similar ticket. https://github.com/apache/trafficcontrol/issues/4043

[mitchell852]

I think the "source of authority" answer is the answer to "what did the perl do?" and i'm pretty sure tenancy failure to GET /deliveryservices/4 returned a 403 on perl. So for the sake of the 1.x contract, we need to respect that.

This does bring up an interesting discussion though on what the response to this should be (when 4 violates tenancy):

GET /deliveryservices?id=4 - i could see an argument for [] or 403...

[ocket8888]

"i'm pretty sure tenancy failure to GET /deliveryservices/4 returned a 403 on perl"

It did, I just checked. So that's definitely a bug, specifically a regression.

[mitchell852]

I, personally, would not classify this as a major as at least the response is empty.

[ocket8888]

... I uh... then why did you classify it as major?

[mitchell852]

... I uh... then why did you classify it as major?

good question. changing back to minor.

[ericholguin]

Requests where the dsID or xmlID pertains to a delivery service owned by the parent tenant of the child tenant requesting it:

GET /api/4.0/deliveryservices/32/regexes

HTTP/1.1 200 OK
Content-Type: application/json
Transfer-Encoding: chunked

{
    "response": []
}

GET /api/4.0/deliveryservices/32/capacity

HTTP/1.1 403 Forbidden
Content-Type: application/json
Transfer-Encoding: chunked

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

GET /api/4.0/deliveryservices/32/health

HTTP/1.1 403 Forbidden
Content-Type: application/json
Transfer-Encoding: chunked

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

GET /api/4.0/deliveryservices/32/routing

HTTP/1.1 403 Forbidden
Content-Type: application/json
Transfer-Encoding: chunked

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

PUT /api/4.0/deliveryservices/32/safe

HTTP/1.1 403 Forbidden
Content-Type: application/json
Transfer-Encoding: chunked

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

GET /api/4.0/deliveryservices/32/servers

HTTP/1.1 200 OK
Content-Type: application/json
Transfer-Encoding: chunked

{
    "response": [
        {
           <server info here>
        }
    ]
}

GET /api/4.0/deliveryservices/32/servers/eligible

HTTP/1.1 403 Forbidden
Content-Type: application/json
Transfer-Encoding: chunked

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

GET /api/4.0/deliveryservices/32/urlkeys

HTTP/1.1 403 Forbidden
Content-Type: application/json
Transfer-Encoding: chunked

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

POST /api/4.0/deliveryservices/ds1/servers

HTTP/1.1 403 Forbidden
Content-Type: application/json
Transfer-Encoding: chunked

{
    "alerts": [
        {
            "text": "Forbidden",
            "level": "error"
        }
    ]
}

GET, POST, PUT, DELETE /api/4.0/deliveryservices/ds1/urisignkeys

HTTP/1.1 403 Forbidden
Content-Type: application/json
Transfer-Encoding: chunked

{
    "alerts": [
        {
            "text": "Forbidden",
            "level": "error"
        }
    ]
}

GET, DELETE /api/4.0//deliveryservices/xmlId/ds1/sslkeys

HTTP/1.1 403 Forbidden
Content-Type: application/json
Transfer-Encoding: chunked

{
    "alerts": [
        {
            "text": "Forbidden",
            "level": "error"
        }
    ]
}

GET, DELETE /api/4.0//deliveryservices/xmlId/ds1/urlkeys

HTTP/1.1 403 Forbidden
Content-Type: application/json
Transfer-Encoding: chunked

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

POST deliveryservices/xmlId/ds1/urlkeys/generate

HTTP/1.1 403 Forbidden
Content-Type: application/json
Transfer-Encoding: chunked

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

GET /staticdnsentries

HTTP/1.1 200 OK
Content-Type: application/json
Transfer-Encoding: chunked

{
    "response": [
        {
            "address": "test.",
            "cachegroup": null,
            "cachegroupId": null,
            "deliveryservice": "ds1",
            "deliveryserviceId": 32,
            "host": "test",
            "id": 1,
            "lastUpdated": "2022-07-12 16:01:19+00",
            "ttl": 10,
            "type": "CNAME_RECORD",
            "typeId": 41
        }
    ]
}

DELETE /staticdnsentries?id=1

HTTP/1.1 200 OK
Content-Type: application/json
Transfer-Encoding: chunked

{
    "alerts": [
        {
            "text": "staticDNSEntry was deleted.",
            "level": "success"
        }
    ]
}

These same request/response apply to 3.0. The endpoints that seem to have issues are deliveryservices/{dsID}/servers and all the /staticdnsentries methods. Someone else would need to confirm that this is not the correct behavior.