Methods for create and call context specific tenanted keystores #6189

Thumimku · 2024-12-10T04:44:38Z

Proposed changes in this pull request

Public Issue: wso2/product-is#21985

The pull request titled "Create and Use Context KeyStore" introduces a method, generateContextKeyStore, designed to generate a context-specific KeyStore for a given tenant domain. This method ensures that each tenant has a unique KeyStore associated with a specific context, enhancing security and isolation between tenants.

Key Features of the Implementation:

Tenant Identification: Retrieves the tenant ID using IdentityTenantUtil.getTenantId(tenantDomain) to uniquely identify the tenant.
KeyStore Manager Initialization: Initializes the KeyStoreManager instance for the specific tenant, facilitating KeyStore operations.
Registry Initialization: Calls IdentityTenantUtil.initializeRegistry(tenantId) to set up the tenant's registry space, preparing the environment for KeyStore operations.
Existence Check: Before proceeding with creation, checks if a KeyStore already exists for the given context using isContextKeyStoreExists(context). If it exists, the method exits early to prevent duplication.
KeyStore Creation: If the KeyStore does not exist:
- Generates a secure password.
- Obtains a KeyStore instance appropriate for the tenant's domain.
- Loads the KeyStore with the generated password.
- Generates a context-specific key pair.
- Persists the KeyStore for future use.
Exception Handling: Catches exceptions during the process, logs detailed error messages, and throws a KeyStoreManagementException to signal issues in KeyStore creation.

Hi all,
I tested the following scenarios.
For Super Tenant and Tenant with an Existing Context Keystore:

Call setCookie normally without any issues.
Call getCookie after creation to validate the cookie.
Call setCookie after a restart to ensure the keystore loads correctly.
Call getCookie after a restart to validate the cookie again.

For a Tenant with No Context Keystore:

Call getCookie before creation to ensure that validation fails without throwing exception.
Call setCookie normally to trigger keystore creation.
Call getCookie after creation to validate the cookie.
Restart the pack to check keystone loaded and invalidate caches and do above three steps again.

Edge Cases:
Expired Cookies: Tested scenarios where a cookie generated by the super tenant exists without a valid session (since the cookie is set for six months, this scenario is possible). In this case, validation should fail gracefully without throwing an error. When the setCookie method is called after this, the MFA context keystore is created, and a new cookie is set.
-Subsequent Requests: Validated that subsequent requests pass the cookie validation successfully.

Todo:

Unit tests
Unit Test Coverage for Created Class

codecov · 2024-12-10T05:14:27Z

Codecov Report

Attention: Patch coverage is 64.41718% with 58 lines in your changes missing coverage. Please review.

Project coverage is 40.84%. Comparing base (c39855e) to head (d8e9659).
Report is 67 commits behind head on master.

Files with missing lines	Patch %	Lines
...carbon/identity/core/IdentityKeyStoreResolver.java	3.44%	27 Missing and 1 partial ⚠️
...eystore/service/IdentityKeyStoreGeneratorImpl.java	80.20%	14 Missing and 5 partials ⚠️
...g/wso2/carbon/identity/core/util/IdentityUtil.java	77.41%	5 Missing and 2 partials ⚠️
...entity/core/util/IdentityKeyStoreResolverUtil.java	40.00%	2 Missing and 1 partial ⚠️
...security/internal/SecurityMgtServiceComponent.java	0.00%	1 Missing ⚠️

Additional details and impacted files

@@             Coverage Diff              @@
##             master    #6189      +/-   ##
============================================
- Coverage     41.03%   40.84%   -0.19%     
- Complexity    15398    15526     +128     
============================================
  Files          1796     1797       +1     
  Lines        122999   124792    +1793     
  Branches      21403    22002     +599     
============================================
+ Hits          50475    50975     +500     
- Misses        65034    66274    +1240     
- Partials       7490     7543      +53

Flag	Coverage Δ
unit	`27.77% <64.41%> (+0.10%)`	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

...rbon.identity.core/src/main/java/org/wso2/carbon/identity/core/IdentityKeyStoreResolver.java

