Repository navigation
Conversation
| |=== | ||
| h|Operation Name 4+e|[[GetThumbnail]]GetThumbnail | ||
| h|Explanation 4+a|Returns the thumbnail file | ||
| h|Explanation 4+a|Returns the thumbnail file. For an external thumbnail, the HTTP/REST binding redirects the client as defined in xref:http-rest-api/http-rest-api.adoc#retrieving-files[Retrieving Files and Thumbnails]. |
There was a problem hiding this comment.
and what happens with a relative path?
There was a problem hiding this comment.
That will stay as before -> Returns the thumbnail file
There was a problem hiding this comment.
I think the new return codes must be added to https://industrialdigitaltwin.io/aas-specifications/IDTA-01002/v3.1.1/http-rest-api/http-rest-api.html#mapping-of-status-codes, I did not see this.
It seems these changes are not backward compatible, are they? It is not only a bugfix for 3.2 but also for 3.1 and 3.0
thanks, I will add that.
I see two options here. We could either handle this as a bugfix or we could see it as a feature. If it's a bugfix, you are right, it should then also be added to 3.1 and 3.0. Alternatively, we could add this to a new branch/milestone 3.3. What would you say is the better way @BirgitBoss? |
|
from today's API meeting:
|
|
It is not really a bugfix, that is true. I will open the PR again for the 3.3 working branch and do the necessary changes for that. Thanks for the feedback @sebastiankb |
|
|
||
| After applying the authorization checks for the requested operation, the server shall handle the location as follows: | ||
|
|
||
| * If the location is an absolute URL with the scheme `http` or `https`, the server shall return `302 Found` with that URL in the `Location` response header, as defined in https://www.rfc-editor.org/rfc/rfc9110.html#section-15.4.3[RFC 9110, Section 15.4.3]. |
There was a problem hiding this comment.
Using shall, i.e., making it mandatory, would mean the spec defines internal behavior/implementation of the server which I think is outside the scope of the spec.
I therefore recommend to change it to should with a big warning about the dangers when doing it diffrently.
There was a problem hiding this comment.
Good point @mjacoby. How the server gets the content is an implementation detail, so I changed the redirect to a recommendation: for external HTTP(S) locations the server should return 302 Found. The "shall not fetch" and "resolve only against managed storage" requirements are gone. Instead there's a [WARNING] block covering the risks of the alternatives: server-side fetching can lead to SSRF (it also lists the usual protections, like allow-lists, blocking internal addresses and not forwarding credentials), and looking up locations outside the managed file storage can expose files. It also says that returning 302 avoids the fetch and is the recommended way.
I kept as "shall" only what a client can actually see: authorization is checked before the content or the redirect URL is revealed, managed files come back with 200, and missing files return 404. The conformance cases HTTP.file.002/003 now say SHOULD instead of MUST.
Define 302 Found with a Location header for HTTP(S) URLs in File.value and the default thumbnail path. This lets clients use the attachment and thumbnail endpoints without making the server fetch external files. Align all affected OpenAPI profiles, clarify authorization and client responsibilities, and add conformance cases and a V3.2.1 changelog entry. Fixes #495
Add RedirectionFound to the generic status codes and map it to HTTP 302 Found. Keep the Location header explanation and changelog aligned.
Make 302 Found for external HTTP(S) locations a recommendation instead of a requirement, since how a server obtains file content is an implementation decision. Add a warning on the SSRF and file disclosure risks of server-side fetching and of resolving locations outside the managed file storage. Keep authorization before disclosure and 404 for missing files mandatory, and align the conformance cases and changelog.
900935f to
4ecc951
Compare
Closes #495.
Clients should be able to use the
/attachmentendpoint without first checking whether a file is stored by the server or referenced by an external URL. For HTTP(S) URLs inFile.value, this PR recommends302 Foundwith the URL in theLocationheader. The same behavior applies to/thumbnailandAssetInformation.defaultThumbnail.path, as already implemented in BaSyx Go.The redirect is a recommendation (
should), not a requirement, because how a server obtains the content it returns is an implementation decision. A warning explains the risks of the alternatives: fetching URLs supplied through writable model data can expose the server to SSRF, and resolving locations outside the managed file storage can disclose files. Authorization is still checked before the content or the redirect URL is disclosed. Clients decide whether to follow the redirect under their own security policy and must not forward credentials intended for the AAS server to another origin. Managed files are still returned with200 OK; missing locations or missing managed files return404 Not Found.The change adds the HTTP/REST explanation and an example, adds the generic status code
RedirectionFoundand its HTTP mapping, aligns the abstract payload definitions, and adds a shared redirect response with a requiredLocationheader to all 22 affected GET definitions across 11 service profiles and the combined API. It also updates the conformance cases and adds a V3.3 changelog entry. The existing write operations are unchanged.This PR now targets
IDTA-01002-3-3_workinginstead ofIDTA-01002-3-2-1_working, since it changes behavior and is not a bugfix (see the discussion below).Validation
python tools/validate_spec_artifacts.pypython tools/validate_multipart_uploads.pyopenapi-spec-validatorpasses for all 11 affected profiles and the combined APIgit diff --check