Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Closed
pmauduit opened this issue Jun 5, 2024 · 3 comments
Closed

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

pmauduit opened this issue Jun 5, 2024 · 3 comments

Comments

@pmauduit
Copy link
Member

pmauduit commented Jun 5, 2024

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
Copy link
Member

edevosc2c commented Jun 5, 2024

image

Related: traefik/traefik#466

@pmauduit
Copy link
Member Author

pmauduit commented Jun 5, 2024

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
Copy link
Member Author

pmauduit commented Jun 5, 2024

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

pmauduit added a commit that referenced this issue Jun 5, 2024
pmauduit added a commit that referenced this issue Jun 13, 2024
…sensitive-125

preauth - http header names are case insensitive (#125)
@f-necas f-necas closed this as completed Jun 13, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants