extensions: make jwt_authn filter token-parsing more flexible (#6355)#6384
Conversation
Skips non-legit JWT chars after the "value-prefix" is found, then returns a substring up to the first non-JWT char, or the end of line. Signed-off-by: Larry West <Larry_West@intuit.com>
Signed-off-by: Larry West <Larry_West@intuit.com>
No functional changes. Signed-off-by: Larry West <Larry_West@intuit.com>
Duplicated #includes, line-length limit. Signed-off-by: Larry West <Larry_West@intuit.com>
Signed-off-by: Larry West <Larry_West@intuit.com>
…s ago (while fixing style in 'extractor_test.cc'). Signed-off-by: Larry West <Larry_West@intuit.com>
Hopefully quoting the values will prevent them from being spell-checked. (Though the extractor.cc complaint about "Bearer eyJhbG...." puzzles me.) Signed-off-by: Larry West <Larry_West@intuit.com>
Signed-off-by: Larry West <Larry_West@intuit.com>
|
Thanks, @qiwzhang, I'll push a "final" couple commits after code-coverage complete, and I finish updating the api/.../jwt_authn/v2alpha/README.md |
...redundant tests of removeJwt(). Signed-off-by: Larry West <Larry_West@intuit.com>
To describe the (updated) functionality of the value_prefix field. Signed-off-by: Larry West <Larry_West@intuit.com>
|
I don't mean to seem impatient, but it looks like the final commit (6ad1680) didn't trigger CircleCI: all of the 12 ci/circleci steps are "Expected", not active. The final commit here was at 3:03pm PDT, and several other PRs have run various steps at circleci.com successfully in the meantime — the oldest job in Running state started at 3:54pm. If this is still stuck at the top of the hour, and if no one objects, I'll push an empty commit. |
(None of the 12 CircleCI tasks had begun in the three hours since the previous commit) Signed-off-by: Larry West <Larry_West@intuit.com>
|
@lizan I am confused by the "ci/circle: coverage — Your tests failed on CircleCI." status in this case. When I go to the Details link, I see no indication of why it failed, just "FAILED" in a red rounded box in the upper-left. The last line of the output from the fourth and final step, "ci/do_circle_ci.sh bazel.coverage", is just There's nothing I can see here to click on that yields anything that looks related to code coverage. There is a message at the top, |
Code Coverage 'failed' with no actual indication of how or why on earlier job, 189678 - https://circleci.com/gh/envoyproxy/envoy/189678?utm_campaign=vcs-integration-link&utm_medium=referral&utm_source=github-build-link Signed-off-by: Larry West <Larry_West@intuit.com>
|
@larrywest Don't worry about |
...for extractJWT() only. I.e., pass by copying the value rather than the reference. Per review comment by lizan. Signed-off-by: Larry West <Larry_West@intuit.com>
...to use absl::StrCat() rather than C++ string '+' concatenation. Per PR#6384 review comments by lizan. Signed-off-by: Larry West <Larry_West@intuit.com>
Signed-off-by: Larry West <Larry_West@intuit.com>
lizan
left a comment
There was a problem hiding this comment.
LGTM modulo one nit. Thank you!
Reversing a change made earlier. Based on a review comment from lizan. Signed-off-by: Larry West <Larry_West@intuit.com>
See the Absiel Tip of the Week #1: https://abseil.io/tips/1 for explanation. Change requested by lizan. Signed-off-by: Larry West <Larry_West@intuit.com>
|
Does anything more need to be done? |
|
Ah sorry one last thing, can you add a line of brief description into version history? |
|
@larrywest just add it in this PR? |
|
I naively followed the link and edited the text in github. I will merge those tiny changes over to the branch for this PR. |
See Issue envoyproxy#6355 Signed-off-by: Larry West <Larry_West@intuit.com>
first token must be alphabetic, no "/". Signed-off-by: Larry West <Larry_West@intuit.com>
| * mysql: added a MySQL proxy filter that is capable of parsing SQL queries over MySQL wire protocol. Refer to ::ref:`MySQL proxy<config_network_filters_mysql_proxy>` for more details. | ||
| * http: added :ref:`max request headers size <envoy_api_field_config.filter.network.http_connection_manager.v2.HttpConnectionManager.max_request_headers_kb>`. The default behaviour is unchanged. | ||
| * http: added modifyDecodingBuffer/modifyEncodingBuffer to allow modifying the buffered request/response data. | ||
| * http: added encodeComplete/decodeComplete. These are invoked at the end of the stream, after all data has been encoded/decoded respectively. Default implementation is a no-op. |
There was a problem hiding this comment.
Looks like the other lines are irrelevant, can you merge master?
Also start the jwt line with jwt_authn: , and sort please.
There was a problem hiding this comment.
BTW, line 82 has tracing: in the midst of a few upstream: lines.
There was a problem hiding this comment.
ah thanks, we will fix that during the release process.
…ue_6355-jwt_authn-token_parsing Signed-off-by: Larry West <Larry_West@intuit.com>
begin with "jwt_authn:" rather than "http:" Signed-off-by: Larry West <Larry_West@intuit.com>
Probably an artifact of my merge. Signed-off-by: Larry West <Larry_West@intuit.com>
|
@lizan What's the next step? |
|
@lizan It looks like this was merged just after the 1.10.0 release, so the merge resulted in Should I make the change to move that line to "1.11.0 (pending)" as a separate PR? |
|
Ah nice catch, that sounds good, let's fix the version history at least for master. |
The PR (envoyproxy#6384) for the enhancement to jwt_authn was accepted before 1.10.0 was cut, and so the history was edited with 1.10.0 in mind, but the merge of happened just after 1.10.0 was cut. Therefore, this change moves the jwt_authn line from the "1.10.0 (April 5, 2019)" section to the "1.11.0 (Pending)" section.
The PR (envoyproxy#6384) for the enhancement to jwt_authn was accepted before 1.10.0 was cut, and so the history was edited with 1.10.0 in mind, but the merge of happened just after 1.10.0 was cut. Therefore, this change moves the jwt_authn line from the "1.10.0 (April 5, 2019)" section to the "1.11.0 (Pending)" section. Signed-off-by: Larry West <Larry_West@intuit.com>
The PR (envoyproxy#6384) for the enhancement to jwt_authn was accepted before 1.10.0 was cut, and so the history was edited with 1.10.0 in mind, but the merge of happened just after 1.10.0 was cut. Therefore, this change moves the jwt_authn line from the "1.10.0 (April 5, 2019)" section to the "1.11.0 (Pending)" section. Signed-off-by: Larry West <Larry_West@intuit.com>
The PR (#6384) for the enhancement to jwt_authn was accepted before 1.10.0 was cut, and so the history was edited with 1.10.0 in mind, but the merge of happened just after 1.10.0 was cut. Therefore, this change moves the jwt_authn line from the "1.10.0 (April 5, 2019)" section to the "1.11.0 (Pending)" section. Signed-off-by: Larry West <Larry_West@intuit.com>
Description: modifies jwt_authn filter's ExtractorImpl extract method to use the
from_headers'svalue_prefixtag more precisely, allowing syntax like "tag=<JWT>,other=xxx" rather than simply taking the remainder of the string as the JWT candidate.See Issue #6355 for full description. Should be backwards-compatible with existing uses of jwt_authn.
Risk Level: Medium (scope: affects JWT authentication)
Testing: Unit testing was added to test/.../jwt_authn/extractor_test.cc. Since
Extractoritself does not validate the JWT, only the parsing is tested.Docs Changes: see "Further header options" section added to api/envoy/config/filter/http/jwt_authn/v2alpha/README.md
Release Notes:
Adds enhancement per Issue #6355, so that deployments can use a wider variety of HTTP header syntaxes to pass JWTs and have them authenticated by the
jwt_authnfilter. Backwards-compatible with existing usage.JWT authentication with the
jwt_authnHTTP filter now permits header syntax like the following: