Re: svn commit: r898965 - in /ofbiz/branches/executioncontext20091231: ./ framework/api/src/org/ofbiz/api/context/ framework/context/src/org/ofbiz/context/ framework/example/data/ themes/bizznesstime/includes/ themes/bluelight/includes/ themes/droppingcrum...

[Adam Heath-2]

[hidden email] wrote:

> Author: adrianc
> Date: Wed Jan 13 22:06:46 2010
> New Revision: 898965
>
> URL: http://svn.apache.org/viewvc?rev=898965&view=rev
> Log:
> Implemented permission filters. Added a user group to the Example component. Main navigation is controlled by the new security design.
>
> Added:
>     ofbiz/branches/executioncontext20091231/framework/context/src/org/ofbiz/context/ContextUtil.java   (with props)
> Modified:
>     ofbiz/branches/executioncontext20091231/BranchReadMe.txt
>     ofbiz/branches/executioncontext20091231/framework/api/src/org/ofbiz/api/context/ThreadContext.java
>     ofbiz/branches/executioncontext20091231/framework/context/src/org/ofbiz/context/AccessControllerImpl.java
>     ofbiz/branches/executioncontext20091231/framework/example/data/ExampleSecurityData.xml
>     ofbiz/branches/executioncontext20091231/themes/bizznesstime/includes/appbar.ftl
>     ofbiz/branches/executioncontext20091231/themes/bizznesstime/includes/secondary-appbar.ftl
>     ofbiz/branches/executioncontext20091231/themes/bluelight/includes/appbarOpen.ftl
>     ofbiz/branches/executioncontext20091231/themes/droppingcrumbs/includes/appbarOpen.ftl
>     ofbiz/branches/executioncontext20091231/themes/flatgrey/includes/appbar.ftl
>     ofbiz/branches/executioncontext20091231/themes/flatgrey/includes/footer.ftl

> Added: ofbiz/branches/executioncontext20091231/framework/context/src/org/ofbiz/context/ContextUtil.java
> URL: http://svn.apache.org/viewvc/ofbiz/branches/executioncontext20091231/framework/context/src/org/ofbiz/context/ContextUtil.java?rev=898965&view=auto
> ==============================================================================
> --- ofbiz/branches/executioncontext20091231/framework/context/src/org/ofbiz/context/ContextUtil.java (added)
> +++ ofbiz/branches/executioncontext20091231/framework/context/src/org/ofbiz/context/ContextUtil.java Wed Jan 13 22:06:46 2010
> @@ -0,0 +1,57 @@
> + *******************************************************************************/
> +package org.ofbiz.context;
> +
> +import static org.ofbiz.api.authorization.BasicPermissions.Access;
> +
> +import java.util.List;
> +
> +import javolution.util.FastList;
> +
> +import org.ofbiz.api.authorization.AccessController;
> +import org.ofbiz.api.context.ArtifactPath;
> +import org.ofbiz.api.context.ThreadContext;
> +import org.ofbiz.base.component.ComponentConfig;
> +import org.ofbiz.base.component.ComponentConfig.WebappInfo;
> +
> +/**
> + * ExecutionContext utility methods.
> + *
> + */
> +public class ContextUtil {
> +
> +    public static List<WebappInfo> getAppBarWebInfos(String serverName, String menuName) {
> +        List<WebappInfo> webInfos = ComponentConfig.getAppBarWebInfos(serverName, menuName);
> +        String [] pathArray = {ArtifactPath.PATH_ROOT_NODE_NAME, null};
> +        ArtifactPath artifactPath = new ArtifactPath(pathArray);
> +        AccessController accessController = ThreadContext.getAccessController();
> +        List<WebappInfo> resultList = FastList.newInstance();
> +        for (WebappInfo webAppInfo : webInfos) {
> +            pathArray[1] = webAppInfo.getContextRoot().replace("/", "");
> +            artifactPath.saveState();
> +            try {
> +                accessController.checkPermission(Access, artifactPath);
> +                resultList.add(webAppInfo);
> +            } catch (Exception e) {}
> +            artifactPath.restoreState();
> +        }
> +        return resultList;
> +    }
> +
> +}

restoreState should be in finally.  You don't handle runtime
exception.  If it was in finally, you wouldn't need the catch.  It's
also bad that you don't log the exception, or rethrow it.

[Adrian Crum-2]

--- On Wed, 1/13/10, Adam Heath <[hidden email]> wrote:

