Add alert service to block tenants #7574
Draft
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
![wso2-engineering[bot]](https://avatars.githubusercontent.com/in/1856828?s=60&v=4){kind=small}
Comment on lines
+52
to
+62
| */ | ||
| public JSExecutionAlert(String tenantDomain, String serviceProvider, AlertType alertType, | ||
| long resourceValue, long threshold, String threadName) { | ||
|
|
||
| this.tenantDomain = tenantDomain; | ||
| this.serviceProvider = serviceProvider; | ||
| this.alertType = alertType; | ||
| this.resourceValue = resourceValue; | ||
| this.threshold = threshold; | ||
| this.threadName = threadName; | ||
| this.timestamp = System.currentTimeMillis(); |
Contributor
There was a problem hiding this comment.
Log Improvement Suggestion No: 1
Suggested change
| */ | |
| public JSExecutionAlert(String tenantDomain, String serviceProvider, AlertType alertType, | |
| long resourceValue, long threshold, String threadName) { | |
| this.tenantDomain = tenantDomain; | |
| this.serviceProvider = serviceProvider; | |
| this.alertType = alertType; | |
| this.resourceValue = resourceValue; | |
| this.threshold = threshold; | |
| this.threadName = threadName; | |
| this.timestamp = System.currentTimeMillis(); | |
| public JSExecutionAlert(String tenantDomain, String serviceProvider, AlertType alertType, | |
| long resourceValue, long threshold, String threadName) { | |
| if (log.isDebugEnabled()) { | |
| log.debug("Creating JS execution alert for tenant: " + tenantDomain + ", SP: " + serviceProvider + | |
| ", alert type: " + alertType + ", resource value: " + resourceValue + | |
| ", threshold: " + threshold); | |
| } | |
| this.tenantDomain = tenantDomain; | |
| this.serviceProvider = serviceProvider; | |
| this.alertType = alertType; | |
| this.resourceValue = resourceValue; | |
| this.threshold = threshold; | |
| this.threadName = threadName; | |
| this.timestamp = System.currentTimeMillis(); | |
| } |
Comment on lines
+81
to
+93
| * Create a new instance of JSExecutionEnforcer by loading configuration from the system. | ||
| * | ||
| * @return A new instance of JSExecutionEnforcer with loaded configuration. | ||
| */ | ||
| public static JSExecutionEnforcer createFromConfiguration() { | ||
|
|
||
| int maxViolationsPerTenant = loadMaxViolationsPerTenant(); | ||
| long violationWindowInMillis = loadViolationWindow(); | ||
| long blockDurationInMillis = loadBlockDuration(); | ||
| long criticalMemoryLimit = loadCriticalMemoryLimit(); | ||
|
|
||
| if (LOG.isDebugEnabled()) { | ||
| LOG.debug(String.format("Initialized JS Execution Alert Service with config: " + |
Contributor
There was a problem hiding this comment.
Log Improvement Suggestion No: 2
Suggested change
| * Create a new instance of JSExecutionEnforcer by loading configuration from the system. | |
| * | |
| * @return A new instance of JSExecutionEnforcer with loaded configuration. | |
| */ | |
| public static JSExecutionEnforcer createFromConfiguration() { | |
| int maxViolationsPerTenant = loadMaxViolationsPerTenant(); | |
| long violationWindowInMillis = loadViolationWindow(); | |
| long blockDurationInMillis = loadBlockDuration(); | |
| long criticalMemoryLimit = loadCriticalMemoryLimit(); | |
| if (LOG.isDebugEnabled()) { | |
| LOG.debug(String.format("Initialized JS Execution Alert Service with config: " + | |
| public static JSExecutionEnforcer createFromConfiguration() { | |
| int maxViolationsPerTenant = loadMaxViolationsPerTenant(); | |
| long violationWindowInMillis = loadViolationWindow(); | |
| long blockDurationInMillis = loadBlockDuration(); | |
| long criticalMemoryLimit = loadCriticalMemoryLimit(); | |
| LOG.info(String.format("Initializing JSExecutionEnforcer with configuration: maxViolationsPerTenant=%d, " + | |
| "violationWindow=%dms, blockDuration=%dms, criticalMemoryLimit=%d bytes", | |
| maxViolationsPerTenant, violationWindowInMillis, blockDurationInMillis, criticalMemoryLimit)); |
Comment on lines
+340
to
+341
| long expiryThreshold = currentTime - violationWindowInMillis; | ||
|
|
Contributor
There was a problem hiding this comment.
Log Improvement Suggestion No: 3
Suggested change
| long expiryThreshold = currentTime - violationWindowInMillis; | |
| private void cleanupExpiredAlerts() { | |
| if (LOG.isDebugEnabled()) { | |
| LOG.debug("Starting cleanup of expired alerts and blocks"); | |
| } |
Comment on lines
+102
to
+105
| public void setExecutionEnforcer(JSExecutionEnforcer executionEnforcer) { | ||
|
|
||
| this.executionEnforcer = executionEnforcer; | ||
| } |
Contributor
There was a problem hiding this comment.
Log Improvement Suggestion No: 4
Suggested change
| public void setExecutionEnforcer(JSExecutionEnforcer executionEnforcer) { | |
| this.executionEnforcer = executionEnforcer; | |
| } | |
| public void setExecutionEnforcer(JSExecutionEnforcer executionEnforcer) { | |
| this.executionEnforcer = executionEnforcer; | |
| if (log.isDebugEnabled()) { | |
| log.debug("JSExecutionEnforcer has been set for monitoring script executions."); | |
| } | |
| } |
Comment on lines
+317
to
+323
| // Push alert to the enforcer. | ||
| if (executionEnforcer != null) { | ||
| long threshold = (monitorType == MONITOR_TYPE_TIME) ? timeoutInMillis : memoryLimitInBytes; | ||
| JSExecutionAlert alert = new JSExecutionAlert(tenantDomain, serviceProvider, alertType, | ||
| consumedResourceValue, threshold, originalThread.getName()); | ||
| executionEnforcer.pushAlert(alert); | ||
| } |
Contributor
There was a problem hiding this comment.
Log Improvement Suggestion No: 5
Suggested change
| // Push alert to the enforcer. | |
| if (executionEnforcer != null) { | |
| long threshold = (monitorType == MONITOR_TYPE_TIME) ? timeoutInMillis : memoryLimitInBytes; | |
| JSExecutionAlert alert = new JSExecutionAlert(tenantDomain, serviceProvider, alertType, | |
| consumedResourceValue, threshold, originalThread.getName()); | |
| executionEnforcer.pushAlert(alert); | |
| } | |
| // Push alert to the enforcer. | |
| if (executionEnforcer != null) { | |
| long threshold = (monitorType == MONITOR_TYPE_TIME) ? timeoutInMillis : memoryLimitInBytes; | |
| JSExecutionAlert alert = new JSExecutionAlert(tenantDomain, serviceProvider, alertType, | |
| consumedResourceValue, threshold, originalThread.getName()); | |
| executionEnforcer.pushAlert(alert); | |
| if (log.isDebugEnabled()) { | |
| log.debug(String.format("Pushed %s alert for service provider: %s, tenant: %s", | |
| alertType, serviceProvider, tenantDomain)); | |
| } | |
| } |
Comment on lines
+81
to
+83
| * @param violationCount The new violation count. | ||
| */ | ||
| public void updateViolationCount(String tenantDomain, int violationCount) { |
Contributor
There was a problem hiding this comment.
Log Improvement Suggestion No: 6
Suggested change
| * @param violationCount The new violation count. | |
| */ | |
| public void updateViolationCount(String tenantDomain, int violationCount) { | |
| public void updateViolationCount(String tenantDomain, int violationCount) { | |
| if (LOG.isInfoEnabled()) { | |
| LOG.info(String.format("Updating violation count for tenant: %s to %d", tenantDomain, violationCount)); | |
| } | |
| try { |
Comment on lines
+99
to
+101
| resourceAdd.setName(tenantDomain); | ||
| resourceAdd.setAttributes(attributes); | ||
|
|
Contributor
There was a problem hiding this comment.
Log Improvement Suggestion No: 7
Suggested change
| resourceAdd.setName(tenantDomain); | |
| resourceAdd.setAttributes(attributes); | |
| getConfigurationManager().replaceResource(RESOURCE_TYPE_NAME, resourceAdd); | |
| LOG.info(String.format("Successfully updated violation count for tenant %s: %d", tenantDomain, violationCount)); | |
| if (LOG.isDebugEnabled()) { |
Comment on lines
+875
to
+877
| JSExecutionEnforcer jsExecutionEnforcer = | ||
| FrameworkServiceDataHolder.getInstance().getJsExecutionSupervisor().getExecutionEnforcer(); | ||
| if (jsExecutionEnforcer.isTenantBlocked(PrivilegedCarbonContext.getThreadLocalCarbonContext() |
Contributor
There was a problem hiding this comment.
Log Improvement Suggestion No: 8
Suggested change
| JSExecutionEnforcer jsExecutionEnforcer = | |
| FrameworkServiceDataHolder.getInstance().getJsExecutionSupervisor().getExecutionEnforcer(); | |
| if (jsExecutionEnforcer.isTenantBlocked(PrivilegedCarbonContext.getThreadLocalCarbonContext() | |
| JSExecutionEnforcer jsExecutionEnforcer = | |
| FrameworkServiceDataHolder.getInstance().getJsExecutionSupervisor().getExecutionEnforcer(); | |
| if (jsExecutionEnforcer.isTenantBlocked(PrivilegedCarbonContext.getThreadLocalCarbonContext() | |
| .getTenantDomain())) { | |
| log.warn("Authentication blocked for tenant {} due to adaptive script execution violations.", | |
| PrivilegedCarbonContext.getThreadLocalCarbonContext().getTenantDomain()); |
Comment on lines
441
to
443
| FrameworkServiceDataHolder.getInstance() | ||
| .setJsExecutionSupervisor(new JSExecutionSupervisor(threadCount, timeOutEnabled, timeoutInMillis, | ||
| memoryLimitInBytes)); |
Contributor
There was a problem hiding this comment.
Log Improvement Suggestion No: 9
Suggested change
| FrameworkServiceDataHolder.getInstance() | |
| .setJsExecutionSupervisor(new JSExecutionSupervisor(threadCount, timeOutEnabled, timeoutInMillis, | |
| memoryLimitInBytes)); | |
| FrameworkServiceDataHolder.getInstance() | |
| .setJsExecutionSupervisor(new JSExecutionSupervisor(threadCount, timeOutEnabled, timeoutInMillis, | |
| memoryLimitInBytes)); | |
| log.info("JS execution supervisor initialized with threadCount: " + threadCount + ", timeoutEnabled: " + timeOutEnabled + ", timeout: " + timeoutInMillis + "ms"); |
Comment on lines
+445
to
+447
| // Initialize JS execution alert service. | ||
| JSExecutionEnforcer executionEnforcer = JSExecutionEnforcer.createFromConfiguration(); | ||
| FrameworkServiceDataHolder.getInstance().getJsExecutionSupervisor().setExecutionEnforcer(executionEnforcer); |
Contributor
There was a problem hiding this comment.
Log Improvement Suggestion No: 10
Suggested change
| // Initialize JS execution alert service. | |
| JSExecutionEnforcer executionEnforcer = JSExecutionEnforcer.createFromConfiguration(); | |
| FrameworkServiceDataHolder.getInstance().getJsExecutionSupervisor().setExecutionEnforcer(executionEnforcer); | |
| // Initialize JS execution alert service. | |
| JSExecutionEnforcer executionEnforcer = JSExecutionEnforcer.createFromConfiguration(); | |
| FrameworkServiceDataHolder.getInstance().getJsExecutionSupervisor().setExecutionEnforcer(executionEnforcer); | |
| log.info("JS execution enforcer initialized and configured"); |
Contributor
There was a problem hiding this comment.
AI Agent Log Improvement Checklist
- The log-related comments and suggestions in this review were generated by an AI tool to assist with identifying potential improvements. Purpose of reviewing the code for log improvements is to improve the troubleshooting capabilities of our products.
- Please make sure to manually review and validate all suggestions before applying any changes. Not every code suggestion would make sense or add value to our purpose. Therefore, you have the freedom to decide which of the suggestions are helpful.
✅ Before merging this pull request:
- Review all AI-generated comments for accuracy and relevance.
- Complete and verify the table below. We need your feedback to measure the accuracy of these suggestions and the value they add. If you are rejecting a certain code suggestion, please mention the reason briefly in the suggestion for us to capture it.
| public static final boolean DEFAULT_EXECUTION_SUPERVISOR_TIMEOUT_ENABLE = true; | ||
| public static final long DEFAULT_EXECUTION_SUPERVISOR_MEMORY_LIMIT = -1; | ||
| public static final int DEFAULT_ALERT_MAX_VIOLATIONS_PER_TENANT = 5; | ||
| // Default tracking window set to 30 seconds. |
There was a problem hiding this comment.
Comment does not start with a capital letter. Update to: "Default tracking window set to 30 seconds."
| // Default tracking window set to 30 seconds. | ||
| public static final long DEFAULT_EXECUTION_SUPERVISOR_VIOLATION_TRACKING_WINDOW_MILLIS = 30000L; | ||
| public static final long DEFAULT_EXECUTION_SUPERVISOR_BLOCK_DURATION_MILLIS = 600000L; | ||
| // Critical memory limit set to 500MB by default. |
There was a problem hiding this comment.
Comment does not start with a capital letter. Update to: "Critical memory limit set to 500MB by default."
| .setJsExecutionSupervisor(new JSExecutionSupervisor(threadCount, timeOutEnabled, timeoutInMillis, | ||
| memoryLimitInBytes)); | ||
|
|
||
| // Initialize JS execution alert service. |
There was a problem hiding this comment.
Comment does not start with a capital letter and does not end with a period. Update to: "Initialize JS execution alert service."
|
|
||
| private static final Log LOG = LogFactory.getLog(TenantViolationStore.class); | ||
|
|
||
| // Resource type for JS execution alerts. |
There was a problem hiding this comment.
Comment does not start with a capital letter. Update to: "Resource type for JS execution alerts."
| // Resource type for JS execution alerts. | ||
| private static final String RESOURCE_TYPE_NAME = "JSExecutionAlerts"; | ||
|
|
||
| // Attribute keys. |
There was a problem hiding this comment.
Comment does not start with a capital letter. Update to: "Attribute keys."
| return false; | ||
| } | ||
|
|
||
| public int getAlertCount(String tenantDomain) { |
There was a problem hiding this comment.
Missing docstring for public method getAlertCount. Add a docstring describing the method's purpose, parameters, and return value.
| return getNonExpiredAlertCount(tenantDomain); | ||
| } | ||
|
|
||
| public void clearAlerts(String tenantDomain) { |
There was a problem hiding this comment.
Missing docstring for public method clearAlerts. Add a docstring describing the method's purpose and parameters.
| } | ||
| } | ||
|
|
||
| public void blockTenant(String tenantDomain) { |
There was a problem hiding this comment.
Missing docstring for public method blockTenant. Add a docstring describing the method's purpose and parameters.
...on/identity/application/authentication/framework/config/model/graph/JSExecutionEnforcer.java
Show resolved
Hide resolved
...on/identity/application/authentication/framework/config/model/graph/JSExecutionEnforcer.java
Show resolved
Hide resolved
|
Codecov Report❌ Patch coverage is ❌ Your patch check has failed because the patch coverage (3.27%) is below the target coverage (80.00%). You can increase the patch coverage or adjust the target coverage. Additional details and impacted files@@ Coverage Diff @@
## master #7574 +/- ##
============================================
- Coverage 50.57% 50.55% -0.03%
+ Complexity 19117 18936 -181
============================================
Files 2094 2097 +3
Lines 122790 122950 +160
Branches 25559 25551 -8
============================================
+ Hits 62106 62152 +46
- Misses 52722 52815 +93
- Partials 7962 7983 +21
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
$subject