-
Notifications
You must be signed in to change notification settings - Fork 43
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
Decouple crypto engine from encryption algorithm #3293
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
✅ No Invisible Unicode Characters Detected.
Fixes #3292 Previously the crypto engine was coupled to the AES-CFB algorithm. This PR introduces a new EncryptionAlgorithm interface which represents different cryptographic algorithms. The Engine is now instantiated with an instance of this algorithm. Future PRs will make it possible to support more than one algorithm, and allow the selection of available algorithms to be driven through configuration.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
✅ No Invisible Unicode Characters Detected.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
✅ No Mixed Scripts Detected.
@@ -0,0 +1,61 @@ | |||
// Copyright 2024 Stacklok, Inc. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This code was previously in the same file as the crypto engine. Since it is unrelated, move to its own file.
|
||
// Engine provides all functions to encrypt and decrypt data | ||
type Engine interface { | ||
EncryptOAuthToken(token *oauth2.Token) (string, error) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Previously this method required callers to marshal the token to JSON, and then base64 the result of the encryption method. This logic has been moved inside this method which simplifies its use.
|
||
// AlgorithmTypeFromString converts a string to an EncryptionAlgorithmType | ||
// or returns errUnknownAlgorithm. | ||
func AlgorithmTypeFromString(input string) (EncryptionAlgorithmType, error) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Some of this code is not used yet, but will be used in the next PR.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
✅ No Invisible Unicode Characters Detected.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Relates to #3292
Previously the crypto engine was coupled to the AES-CFB algorithm. This PR introduces a new EncryptionAlgorithm interface which represents different cryptographic algorithms. The Engine is now instantiated with an instance of this algorithm.
Future PRs will make it possible to support more than one algorithm, and allow the selection of available algorithms to be driven through configuration.
Summary
Provide a brief overview of the changes and the issue being addressed.
Explain the rationale and any background necessary for understanding the changes.
List dependencies required by this change, if any.
Fixes #(related issue)
Change Type
Mark the type of change your PR introduces:
Testing
Outline how the changes were tested, including steps to reproduce and any relevant configurations.
Attach screenshots if helpful.
Review Checklist: