-
Notifications
You must be signed in to change notification settings - Fork 454
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
feat(plugin): implement golang version of plugin jwt-auth #743
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #743 +/- ##
==========================================
- Coverage 38.27% 38.24% -0.04%
==========================================
Files 60 60
Lines 9979 9979
==========================================
- Hits 3819 3816 -3
- Misses 5869 5871 +2
- Partials 291 292 +1 |
45ad2f7
to
bbacf81
Compare
@Ink-33 Is this draft PR ready for review, or is there still unfinished work? |
Yes it's ready for review and I will update the branch later today. |
@Ink-33 Please add an e2e test for it. |
@Ink-33 Please add e2e test cases, refer to: https://github.com/alibaba/higress/pull/759/files#diff-00090f4cf6417f2566ebc91e56b6b5d0e12a834a184f4b814d51bc89581d796b |
got it. I would like to add three parts of test cases including normal, rejected, and normal with all features one by one. btw, how to run e2e test case on my local dev environments? |
Please refer this guide: https://github.com/alibaba/higress/tree/main/test
|
Signed-off-by: Ink33 <Ink33@smlk.org>
Signed-off-by: Ink33 <Ink33@smlk.org>
Signed-off-by: Ink33 <Ink33@smlk.org>
Signed-off-by: Ink33 <Ink33@smlk.org>
Signed-off-by: Ink33 <Ink33@smlk.org>
v4 require go1.21 but we use go1.19 Signed-off-by: Ink33 <Ink33@smlk.org>
Signed-off-by: Ink33 <Ink33@smlk.org>
Signed-off-by: Ink33 <Ink33@smlk.org>
Signed-off-by: Ink33 <Ink33@smlk.org>
Signed-off-by: Ink33 <Ink33@smlk.org>
Signed-off-by: Ink33 <Ink33@smlk.org>
Signed-off-by: Ink33 <Ink33@smlk.org>
Signed-off-by: Ink33 <Ink33@smlk.org>
@johnlanni allow 里面配置多个 consumer 且全部验证失败应该返回哪个错误码给请求,或者直接用 verify failed 兜底呢? |
返回 403 即可,错误原因可以用 not allowed,跟jwt本身凭证不合法区分下 |
Signed-off-by: Ink33 <Ink33@smlk.org>
Signed-off-by: Ink33 <Ink33@smlk.org>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, Thanks!
Ⅰ. Describe what this PR did
This merge request contains the
jwt-auth
plugin implemented in golang.Ⅱ. Does this pull request fix one issue?
#232 , #591
Ⅲ. Why don't you add test cases (unit test/integration test)?
Test cases are still in progress due to the complexity.
Ⅳ. Describe how to verify it
As #589, all behaviors of this plugin should be same with the currently cpp verison of
jwt-auth
. Maybe we need to use plently of tese cases to verify it.Ⅴ. Special notes for reviews
Same with IV. Since I am not particularly familiar with cpp, I hope that reviewers will understand some errors and I will correct them in time🙏