Skip to content

Java: Promote Hardcoded JWT credential query - #12032

Merged
egregius313 merged 7 commits into
github:mainfrom
egregius313:egregius313/promote-hardcoded-jwt-credential
Feb 27, 2023
Merged

Java: Promote Hardcoded JWT credential query#12032
egregius313 merged 7 commits into
github:mainfrom
egregius313:egregius313/promote-hardcoded-jwt-credential

Conversation

@egregius313

@egregius313 egregius313 commented Jan 30, 2023

Copy link
Copy Markdown
Contributor

Query promotion for the experimental java/hardcoded-jwt-key by integrating it into java/hardcoded-credential-api-call.

Promotion of #9036.

@github-actions github-actions Bot added the Java label Jan 30, 2023
@owen-mc owen-mc changed the title Promote Hardcoded JWT credential query Jan 31, 2023
@egregius313
egregius313 marked this pull request as ready for review February 15, 2023 17:27
@egregius313
egregius313 requested a review from a team as a code owner February 15, 2023 17:27

@jcogs33 jcogs33 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hey Ed, I’ve added a few comments/questions below, but I’ll leave a full review and approval to someone else. 🙂

  • This will probably need a change note under lib for the new sinks. (FYI, it’s standard to mention the original author of the experimental query in the change note when doing a promotion, see this discussion)
  • Where is the java/ql/src/Security/CWE/CWE-798/HardcodedJwtKey.java file being used? Was this meant to be added as an example in HardcodedCredentialsApiCall.qhelp?
  • Would it make sense to include test cases for all of the new sinks? e.g. HMAC384 and HMAC512 as well?
Comment thread java/ql/test/experimental/query-tests/security/CWE-321/HardcodedJwtKey.java Outdated
@github-actions

Copy link
Copy Markdown
Contributor

QHelp previews:

@egregius313
egregius313 force-pushed the egregius313/promote-hardcoded-jwt-credential branch from bf409a7 to b259088 Compare February 17, 2023 15:24

@atorralba atorralba left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The HardcodedJwtKey.java file under Security/CWE/CWE-798 isn't doing much unless you reference it in the QHelp. So I'd recommend doing that, or just removing it.

But otherwise this looks good to me! 👍

@egregius313

Copy link
Copy Markdown
Contributor Author

The HardcodedJwtKey.java file under Security/CWE/CWE-798 isn't doing much unless you reference it in the QHelp. So I'd recommend doing that, or just removing it.

Ok I have removed the unneeded file.

@atorralba atorralba added ready-for-doc-review This PR requires and is ready for review from the GitHub docs team. and removed ready-for-doc-review This PR requires and is ready for review from the GitHub docs team. labels Feb 21, 2023
The HMAC* constructors of the com.auth0.jwt.algorithm.Algorithm class
take a secret as a parameter. Therefore, the arguments should be added
to be checked for hardcoded credentials.
@atorralba
atorralba force-pushed the egregius313/promote-hardcoded-jwt-credential branch from d1e2756 to ed1aac1 Compare February 27, 2023 11:16

@atorralba atorralba left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

DCA looks good! 👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

3 participants