georchestra / georchestra-gateway

GNU General Public License v3.0
0 stars 5 forks source link

preauth - http headers are evaluated in a case-sensitive manner #125

Closed pmauduit closed 2 months ago

pmauduit commented 2 months ago

Relying on the http specifications, the HTTP header names should be considered as case-insensitive, meaning "preauth-username should be the same as "Preauth-Username".

in the following code snippet: https://github.com/georchestra/georchestra-gateway/blob/main/gateway/src/main/java/org/georchestra/gateway/security/preauth/PreauthAuthenticationManager.java#L57-L74

the HttpHeaders object is following the specs correctly (calling getFirst("preauth-username") returns the same result as calling getFirst("Preauth-Username")), but as soon as we built our own hashmap (method extract()), we end up with a map in which we expect the headers names to be lowercased.

If the upstream proxy (say traefik) is sending normalized versions of the headers (like Preauth-Username), no matter how it is actually specified in its configuration, then we will miss the expected headers in the hashmap being built by the extract() method.

One workaround could be to lowercase the key in the extract() method.

edevosc2c commented 2 months ago

image

Related: https://github.com/traefik/traefik/issues/466

pmauduit commented 2 months ago

using the following patch is fixing the behaviour:

     private Map<String, String> extract(HttpHeaders headers) {
-        return headers.toSingleValueMap().entrySet().stream().filter(e -> e.getKey().startsWith("preauth-"))
-                .collect(Collectors.toMap(Map.Entry::getKey, Map.Entry::getValue));
+        return headers.toSingleValueMap().entrySet().stream()
+                .filter(e -> e.getKey().toLowerCase().startsWith("preauth-"))
+                .collect(Collectors.toMap(e -> e.getKey().toLowerCase(), Map.Entry::getValue));
     }

but we might want to avoid the call to extract(), and use the HttpHeaders object provided by the framework directly in the whole class.

pmauduit commented 2 months ago

Since the hashmap is passed when instanciating the PreauthAuthenticationToken object (https://github.com/georchestra/georchestra-gateway/blob/main/gateway/src/main/java/org/georchestra/gateway/security/preauth/PreauthAuthenticationManager.java#L64-L65), we could still create a hashmap (e.g. keeping the extract method), but with the keys being lowercased.

The map will be reused afterwards here, when mapping the headers onto a GeorchestraUser: https://github.com/georchestra/georchestra-gateway/blob/main/gateway/src/main/java/org/georchestra/gateway/security/preauth/PreauthenticatedUserMapperExtension.java#L37