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.

Sign up for GitHub

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

Jump to bottom

JWT Authenticator supports public-keys variable #2463

Merged
merged 1 commit into from
Jan 16, 2022
Merged

JWT Authenticator supports public-keys variable #2463

merged 1 commit into from
Jan 16, 2022

Conversation

sashaCher
Copy link
Contributor

@sashaCher sashaCher commented Jan 13, 2022 •
edited
Loading

Desired Outcome

Enable JWT authenticator to receive static signing keys via public-keys variable.

Implemented Changes

  • CreateSigningKeyProvider creates and returns FetchPublicKeysSigningKey class
  • Enable relevant integration tests

Connected Issue/Story

ONYX-15388

Definition of Done

  • Desired outcome is achieved
  • No integration tests are broken

Changelog

  • The CHANGELOG has been updated, or
  • This PR does not include user-facing changes and doesn't require a
    CHANGELOG update

Test coverage

  • This PR includes new unit and integration tests to go with the code
    changes, or
  • The changes in this PR do not require tests

Documentation

  • Docs (e.g. READMEs) were updated in this PR
  • A follow-up issue to update official docs has been filed here: insert issue ID
  • This PR does not require updating any documentation

Behavior

  • This PR changes product behavior and has been reviewed by a PO, or
  • These changes are part of a larger initiative that will be reviewed later, or
  • No behavior was changed with this PR

Security

  • Security architect has reviewed the changes in this PR,
  • These changes are part of a larger initiative with a separate security review, or
  • There are no security aspects to these changes

@sashaCher sashaCher requested a review from a team January 13, 2022 19:55
tzheleznyak
tzheleznyak previously approved these changes Jan 14, 2022
View reviewed changes
Copy link
Contributor

@tzheleznyak tzheleznyak left a comment

Choose a reason for hiding this comment

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

LGTM

@sashaCher sashaCher force-pushed the authn-jwt-all-signing-key-settings branch from 54d0c93 to 3d446df Compare January 14, 2022 20:32
@sashaCher sashaCher force-pushed the authn-jwt-all-signing-key-settings branch from 3d446df to ba79c6f Compare January 14, 2022 20:51
@sashaCher sashaCher force-pushed the authn-jwt-all-signing-key-settings branch from ba79c6f to 5fabe9a Compare January 16, 2022 09:36
Base automatically changed from authn-jwt-all-signing-key-settings to master January 16, 2022 10:14
@sashaCher sashaCher marked this pull request as ready for review January 16, 2022 10:19
@sashaCher sashaCher requested a review from a team as a code owner January 16, 2022 10:19
tzheleznyak
tzheleznyak previously approved these changes Jan 16, 2022
View reviewed changes
Copy link
Contributor

@tzheleznyak tzheleznyak left a comment

Choose a reason for hiding this comment

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

LGTM

codeclimate[bot]
codeclimate bot reviewed Jan 16, 2022
View reviewed changes
CHANGELOG.md
@@ -6,6 +6,18 @@ and this project adheres to [Semantic Versioning](http://semver.org/spec/v2.0.0.

## [Unreleased]

### Added
- Added an ability to JWT generic vendor configuration to receive signing keys for
Copy link

Choose a reason for hiding this comment

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

Lists should be surrounded by blank lines

@sashaCher
Integrate public-keys variable into full JWT Authentication flow
2a2354d 
When Conjur is unable to reach remote JWKS URI endpoint public-keys variable can be used in order to provide static JWK Set to the authenticator.
Variable value is a JSON object contains two fields: type and value. The type filed should be set to "jwks" string and the value field should be set to a relevant JWKS set.
tzheleznyak
tzheleznyak approved these changes Jan 16, 2022
View reviewed changes
Copy link
Contributor

@tzheleznyak tzheleznyak left a comment

Choose a reason for hiding this comment

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

LGTM

@codeclimate
Copy link

codeclimate bot commented Jan 16, 2022

Code Climate has analyzed commit 2a2354d and detected 1 issue on this pull request.

Here's the issue category breakdown:

Category Count
Style 1

The test coverage on the diff in this pull request is 100.0% (50% is the threshold).

This pull request will bring the total coverage in the repository to 91.2% (0.0% change).

View more on Code Climate.

@sashaCher sashaCher merged commit b8f2a97 into master Jan 16, 2022
@sashaCher sashaCher deleted the integrate-public-keys-flow branch January 16, 2022 11:47
@jtuttle jtuttle mentioned this pull request Jul 12, 2022
Merged
@gl-johnson gl-johnson mentioned this pull request Aug 23, 2022
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@codeclimate CodeClimate codeclimate left review comments

@tzheleznyak tzheleznyak tzheleznyak approved these changes

Assignees
No one assigned
Labels
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@sashaCher @tzheleznyak