...tity.core/src/main/java/org/wso2/carbon/identity/core/util/IdentityKeyStoreResolverUtil.java

...t/src/main/java/org/wso2/carbon/security/keystore/service/IdentityKeyStoreGeneratorImpl.java

janakamarasena · 2024-12-10T05:34:01Z

...t/src/main/java/org/wso2/carbon/security/keystore/service/IdentityKeyStoreGeneratorImpl.java

+     * @param context      the context for which the KeyStore is to be generated.
+     * @throws KeyStoreManagementException if an error occurs during KeyStore creation or initialization.
+     */
+    public void generateContextKeyStore(String tenantDomain, String context) throws KeyStoreManagementException {


Can't we reuse an already available keystore generator from the kernel?

We dont have a keystore generation method in the kernel, it is currently in multitenancy repo.

Multi tenancy has key store generation logic once this fix is merged, we can improve further so that multi tenancy will call this logic to create tenanted keystore

...t/src/main/java/org/wso2/carbon/security/keystore/service/IdentityKeyStoreGeneratorImpl.java

...rbon.identity.core/src/main/java/org/wso2/carbon/identity/core/IdentityKeyStoreResolver.java

...t/src/main/java/org/wso2/carbon/security/keystore/service/IdentityKeyStoreGeneratorImpl.java

jenkins-is-staging · 2024-12-10T11:15:30Z

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

...rbon.identity.core/src/main/java/org/wso2/carbon/identity/core/IdentityKeyStoreResolver.java

...t/src/main/java/org/wso2/carbon/security/keystore/service/IdentityKeyStoreGeneratorImpl.java

jenkins-is-staging · 2024-12-10T13:11:36Z

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

jenkins-is-staging

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

...t/src/main/java/org/wso2/carbon/security/keystore/service/IdentityKeyStoreGeneratorImpl.java

jenkins-is-staging · 2024-12-11T12:53:50Z

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

jenkins-is-staging · 2024-12-11T12:55:24Z

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

jenkins-is-staging · 2024-12-11T17:28:19Z

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

jenkins-is-staging · 2024-12-11T17:29:47Z

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

Thumimku · 2024-12-11T17:49:04Z

The reason for above two failures are I am adding a jks file for unit tests and patch diff is failing.

error: cannot apply binary patch to 'components/security-mgt/org.wso2.carbon.security.mgt/src/test/resources/repository/resources/security/carbon-super--cookie.jks' without full index line
error: components/security-mgt/org.wso2.carbon.security.mgt/src/test/resources/repository/resources/security/carbon-super--cookie.jks: patch does not apply
Applying diff failed. Exiting...
Error: Applying diff failed.

jenkins-is-staging · 2024-12-11T17:52:15Z

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

jenkins-is-staging · 2024-12-11T20:10:34Z

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

jenkins-is-staging

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

sonarcloud · 2024-12-12T05:03:29Z

Quality Gate passed

Issues
9 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

hwupathum · 2024-12-12T15:09:16Z

...rbon.identity.core/src/main/java/org/wso2/carbon/identity/core/IdentityKeyStoreResolver.java

+     * @param context the context
+     * @return a concatenated string in the format tenantDomain:context
+     */
+    private String buildDomainWithContext(int tenantId, String context) {


Shall we rename the method to avoid confusion with buildDomainWithContext(int tenantId, String context) from IdentityKeyStoreResolverUtil?

hwupathum · 2024-12-13T06:37:06Z

...t/src/main/java/org/wso2/carbon/security/keystore/service/IdentityKeyStoreGeneratorImpl.java

+import java.security.cert.X509Certificate;
+import java.util.Date;
+
+import static org.wso2.carbon.security.SecurityConstants.KeyStoreMgtConstants.KEY_STORE_CONTEXT_SEPARATOR;


We can use

import static org.wso2.carbon.identity.core.util.IdentityKeyStoreResolverConstants.KEY_STORE_CONTEXT_SEPARATOR

instead

Thumimku added 2 commits December 10, 2024 08:36

methods for create and call context specific tenanted keystores

f8e8c26

update doc

dd985fb