[rfc][icedtea-web] PolicyEditor

Andrew Azores aazores at redhat.com
Thu Feb 13 14:14:35 PST 2014


On 02/13/2014 08:55 AM, Jiri Vanek wrote:

>
> Hi!
>
> I have no general issues with this.Considering it was done n best mind;)
> After reading it, I'm against making it separate project. I would much 
> ratehr keep it in itew.. and suggestion .. add new luncher? next to 
> itw-settings javaws.. SimplePolicyEditor?

I didn't get around to that with this patch, but I'll work on that next. 
Either a new launcher beside itweb-settings and javaws, or a standalone 
launcher as a separate project... ;) I really do think this idea has 
some merit. Maybe the editor seems small and simple enough now to stay 
within ITW, but I think it's already got enough utility to be a 
replacement for PolicyTool for a lot of users. Most of whom probably are 
using IcedTea-Web anyway, but not necessarily.

>
> This is ok to head, so you cna continue in run in sandbox more promptly.
>

Not yet ;) and the RunInSandbox/PartiallySigned stuff is still not in 
yet anyway, so hooking this in with those already doesn't make sense to 
me. Anyway, it's already all in-place in the PolicyEditor, what's left 
is just adding a button or something to those dialogs and having them 
launch a PolicyEditor instance, then call addNewCodebase(String) on it 
or provide the -codebase flag in its args.

>
> few notes:
>     permission java.io.FilePermission "${user.home}${/}*", "read";
>     permission java.io.FilePermission "${user.home}${/}*", "write";
>     permission java.io.FilePermission "*", "read";
>     permission java.io.FilePermission "${io.tmpdir}${/}*", "read";
>     permission java.io.FilePermission "${io.tmpdir}${/}*", "write";
>
> are granted by checkboxes which do not fully descibee them (imho) 
> Maybew tooltip should be enough.... 

There are tooltips, for every checkbox...

> Onemore think I noted After I click the send - yo are allowing the app 
> to open multiple simple editor windows in time. This may be dangerous. 
> - maybe some check "underlying file have changed, reload?)
> (but yah, advaced tool do the same )I'm for -
>  - have it as modal dialogue in itw-settings
>  - have it on frame in case of seaprate application (see suggestion in 
> previous email)
>  -  otherwse do as you wish ::)

Hmm. I don't think I like the idea of making it modal in this situation 
(just seems like it is not necessary, and unnecessary modality is really 
irritating to deal with as a user IMO), but "file has changed, 
reload/ignore/quit" does sound good. I will look into that next. 
Besides, even if we make it modal, that doesn't actually protect us 
against concurrent modification very much. Perhaps itweb-settings 
control panel can be made to handle the PolicyEditor in the same way 
that PolicyEditor handles CustomPolicyViewer - if there is no instance, 
create one. If there is an instance, focus it rather than creating another.

I spent a lot more time on this already, so I'm attaching another patch 
with the progress so far.

Notable changes:
- can view "custom permissions" aka permissions in the file that don't 
match any of the checkboxes.
-- can also add/remove these. Adding them is not nearly as powerful as 
the big old PolicyTool, but this is meant as complement, not 
replacement, so that's fine by me
-- only one of these windows is displayed at a time, but I really am not 
sure about the method I used to achieve this. I don't want to make it 
blocking (modal), so is there any other better way than what I've done?
- much, much better model for "default" (checkbox-provided) permissions. 
Not just hardcoded Strings anymore
-- as part of this, the "parsing" of policy files became a bit more 
relaxed. For permissions that specify multiple actions, eg the NetAll 
default permission, the permission will still be recognized as a default 
even if the actions are reordered
-- considering making the "parsing" step case-insensitive but not sure 
on that
-- newline isn't just \n anymore, actually using system line separator 
when "serializing"
- vertical scrollbars always shown just so it's clear that those are 
indeed list views. horizontal shown as needed
- mnemonics and accelerators, even more.

Other notes:
- still no handling for SignedBy or Principal or multiple codebases per 
"block", and I'm not really planning on supporting this. Leave that for 
the heavyweight tool and power users, IMO. If you create an entry like 
this, it will simply be clobbered when you next run PolicyEditor
- "parsing"/"deserializing" is not the most robust. I haven't tested it 
with random garbage malformed text or with comments in the middle of a 
block, etc. These hardening improvements will come a bit later on, right 
now I want to nail down the functionality/visuals. I may end up 
refactoring the models even further, so hardening it now might be a 
waste of effort

I'm planning to add plenty of unit testing to the models introduced with 
this patch, but I'm out of time today and am taking a short day 
tomorrow, so I'm not sure if I'll get those done before the end of the 
week, and I wanted to get this out for at least visuals feedback first.

Thanks,

-- 
Andrew A

-------------- next part --------------
A non-text attachment was scrubbed...
Name: policy-editor-4.patch
Type: text/x-patch
Size: 109743 bytes
Desc: not available
Url : http://mail.openjdk.java.net/pipermail/distro-pkg-dev/attachments/20140213/a1ad4d94/policy-editor-4-0001.patch 


More information about the distro-pkg-dev mailing list