[rfc][icedtea-web] Temporary permissions

[Andrew Azores]

On 03/27/2014 10:30 AM, Jiri Vanek wrote:
> ..snip...
>>>
>>
>> As you said you preferred on IRC, here's the way where each temporary 
>> permission is just newly
>> defined, rather than being directly converted from 
>> PolicyEditorPermissions. I do reuse what I can
>> from them, just to eliminate as much extra hardcoding of Strings as 
>> possible, but the new
>> Permissions are all defined manually. It took a while to figure out a 
>> way to make this look clean
>> and be easy to extend in the future, as well as to be able to nicely 
>> define permissions as
>> reductions from the All Permissions set, but I think I've managed it.
>>
>
> I like this patch.
>
> one mayor nit - it broke 8 tests:
>
> FAILED: 
> testNormalPolicyWithMixedEndings(net.sourceforge.jnlp.security.policyeditor.PolicyEditorParsingTest) 
> Permissions should include READ_LOCAL_FILES
> FAILED: 
> testNormalPolicyWithLFEndings(net.sourceforge.jnlp.security.policyeditor.PolicyEditorParsingTest) 
> Permissions should include READ_LOCAL_FILES
> FAILED: 
> testCommentAfterPermission(net.sourceforge.jnlp.security.policyeditor.PolicyEditorParsingTest) 
> Permissions should include READ_LOCAL_FILES
> FAILED: 
> testPolicyWithCodebase(net.sourceforge.jnlp.security.policyeditor.PolicyEditorParsingTest) 
> Permissions should include READ_LOCAL_FILES
> Passed: 
> net.sourceforge.jnlp.security.policyeditor.PolicyEditorParsingTest.testCommentedLine
> FAILED: 
> testMultiplePermissions(net.sourceforge.jnlp.security.policyeditor.PolicyEditorParsingTest) 
> Permissions should include READ_LOCAL_FILES
> FAILED: 
> testNormalPolicy(net.sourceforge.jnlp.security.policyeditor.PolicyEditorParsingTest) 
> Permissions should include READ_LOCAL_FILES
> FAILED: 
> testNormalPolicyWithHeader(net.sourceforge.jnlp.security.policyeditor.PolicyEditorParsingTest) 
> Permissions should include READ_LOCAL_FILES
> FAILED: 
> testNormalPolicyWithCRLFEndings(net.sourceforge.jnlp.security.policyeditor.PolicyEditorParsingTest) 
> Permissions should include READ_LOCAL_FILES

Ahh good catch, thanks. I slightly changed the PermissionTarget for the 
user home directory and forgot to mirror this in the unit tests, that's all.

>
> ...
>> +
>> +    private class PolicyEditorPopupListener implements MouseListener {
>> +        private final Component parent;
>> +
>> +        public PolicyEditorPopupListener(final Component parent) {
>> +            this.parent = parent;
>> +        }
>> +
>> +        @Override
>> +        public void mouseClicked(final MouseEvent e) {
>> +            menu.show(parent, e.getX(), e.getY());
>> +        }
>> +
>> +        @Override
>> +        public void mousePressed(final MouseEvent e) {
>> +        }
>> +
>> +        @Override
>> +        public void mouseReleased(final MouseEvent e) {
>> +        }
>> +
>> +        @Override
>> +        public void mouseEntered(final MouseEvent e) {
>> +        }
>> +
>> +        @Override
>> +        public void mouseExited(final MouseEvent e) {
>> +        }
>> +    }
>
>
> please use mouse adapter :))
>
>
> ...

Learn something useful every day :)

>
>> @@ -53,6 +53,9 @@ public enum PolicyEditorPermissions {
>>       WRITE_LOCAL_FILES(R("PEWriteFiles"), R("PEWriteFilesDetail"),
>>               PermissionType.FILE_PERMISSION, 
>> PermissionTarget.USER_HOME, PermissionActions.WRITE),
>>
>> +    DELETE_LOCAL_FILES(R("PEDeleteFiles"), R("PEDeleteFilesDetail"),
>> +            PermissionType.FILE_PERMISSION, 
>> PermissionTarget.USER_HOME, PermissionActions.DELETE),
>> +
>>       READ_PROPERTIES(R("PEReadProps"), R("PEReadPropsDetail"),
>>               PermissionType.PROPERTY_PERMISSION, 
>> PermissionTarget.ALL, PermissionActions.READ),
>>
>> @@ -71,6 +74,9 @@ public enum PolicyEditorPermissions {
>>       WRITE_TMP_FILES(R("PEWriteTempFiles"), 
>> R("PEWriteTempFilesDetail"),
>>               PermissionType.FILE_PERMISSION, 
>> PermissionTarget.TMPDIR, PermissionActions.WRITE),
>>
>> +    DELETE_TMP_FILES(R("PEDeleteTempFiles"), 
>> R("PEDeleteTempFilesDetail"),
>> +            PermissionType.FILE_PERMISSION, PermissionTarget.TMPDIR, 
>> PermissionActions.DELETE),
>> +
>>       JAVA_REFLECTION(R("PEReflection"), R("PEReflectionDetail"),
>>               PermissionType.REFLECT_PERMISSION, 
>> PermissionTarget.REFLECT, PermissionActions.NONE),
>>
>>
> Those should (if you wish) be gtrouped undeer write system group.

Done.

>
> Final decissopn - minors are ok. Once fixed this can go in.
>
>
> The unittets  - please fixed. If fixes are minor, then push once 
> fixed.If they will grow, feel free to post another round.
>
> jsut double answer - the unittestsmust be fixed befor push :)
>
> J.

Thanks,

-- 
Andrew A