Skip to content

Conversation

reza-ebrahimi
Copy link

@reza-ebrahimi reza-ebrahimi commented May 27, 2023

Add private key validation check for multi-line string content specifically when set using environment variables as multi-line string.

Discussion (Thanks to @gr2m ): gr2m/universal-github-app-jwt#71

Resolves #465 #71

Behavior

Before the change?

Failing due to passing multiline string to environment variables, See #465 (comment)

Pull request checklist

  • Tests for the changes have been added (for bug fixes / features)
  • Docs have been reviewed and added / updated if needed (for bug fixes / features)
  • Added the appropriate label for the given change

Does this introduce a breaking change?

Please see our docs on breaking changes to help!

  • Yes (Please add the Type: Breaking change label)
  • No

If Yes, what's the impact:

  • N/A

Pull request type

Please add the corresponding label for change this PR introduces:

  • Bugfix: Type: Bug
  • Feature/model/API additions: Type: Feature
  • Updates to docs or samples: Type: Documentation
  • Dependencies/code cleanup: Type: Maintenance

@reza-ebrahimi
Copy link
Author

There is already a test for this case: https://github.com/reza-ebrahimi/auth-app.js/blob/pk_check_multiline_string/test/index.test.ts#L105

test("throws if incomplete Private Key is provided", async () => {
  const auth = createAppAuth({
    appId: APP_ID,
    privateKey: "-----BEGIN RSA PRIVATE KEY-----",
  });

  await expect(auth({ type: "app" })).rejects.toEqual(
    new Error(
      "The 'privateKey` option contains only the first line '-----BEGIN RSA PRIVATE KEY-----'. If you are setting it using a `.env` file, make sure it is set on a single line with newlines replaced by '\n'"
    )
  );
});

@wolfy1339 wolfy1339 added the Type: Bug Something isn't working as documented, or is being fixed label May 27, 2023
@reza-ebrahimi
Copy link
Author

Change the check logic from regex to a multi-step check function since the regex doesn't support some edge cases like -----BEGIN RSA PRIVATE KEY-----.

@gr2m
Copy link
Contributor

gr2m commented May 27, 2023

There is already a test for this case

Ha, what do you know 🤣

I think we should

  1. Implement this fix in https://github.com/gr2m/universal-github-app-jwt
  2. Remove the special error handling from this library

Do you see any reason not to do it this way?

@kfcampbell
Copy link
Member

@reza-ebrahimi I'd like to bring this up again. Do you have the inclination to make the fix in gr2m/universal-github-app-jwt?

@reza-ebrahimi
Copy link
Author

@kfcampbell I'm not working on this for a long time, feel free to push this code in gr2m/universal-github-app-jwt or close it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Type: Bug Something isn't working as documented, or is being fixed
Projects
Status: 🛑 Blocked/Awaiting Response
Development

Successfully merging this pull request may close these issues.

[BUG]: secretOrPrivateKey must be an asymmetric key when using RS256
4 participants