> From: Adam Heath <[hidden email]>
> Subject: Re: svn commit: r898965 - in /ofbiz/branches/executioncontext20091231: ./ framework/api/src/org/ofbiz/api/context/ framework/context/src/org/ofbiz/context/ framework/example/data/ themes/bizznesstime/includes/ themes/bluelight/includes/ themes/droppingcrum...
> To: [hidden email]
> Date: Wednesday, January 13, 2010, 6:31 PM
> [hidden email]
> wrote:
> > Author: adrianc
> > Date: Wed Jan 13 22:06:46 2010
> > New Revision: 898965
> >
> > URL: http://svn.apache.org/viewvc?rev=898965&view=rev
> > Log:
> > Implemented permission filters. Added a user group to
> the Example component. Main navigation is controlled by the
> new security design.
> >
> > Added:
> > 
>    ofbiz/branches/executioncontext20091231/framework/context/src/org/ofbiz/context/ContextUtil.java   (with
> props)
> > Modified:
> > 
>    ofbiz/branches/executioncontext20091231/BranchReadMe.txt
> > 
>    ofbiz/branches/executioncontext20091231/framework/api/src/org/ofbiz/api/context/ThreadContext.java
> > 
>    ofbiz/branches/executioncontext20091231/framework/context/src/org/ofbiz/context/AccessControllerImpl.java
> > 
>    ofbiz/branches/executioncontext20091231/framework/example/data/ExampleSecurityData.xml
> > 
>    ofbiz/branches/executioncontext20091231/themes/bizznesstime/includes/appbar.ftl
> > 
>    ofbiz/branches/executioncontext20091231/themes/bizznesstime/includes/secondary-appbar.ftl
> > 
>    ofbiz/branches/executioncontext20091231/themes/bluelight/includes/appbarOpen.ftl
> > 
>    ofbiz/branches/executioncontext20091231/themes/droppingcrumbs/includes/appbarOpen.ftl
> > 
>    ofbiz/branches/executioncontext20091231/themes/flatgrey/includes/appbar.ftl
> > 
>    ofbiz/branches/executioncontext20091231/themes/flatgrey/includes/footer.ftl
>
> > Added:
> ofbiz/branches/executioncontext20091231/framework/context/src/org/ofbiz/context/ContextUtil.java
> > URL: http://svn.apache.org/viewvc/ofbiz/branches/executioncontext20091231/framework/context/src/org/ofbiz/context/ContextUtil.java?rev=898965&view=auto
> >
> ==============================================================================
> > ---
> ofbiz/branches/executioncontext20091231/framework/context/src/org/ofbiz/context/ContextUtil.java
> (added)
> > +++
> ofbiz/branches/executioncontext20091231/framework/context/src/org/ofbiz/context/ContextUtil.java
> Wed Jan 13 22:06:46 2010
> > @@ -0,0 +1,57 @@
> > +
> *******************************************************************************/
> > +package org.ofbiz.context;
> > +
> > +import static
> org.ofbiz.api.authorization.BasicPermissions.Access;
> > +
> > +import java.util.List;
> > +
> > +import javolution.util.FastList;
> > +
> > +import org.ofbiz.api.authorization.AccessController;
> > +import org.ofbiz.api.context.ArtifactPath;
> > +import org.ofbiz.api.context.ThreadContext;
> > +import org.ofbiz.base.component.ComponentConfig;
> > +import
> org.ofbiz.base.component.ComponentConfig.WebappInfo;
> > +
> > +/**
> > + * ExecutionContext utility methods.
> > + *
> > + */
> > +public class ContextUtil {
> > +
> > +    public static List<WebappInfo>
> getAppBarWebInfos(String serverName, String menuName) {
> > +        List<WebappInfo>
> webInfos = ComponentConfig.getAppBarWebInfos(serverName,
> menuName);
> > +        String [] pathArray =
> {ArtifactPath.PATH_ROOT_NODE_NAME, null};
> > +        ArtifactPath artifactPath
> = new ArtifactPath(pathArray);
> > +        AccessController
> accessController = ThreadContext.getAccessController();
> > +        List<WebappInfo>
> resultList = FastList.newInstance();
> > +        for (WebappInfo
> webAppInfo : webInfos) {
> > +           
> pathArray[1] = webAppInfo.getContextRoot().replace("/",
> "");
> > +           
> artifactPath.saveState();
> > +            try {
> > +             
>   accessController.checkPermission(Access,
> artifactPath);
> > +             
>   resultList.add(webAppInfo);
> > +            } catch
> (Exception e) {}
> > +           
> artifactPath.restoreState();
> > +        }
> > +        return resultList;
> > +    }
> > +
> > +}
>
>
> restoreState should be in finally.  You don't handle
> runtime
> exception.  If it was in finally, you wouldn't need
> the catch.  It's
> also bad that you don't log the exception, or rethrow it.

I think you're not understanding the application. This might help:

http://java.sun.com/javase/6/docs/api/java/security/AccessController.html#checkPermission%28java.security.Permission%29

-Adrian

[Adam Heath-2]

Adrian Crum wrote:

