gnodet-bot commented on code in PR #28570:
URL: https://github.com/apache/camel/pull/28570#discussion_r4389944379
##########
components/camel-shiro/src/main/java/org/apache/camel/component/shiro/security/ShiroSecurityProcessor.java:
##########
@@ -114,18 +120,75 @@ private void applySecurityPolicy(Exchange exchange)
throws Exception {
ByteSourceBroker decryptedToken =
policy.getCipherService().decrypt(encryptedToken.getBytes(),
policy.getPassPhrase());
ShiroSecurityToken securityToken =
ShiroSecurityHelper.deserialize(decryptedToken.getClonedBytes());
- Subject currentUser = SecurityUtils.getSubject();
+ SubjectThreadState threadState = null;
+ Subject currentUser = null;
+ boolean success = false;
- // Authenticate user if not authenticated
try {
+ Subject threadSubject = ThreadContext.getSubject();
+ if (!policy.isAlwaysReauthenticate()
+ && threadSubject instanceof DelegatingSubject
delegatingSubject
+ && policy.getSecurityManager() != null
+ && delegatingSubject.getSecurityManager() ==
policy.getSecurityManager()) {
+ currentUser = threadSubject;
+ } else {
+ cleanUpManagedSubject(exchange);
+
+ if (policy.getSecurityManager() != null) {
+ currentUser = new
Subject.Builder(policy.getSecurityManager()).buildSubject();
+ threadState = new SubjectThreadState(currentUser);
+ threadState.bind();
+ } else {
+ currentUser = SecurityUtils.getSubject();
+ }
+ }
+
+ // Authenticate user if not authenticated
authenticateUser(currentUser, securityToken);
// Test whether user's role is authorized to perform functions in
the permissions list
authorizeUser(currentUser, exchange);
+ success = true;
} finally {
- if (policy.isAlwaysReauthenticate()) {
- currentUser.logout();
+ if (policy.isAlwaysReauthenticate() || !success) {
+ try {
+ if (currentUser != null) {
+ currentUser.logout();
+ }
+ } finally {
+ ThreadContext.remove(MANAGED_SUBJECT);
+ ThreadContext.remove(MANAGED_EXCHANGE);
+ if (threadState != null) {
+ threadState.restore();
+ } else {
+ ThreadContext.unbindSubject();
+ ThreadContext.unbindSecurityManager();
+ }
+ }
+ } else {
+ // When alwaysReauthenticate=false and the exchange succeeds,
the subject remains bound for subsequent
+ // exchanges on this thread. cleanUpManagedSubject will log it
out when a different exchange arrives.
+ ThreadContext.put(MANAGED_SUBJECT, currentUser);
+ ThreadContext.put(MANAGED_EXCHANGE, exchange);
+ }
+ }
+ }
+
+ private void cleanUpManagedSubject(Exchange currentExchange) {
+ Object managedExchange = ThreadContext.get(MANAGED_EXCHANGE);
+ // Use reference inequality (!=) to skip cleanup during synchronous
nested routing on the same exchange
+ // (such as direct: routes), avoiding premature logout of the outer
subject.
+ if (managedExchange != null && managedExchange != currentExchange) {
+ Object managed = ThreadContext.get(MANAGED_SUBJECT);
+ if (managed instanceof Subject oldSubject) {
+ try {
+ oldSubject.logout();
+ } catch (Exception e) {
+ LOG.debug("Error logging out previous managed subject", e);
+ }
}
+ ThreadContext.remove(MANAGED_SUBJECT);
+ ThreadContext.remove(MANAGED_EXCHANGE);
Review Comment:
💡 **Minor (low severity):** `cleanUpManagedSubject` removes the custom
`MANAGED_SUBJECT`/`MANAGED_EXCHANGE` keys but leaves the standard Shiro
`SUBJECT_KEY`/`SECURITY_MANAGER_KEY` bindings on `ThreadContext`. The
immediately-following `SubjectThreadState.bind()` call (line ~128) snapshots
`ThreadContext.getResources()` into `originalResources` **before** clearing, so
it captures these stale (logged-out) bindings. On the **success path** this is
harmless — `threadState.restore()` is never called. On the **failure path**
(exception in the finally block), `threadState.restore()` would reinstate the
logged-out subject — but since the subject is already logged out, it's
effectively inert.
Defensive fix: call `ThreadContext.unbindSubject()` and
`ThreadContext.unbindSecurityManager()` here (alongside the `remove(MANAGED_*)`
calls) to ensure a clean slate.
```suggestion
ThreadContext.remove(MANAGED_SUBJECT);
ThreadContext.remove(MANAGED_EXCHANGE);
ThreadContext.unbindSubject();
ThreadContext.unbindSecurityManager();
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
