Review Board 1.7.22


Resouces(js,css) Requests through the shindig /concat servlet/proxy servlet are not signed

Review Request #6085 - Created July 23, 2012 and updated

Zhi Hong Yang
java
SHINDIG-1822
Reviewers
ddumont, rbaxter, ssievers
shindig
see https://issues.apache.org/jira/browse/SHINDIG-1822

Create a gadget that just has a ?
<Content><[CDATA[
<script src="SOME-JS-FILE.js" type="text/javascript"></script>
<link rel="stylesheet" type="text/css" href="SOME-CSS-FILE.css" />
]]>
 </Content>
 
During the content rewrite, the container will create a js link to the Concat servlet that includes that JS or create a css link to proxy servlet that includes that CSS.

A config option as below will be added to container.js so that URLs to the concat/proxy servlet include a security token (st=<gadet-security-token>), via this security token, the request to those js/css file from gadget can be idendified and authorized.

//Enables/Disables securiry token for js, css resources loaded by concat servlet/proxy servlet
"gadgets.admin.enableSecuryTokenForConcat" : "false", 

 
Total:
27
Open:
7
Resolved:
20
Dropped:
0
Status:
From:
Description From Last Updated Status
whitespace Dan Dumont Aug. 2, 2012, 1:40 p.m. Open
whitespace Dan Dumont Aug. 2, 2012, 1:40 p.m. Open
whitespace Dan Dumont Aug. 2, 2012, 1:40 p.m. Open
whitespace Dan Dumont Aug. 2, 2012, 1:40 p.m. Open
whitespace Dan Dumont Aug. 2, 2012, 1:40 p.m. Open
whitespace Dan Dumont Aug. 2, 2012, 1:40 p.m. Open
Rather than introduce a new parameter I would rather reuse shindig.allowUnauthenticated. When set to false it would require STs on ... Ryan Baxter Aug. 15, 2012, 2:21 a.m. Open
Review request changed
Updated (Sept. 3, 2012, 2:03 p.m.)
Posted (Sept. 4, 2012, 4:12 p.m.)
I'm almost of the mind that we should always put an security token on the content proxy url.
I think we might just be able to reuse the anon one, the specific named one really doesn't seem to buy us much.

The basic idea behind it is that it's a request pattern generated by the container to be used later to request stuff from the container.  I think that always forcing a security token on the concat proxy would be fine, and we should also make sure the urls are removed (or don't put them there to begin with) from the concat url so that they don't take up twice the space.

Any thoughts on this?   Ryan? Stanton?
Can you explain this method?  Why is this necessary, and is there code elsewhere that handles this?
  1. it is used in this method:
      private UriBuilder makeUriBuilder(ConcatUri ctx, String authority, String path,Uri resourceUri) {
    	 UriBuilder uriBuilder = ctx.makeQueryParams(null, null);
    	 uriBuilder.setAuthority(authority);
    	 uriBuilder.setPath(path);
    	 uriBuilder.addQueryParameter(Param.TYPE.getKey(), ctx.getType().getType());
    	 // Handle addition of security token to the URI param
    	 if (wantsSecurityToken(ctx.getGadgetIns())) {
    	      boolean securityTokenOnQuery = isTokenNeededForRendering(ctx.getGadgetIns());
    
    	      String securityToken = generateSecurityToken(ctx,resourceUri);
    	      addParam(uriBuilder, Param.SECURITY_TOKEN.getKey(), securityToken, securityToken == null,
    	          !securityTokenOnQuery);
    	 }
    	 return uriBuilder;
      }
    
    I reference and copy this method from DefaultIframeUriManager.java. 
    
I don't think the default impl for this should be true.  You might look up in the feature registry to see if the gadget ends up requiring the security token feature.
  1. Copied same implementation from DefaultIframeUriManager.java,  and there is a comment about why return true:
      // This method should be overridden to provide better caching characteristics
      // for rendering Uris. In particular, it should return true only when the gadget
      // uses server-side processing of such things as OpenSocial templates, Data pipelining,
      // and Preloads. The default implementation is naive, returning true for all URIs,
      // as this is the conservative result that will always functionally work.
    
    I think we can fix it later for all those implmentation in another defect.