> --- On Wed, 1/13/10, Adam Heath <[hidden email]> wrote:
>
>> From: Adam Heath <[hidden email]>
>> Subject: Re: svn commit: r898965 - in /ofbiz/branches/executioncontext20091231: ./ framework/api/src/org/ofbiz/api/context/ framework/context/src/org/ofbiz/context/ framework/example/data/ themes/bizznesstime/includes/ themes/bluelight/includes/ themes/droppingcrum...
>> To: [hidden email]
>> Date: Wednesday, January 13, 2010, 6:31 PM
>> [hidden email]
>> wrote:
>>> Author: adrianc
>>> Date: Wed Jan 13 22:06:46 2010
>>> New Revision: 898965
>>>
>>> URL: http://svn.apache.org/viewvc?rev=898965&view=rev
>>> Log:
>>> Implemented permission filters. Added a user group to
>> the Example component. Main navigation is controlled by the
>> new security design.
>>> Added:
>>>  
>>    ofbiz/branches/executioncontext20091231/framework/context/src/org/ofbiz/context/ContextUtil.java   (with
>> props)
>>> Modified:
>>>  
>>    ofbiz/branches/executioncontext20091231/BranchReadMe.txt
>>>  
>>    ofbiz/branches/executioncontext20091231/framework/api/src/org/ofbiz/api/context/ThreadContext.java
>>>  
>>    ofbiz/branches/executioncontext20091231/framework/context/src/org/ofbiz/context/AccessControllerImpl.java
>>>  
>>    ofbiz/branches/executioncontext20091231/framework/example/data/ExampleSecurityData.xml
>>>  
>>    ofbiz/branches/executioncontext20091231/themes/bizznesstime/includes/appbar.ftl
>>>  
>>    ofbiz/branches/executioncontext20091231/themes/bizznesstime/includes/secondary-appbar.ftl
>>>  
>>    ofbiz/branches/executioncontext20091231/themes/bluelight/includes/appbarOpen.ftl
>>>  
>>    ofbiz/branches/executioncontext20091231/themes/droppingcrumbs/includes/appbarOpen.ftl
>>>  
>>    ofbiz/branches/executioncontext20091231/themes/flatgrey/includes/appbar.ftl
>>>  
>>    ofbiz/branches/executioncontext20091231/themes/flatgrey/includes/footer.ftl
>>
>>> Added:
>> ofbiz/branches/executioncontext20091231/framework/context/src/org/ofbiz/context/ContextUtil.java
>>> URL: http://svn.apache.org/viewvc/ofbiz/branches/executioncontext20091231/framework/context/src/org/ofbiz/context/ContextUtil.java?rev=898965&view=auto
>>>
>> ==============================================================================
>>> ---
>> ofbiz/branches/executioncontext20091231/framework/context/src/org/ofbiz/context/ContextUtil.java
>> (added)
>>> +++
>> ofbiz/branches/executioncontext20091231/framework/context/src/org/ofbiz/context/ContextUtil.java
>> Wed Jan 13 22:06:46 2010
>>> @@ -0,0 +1,57 @@
>>> +
>> *******************************************************************************/
>>> +package org.ofbiz.context;
>>> +
>>> +import static
>> org.ofbiz.api.authorization.BasicPermissions.Access;
>>> +
>>> +import java.util.List;
>>> +
>>> +import javolution.util.FastList;
>>> +
>>> +import org.ofbiz.api.authorization.AccessController;
>>> +import org.ofbiz.api.context.ArtifactPath;
>>> +import org.ofbiz.api.context.ThreadContext;
>>> +import org.ofbiz.base.component.ComponentConfig;
>>> +import
>> org.ofbiz.base.component.ComponentConfig.WebappInfo;
>>> +
>>> +/**
>>> + * ExecutionContext utility methods.
>>> + *
>>> + */
>>> +public class ContextUtil {
>>> +
>>> +    public static List<WebappInfo>
>> getAppBarWebInfos(String serverName, String menuName) {
>>> +        List<WebappInfo>
>> webInfos = ComponentConfig.getAppBarWebInfos(serverName,
>> menuName);
>>> +        String [] pathArray =
>> {ArtifactPath.PATH_ROOT_NODE_NAME, null};
>>> +        ArtifactPath artifactPath
>> = new ArtifactPath(pathArray);
>>> +        AccessController
>> accessController = ThreadContext.getAccessController();
>>> +        List<WebappInfo>
>> resultList = FastList.newInstance();
>>> +        for (WebappInfo
>> webAppInfo : webInfos) {
>>> +          
>> pathArray[1] = webAppInfo.getContextRoot().replace("/",
>> "");
>>> +          
>> artifactPath.saveState();
>>> +            try {
>>> +            
>>   accessController.checkPermission(Access,
>> artifactPath);
>>> +            
>>   resultList.add(webAppInfo);
>>> +            } catch
>> (Exception e) {}
>>> +          
>> artifactPath.restoreState();
>>> +        }
>>> +        return resultList;
>>> +    }
>>> +
>>> +}
>>
>> restoreState should be in finally.  You don't handle
>> runtime
>> exception.  If it was in finally, you wouldn't need
>> the catch.  It's
>> also bad that you don't log the exception, or rethrow it.
>
> I think you're not understanding the application. This might help:
>
> http://java.sun.com/javase/6/docs/api/java/security/AccessController.html#checkPermission%28java.security.Permission%29

That has no bearing whatsoever.  RuntimeException and Error can happen
at any point.