This is an automated email from the ASF dual-hosted git repository.
markt pushed a commit to branch 10.1.x
in repository https://gitbox.apache.org/repos/asf/tomcat.git
The following commit(s) were added to refs/heads/10.1.x by this push:
new ae3c73ad81 Don't create multiple GenericPrincipals per Subject
ae3c73ad81 is described below
commit ae3c73ad818fa892e8a815d6c1b2c078a80046bb
Author: Mark Thomas <ma...@apache.org>
AuthorDate: Wed Sep 18 21:48:33 2024 +0100
Don't create multiple GenericPrincipals per Subject
---
.../authenticator/jaspic/CallbackHandlerImpl.java | 74 ++++++++++++++--------
webapps/docs/changelog.xml | 7 +-
2 files changed, 52 insertions(+), 29 deletions(-)
diff --git
a/java/org/apache/catalina/authenticator/jaspic/CallbackHandlerImpl.java
b/java/org/apache/catalina/authenticator/jaspic/CallbackHandlerImpl.java
index 17d97c683c..e95654c4bc 100644
--- a/java/org/apache/catalina/authenticator/jaspic/CallbackHandlerImpl.java
+++ b/java/org/apache/catalina/authenticator/jaspic/CallbackHandlerImpl.java
@@ -18,9 +18,11 @@ package org.apache.catalina.authenticator.jaspic;
import java.io.IOException;
import java.security.Principal;
+import java.util.ArrayList;
import java.util.Arrays;
import java.util.Collections;
import java.util.List;
+import java.util.Set;
import javax.security.auth.Subject;
import javax.security.auth.callback.Callback;
@@ -58,9 +60,11 @@ public class CallbackHandlerImpl implements CallbackHandler,
Contained {
String[] groups = null;
if (callbacks != null) {
- // Need to combine data from multiple callbacks so use this to hold
- // the data
- // Process the callbacks
+ /*
+ * There may be multiple callbacks passed to this method and/or
multiple calls to this method. The Jakarta
+ * Authentication specification recommends that this class does
not maintain state for individual requests
+ * and that the Subject is used to maintain state.
+ */
for (Callback callback : callbacks) {
if (callback instanceof CallerPrincipalCallback) {
CallerPrincipalCallback cpc = (CallerPrincipalCallback)
callback;
@@ -88,36 +92,50 @@ public class CallbackHandlerImpl implements
CallbackHandler, Contained {
}
}
- // Create the GenericPrincipal
- Principal gp = getPrincipal(principal, name, groups);
- if (subject != null && gp != null) {
- subject.getPrivateCredentials().add(gp);
- }
- }
- }
+ // If subject is null, there is nothing to do
+ if (subject != null) {
+ // Need a name to create a Principal
+ if (name == null && principal != null) {
+ name = principal.getName();
+ }
- private Principal getPrincipal(Principal principal, String name, String[]
groups) {
- // If the Principal is cached in the session JASPIC may simply return
it
- if (principal instanceof GenericPrincipal) {
- return principal;
- }
- if (name == null && principal != null) {
- name = principal.getName();
- }
- if (name == null) {
- return null;
- }
- List<String> roles;
- if (groups == null || groups.length == 0) {
- roles = Collections.emptyList();
- } else {
- roles = Arrays.asList(groups);
+ if (name != null) {
+ // If the Principal has been cached in the session, just
return it.
+ if (principal instanceof GenericPrincipal) {
+ // Duplicates are unlikely and will be handled in
AuthenticatorBase.getPrincipal()
+ subject.getPrivateCredentials().add(principal);
+ } else {
+ /*
+ * There should only be a single GenericPrincipal in
the private credentials for the Subject. If
+ * one is already present, merge the groups to create
a new GenericPrincipal. The code assumes
+ * that the name and principal (if any) will be the
same.
+ */
+ List<String> mergedRoles = new ArrayList<>();
+
+ Set<GenericPrincipal> gps =
subject.getPrivateCredentials(GenericPrincipal.class);
+ if (!gps.isEmpty()) {
+ GenericPrincipal gp = gps.iterator().next();
+ mergedRoles.addAll(Arrays.asList(gp.getRoles()));
+ // Remove the existing GenericPrincipal
+ subject.getPrivateCredentials().remove(gp);
+ }
+ if (groups != null) {
+ mergedRoles.addAll(Arrays.asList(groups));
+ }
+
+ if (mergedRoles.size() == 0) {
+ mergedRoles = Collections.emptyList();
+ }
+
+ subject.getPrivateCredentials().add(new
GenericPrincipal(name, mergedRoles, principal));
+ }
+ }
+ }
}
-
- return new GenericPrincipal(name, roles, principal);
}
+
// Contained interface methods
@Override
public Container getContainer() {
diff --git a/webapps/docs/changelog.xml b/webapps/docs/changelog.xml
index 7b09500918..2367fb89a6 100644
--- a/webapps/docs/changelog.xml
+++ b/webapps/docs/changelog.xml
@@ -109,7 +109,12 @@
<changelog>
<fix>
Ensure that <code>ServerAuthModule.initialize()</code> is called when
- the module is configured via <code>registerServerAuthModule()</code>.
+ a Jakarta Authentication module is configured via
+ <code>registerServerAuthModule()</code>. (markt)
+ </fix>
+ <fix>
+ Ensure that the Jakarta Authentication <code>CallbackHandler</code>
only
+ creates one <code>GenericPrincipal</code> in the <code>Subject</code>.
(markt)
</fix>
</changelog>
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org
For additional commands, e-mail: dev-h...@tomcat.apache.org
