costin 01/09/09 23:43:02
Modified: src/share/org/apache/tomcat/core ContextManager.java
Request.java
src/share/org/apache/tomcat/modules/aaa
AccessInterceptor.java
Log:
Fix 3441, small enhancement in security constraints handling.
The problem was that ContextManager only checked for user roles to decide if
authorization
is needed.
Now any security-related property will triger the authorize hook ( well, right now
there are
only 2 kinds - transport and user, but it's better to be flexible ). A new field,
securityContext
will hold all the secuirity-related properties for the request and will be set by
AccessInterceptor.
No other changes are needed, except for modules that implement authorize() - they
must
be prepared to deal with situations when only transport constraints are required.
Regarding the transport - we just report an error - we could do a more advanced
operation
like redirect, but that can also be done by users using error directives - or later,
by
a more advanced module.
Revision Changes Path
1.193 +6 -9
jakarta-tomcat/src/share/org/apache/tomcat/core/ContextManager.java
Index: ContextManager.java
===================================================================
RCS file:
/home/cvs/jakarta-tomcat/src/share/org/apache/tomcat/core/ContextManager.java,v
retrieving revision 1.192
retrieving revision 1.193
diff -u -r1.192 -r1.193
--- ContextManager.java 2001/09/03 02:20:08 1.192
+++ ContextManager.java 2001/09/10 06:43:01 1.193
@@ -887,20 +887,17 @@
return;
}
- String roles[]=req.getRequiredRoles();
- if(roles != null ) {
+ Container sct=req.getSecurityContext();
+ if(sct != null ) {
status=0;
BaseInterceptor reqI[];
- if( req.getContext()==null )
- reqI=getContainer().
- getInterceptors( Container.H_handleError );
- else
- reqI = req.getContext().getContainer().
- getInterceptors(Container.H_authorize);
+ // assert( req.getContext()!=null ) - checked in processRequest
+ reqI = req.getContext().getContainer().
+ getInterceptors(Container.H_authorize);
// Call all authorization callbacks.
for( int i=0; i< reqI.length; i++ ) {
- status = reqI[i].authorize( req, res, roles );
+ status = reqI[i].authorize( req, res, sct.getRoles() );
if ( status != BaseInterceptor.DECLINED ) {
break;
}
1.112 +22 -0 jakarta-tomcat/src/share/org/apache/tomcat/core/Request.java
Index: Request.java
===================================================================
RCS file: /home/cvs/jakarta-tomcat/src/share/org/apache/tomcat/core/Request.java,v
retrieving revision 1.111
retrieving revision 1.112
diff -u -r1.111 -r1.112
--- Request.java 2001/09/01 00:57:16 1.111
+++ Request.java 2001/09/10 06:43:01 1.112
@@ -190,6 +190,9 @@
protected Principal principal;
// active roles for the current user
protected String userRoles[];
+
+ // Security properties required by the config ( web.xml constraints )
+ protected Container security;
protected String reqRoles[];
// Association with other tomcat comp.
@@ -597,10 +600,28 @@
reqRoles=roles;
}
+ /** Return the associated security properties for
+ * the request. This is set up during mapping using the configured
+ * constraints. The container holds various security properties that
+ * are checked using authorize() hook. If no security context is set
+ * the authorize hook will not be called.
+ */
+ public Container getSecurityContext() {
+ return security;
+ }
+
+ public void setSecurityContext( Container ct ) {
+ security=ct;
+ }
+
+ /** @deprecated use getSecurityContext
+ */
public String[] getRequiredRoles( ) {
return reqRoles;
}
+ /** @deprecated use setSecurityContext
+ */
public void setUserRoles( String roles[] ) {
userRoles=roles;
}
@@ -1048,6 +1069,7 @@
notAuthenticated=true;
userRoles=null;
reqRoles=null;
+ security=null;
uriMB.recycle();
unparsedURIMB.recycle();
1.15 +34 -1
jakarta-tomcat/src/share/org/apache/tomcat/modules/aaa/AccessInterceptor.java
Index: AccessInterceptor.java
===================================================================
RCS file:
/home/cvs/jakarta-tomcat/src/share/org/apache/tomcat/modules/aaa/AccessInterceptor.java,v
retrieving revision 1.14
retrieving revision 1.15
diff -u -r1.14 -r1.15
--- AccessInterceptor.java 2001/08/24 23:57:45 1.14
+++ AccessInterceptor.java 2001/09/10 06:43:02 1.15
@@ -283,6 +283,9 @@
for( int i=0; i< ctxSec.patterns ; i++ ) {
Container ct=ctxSec.securityPatterns[i];
if( match( ct, path, method ) ) {
+ req.setSecurityContext( ct );
+
+ // Backward compat
String roles[]=ct.getRoles();
String methods[]=ct.getMethods();
String transport=ct.getTransport();
@@ -317,11 +320,40 @@
*/
public int authorize( Request req, Response response, String roles[] )
{
- if( roles==null || roles.length==0 ) {
+ if( req.getSecurityContext() == null && roles==null) {
// request doesn't need authentication
return OK;
}
+ if( roles==null )
+ roles=req.getSecurityContext().getRoles();
+
+ String transp=null;
+ if( req.getSecurityContext() != null ) {
+ transp=(String)req.getNote( reqTransportNote );
+ }
+
+ // Check transport. We only verify "CONFIDENTIAL", other auth modules
+ // could do other tests
+ if( debug > 0 ) log( "Transport " + transp );
+ if( "CONFIDENTIAL".equalsIgnoreCase(transp) ) {
+ if( ! req.scheme().equals("https")) {
+ // We could redirect or do something advanced - but the spec
+ // only requires us to deny access. A nice error handler
+ // would also be nice
+ response.setContentType("text/html");
+ response.setStatus( 403 );
+ req.setAttribute("javax.servlet.error.message",
+ "Invalid transport, CONFIDENTIAL required");
+ return 403;// FORBIDEN
+ }
+ }
+
+ // no roles - the request may have only transport constraints
+ if( roles == null || roles.length==0 ) {
+ return OK;
+ }
+
// will call authenticate() hooks to get the user
String user=req.getRemoteUser();
if( user==null )
@@ -606,3 +638,4 @@
contextM.handleStatus( req, res, 302 ); // redirect
}
}
+
