Skip to content

Java: Promote Hardcoded JWT credential query #12032

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

Conversation

egregius313
Copy link
Contributor

@egregius313 egregius313 commented Jan 30, 2023

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 Java: 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
Copy link
Contributor

@jcogs33 jcogs33 left a comment

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?

@github-actions
Copy link
Contributor

QHelp previews:

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

@atorralba atorralba left a comment

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
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
Copy link
Contributor

@atorralba atorralba left a comment

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
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants