Skip to content

Conversation

dcleao
Copy link
Contributor

@dcleao dcleao commented Sep 24, 2025

Please, check each commit for a more local description (title) and set of changes.
Besides actual features, there are a couple of commits dealing with fixes and small performance optimizations.

  1. feat(auth): switch legacy IAuthorizationPolicy to IAuthorizationService impl [PPUC-318]
  2. feat(auth): caching support for AuthorizationService [PPUC-318]
  3. fix(auth): SecurityHelper.runAsAnonymous not destroying StandaloneSession [PPUC-318]
  4. fix: CacheManager onLogout not properly clearing the session cache [PPUC-318]
  5. feat(auth): add default log configuration for authorization classes [PPUC-318]
  6. feat(auth): caching support for PentahoSystemAuthorizationActionService [PPUC-318]
  7. feat(auth): invalidate authorization cache from JcrRoleAuthorizationPolicyRoleBindingDao [PPUC-318]
  8. feat(auth): invalidate authorization cache from JcrUserRoleDao [PPUC-318]
  9. chore(auth): performance of default authorization options [PPUC-318]
  10. chore(auth): performance of authorization related hash code and equals impls [PPUC-318]
  11. feat(auth): endpoint and old PUC menu item to invalidate authorization cache [PPUC-318]
    PPUC-318-Menu-Tools-Refresh-AuthCache

Issue: https://hv-eng.atlassian.net/browse/PPUC-318

To be merged with:

Copilot Summary

This pull request introduces a new caching mechanism for authorization decisions and refactors how default authorization options are provided. It also updates Spring configuration files to wire up the new cache and related services, improves logging for authorization components, and makes a minor resource management fix in the security helper.

Authorization Decision Caching

  • Added new interfaces IAuthorizationDecisionCache and IAuthorizationDecisionCacheKey to support caching of authorization decisions, following a loading cache pattern. [1] [2]
  • Registered a new bean authorizationDecisionCache using MemoryAuthorizationDecisionCache in Spring configuration, with configurable expiration and size.

Service and Options Refactoring

  • Refactored IAuthorizationOptions.getDefault() to return a singleton DefaultAuthorizationOptions.INSTANCE instead of an anonymous inner class, moving the implementation to a dedicated class. [1] [2] [3]

Spring Configuration Updates

  • Updated beans to use the new caching authorization service (CachingAuthorizationService) and to inject the decision cache where needed, including repository and authorization policy beans. [1] [2] [3]
  • Updated CSRF configuration to allow refreshing the authorization decision cache via API.

Logging Improvements

  • Added a dedicated logger for org.pentaho.platform.engine.security.authorization at the ERROR level in log4j2.xml for better visibility of authorization-related issues.

Code Cleanup and Resource Management

  • Improved resource management in SecurityHelper.runAsAnonymous by ensuring the anonymous session is destroyed after use. Also reordered imports for clarity. [1] [2] [3] [4]

These changes collectively improve authorization performance and maintainability, and prepare the platform for more robust security features.

This comment has been minimized.

@buildguy

This comment has been minimized.

@dcleao dcleao force-pushed the PPUC-122-6-TheSwitch branch from 17f96d9 to 05e472b Compare September 25, 2025 20:33

This comment has been minimized.

@buildguy

This comment has been minimized.

@dcleao
Copy link
Contributor Author

dcleao commented Sep 30, 2025

Commit fixing system IT test issues was moved to: #6036.

logs/
target/
http:/
c:/
Copy link
Contributor

Choose a reason for hiding this comment

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

Is this correct: C:/? I'm just wondering.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, the IT tests create these weird files/folders!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

In any case, those changes have moved to #6036.

@dcleao dcleao force-pushed the PPUC-122-6-TheSwitch branch from 05e472b to e48deeb Compare October 1, 2025 15:11

This comment has been minimized.

@buildguy

This comment has been minimized.

@dcleao dcleao force-pushed the PPUC-122-6-TheSwitch branch from e48deeb to 0868674 Compare October 3, 2025 10:30
@dcleao dcleao changed the title feat: direct legacy IAuthorizationPolicy to IAuthorizationService [PPUC-122] feat: direct legacy IAuthorizationPolicy to IAuthorizationService [PPUC-318] Oct 3, 2025
@dcleao
Copy link
Contributor Author

