Open-EO / openeo-api

The openEO API specification
http://api.openeo.org
Apache License 2.0
91 stars 11 forks source link

error code for expired file access #379

Closed soxofaan closed 3 years ago

soxofaan commented 3 years ago

@laxiwuka is working on implementing signed result download urls (Open-EO/openeo-python-driver#50)

he is also incorporating expiring of download urls, but as far as we know there is no dedicated error code to return when requesting a file that is expired.

Can we add some kind of FileExpired error code?

m-mohr commented 3 years ago

This is intentionally not in the API spec, because these signed URLs could be hosted anywhere and the endpoint where the files reside is part of the API, too. Like VITO you could store the results on a S3 bucket and there you can't provide a specific error code and the files will likely just disappear (HTTP 404). So we could only recommend an error code, but clients can't rely on it so the purpose of a pre-defined error code may be limited (i.e. back-ends can just make a custom one). If you still think it's useful, we could add a recommended error code, I'm not advocating against having one, but I'm also not seeing a real advantage of having one.

soxofaan commented 3 years ago

In practice all error codes are just recommendations anyway, right? A sloppy backend can choose to raise "500 Internal" everywhere where a better choice is available, and still be "compliant": https://github.com/Open-EO/openeo-api/blob/81dd248b9e8b8caeedbdd96c6a9dd8f3f56d0eb7/openapi.yaml#L75

The standardized error codes are mainly about making the development/debug/maintenance experience better, even if they are un-handled by a client. In that sense it is useful to define this error code, I think.

Which alternative would you recommend? A custom error with status 403 (forbidden) or 410 (Gone), code FileExpired and message like File access has expired for {path}?

m-mohr commented 3 years ago

Yes, correct.

I think I'd define it as follows:

    "LinkExpired": {
        "description": "The signed URLs for batch job results have expired. Please send a request to `GET /jobs/{job_id}/results` to refresh the links.",
        "message": "The links to the batch job results have expired. Please request the results again.",
        "http": 410,
        "tags": [
            "Batch Jobs"
        ]
    },

and mention it as recommendation in the GET /jobs/{job_id}/results endpoint.

I realized just now, that the API doesn't say what to do after expiration. I guess it's implicitly clear that GET /jobs/{job_id}/results always returns valid links so that re-requesting that endpoint returns new links if required, but it's not clearly stated.

soxofaan commented 3 years ago

Interesting, I didn't consider that you can just "refresh" the urls by calling .../results again. That would actually be a reason to use a relatively small expiry timespan (couple of minutes) to improve security. If so, that would be interesting to explicitly add as hint or recommendation in the spec as you mention.

m-mohr commented 3 years ago

see PR #380

I wouldn't do a too short timespan. Usecase is that you can read it with STAC clients, e.g. in QGIS. If that expires too fast, it will get annoying. So I'd do at least an hour...

soxofaan commented 3 years ago

sure, order of magnitude of hours is fine too. I originally had days or weeks in mind for those urls.

m-mohr commented 3 years ago

Days or weeks should also be fine, it works for Google Docs, too. But restricting it to some hours (maybe 8 hours for a working day ;-) ) is probably also fine.