Skip to content

[2.x] Fix permission issue when creating JWTParser after jjwt upgrade#3189

Merged
stephen-crawford merged 1 commit intoopensearch-project:2.xfrom
cwperks:fix-jwt-integ-tests
Aug 16, 2023
Merged

[2.x] Fix permission issue when creating JWTParser after jjwt upgrade#3189
stephen-crawford merged 1 commit intoopensearch-project:2.xfrom
cwperks:fix-jwt-integ-tests

Conversation

@cwperks
Copy link
Copy Markdown
Member

@cwperks cwperks commented Aug 16, 2023

Description

After the upgrade of JJWT from #3092, the JWT Integration Tests in security-dashboards-plugin started failing. See details on: opensearch-project/security-dashboards-plugin#1555

The output from the tests in Github actions was not very revealing, but @RyanL1997 pointed out that when running the test locally after creating a local distro of core + security plugin with 2.x it would output a permissions error:

Caused by: java.security.AccessControlException: access denied ("java.lang.RuntimePermission" "getClassLoader")
	at java.security.AccessControlContext.checkPermission(AccessControlContext.java:485) ~[?:?]
	at java.security.AccessController.checkPermission(AccessController.java:1068) ~[?:?]
	at java.lang.SecurityManager.checkPermission(SecurityManager.java:416) ~[?:?]
	at java.lang.ClassLoader.checkClassLoaderPermission(ClassLoader.java:2060) ~[?:?]
	at java.lang.Thread.getContextClassLoader(Thread.java:1493) ~[?:?]
	at io.jsonwebtoken.impl.lang.Services$1.getClassLoader(Services.java:37) ~[jjwt-impl-0.11.5.jar:0.11.5]
	at io.jsonwebtoken.impl.lang.Services.loadFirst(Services.java:100) ~[jjwt-impl-0.11.5.jar:0.11.5]
	at io.jsonwebtoken.impl.DefaultJwtParserBuilder.build(DefaultJwtParserBuilder.java:191) ~[jjwt-impl-0.11.5.jar:0.11.5]
	at com.amazon.dlic.auth.http.jwt.HTTPJwtAuthenticator.<init>(HTTPJwtAuthenticator.java:135) ~[opensearch-security-2.10.0.0-SNAPSHOT.jar:2.10.0.0-SNAPSHOT]
        ...

This PR surrounds the call to build the parser with AccessController.doPrivileged(...) to ensure the call has the privileges needed to create the parser.

This change should also be applied to main. For some reason, the tests are not failing when running on the main branch but are on 2.x.

  • Category (Enhancement, New feature, Bug fix, Test fix, Refactoring, Maintenance, Documentation)

Bug fix

Issues Resolved

opensearch-project/security-dashboards-plugin#1555

Check List

  • New functionality includes testing
  • New functionality has been documented
  • 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.

Signed-off-by: Craig Perkins <craig5008@gmail.com>
@codecov
Copy link
Copy Markdown

codecov bot commented Aug 16, 2023

Codecov Report

Merging #3189 (93958fe) into 2.x (ee81de7) will decrease coverage by 0.06%.
The diff coverage is 81.81%.

@@             Coverage Diff              @@
##                2.x    #3189      +/-   ##
============================================
- Coverage     62.22%   62.17%   -0.06%     
+ Complexity     3312     3308       -4     
============================================
  Files           265      265              
  Lines         19502    19507       +5     
  Branches       3328     3329       +1     
============================================
- Hits          12136    12129       -7     
- Misses         5741     5747       +6     
- Partials       1625     1631       +6     
Files Changed Coverage Δ
...mazon/dlic/auth/http/jwt/HTTPJwtAuthenticator.java 80.99% <81.81%> (-3.50%) ⬇️

... and 3 files with indirect coverage changes

@cwperks
Copy link
Copy Markdown
Member Author

cwperks commented Aug 16, 2023

This works on main w/o the same fix and I am trying to determine why.

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

Successfully merging this pull request may close these issues.

3 participants