Skip to content
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

Allow JWT from string #1021

Merged
merged 1 commit into from
Jan 15, 2021
Merged

Allow JWT from string #1021

merged 1 commit into from
Jan 15, 2021

Conversation

bitwiseman
Copy link
Member

Description

  • Push your changes to a branch other than master. Create your PR from that branch.
  • Add JavaDocs and other comments
  • Write tests that run and pass in CI. See CONTRIBUTING.md for details on how to capture snapshot data.
  • Run mvn clean compile locally. This may reformat your code, commit those changes.
  • Run mvn -D enable-ci clean install site locally. If this command doesn't succeed, your change will not pass CI.

When creating a PR:

  • Fill in the "Description" above.
  • Enable "Allow edits from maintainers".

@bitwiseman bitwiseman requested a review from timja January 14, 2021 22:27
@@ -42,11 +42,15 @@
private final String applicationId;

public JWTTokenProvider(String applicationId, File keyFile) throws GeneralSecurityException, IOException {
this(applicationId, loadPrivateKey(keyFile.toPath()));
this(applicationId, keyFile.toPath());
Copy link
Member Author

Choose a reason for hiding this comment

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

Refactoring to get a single code path through all the constructors.

@@ -34,9 +36,12 @@
JWT_PROVIDER_1 = new JWTTokenProvider(TEST_APP_ID_1,
new File(this.getClass().getResource(PRIVATE_KEY_FILE_APP_1).getFile()));
JWT_PROVIDER_2 = new JWTTokenProvider(TEST_APP_ID_2,
new File(this.getClass().getResource(PRIVATE_KEY_FILE_APP_2).getFile()));
new File(this.getClass().getResource(PRIVATE_KEY_FILE_APP_2).getFile()).toPath());
Copy link
Member Author

Choose a reason for hiding this comment

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

Modified tests to make sure all constructors are tested.

@bitwiseman bitwiseman closed this Jan 15, 2021
@bitwiseman bitwiseman reopened this Jan 15, 2021
@bitwiseman bitwiseman merged commit 80124e3 into hub4j:master Jan 15, 2021
@bitwiseman bitwiseman deleted the jwt-string branch January 15, 2021 04:09
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.

1 participant