You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@tomcat.apache.org by ma...@apache.org on 2019/06/28 08:45:38 UTC
[tomcat] branch 7.0.x updated (ecde67c -> 241148f)
This is an automated email from the ASF dual-hosted git repository.
markt pushed a change to branch 7.0.x
in repository https://gitbox.apache.org/repos/asf/tomcat.git.
from ecde67c Align with 8.5.x. Code clean up and improved i18n messages
new 47235d9 Align with 8.5.x. Clean-up. Remove ununsed code.
new d121104 Fix https://bz.apache.org/bugzilla/show_bug.cgi?id=63531
new 739fa61 Align with 8.5.x. Clean-up
new 241148f Add debug logging for session ID change
The 4 revisions listed above as "new" are entirely new to this
repository and will be described in separate emails. The revisions
listed as "add" were already present in the repository and have only
been added to this reference.
Summary of changes:
java/org/apache/catalina/Authenticator.java | 12 +-
.../catalina/authenticator/AuthenticatorBase.java | 171 ++++++++-------------
.../catalina/authenticator/FormAuthenticator.java | 47 ++++--
.../catalina/authenticator/LocalStrings.properties | 1 +
.../authenticator/LocalStrings_fr.properties | 1 +
.../authenticator/LocalStrings_ja.properties | 1 +
webapps/docs/changelog.xml | 6 +
7 files changed, 113 insertions(+), 126 deletions(-)
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org
[tomcat] 02/04: Fix
https://bz.apache.org/bugzilla/show_bug.cgi?id=63531
Posted by ma...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
markt pushed a commit to branch 7.0.x
in repository https://gitbox.apache.org/repos/asf/tomcat.git
commit d1211048a9b4a0cf9e968f1f7a3f8fd09c7c2d94
Author: Mark Thomas <ma...@apache.org>
AuthorDate: Thu Jun 27 23:05:52 2019 +0100
Fix https://bz.apache.org/bugzilla/show_bug.cgi?id=63531
Refactor authenticators so that the session last accessed time is not
updated if the cache attribute is set to false and FORM authentication
is not being used.
---
.../catalina/authenticator/AuthenticatorBase.java | 87 ++++++++--------------
.../catalina/authenticator/FormAuthenticator.java | 36 ++++++++-
webapps/docs/changelog.xml | 6 ++
3 files changed, 70 insertions(+), 59 deletions(-)
diff --git a/java/org/apache/catalina/authenticator/AuthenticatorBase.java b/java/org/apache/catalina/authenticator/AuthenticatorBase.java
index eb9d35e..e67cd20 100644
--- a/java/org/apache/catalina/authenticator/AuthenticatorBase.java
+++ b/java/org/apache/catalina/authenticator/AuthenticatorBase.java
@@ -431,55 +431,13 @@ public abstract class AuthenticatorBase extends ValveBase
}
}
- // Special handling for form-based logins to deal with the case
- // where the login form (and therefore the "j_security_check" URI
- // to which it submits) might be outside the secured area
- String contextPath = this.context.getPath();
- String requestURI = request.getDecodedRequestURI();
- if (requestURI.startsWith(contextPath) &&
- requestURI.endsWith(Constants.FORM_ACTION)) {
- if (!authenticate(request, response, config)) {
- if (log.isDebugEnabled())
- log.debug(" Failed authenticate() test ??" + requestURI );
- return;
- }
- }
-
- // Special handling for form-based logins to deal with the case where
- // a resource is protected for some HTTP methods but not protected for
- // GET which is used after authentication when redirecting to the
- // protected resource.
- // TODO: This is similar to the FormAuthenticator.matchRequest() logic
- // Is there a way to remove the duplication?
- Session session = request.getSessionInternal(false);
- if (session != null) {
- SavedRequest savedRequest =
- (SavedRequest) session.getNote(Constants.FORM_REQUEST_NOTE);
- if (savedRequest != null) {
- String decodedRequestURI = request.getDecodedRequestURI();
- if (decodedRequestURI != null &&
- decodedRequestURI.equals(
- savedRequest.getDecodedRequestURI())) {
- if (!authenticate(request, response)) {
- if (log.isDebugEnabled()) {
- log.debug(" Failed authenticate() test");
- }
- /*
- * ASSERT: Authenticator already set the appropriate
- * HTTP status code, so we do not have to do anything
- * special
- */
- return;
- }
- }
- }
- }
+ boolean authRequired = isContinuationRequired(request);
Realm realm = this.context.getRealm();
// Is this request URI subject to a security constraint?
SecurityConstraint[] constraints = realm.findSecurityConstraints(request, this.context);
- if (constraints == null && !context.getPreemptiveAuthentication()) {
+ if (constraints == null && !context.getPreemptiveAuthentication() && !authRequired) {
if (log.isDebugEnabled()) {
log.debug(" Not subject to any constraint");
}
@@ -520,23 +478,25 @@ public abstract class AuthenticatorBase extends ValveBase
// Since authenticate modifies the response on failure,
// we have to check for allow-from-all first.
- boolean authRequired;
- if (constraints == null) {
- authRequired = false;
- } else {
- authRequired = true;
- for(int i = 0; i < constraints.length && authRequired; i++) {
- if(!constraints[i].getAuthConstraint()) {
- authRequired = false;
- } else if(!constraints[i].getAllRoles()) {
- String [] roles = constraints[i].findAuthRoles();
- if(roles == null || roles.length == 0) {
- authRequired = false;
+ boolean hasAuthConstraint = false;
+ if (constraints != null) {
+ hasAuthConstraint = true;
+ for (int i = 0; i < constraints.length && hasAuthConstraint; i++) {
+ if (!constraints[i].getAuthConstraint()) {
+ hasAuthConstraint = false;
+ } else if (!constraints[i].getAllRoles()) {
+ String[] roles = constraints[i].findAuthRoles();
+ if (roles == null || roles.length == 0) {
+ hasAuthConstraint = false;
}
}
}
}
+ if (!authRequired && hasAuthConstraint) {
+ authRequired = true;
+ }
+
if (!authRequired && context.getPreemptiveAuthentication()) {
authRequired =
request.getCoyoteRequest().getMimeHeaders().getValue("authorization") != null;
@@ -593,6 +553,21 @@ public abstract class AuthenticatorBase extends ValveBase
// ------------------------------------------------------ Protected Methods
/**
+ * Does this authenticator require that {@link #authenticate(Request,
+ * HttpServletResponse)} is called to continue an authentication process
+ * that started in a previous request?
+ *
+ * @param request The request currently being processed
+ *
+ * @return {@code true} if authenticate() must be called, otherwise
+ * {@code false}
+ */
+ protected boolean isContinuationRequired(Request request) {
+ return false;
+ }
+
+
+ /**
* Look for the X509 certificate chain in the Request under the key
* <code>javax.servlet.request.X509Certificate</code>. If not found, trigger
* extracting the certificate chain from the Coyote request.
diff --git a/java/org/apache/catalina/authenticator/FormAuthenticator.java b/java/org/apache/catalina/authenticator/FormAuthenticator.java
index 809556e..5b55664 100644
--- a/java/org/apache/catalina/authenticator/FormAuthenticator.java
+++ b/java/org/apache/catalina/authenticator/FormAuthenticator.java
@@ -352,13 +352,43 @@ public class FormAuthenticator
}
+ // ------------------------------------------------------ Protected Methods
+
@Override
- protected String getAuthMethod() {
- return HttpServletRequest.FORM_AUTH;
+ protected boolean isContinuationRequired(Request request) {
+ // Special handling for form-based logins to deal with the case
+ // where the login form (and therefore the "j_security_check" URI
+ // to which it submits) might be outside the secured area
+ String contextPath = this.context.getPath();
+ String decodedRequestURI = request.getDecodedRequestURI();
+ if (decodedRequestURI.startsWith(contextPath) &&
+ decodedRequestURI.endsWith(Constants.FORM_ACTION)) {
+ return true;
+ }
+
+ // Special handling for form-based logins to deal with the case where
+ // a resource is protected for some HTTP methods but not protected for
+ // GET which is used after authentication when redirecting to the
+ // protected resource.
+ // TODO: This is similar to the FormAuthenticator.matchRequest() logic
+ // Is there a way to remove the duplication?
+ Session session = request.getSessionInternal(false);
+ if (session != null) {
+ SavedRequest savedRequest = (SavedRequest) session.getNote(Constants.FORM_REQUEST_NOTE);
+ if (savedRequest != null &&
+ decodedRequestURI.equals(savedRequest.getDecodedRequestURI())) {
+ return true;
+ }
+ }
+
+ return false;
}
- // ------------------------------------------------------ Protected Methods
+ @Override
+ protected String getAuthMethod() {
+ return HttpServletRequest.FORM_AUTH;
+ }
/**
diff --git a/webapps/docs/changelog.xml b/webapps/docs/changelog.xml
index 64db49f..11afd1e 100644
--- a/webapps/docs/changelog.xml
+++ b/webapps/docs/changelog.xml
@@ -117,6 +117,12 @@
<code>RequestDispatcher</code>. The requested target path is logged as a
warning since this is an application error. (markt)
</add>
+ <fix>
+ <bug>63531</bug>: Refactor authenticators so that the session last
+ accessed time is not updated if the cache attribute is set to
+ <code>false</code> and <code>FORM</code> authentication is not being
+ used. (markt)
+ </fix>
</changelog>
</subsection>
<subsection name="Coyote">
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org
[tomcat] 04/04: Add debug logging for session ID change
Posted by ma...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
markt pushed a commit to branch 7.0.x
in repository https://gitbox.apache.org/repos/asf/tomcat.git
commit 241148fcd1582c79ea6866793f94f1752f1ce5fb
Author: Mark Thomas <ma...@apache.org>
AuthorDate: Fri Jun 28 09:32:44 2019 +0100
Add debug logging for session ID change
---
java/org/apache/catalina/authenticator/AuthenticatorBase.java | 8 ++++++++
java/org/apache/catalina/authenticator/LocalStrings.properties | 1 +
java/org/apache/catalina/authenticator/LocalStrings_fr.properties | 1 +
java/org/apache/catalina/authenticator/LocalStrings_ja.properties | 1 +
4 files changed, 11 insertions(+)
diff --git a/java/org/apache/catalina/authenticator/AuthenticatorBase.java b/java/org/apache/catalina/authenticator/AuthenticatorBase.java
index 1011bbb..0c6ff36 100644
--- a/java/org/apache/catalina/authenticator/AuthenticatorBase.java
+++ b/java/org/apache/catalina/authenticator/AuthenticatorBase.java
@@ -787,9 +787,17 @@ public abstract class AuthenticatorBase extends ValveBase
// If the principal is null then this is a logout. No need to change
// the session ID. See BZ 59043.
if (changeSessionIdOnAuthentication && principal != null) {
+ String oldId = null;
+ if (log.isDebugEnabled()) {
+ oldId = session.getId();
+ }
Manager manager = request.getContext().getManager();
manager.changeSessionId(session);
request.changeSessionId(session.getId());
+ if (log.isDebugEnabled()) {
+ log.debug(sm.getString("authenticator.changeSessionId",
+ oldId, session.getId()));
+ }
}
} else if (alwaysUseSession) {
session = request.getSessionInternal(true);
diff --git a/java/org/apache/catalina/authenticator/LocalStrings.properties b/java/org/apache/catalina/authenticator/LocalStrings.properties
index 62dee48..49cfa4e 100644
--- a/java/org/apache/catalina/authenticator/LocalStrings.properties
+++ b/java/org/apache/catalina/authenticator/LocalStrings.properties
@@ -14,6 +14,7 @@
# limitations under the License.
authenticator.certificates=No client certificate chain in this request
+authenticator.changeSessionId=Session ID changed on authentication from [{0}] to [{1}]
authenticator.check.authorize=User name [{0}] obtained from the Connector and trusted to be valid. Obtaining roles for this user from the Tomcat Realm.
authenticator.check.authorizeFail=Realm did not recognise user [{0}]. Creating a Principal with that name and no roles.
authenticator.check.found=Already authenticated [{0}]
diff --git a/java/org/apache/catalina/authenticator/LocalStrings_fr.properties b/java/org/apache/catalina/authenticator/LocalStrings_fr.properties
index 58c8ae2..84a7843 100644
--- a/java/org/apache/catalina/authenticator/LocalStrings_fr.properties
+++ b/java/org/apache/catalina/authenticator/LocalStrings_fr.properties
@@ -14,6 +14,7 @@
# limitations under the License.
authenticator.certificates=Aucune chaîne de certificat client (client certificate chain) dans cette requête
+authenticator.changeSessionId=L''id de session a changé suite à l''authntification de [{0}] en [{1}]
authenticator.check.authorize=Le nom d''utilisateur [{0}] obtenu à partir du connecteur est considéré comme de valide et de confiance, les rôles sont obtenus à partir du royaume
authenticator.check.authorizeFail=Le royaume ne reconnait pas l''utilisateur [{0}], un principal a été crée avec ce nom mais sans rôles
authenticator.check.found=Déjà authentifié [{0}]
diff --git a/java/org/apache/catalina/authenticator/LocalStrings_ja.properties b/java/org/apache/catalina/authenticator/LocalStrings_ja.properties
index 0726b01..5301f02 100644
--- a/java/org/apache/catalina/authenticator/LocalStrings_ja.properties
+++ b/java/org/apache/catalina/authenticator/LocalStrings_ja.properties
@@ -14,6 +14,7 @@
# limitations under the License.
authenticator.certificates=このリクエストにはクライアント認証チェーンがありません
+authenticator.changeSessionId=認証時に[{0}]から[{1}]にセッションIDが変更されました。
authenticator.check.authorize=Connector から取得したユーザー名 [{0}] を正当なものとして信頼します。ユーザーのロールは Tomcat Realmから取得します。
authenticator.check.authorizeFail=Realm がユーザー[{0}]を認識しませんでした。 その名前とロールのないプリンシパルを作成します。
authenticator.check.found=既に認証された[{0}]
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org
[tomcat] 01/04: Align with 8.5.x. Clean-up. Remove ununsed code.
Posted by ma...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
markt pushed a commit to branch 7.0.x
in repository https://gitbox.apache.org/repos/asf/tomcat.git
commit 47235d9a2083ab52ea03f44d51f556667636f614
Author: Mark Thomas <ma...@apache.org>
AuthorDate: Thu Jun 27 22:37:02 2019 +0100
Align with 8.5.x. Clean-up. Remove ununsed code.
---
.../catalina/authenticator/AuthenticatorBase.java | 22 ++++++++++++----------
.../catalina/authenticator/FormAuthenticator.java | 11 -----------
2 files changed, 12 insertions(+), 21 deletions(-)
diff --git a/java/org/apache/catalina/authenticator/AuthenticatorBase.java b/java/org/apache/catalina/authenticator/AuthenticatorBase.java
index 2bb4746..eb9d35e 100644
--- a/java/org/apache/catalina/authenticator/AuthenticatorBase.java
+++ b/java/org/apache/catalina/authenticator/AuthenticatorBase.java
@@ -78,10 +78,6 @@ public abstract class AuthenticatorBase extends ValveBase
*/
private static final String DATE_ONE = ConcurrentDateFormat.formatRfc1123(new Date(1));
- public AuthenticatorBase() {
- super(true);
- }
-
/**
* The string manager for this package.
*/
@@ -97,6 +93,14 @@ public abstract class AuthenticatorBase extends ValveBase
*/
protected static final String REALM_NAME = "Authentication required";
+ // ------------------------------------------------------ Constructor
+
+ public AuthenticatorBase() {
+ super(true);
+ }
+
+ // ----------------------------------------------------- Instance Variables
+
/**
* Should a session always be used once a user is authenticated? This may
* offer some performance benefits since the session can then be used to
@@ -497,7 +501,6 @@ public abstract class AuthenticatorBase extends ValveBase
response.setHeader("Expires", DATE_ONE);
}
- int i;
if (constraints != null) {
// Enforce any user data constraint for this security constraint
if (log.isDebugEnabled()) {
@@ -522,7 +525,7 @@ public abstract class AuthenticatorBase extends ValveBase
authRequired = false;
} else {
authRequired = true;
- for(i=0; i < constraints.length && authRequired; i++) {
+ for(int i = 0; i < constraints.length && authRequired; i++) {
if(!constraints[i].getAuthConstraint()) {
authRequired = false;
} else if(!constraints[i].getAllRoles()) {
@@ -545,7 +548,7 @@ public abstract class AuthenticatorBase extends ValveBase
authRequired = certs != null && certs.length > 0;
}
- if(authRequired) {
+ if (authRequired) {
if (log.isDebugEnabled()) {
log.debug(" Calling authenticate()");
}
@@ -817,9 +820,8 @@ public abstract class AuthenticatorBase extends ValveBase
* @param password
* Password used to authenticate (if any)
*/
- public void register(Request request, HttpServletResponse response,
- Principal principal, String authType,
- String username, String password) {
+ public void register(Request request, HttpServletResponse response, Principal principal,
+ String authType, String username, String password) {
if (log.isDebugEnabled()) {
String name = (principal == null) ? "none" : principal.getName();
diff --git a/java/org/apache/catalina/authenticator/FormAuthenticator.java b/java/org/apache/catalina/authenticator/FormAuthenticator.java
index 935486b..809556e 100644
--- a/java/org/apache/catalina/authenticator/FormAuthenticator.java
+++ b/java/org/apache/catalina/authenticator/FormAuthenticator.java
@@ -38,7 +38,6 @@ import org.apache.juli.logging.Log;
import org.apache.juli.logging.LogFactory;
import org.apache.tomcat.util.ExceptionUtils;
import org.apache.tomcat.util.buf.ByteChunk;
-import org.apache.tomcat.util.buf.CharChunk;
import org.apache.tomcat.util.buf.MessageBytes;
import org.apache.tomcat.util.http.MimeHeaders;
@@ -229,9 +228,6 @@ public class FormAuthenticator
}
// Acquire references to objects we will need to evaluate
- MessageBytes uriMB = MessageBytes.newInstance();
- CharChunk uriCC = uriMB.getCharChunk();
- uriCC.setLimit(-1);
String contextPath = request.getContextPath();
String requestURI = request.getDecodedRequestURI();
@@ -575,8 +571,6 @@ public class FormAuthenticator
}
request.getCoyoteRequest().getParameters().recycle();
- request.getCoyoteRequest().getParameters().setQueryStringEncoding(
- request.getConnector().getURIEncoding());
ByteChunk body = saved.getBody();
@@ -611,11 +605,6 @@ public class FormAuthenticator
request.getQueryString();
request.getProtocol();
- request.getCoyoteRequest().queryString().setString
- (saved.getQueryString());
-
- request.getCoyoteRequest().requestURI().setString
- (saved.getRequestURI());
return true;
}
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org
[tomcat] 03/04: Align with 8.5.x. Clean-up
Posted by ma...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
markt pushed a commit to branch 7.0.x
in repository https://gitbox.apache.org/repos/asf/tomcat.git
commit 739fa611e9175632278c24585a2792923a7c9e9b
Author: Mark Thomas <ma...@apache.org>
AuthorDate: Fri Jun 28 09:22:40 2019 +0100
Align with 8.5.x. Clean-up
---
java/org/apache/catalina/Authenticator.java | 12 ++---
.../catalina/authenticator/AuthenticatorBase.java | 56 ++++++----------------
2 files changed, 21 insertions(+), 47 deletions(-)
diff --git a/java/org/apache/catalina/Authenticator.java b/java/org/apache/catalina/Authenticator.java
index 871d471..c49850d 100644
--- a/java/org/apache/catalina/Authenticator.java
+++ b/java/org/apache/catalina/Authenticator.java
@@ -14,8 +14,6 @@
* See the License for the specific language governing permissions and
* limitations under the License.
*/
-
-
package org.apache.catalina;
import java.io.IOException;
@@ -33,19 +31,21 @@ import org.apache.catalina.deploy.LoginConfig;
*
* @author Craig R. McClanahan
*/
-
public interface Authenticator {
/**
* Authenticate the user making this request, based on the login
* configuration of the {@link Context} with which this Authenticator is
- * associated. Return <code>true</code> if any specified constraint has
- * been satisfied, or <code>false</code> if we have created a response
- * challenge already.
+ * associated.
*
* @param request Request we are processing
* @param response Response we are populating
*
+ * @return <code>true</code> if any specified constraints have been
+ * satisfied, or <code>false</code> if one more constraints were not
+ * satisfied (in which case an authentication challenge will have
+ * been written to the response).
+ *
* @exception IOException if an input/output error occurs
*/
public boolean authenticate(Request request, HttpServletResponse response)
diff --git a/java/org/apache/catalina/authenticator/AuthenticatorBase.java b/java/org/apache/catalina/authenticator/AuthenticatorBase.java
index e67cd20..1011bbb 100644
--- a/java/org/apache/catalina/authenticator/AuthenticatorBase.java
+++ b/java/org/apache/catalina/authenticator/AuthenticatorBase.java
@@ -550,6 +550,21 @@ public abstract class AuthenticatorBase extends ValveBase
}
+ @Override
+ public boolean authenticate(Request request, HttpServletResponse httpResponse)
+ throws IOException {
+ if (context == null || context.getLoginConfig() == null) {
+ return true;
+ }
+ return authenticate(request, httpResponse, context.getLoginConfig());
+ }
+
+
+ @Override
+ public abstract boolean authenticate(Request request, HttpServletResponse response,
+ LoginConfig config) throws IOException;
+
+
// ------------------------------------------------------ Protected Methods
/**
@@ -616,47 +631,6 @@ public abstract class AuthenticatorBase extends ValveBase
/**
- * Authenticate the user making this request, based on the login
- * configuration of the {@link Context} with which this Authenticator is
- * associated. Return <code>true</code> if any specified constraint has
- * been satisfied, or <code>false</code> if we have created a response
- * challenge already.
- *
- * @param request Request we are processing
- * @param response Response we are populating
- *
- * @exception IOException if an input/output error occurs
- */
- @Override
- public boolean authenticate(Request request, HttpServletResponse response)
- throws IOException {
- if (context == null || context.getLoginConfig() == null) {
- return true;
- }
- return authenticate(request, response, context.getLoginConfig());
- }
-
- /**
- * Authenticate the user making this request, based on the specified
- * login configuration. Return <code>true</code> if any specified
- * constraint has been satisfied, or <code>false</code> if we have
- * created a response challenge already.
- *
- * @param request Request we are processing
- * @param response Response we are populating
- * @param config Login configuration describing how authentication
- * should be performed
- *
- * @exception IOException if an input/output error occurs
- */
- @Override
- public abstract boolean authenticate(Request request,
- HttpServletResponse response,
- LoginConfig config)
- throws IOException;
-
-
- /**
* Check to see if the user has already been authenticated earlier in the
* processing chain or if there is enough information available to
* authenticate the user without requiring further user interaction.
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org