Duplicate Content-Header for GET /api/vaults/{vaultId}/access-token

chenkins commented 6 months ago

Please agree to the following

[X] I have searched existing issues for duplicates
[X] I agree to follow this project's Code of Conduct

Summary

System Setup

- Hub: 1.4.0-SNAPSHOT
- Keycloak: 23.0.7

Steps to Reproduce

GET /api/vaults/{vaultId}/access-token

Expected Behavior

One Content-Type header field

Actual Behavior

Two Content-Type header field, I suppose one from appliation.properties

quarkus.http.header."Content-Type".value=text/html

and one from @Produces(MediaType.TEXT_PLAIN)

Reproducibility

Always

Relevant Log Output

2024-03-20 07:44:23,961 [main] INFO  ********request - GET /api/vaults/********/access-token?evenIfArchived=false HTTP/1.1
2024-03-20 07:44:23,961 [main] INFO  ********request - Accept: text/plain
2024-03-20 07:44:23,961 [main] INFO  ********request - User-Agent: ********
2024-03-20 07:44:23,961 [main] INFO  ********request - Host: localhost:8280
2024-03-20 07:44:23,961 [main] INFO  ********request - Connection: Keep-Alive
2024-03-20 07:44:23,961 [main] INFO  ********request - Accept-Encoding: gzip,deflate
2024-03-20 07:44:23,961 [main] INFO  ********request - Authorization: ********
2024-03-20 07:44:23,981 [main] INFO  ********response - HTTP/1.1 200 OK
2024-03-20 07:44:23,981 [main] INFO  ********response - Cache-Control: no-cache, no-store, must-revalidate
2024-03-20 07:44:23,981 [main] INFO  ********response - Content-Security-Policy: default-src 'self'; connect-src 'self' api.cryptomator.org localhost:8180; object-src 'none'; child-src 'self'; img-src * data:; frame-ancestors 'none'
2024-03-20 07:44:23,981 [main] INFO  ********response - Content-Type: text/html
2024-03-20 07:44:23,981 [main] INFO  ********response - Cross-Origin-Embedder-Policy: credentialless
2024-03-20 07:44:23,981 [main] INFO  ********response - Cross-Origin-Opener-Policy: same-origin
2024-03-20 07:44:23,981 [main] INFO  ********response - Cross-Origin-Resource-Policy: same-origin
2024-03-20 07:44:23,981 [main] INFO  ********response - Referrer-Policy: no-referrer
2024-03-20 07:44:23,981 [main] INFO  ********response - Strict-Transport-Security: max-age=31536000; includeSubDomains
2024-03-20 07:44:23,981 [main] INFO  ********response - X-Content-Type-Options: nosniff
2024-03-20 07:44:23,981 [main] INFO  ********response - X-Frame-Options: deny
2024-03-20 07:44:23,981 [main] INFO  ********response - X-Permitted-Cross-Domain-Policies: none
2024-03-20 07:44:23,981 [main] INFO  ********response - Content-Type: text/plain;charset=UTF-8
2024-03-20 07:44:23,981 [main] INFO  ********response - content-length: 416
2024-03-20 07:44:23,981 [main] INFO  ********response - Hub-Subscription-State: INACTIVE

Anything else?

Introduced in https://github.com/cryptomator/hub/commit/2111e9904afdafc639daae6b347be97c4073c006

patch:

--- a/backend/src/main/java/org/cryptomator/hub/api/VaultResource.java
+++ b/backend/src/main/java/org/cryptomator/hub/api/VaultResource.java
@@ -317,7 +317,6 @@ public class VaultResource {
        @RolesAllowed("user")
        @VaultRole({VaultAccess.Role.MEMBER, VaultAccess.Role.OWNER}) // may throw 403
        @Transactional
-       @Produces(MediaType.TEXT_PLAIN)
        @Operation(summary = "get the user-specific vault key", description = "retrieves a jwe containing the vault key, encrypted for the current user")
        @APIResponse(responseCode = "200")
        @APIResponse(responseCode = "402", description = "license expired or number of effective vault users that have a token exceeds available license seats")
@@ -347,7 +346,7 @@ public class VaultResource {
                        AuditEventVaultKeyRetrieve.log(jwt.getSubject(), vaultId, AuditEventVaultKeyRetrieve.Result.SUCCESS);
                        var subscriptionStateHeaderName = "Hub-Subscription-State";
                        var subscriptionStateHeaderValue = license.isSet() ? "ACTIVE" : "INACTIVE"; // license expiration is not checked here, because it is checked in the ActiveLicense filter
-                       return Response.ok(access.vaultKey).header(subscriptionStateHeaderName, subscriptionStateHeaderValue).build();
+                       return Response.ok(access.vaultKey, MediaType.TEXT_PLAIN_TYPE).header(subscriptionStateHeaderName, subscriptionStateHeaderValue).build();
                } else if (Vault.findById(vaultId) == null) {
                        throw new NotFoundException("No such vault.");
                } else {

overheadhunter commented 6 months ago

@JaniruTEC any particular reason for this change?

we shouldn't set such a header globally in application.properties, though. Just for specific resources.

chenkins commented 6 months ago

application properties is old, but the change from returning String to Resource without setting the content type is recent

overheadhunter commented 6 months ago

I know, but still I wonder why we're defaulting to text/html.

JaniruTEC commented 5 months ago

2111e99 needs to be considered in the context of its PR (#250) and the next commit after (289014b). It was created for the purpose of enabling that next commit. The content type was already defined at that point in time, tho: https://github.com/cryptomator/hub/commit/2111e9904afdafc639daae6b347be97c4073c006#diff-2379b203b20f6c78d9aaf2d7437eaff6d257e2c82136a2c31e770e975afd1bf2R291

overheadhunter commented 1 month ago

fyi: I just retested with Quarkus 3.8.5: There is now just one Content-Type header field in the response (but sadly not the one specified by the annotation).

cryptomator / hub