-
Notifications
You must be signed in to change notification settings - Fork 280
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
[Feature/Extension] Add integration test case for OBO hostmapping #3270
[Feature/Extension] Add integration test case for OBO hostmapping #3270
Conversation
Signed-off-by: Ryan Liang <jiallian@amazon.com>
Codecov Report
@@ Coverage Diff @@
## main #3270 +/- ##
============================================
+ Coverage 63.15% 64.14% +0.98%
- Complexity 3448 3470 +22
============================================
Files 263 263
Lines 20024 20112 +88
Branches 3341 3359 +18
============================================
+ Hits 12647 12901 +254
+ Misses 5748 5532 -216
- Partials 1629 1679 +50 |
String[] splitToken = oboToken.split("\\."); | ||
|
||
String unsignedToken = splitToken[0] + "." + splitToken[1] + "."; | ||
Claims claims = Jwts.parserBuilder().build().parseClaimsJwt(unsignedToken).getBody(); |
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.
Is splitting the token necessary for calling parseClaimsJwt
?
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.
Yes, otherwise it won't recognize the verify signature part of the token, and throw an error:
Signed JWSs are not supported.
io.jsonwebtoken.UnsupportedJwtException: Signed JWSs are not supported.
...
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.
A couple lines down you are creating an instance of EncryptionDecryptionUtil
with the expected encryption key. Can you do something similar with the JWTParser to avoid having to split the token?
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.
That's a good point. I just updated.
src/integrationTest/java/org/opensearch/security/http/OnBehalfOfJwtAuthenticationTest.java
Outdated
Show resolved
Hide resolved
Signed-off-by: Ryan Liang <jiallian@amazon.com>
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.
Thank you @RyanL1997. This PR looks good to me. I left 2 comments to be addressed and will approve this PR once addressed or responded to.
src/integrationTest/java/org/opensearch/security/http/OnBehalfOfJwtAuthenticationTest.java
Outdated
Show resolved
Hide resolved
String[] splitToken = oboToken.split("\\."); | ||
|
||
String unsignedToken = splitToken[0] + "." + splitToken[1] + "."; | ||
Claims claims = Jwts.parserBuilder().build().parseClaimsJwt(unsignedToken).getBody(); |
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.
A couple lines down you are creating an instance of EncryptionDecryptionUtil
with the expected encryption key. Can you do something similar with the JWTParser to avoid having to split the token?
Signed-off-by: Ryan Liang <jiallian@amazon.com>
…ensearch-project#3270) Add integration test case for OBO hostmapping * Category (Enhancement, New feature, Bug fix, Test fix, Refactoring, Maintenance, Documentation) Test Enhancement * Resolve opensearch-project#3222 - [x] New functionality includes testing - [ ] New functionality has been documented - [x] Commits are signed per the DCO using --signoff By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license. For more information on following Developer Certificate of Origin and signing off your commits, please check [here](https://github.com/opensearch-project/OpenSearch/blob/main/CONTRIBUTING.md#developer-certificate-of-origin). --------- Signed-off-by: Ryan Liang <jiallian@amazon.com>
Description
Add integration test case for OBO hostmapping
Test Enhancement
Issues Resolved
Check List
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
For more information on following Developer Certificate of Origin and signing off your commits, please check here.