dcleao commented Oct 3, 2025

(reworded the commit and PR description, from PPUC-122, to the new story, PPUC-318, but kept the branch name, to avoid having to close this PR)

This comment has been minimized.

@buildguy

This comment has been minimized.

@dcleao
Copy link
Contributor Author

dcleao commented Oct 10, 2025

Updated the PR to include caching support for the authorization service and authorization action service.
Also, included some performance optimizations.
Also, included a new endpoint and old PUC menu item to allow an "administer security" to explicitly clear the authorization service cache.

@dcleao dcleao changed the title feat: direct legacy IAuthorizationPolicy to IAuthorizationService [PPUC-318] feat(auth): switch legacy IAuthorizationPolicy to IAuthorizationService impl [PPUC-318] Oct 10, 2025
@dcleao dcleao requested a review from mbrasil October 13, 2025 10:07
@dcleao dcleao marked this pull request as ready for review October 13, 2025 10:07
@dcleao dcleao requested a review from a team as a code owner October 13, 2025 10:07
Copy link
Contributor

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This pull request introduces an authorization decision caching mechanism to improve the performance of authorization operations in the Pentaho platform. The PR switches from the legacy IAuthorizationPolicy to a new IAuthorizationService implementation with comprehensive caching support.

  • Added authorization decision caching infrastructure with cache invalidation capabilities
  • Refactored authorization service to use caching and updated Spring configuration
  • Added management endpoints and UI menu items for cache administration

Reviewed Changes

Copilot reviewed 29 out of 29 changed files in this pull request and generated 5 comments.

Show a summary per file
File Description
core/src/main/java/org/pentaho/platform/engine/security/authorization/core/caching/MemoryAuthorizationDecisionCache.java New authorization decision cache implementation using Guava cache with session-based invalidation
core/src/main/java/org/pentaho/platform/engine/security/authorization/core/CachingAuthorizationService.java Caching wrapper for authorization service that uses the decision cache
core/src/main/java/org/pentaho/platform/engine/security/authorization/PentahoSystemAuthorizationActionService.java Refactored service to use plugin manager listener for cache refresh on plugin changes
assemblies/pentaho-solutions/src/main/resources/pentaho-solutions/system/pentahoObjects.spring.xml Spring configuration updates to wire caching authorization service and decision cache
user-console/src/main/java/org/pentaho/mantle/client/commands/PurgeAuthorizationDecisionCacheCommand.java New GWT command for purging authorization cache via UI
extensions/src/main/java/org/pentaho/platform/web/http/api/resources/SystemRefreshResource.java REST endpoint for authorization cache management
repository/src/main/java/org/pentaho/platform/security/userroledao/jackrabbit/AbstractJcrBackedUserRoleDao.java Cache invalidation integration for user role changes
core/src/main/java/org/pentaho/platform/engine/security/SecurityHelper.java Fixed session resource leak in runAsAnonymous method

Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

Copy link

Failed

  • 26.80% Coverage on New Code (is less than 80.00%)
  • 0.00% Security Hotspots Reviewed on New Code (is less than 100.00%)
  • 9 New Issues (is greater than 0)

Analysis Details

10 Issues

  • Bug 0 Bugs
  • Vulnerability 1 Vulnerability
  • Code Smell 9 Code Smells

Coverage and Duplications

  • Coverage 26.80% Coverage (30.30% Estimated after merge)
  • Duplications 2.76% Duplicated Code (6.90% Estimated after merge)

Project ID: pentaho:pentaho-platform-ce-parent

View in SonarQube

@buildguy
Copy link
Collaborator

👍 Frogbot scanned this pull request and did not find any new security issues.

Note:

Frogbot also supports Contextual Analysis, Secret Detection, IaC and SAST Vulnerabilities Scanning. This features are included as part of the JFrog Advanced Security package, which isn't enabled on your system.


@buildguy
Copy link
Collaborator

✅ Build finished in 15h 39m 50s

Build command:

mvn clean verify -B -e -Daudit -Djs.no.sandbox -pl \
api,assemblies/pentaho-solutions,assemblies/pentaho-war,core,extensions,repository,user-console

👌 All tests passed!

Tests run: 3283, Failures: 0, Skipped: 1    Test Results


ℹ️ This is an automatic message

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