Discover and use the cache authorization token #24
Conversation
If the plugin is configured to use the cache authorization token, then read it periodically from the file and add it to the HTTP request via the query parameters. Includes relevant unit and integration tests.
| // Configure a curl handle to use the current cache token | ||
| // | ||
| // Adds the token to the curl handle URL's query string | ||
| // parameter `access_token`, as specified in RFC 6750, Sec 2.3. |
There was a problem hiding this comment.
Two sets of questions here:
- Section 2.3 of the RFC you link to warns:
Because of the security weaknesses associated with the URI method,
including the high likelihood that the URL containing the access token
will be logged, it SHOULD NOT be used unless it is impossible to
transport the access token in the "Authorization" request header field
or the HTTP request entity-body.
My assumption is that you're using the query param instead of the Authz header because the Authz header likely contains a JWT for the origin. Can you confirm or deny that with a brief comment? If I'm not correct, what's the logic for doing something the RFC explicitly warns against?
- Does this imply the origin receives the two tokens, one client token potentially in the Authz header and one cache token as a URL query param under
access_token? Why not use a simple JWT like we use everywhere else in Pelican Authz?
There was a problem hiding this comment.
We need to get two tokens to the origin -- one identifying the cache, one showing that there was at least one valid user request to the cache.
I started with the idea of putting both in the Authorization header (as suggested in the RFC text you quote). However, that ran aground in upstream XRootD as sending two Authorization headers is not kosher: you're only supposed to send two headers in very limited cases (where the header is explicitly allowed to repeat or where the value is comma-separated).
So, eliminating that, this was the remaining option. The "good news" here is that we already have log scrubbing for tokens, meaning there's mitigation for the explicit issue called out. This URL is also not exposed to the browser, meaning there's no likelihood of a user copy/pasting it.
| std::string_view url{url_char}; | ||
| auto has_query_string = url.find('?') != std::string::npos; | ||
| std::string final_url{url}; | ||
| final_url += has_query_string ? "&" : "?"; | ||
| final_url += "access_token="; | ||
| final_url += token; |
There was a problem hiding this comment.
This type of URL parsing feels like it should be common enough (if not already, then eventually) to make a utility function.
This comment was marked as duplicate.
This comment was marked as duplicate.
Sorry, something went wrong.
This comment was marked as duplicate.
This comment was marked as duplicate.
Sorry, something went wrong.
There was a problem hiding this comment.
Agreed!
This has a lot of the characteristics of a generic URL parser -- but only targets extracting a a single parameter. I think I'm going to keep it as-is for this PR but tackle things next time we need a "one-off".
|
Looks like GitHub was having A Morning today and managed to duplicate my comments a few times! Let's see what happens when I merge... 😆 |
If a cache authorization token file is provided in the plugin's configuration, then periodically read it out and use it in the generated curl requests.