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

Introduce KeyStoreDAO and database based keystore persistence manager implementation #4130

Draft
wants to merge 2 commits into
base: 4.10.x
Choose a base branch
from

Conversation

UdeshAthukorala
Copy link

@UdeshAthukorala UdeshAthukorala commented Dec 4, 2024

Purpose

  • Introduce KeyStoreDAO and database based keystore persistence manager implementation.
  • Introduce a config to select keystore persistence manager(weather keystore data stores in database or registry)
[keystores]
data_storage_type = "database"

Part of: wso2/product-is#21206

Details

This pull request introduces several significant changes to the codebase, primarily focusing on enhancing the key store management functionalities by adding a new persistence manager that utilizes a database for storing key stores. The most important changes include updating dependencies, modifying the CarbonCoreDataHolder class, adding a new JDBCKeyStorePersistenceManager class, and creating a factory for the key store persistence manager.

Enhancements to key store management:

Updates to CarbonCoreDataHolder:

New JDBCKeyStorePersistenceManager class:

Creation of a factory for key store persistence manager:

Addition of constants for persistence manager:

@jenkins-is-staging
Copy link

PR builder started
Link: https://github.com/wso2/product-is/actions/runs/12159509316

@jenkins-is-staging
Copy link

PR builder completed
Link: https://github.com/wso2/product-is/actions/runs/12159509316
Status: success

Copy link

@jenkins-is-staging jenkins-is-staging left a comment

Choose a reason for hiding this comment

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

Approving the pull request based on the successful pr build https://github.com/wso2/product-is/actions/runs/12159509316

private static final KeyStoreDAO keyStoreDAO = new KeyStoreDAO();

/**
* Add the key store to the database.
Copy link
Contributor

Choose a reason for hiding this comment

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

Lets avoid duplicating method comments

import java.util.List;
import java.util.Optional;

public class JDBCKeyStorePersistenceManager implements KeyStorePersistenceManager {
Copy link
Contributor

Choose a reason for hiding this comment

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

Lets add a class comment

@@ -458,6 +458,10 @@
<!-- Keystore type (JKS/PKCS12 etc.)-->
<Type>PKCS12</Type>
</TenantKeyStore>
<KeyStores>
<!-- Keystore file location-->
Copy link
Contributor

Choose a reason for hiding this comment

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

<!-- Keystore file location--> -> <!-- Keystore storage type-->

CREATE TABLE KEY_STORE (
ID INTEGER DEFAULT NEXTVAL('KEY_STORE_PK_SEQ'),
NAME VARCHAR(255) NOT NULL,
TYPE VARCHAR(255),
Copy link
Contributor

Choose a reason for hiding this comment

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

Do we need 255 characters here?

NAME VARCHAR(255) NOT NULL,
TYPE VARCHAR(255),
PROVIDER VARCHAR(255),
PASSWORD VARCHAR(1000),
Copy link
Contributor

Choose a reason for hiding this comment

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

Length of the password going to be depend on the key size.

return defaultPolicyPersistenceManager;
}
}
return defaultPolicyPersistenceManager;
Copy link
Contributor

Choose a reason for hiding this comment

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

Add a (debug) log..

public static final String CONTENT = "CONTENT";
public static final String TENANT_ID = "TENANT_ID";
public static final String PUB_CERT_ID = "PUB_CERT_ID";
public static final String CREATED_AT = "CREATED_AT";
Copy link
Contributor

Choose a reason for hiding this comment

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

Lets use the VERSION -> CREATED_AT -> UPDATED_AT order for the consistency.

}

public static final String ADD_KEY_STORE = "INSERT INTO KEY_STORE (NAME, TYPE, PROVIDER, PASSWORD, " +
"PRIVATE_KEY_ALIAS, PRIVATE_KEY_PASS, CONTENT, PUB_CERT_ID, TENANT_ID, CREATED_AT, UPDATED_AT," +
Copy link
Contributor

Choose a reason for hiding this comment

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

Lets use the VERSION -> CREATED_AT -> UPDATED_AT order for the consistency.

"VERSION) VALUES (:NAME;, :TYPE;, :PROVIDER;, :PASSWORD;, :PRIVATE_KEY_ALIAS;, :PRIVATE_KEY_PASS;, " +
":CONTENT;, :PUB_CERT_ID;, :TENANT_ID;, :CREATED_AT;, :UPDATED_AT;, :VERSION;);";
public static final String GET_KEY_STORE_BY_NAME =
"SELECT * FROM KEY_STORE WHERE NAME = :NAME; AND TENANT_ID = :TENANT_ID;";
Copy link
Contributor

Choose a reason for hiding this comment

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

Lets avoid using select *

*/
public void addKeystore(KeyStoreModel keyStoreModel) {

try (Connection connection = DatabaseUtil.getDBConnection(dataSource)) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Lets use NamedJdbcTemplate

@UdeshAthukorala UdeshAthukorala marked this pull request as draft December 16, 2024 05:04
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