Skip to content

Validate OIDC identity provider public/private keys#11612

Merged
mitchellhenke merged 1 commit intomainfrom
mitchellhenke/validate-openid-connect-certs
Dec 10, 2024
Merged

Validate OIDC identity provider public/private keys#11612
mitchellhenke merged 1 commit intomainfrom
mitchellhenke/validate-openid-connect-certs

Conversation

@mitchellhenke
Copy link
Copy Markdown
Contributor

As part of work to allow zero down-time rotation of OIDC keys, this is an initial step to validate that the keys are minimally correct before we potentially add more keys into the mix. It's brief, but ensures that the private key can sign data that is verifiable by the public key.

@mitchellhenke mitchellhenke requested review from a team and Sgtpluck December 9, 2024 22:43
@mitchellhenke mitchellhenke force-pushed the mitchellhenke/validate-openid-connect-certs branch from 3dc24e5 to 4e18441 Compare December 9, 2024 22:47
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.

What if we provided a default data generator? the bit in app_artifacts.rb seems arbitrary and isn't clear what it's used for

Suggested change
def self.valid?(private_key:, public_key:, data:)
def self.valid?(private_key:, public_key:, data: SecureRandom.hex)

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

yep, makes sense

Copy link
Copy Markdown
Contributor

@Sgtpluck Sgtpluck left a comment

Choose a reason for hiding this comment

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

agree with zach's suggestion, but looks good

@mitchellhenke mitchellhenke force-pushed the mitchellhenke/validate-openid-connect-certs branch from 4e18441 to cdf3f70 Compare December 10, 2024 14:07
changelog: Internal, OpenID Connect, Validate identity provider public/private keys

Co-authored-by: Zach Margolis <zachmargolis@users.noreply.github.com>
@mitchellhenke mitchellhenke force-pushed the mitchellhenke/validate-openid-connect-certs branch from cdf3f70 to 89dfd05 Compare December 10, 2024 14:26
@mitchellhenke mitchellhenke merged commit 5a224c8 into main Dec 10, 2024
@mitchellhenke mitchellhenke deleted the mitchellhenke/validate-openid-connect-certs branch December 10, 2024 15:12
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.

3 participants