This is an automated email from the ASF dual-hosted git repository.
Croway pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/camel.git
The following commit(s) were added to refs/heads/main by this push:
new 6f72ae648eb0 CAMEL-24439: camel-shiro - verify the presented
credentials on every exchange (#25822)
6f72ae648eb0 is described below
commit 6f72ae648eb0c6e54a913f7be33cd2c020355b95
Author: Andrea Cosentino <[email protected]>
AuthorDate: Fri Aug 28 12:10:49 2026 +0200
CAMEL-24439: camel-shiro - verify the presented credentials on every
exchange (#25822)
* CAMEL-24439: camel-shiro - verify the presented credentials on every
exchange
ShiroSecurityProcessor.authenticateUser() called login() only when the
thread-bound
subject was not already authenticated for the same username as the incoming
ShiroSecurityToken:
if (!authenticated || !sameUser) { ... currentUser.login(token); }
That conflates "same principal name" with "same credentials". Once a user
had
authenticated on a worker thread, a later exchange presenting that username
with any
password was accepted for as long as the subject stayed bound, because the
password
was never checked.
The default alwaysReauthenticate=true masks it, since the processor calls
logout() in a
finally block after each exchange. With alwaysReauthenticate=false the skip
is
reachable, and that mode deliberately sets rememberMe(true) to keep subjects
long-lived on Camel's shared worker threads.
Call login() for every exchange with the credentials that exchange
presented. Shiro
offers no way to compare presented credentials against a bound subject, so
the
principal-name comparison could not be made sound and is removed rather
than narrowed.
The added test sends a valid token for ringo, then the same username with a
wrong
password on the same thread; without the fix both reach mock:success.
Co-Authored-By: Claude Opus 5 (1M context) <[email protected]>
Signed-off-by: Andrea Cosentino <[email protected]>
* CAMEL-24439: Simplify test encryption key
---------
Signed-off-by: Andrea Cosentino <[email protected]>
Co-authored-by: Claude Opus 5 (1M context) <[email protected]>
Co-authored-by: Croway <[email protected]>
Co-authored-by: Federico Mariani <[email protected]>
---
.../shiro/security/ShiroSecurityProcessor.java | 29 +++----
...oAuthenticationCredentialAlwaysCheckedTest.java | 93 ++++++++++++++++++++++
.../ROOT/pages/camel-4x-upgrade-guide-4_23.adoc | 16 ++++
3 files changed, 122 insertions(+), 16 deletions(-)
diff --git
a/components/camel-shiro/src/main/java/org/apache/camel/component/shiro/security/ShiroSecurityProcessor.java
b/components/camel-shiro/src/main/java/org/apache/camel/component/shiro/security/ShiroSecurityProcessor.java
index 29b1271fa2c5..98f18bec1221 100644
---
a/components/camel-shiro/src/main/java/org/apache/camel/component/shiro/security/ShiroSecurityProcessor.java
+++
b/components/camel-shiro/src/main/java/org/apache/camel/component/shiro/security/ShiroSecurityProcessor.java
@@ -130,24 +130,21 @@ public class ShiroSecurityProcessor extends
DelegateAsyncProcessor {
}
private void authenticateUser(Subject currentUser, ShiroSecurityToken
securityToken) {
- boolean authenticated = currentUser.isAuthenticated();
- boolean sameUser =
securityToken.getUsername().equals(currentUser.getPrincipal());
- LOG.trace("Authenticated: {}, same Username: {}", authenticated,
sameUser);
+ // The login is not skipped when the thread-bound subject already
carries the same principal name.
+ // A matching username says nothing about the password presented with
this exchange, and with
+ // alwaysReauthenticate=false the subject is deliberately long-lived
on a shared worker thread, so
+ // skipping would let a wrong password through for as long as that
subject stays bound.
+ LOG.trace("Authenticating {} (subject currently authenticated: {})",
+ securityToken.getUsername(), currentUser.isAuthenticated());
- if (!authenticated || !sameUser) {
- UsernamePasswordToken token = new
UsernamePasswordToken(securityToken.getUsername(), securityToken.getPassword());
- if (policy.isAlwaysReauthenticate()) {
- token.setRememberMe(false);
- } else {
- token.setRememberMe(true);
- }
+ UsernamePasswordToken token = new
UsernamePasswordToken(securityToken.getUsername(), securityToken.getPassword());
+ token.setRememberMe(!policy.isAlwaysReauthenticate());
- try {
- currentUser.login(token);
- LOG.debug("Current user {} successfully authenticated",
currentUser.getPrincipal());
- } catch (AuthenticationException ae) {
- throw new AuthenticationException("Authentication Failed.",
ae.getCause());
- }
+ try {
+ currentUser.login(token);
+ LOG.debug("Current user {} successfully authenticated",
currentUser.getPrincipal());
+ } catch (AuthenticationException ae) {
+ throw new AuthenticationException("Authentication Failed.",
ae.getCause());
}
}
diff --git
a/components/camel-shiro/src/test/java/org/apache/camel/component/shiro/security/ShiroAuthenticationCredentialAlwaysCheckedTest.java
b/components/camel-shiro/src/test/java/org/apache/camel/component/shiro/security/ShiroAuthenticationCredentialAlwaysCheckedTest.java
new file mode 100644
index 000000000000..7e32cc11db2b
--- /dev/null
+++
b/components/camel-shiro/src/test/java/org/apache/camel/component/shiro/security/ShiroAuthenticationCredentialAlwaysCheckedTest.java
@@ -0,0 +1,93 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.camel.component.shiro.security;
+
+import java.nio.charset.StandardCharsets;
+
+import org.apache.camel.EndpointInject;
+import org.apache.camel.Exchange;
+import org.apache.camel.builder.RouteBuilder;
+import org.apache.camel.component.mock.MockEndpoint;
+import org.apache.camel.test.junit6.CamelTestSupport;
+import org.apache.shiro.authc.AuthenticationException;
+import org.apache.shiro.authc.IncorrectCredentialsException;
+import org.apache.shiro.authc.LockedAccountException;
+import org.apache.shiro.authc.UnknownAccountException;
+import org.junit.jupiter.api.Test;
+
+/**
+ * With {@code alwaysReauthenticate=false} the Shiro subject stays bound to
the thread between exchanges, which is the
+ * point of the option. The password carried by each exchange still has to be
verified: a username that happens to match
+ * the bound subject's principal is not evidence about the credentials
presented this time.
+ */
+class ShiroAuthenticationCredentialAlwaysCheckedTest extends CamelTestSupport {
+
+ private static final byte[] TEST_KEY =
"0123456789abcdef".getBytes(StandardCharsets.US_ASCII);
+
+ @EndpointInject("mock:success")
+ protected MockEndpoint successEndpoint;
+
+ @EndpointInject("mock:authenticationException")
+ protected MockEndpoint failureEndpoint;
+
+ @Test
+ void aWrongPasswordIsRejectedEvenAfterTheSameUserAuthenticated() throws
Exception {
+ successEndpoint.expectedMessageCount(1);
+ failureEndpoint.expectedMessageCount(1);
+
+ // Authenticates and leaves the subject bound to this thread
+ template.send("direct:secureEndpoint", injector("ringo", "starr"));
+ // Same username, wrong password - must not ride the bound subject
through
+ template.send("direct:secureEndpoint", injector("ringo", "not-starr"));
+
+ successEndpoint.assertIsSatisfied();
+ failureEndpoint.assertIsSatisfied();
+ }
+
+ private TestShiroSecurityTokenInjector injector(String user, String
password) {
+ return new TestShiroSecurityTokenInjector(new ShiroSecurityToken(user,
password), TEST_KEY);
+ }
+
+ @Override
+ protected RouteBuilder createRouteBuilder() {
+ final ShiroSecurityPolicy securityPolicy
+ = new
ShiroSecurityPolicy("./src/test/resources/securityconfig.ini", TEST_KEY, false);
+
+ return new RouteBuilder() {
+ @Override
+ public void configure() {
+ onException(UnknownAccountException.class,
IncorrectCredentialsException.class,
+ LockedAccountException.class,
AuthenticationException.class).to("mock:authenticationException");
+
+
from("direct:secureEndpoint").policy(securityPolicy).to("mock:success");
+ }
+ };
+ }
+
+ private static class TestShiroSecurityTokenInjector extends
ShiroSecurityTokenInjector {
+
+ TestShiroSecurityTokenInjector(ShiroSecurityToken shiroSecurityToken,
byte[] bytes) {
+ super(shiroSecurityToken, bytes);
+ }
+
+ @Override
+ public void process(Exchange exchange) {
+
exchange.getIn().setHeader(ShiroSecurityConstants.SHIRO_SECURITY_TOKEN,
encrypt());
+ exchange.getIn().setBody("Beatle Mania");
+ }
+ }
+}
diff --git
a/docs/user-manual/modules/ROOT/pages/camel-4x-upgrade-guide-4_23.adoc
b/docs/user-manual/modules/ROOT/pages/camel-4x-upgrade-guide-4_23.adoc
index 30e8dfb1d3ec..74b7f414031b 100644
--- a/docs/user-manual/modules/ROOT/pages/camel-4x-upgrade-guide-4_23.adoc
+++ b/docs/user-manual/modules/ROOT/pages/camel-4x-upgrade-guide-4_23.adoc
@@ -703,6 +703,22 @@ inherit values set on `defaultInstance`, and
`defaultInstance` itself remains un
Routes that compared unmarshalled bodies by identity, or that mutated one body
expecting the change to
be visible on another, must be updated.
+=== camel-shiro
+
+`ShiroSecurityProcessor` used to skip the Shiro `login()` call — and therefore
the credential check —
+when the thread-bound subject was already authenticated for the same username
as the incoming
+`ShiroSecurityToken`. The check conflated "same principal name" with "same
credentials", so once a user
+had authenticated on a worker thread, a later exchange presenting that
username with any password was
+accepted for as long as the subject stayed bound.
+
+The default `alwaysReauthenticate=true` masked this, because the processor
calls `logout()` after each
+exchange. With `alwaysReauthenticate=false` — a documented option, which also
sets `rememberMe(true)` to
+keep subjects long-lived — the skip was reachable.
+
+`login()` is now called for every exchange with the credentials that exchange
presented. Deployments
+using `alwaysReauthenticate=false` will see one realm lookup per exchange
where previously matching
+usernames reused the bound subject; correctness aside, that is the same cost
the default already pays.
+
=== camel-oauth
The authorization code flow now sends a `state` parameter and requires it back
on the callback.
