Community
Participate
Working Groups
When building commercial applications it is often necessary to allow or deny access to actions, views, editors, wizards and perspectives based on the role of the user of this application. Since there is no support for this in RCP one has to build custom solutions, sometimes by modifying parts of RCP, sometimes by doing tricks with actions and plugins. My company has just started to work on an concept to integrate role based access control into RCP. We are looking for requirements and, as soon as the first documents are done, feedback. The goal is to find a non intrusive way to add access control to RCP. If there are already other people working on this we would like to get in contact.
Nice indeed, very nice. I'll keep looking forward what you find out. Do you know about the Equinox-Project working on Security? http://mea-bloga.blogspot.com/2007/07/equinox-and-security.html
You can go a long way with activities. See http://blog.rcp-company.com/2007/06/using-activities-for-user-management.html But only as long as you don't use of "open view" or the perspective chooser... To me, the easy solution would be to plug the holes in the activities solution. Possibly via a new configuration setting. Please note that I assume a lot here about the security of the target machine and the lock-down of the OS. If that is not the case, we will need a completely new framework that is resilient to all the possible attaches via Trojan plug-in...
Adding Kim (our activities expert) to the CC list.
Another interesting approach might use AbstractSourceProvider in conjunction with the IHandlerService, IEvaluationService, ... .
On: http://www.ibm.com/developerworks/library/os-ecl-rcpsec/index.html?ca=drs- you can find a more sophisticated example of RCP rights management with activities.
Just to keep you informed: Jan-Hendrik has started some work on this topic as part of his master thesis. We will submit some kind of position paper here early next week. In this paper Jan will discuss a few proposals based on OUR requirements. We will gladly include other requirements and proposals, but we thought it a good idea to have some starting point.
Created attachment 79894 [details] First thoughts We have finally done some evaluation and written up a small proposal. The main purpose is to get feedback if we're going in the right direction. I'll be at the Eclipse Summit RCP workshop on 2007-10-09. I'd love to discuss our proposal with everyone interested.
Well I think that using Activities(solution 2) is not the way to go nor it is customization of plugin.xml(solution 3), putting the rights-management on top of it is not good idea IMHO and solution 3 is not interactive which means that once the application is running you cann't switch anymore. By the way we using the Expressions-Framework which is also hack but works like a charme. For me the only possibility is to make the Core of Eclipse aware of rights and I don't think that another solution can ever get part of eclipse-core but that are only my 2 cents. I'm also at ESE and will attend the RCP-Workshop so we'll meet each other.
I wish we can just find simple way to do this... hopefully with no additional requirements for additional technologies :-) Thus, I'm not too fund of the new Equinox facilities. Too me, activities fit the bill just fine... if we can just solve the problems with the few present holes... I'll be at ESE as well - though not at the workshop - so I'll try to find you to see if we can find a common set of requirements.
Created attachment 80073 [details] Example of an RCP-Application using ISourceProvider to set expression variables
Thanks for the answers. But this discussion could need more input. More pros and contras for any of these points. And it would be good if someone has any valuable input for point four, mentioned in the pdf document. It's the question how the interface of the authorisation-check should work.
Some intial thoughts from my side on the whole as posted to the newsgroup: ---------8<--------- Hi, I already expressed my thoughts on the whole thing on ESE where I already said that I'd use the Expression-Framework because it's the most flexible and dynamic solution. The only thing that needs to be done is to make it work for Editors and Views (too you can by the way as demostrated on ESE get access to the privileges through the IEvaluationContext inside views and editors). IMHO defining the privileges on start up you loose to many things. Take for example an Editor ("PersonEditor") for Persons, if the Person is part of say Group-A you are allowed to Edit it in "PersonEditor" but not if he/she is in Group-C. Taking your solution you only have to choice to enable/disable the "PersonEditor" but not depending on any dynamic constraints. This makes your great work and effort not useable for me. If I where you I'd define the following base classes: - PrivilegedViewPart: abstract void changeToReadonly(); abstract void changeToNotViewable(); abstract void changeToWrite(); - PrivilegedEditorPart abstract void changeToReadonly(); abstract void changeToNotViewable(); abstract void changeToWrite(); These base classes hide the Expression-Framework implementations behind them and informs the real implementation about the current privilege state of the Part which can react upon it (maybe the base). You could probably use delegation to not implement the logic twice, use an Event-System, a registry where all the privileged WorkbenchParts register them upon, ... . I'll also append this to the bug report. Tom ---------8<---------
Created attachment 82451 [details] Example implementation of a RCP user rights check via transformations. This is an example implementation of a RCP user rights check via transformations. More explanations are in the included readme.pdf.
Jan-Hendrik: did you develop that from scratch? If so, do you know about the existing org.eclipse.equinox.transforms* plugins?
(In reply to comment #14) > Jan-Hendrik: did you develop that from scratch? If so, do you know about the > existing org.eclipse.equinox.transforms* plugins? I didn't develop it from scratch. Actually I took the existing plugins and transformed them, more precisely the XSLT example. The example plugins aren't useful for this, because for this simple transformation they are far to complicate and error-prone, they have _much_ more overhead. They do things which are absolute unnecessary in this case, e.g. things like the OSGi registry tracking with LDAP filters. In the end, this special solution is cleaner, easier to understand and easier to maintain.
Isn't this enhancement related to the ongoing equinox security additions (e.g. bug #198814, #202156 and others...see https://bugs.eclipse.org/bugs/buglist.cgi?query_format=advanced&short_desc_type=allwordssubstr&short_desc=%5Bsec%5D&classification=Eclipse&product=Equinox&long_desc_type=allwordssubstr&long_desc=&bug_file_loc_type=allwordssubstr&bug_file_loc=&status_whiteboard_type=allwordssubstr&status_whiteboard=&keywords_type=allwords&keywords=&emailtype1=substring&email1=&emailtype2=substring&email2=&bugidtype=include&bug_id=&votes=&chfieldfrom=&chfieldto=Now&chfieldvalue=&cmdtype=doit&order=Reuse+same+sort+as+last+time&field0-0-0=noop&type0-0-0=noop&value0-0-0= for a complete list). I see in comment #1 that it's mentioned, but I don't see any further reference to it.
IMHO to some extend it is. In fact to me it looks like te example implementation that uses transformers is comparable with the Equinox project. The problem I see is that there has to be more dynamic support then Equinox can provide (think about RAP how do you deal there with the enabling/disabling of commands, views, ...) there's only this one OSGI-Runtime you don't have one for every authentificated user the Innoopract guys could prove me wrong though.
(In reply to comment #16) > Isn't this enhancement related to the ongoing equinox security additions (e.g. > bug #198814, #202156 and others... Yes. Thanks for pointing this out, Scott! I have posted to both developer mailing lists to let people know about the related work.
To solve the problem via Expressions I must add the expression "visibleWhen", with a property tester, to every "basic" UI element ("basic" should stand for Views, Editors, and similar things). An example XML code: <visibleWhen> <test property="org.eclipse.rcpauth.expressions.rightsProperty" args="read"/> </visibleWhen> But I have a question: while it's quite easy to implement it, once you learned how the code in "org.eclipse.ui.internal.registry", "org.eclipse.ui.internal", "org.eclipse.ui", etc. works together, I have too much possibilities where I can implement my solution. The commands (ext.-point "org.eclipse.ui.commands"), have already the ability to process Expressions, i.e. to process them in all XML declarations which use their command declarations (via "commmandId"). Menus do this, for example. But views, editors and such things are another case: they are not commands, neither in their actual implementation nor in their abstract logic. So I implemented the logic to read and bind the visibleWhen Expression to a View, in the "org.eclipse.ui.internal.registry.ViewRegistryReader#readElement", and store the Expression in the "ViewDescriptor". While the logic to read and store the Expressions for _commands_ is handled in "org.eclipse.ui.workbench.HandlerPersistence#read" (and stored in a global list). Although my solution works very good, I'm not sure what fits better to Eclipses (original?) Expressions concept: Should the basic Expressions handling all be done in HandlerPersistence, or locale, in the Registry-Readers of the UI elements itself? Or should there be a new standalone class and registry for all Expressions? Because this can become quite complex if you take it to a wider, more general approach. This needs to be discussed, and I'm already thinking about a second Bug entry I should open for this alone PS: I really want to plan and implement it for 3.4, in the next few month. Spending (almost) my whole time on this.
(In reply to comment #19) > PS: I really want to plan and implement it for 3.4, in the next few month. > Spending (almost) my whole time on this. Great! Keep in mind that our development work is structured by milestones: # Friday Dec. 14, 2007 - Milestone 4 (3.4 M4) - stable build # Friday Feb. 8, 2008 - Milestone 5 (3.4 M5) - stable build New functionality cannot be crammed in at the last minute. It should be in the codebase well before the M5 milestone. So therefore you should try to have a first prototype ready at the end of M4 which would give us a full six-week milestone cycle to iterate.
(In reply to comment #20) > Friday Dec. 14, 2007 - Milestone 4 (3.4 M4) - stable build > Friday Feb. 8, 2008 - Milestone 5 (3.4 M5) - stable build > New functionality cannot be crammed in at the last minute. It should be in the > codebase well before the M5 milestone. So therefore you should try to have a > first prototype ready at the end of M4 which would give us a full six-week > milestone cycle to iterate. That OK. Please: No one should ever get the impression I'm someone who wants to cram in some code. Stability and performance are important for me. I'm myself an Eclipse User and very satisfied with it. Absolute no interest to ruin this experience by buggy and/or not maintainable code.
Sorry, what I was trying to say is that features like this cannot be added *at all* after a certain point in the development cycle, regardless of code quality. I did not mean to imply that your contributions would be of low quality.
(In reply to comment #19) > While the logic to read and store the Expressions for _commands_ is handled in > "org.eclipse.ui.workbench.HandlerPersistence#read" (and stored in a global > list). Although my solution works very good, I'm not sure what fits better to > Eclipses (original?) Expressions concept: > Should the basic Expressions handling all be done in HandlerPersistence, or > locale, in the Registry-Readers of the UI elements itself? > > Or should there be a new standalone class and registry for all Expressions? > Because this can become quite complex if you take it to a wider, more general > approach. A more general expression evaluation service was added internally in 3.3 and should hopefully be made API in 3.4. The IEvaluationService. Not well documented at the moment, but basically it takes a core expression which can be generated from declarations using ExpressionConverter#perform(IConfigurationElement) and an IPropertyChangeListener plus property name. Currently, the evaluation service is driven by variable change events and the ISourceProviders, which cause some of them to be re-evaluated. A re-evaluation that causes them to change will fire the property change event with Booleans. While it's been a while since I fooled around with JAAS, once you've authenticated you are a Subject (and the framework must have access to role information as well)? If that was a source to the evaluation service, then adding to core expressions could be: <visibleWhen> <with variable="userRoles"> <iterate operator="or"> <equals value="com.example.roles.Admin"/> </iterate> </with> </visibleWhen> Currently, org.eclipse.ui.menus is backed by the IEvaluationService, as is the enabledWhen from org.eclipse.ui.handlers (but not activeWhen). In 3.4 hopefully all handlers will be as well (we don't need 3 or 4 core expression evaluation services, at least not without a really good reason :-) As for views and editors, would we not need hooks similar to the activities hooks so that when something changes and the core expressions are re-evaluated, they can be either 1) turned invisible in the spirit of visibleWhen or 2) just closed if we pick another name for our core expression element, maybe userRights (only less goofy) ? These are just comments for discussion, take them for what they are worth :-) PW
(In reply to comment #23) > > <visibleWhen> > <with variable="userRoles"> > <iterate operator="or"> > <equals value="com.example.roles.Admin"/> > </iterate> > </with> > </visibleWhen> Oh, I see what you mean, probably more likely: <visibleWhen> <with variable="subject"> <test property="org.eclipse.jaas.auth.permission" args="javax.security.auth.AuthPermission" value="read,write,org.eclipse.ui.show"/> </with> </visibleWhen> where org.eclipse.jaas.auth.permission could be a property tester that queries the permission specified in args for the values. PW
(In reply to comment #23) <stuff deleted> > > While it's been a while since I fooled around with JAAS, once you've > authenticated you are a Subject (and the framework must have access to role > information as well)? Yes. In the JAAS Equinox contribution there will be a method (possibly SecurePlatform.getSubject() but perhaps by some other name), that will return an authenticated Subject instance. The Subject then provides access to an arbitrary number of associated Principals via subject.getPrincipals(). Principals are/can be groups, roles, user identifiers, etc...all assumed to be unique and added to the subject by JAAS login modules during the authentication process. Once authentication is complete, the collection returned by getPrincipals() is made read-only. Incidently, there is an enhancement bug #200299 to provide extensible login modules, so that plugins can add principals and credentials to the authenticated Subject during Equinox authentication without defining a whole new login module.
And as an aside, Bug 206745 is to make the IEvaluationService API PW
While I'm trying to widen the IEvaluationService concept from the menu/command concept to the other UI elements I get growing doubts if this is the right way. See: Menus can, and sometimes should be, context-dependent (triggered on/off, made visible/invisible). But Views and Editors are another thing. Either you're allowed to use them, or you aren't. You login as an user with well-defined rights (e.g. as "user"). If you want to do some administration stuff you login as a second user (maybe even without a logout, but as a second login), do your admin stuff, and work further in your restricted login (e.g. in Linux, Windows XP, MacOS, etc.). In reality it's extremely rare that you can switch the user in a running application. IIRC the EclipseCon-2007 developers agreed on that. If you just want to hide them (user- and not rights-dependent), so that users aren't disturbed by overwhelming options, activities are a nice solution. What I want to say: if you think this way, it's extremely simple and will eat much less CPU power and memory. You can just add an "expression.evaluate(...)" call to every registry method which returns the registered UI elements. And hide the elements which failed the check. It would also be super secure. No access to items which don't exist, without any workaround. This is a comment, nothing I want to enforce. Please give me your thoughts and inputs. I want to add features which are really useful and are really used.
(In reply to comment #27) I agree that unless we think dynamically changing roles or access rights are important, we should go for the simple solution.
(In reply to comment #28) > (In reply to comment #27) > > I agree that unless we think dynamically changing roles or access rights are > important, we should go for the simple solution. Dynamically changing access rights seem to me to be part of the functionaligy (available in other products for years): i.e. The user has a certain set of roles, but one role (the ability to update the system) is only valid between 07:30-17:00. I don't remember if that was dynamic permissions on the role or a dynamic role on the user. PW
(In reply to comment #27) > What I want to say: if you think this way, it's extremely simple and will eat > much less CPU power and memory. You can just add an "expression.evaluate(...)" > call to every registry method which returns the registered UI elements. And > hide the elements which failed the check. But the state for your evaluate must come from somewhere ... and to be useful with menus/commands/handlers the correct information must be available to the IEvaluationService. The registry might be one "hook" place, although I suspect it's not that important a hook and definitely not the only one. You could write your hook with: IEvaluationService s = ...; expression.evaluate(s.getCurrentState()) == EvaluationResult.TRUE // etc But this will make the system extremely un-dynamic ... and if your entry point is the registry then why not use the tranforms to remove the offending XML. I would think the similar workflow to the way we can use sudo would be: 1. user requests Admin-type view 2. authenticate 3. user gets some new principals with new permissions 4. the admin view now shows up Although this specific case could be handled with a hook in showView that used the expression.evaluate(*) as well. PW
We also have to take into consideration that the privileg maybe different for different instances of an UI element. Editors: Privilege depends on the InputSource (e.g. a Persondata-Editor) Views: Privilege depends on the Secondary-Id (e.g. a Persondata-Editor) Another I'd like to see solved is the following: Say we have: - Person A - Person B - Person C - Administrator: - Edit privs on people in A and B - Tom - View privs on people in A and B - Edit privs on people in A When I start the programm the initial situation looks like the following: Company A: - Person A - Person B Company B: - Person C This means I can edit data from A and B but can only view data from C. In the meanwhile the Administrator is adding Person C to Company A and suddenly I have edit rights on this Person. What I'm trying to say is that the privilege has not to be attached the *content* the UI-Element in the context of this discussion the Editor/View (implementation at the widget level (Text-Field/...) is not scope of this bug)
(In reply to comment #30) You're right. An on-the-fly sudo is a very good idea. I fear this feature wish will be come sooner or later to discussion anyway, and my whole code must be thrown out of the window, to implement this dynamic behaviour. Even if it's perfect working for the static stuff. I must reconsider my strategy.
Created attachment 83410 [details] Example implementation of user rights check with Expressions. I thought about it again and came to the conclusion that "sudo" and similar behaviors are very uncommon and unlikely in almost every use-case. So I stuck to the method I suggested first, used the general EvaluationService (registered as OSGi service) and modified the UI elements lists. So that every UI element which has an "visibleWhen" Expression which returns "false", is removed from the return list. The remove happens dynamically, and isn't final, so if you alter the user rights and connect the Expressions cache via a Listener to an ISourceProvider (e.g. something like UserRightsSource) you can clear the Expressions cache, and try again to get your wished UI elements. I worry only about the performance, since the UI element lists are checked and altered every time they are requested. On the other hand this doesn't happen very often, additionally the Expression evaluation is cached and never changes (at least in this solution). In the attachment is an example implementation. To find the altered source just search for the line "It's a me JHD". This is intentionally goofy, to make my changes more noticeable. I want to check the code again, add a few more comments before I send it finally into the Eclipse CVS. All the code change was done in the "org.eclipse.ui.workbench" project. Of course the corresponding "org.eclipse.ui/schema/*.exsd" files must all be altered to, or the manifest editor gives unnecessary warnings (these changes are extreme simple, therefore I didn't include the files). Example declaration for view: <extension point="org.eclipse.ui.views"> <view name="Message" allowMultiple="true" class="org.eclipse.rcpauth.expressions.View" id="org.eclipse.rcpauth.expressions.view"> <visibleWhen> <!-- Test it with a property tester --> <test property="org.eclipse.rcpauth.expressions.rightsProperty" args="write"/> </visibleWhen> </view> </extension> The same for Wizards: <wizard id="expressions.wizards.SampleNewWizard" ... ... name="Multi-page Editor file"> <visibleWhen> <test property="org.eclipse.rcpauth.expressions.rightsProperty" args="read,write"/> </visibleWhen> </wizard> and so on (for Editors and PreferencePages).
To the people who want a general enable/disable visible/invisible feature for all UI Elements: This is something that should be handled separate, in a 2nd bug/enhancement entry. It could maybe something more than just these 3 states: visible & enabled visible & disabled invisible It could be a general integer (like "elementState"), with flags defining its state (state = IElementState.VISIBLE | IElementState.ENABLED | IElementState.STICKY). And the possibility to add own states. Because this state handling stuff is directly connected to the real implementation. If you take a Tree-View for example: Should a disabled tree be totally blocked, ignoring all inputs, or should you at least be allowed to expand/collapse the nodes? That depends on your needs. Or you could go all the way and attach an state object which you can override and implement the wildest things. And in this case makes a general ISourceProvider connection _very much_ sense. Because this is far behind the target of this enhancement, I really suggest to open a new enhancement (for the state-handling), since it sounds useful to me.
(In reply to comment #33) > <extension point="org.eclipse.ui.views"> > <view > name="Message" > allowMultiple="true" > class="org.eclipse.rcpauth.expressions.View" > id="org.eclipse.rcpauth.expressions.view"> > <visibleWhen> > <!-- Test it with a property tester --> > <test property="org.eclipse.rcpauth.expressions.rightsProperty" > args="write"/> > </visibleWhen> > </view> > </extension> Do others on this bug agree that the visibleWhen expression should be nested in the view(/editor/wizard...) declaration? This couples the view definition with the access rights for those views, i.e. you couldn't restrict the visibility of a third-party view.
(In reply to comment #35) > Do others on this bug agree that the visibleWhen expression should be nested in > the view(/editor/wizard...) declaration? This couples the view definition with > the access rights for those views, i.e. you couldn't restrict the visibility of > a third-party view. > Do you suggest something like a "masterPlugin.xml" file, like I implemented for the solution with "Transforms"? So you can alter the right definitions for sealed (i.e. signed) bundles?
I was just asking a question :) One way to do this without tying it to the view/editor/... definitions would be to create a new extension point where you list visibleWhen expressions that refer to a view/editor/... by its type and id. We could then look at all visibleWhen expressions that apply to a certain view, i.e. if all of them evaluate to 'true' the view would be available.
(In reply to comment #37) > One way to do this without tying it to the view/editor/... definitions would be > to create a new extension point where you list visibleWhen expressions that > refer to a view/editor/... by its type and id. We could then look at all > visibleWhen expressions that apply to a certain view, i.e. if all of them > evaluate to 'true' the view would be available. > I would certainly prefer this additional level of indirection. Controlling third-party contributions seems impossible without it.
hm to me both make sense although it would be nice if I could control user rights on 3rd party views. Could this also solve the problem of unwanted menu-contributions from other plugins which is a problem for many RCP developers currently (See the many threads and hacks presented on newsgroups). So I'd vote for externalizing, good catch Boris.
One thing just came to my mind is that in you current solution you can only have one right per element. This is maybe fine for Commands and Wizards but not for Views/Editor where you can have multiple instances of the same View/Editor. Is it somehow possible to bind the right to: - secondaryId of the view - editorInput of the editor (i haven't worked with editors so I don't know exaclty what an editor gets passed in)
Re comment #237, then I currently work with a large banking system, where all security management is handled by a separate group of people than the "ordinary" developers. For this, the current activities are just perfect as you can centralize all the security management is a number of separate plug-ins that does not change the "code" plug-ins... (and yes, we have checks in place that means the application cannot run in the production environment unless the security plug-ins are present and properly signed... :-)) So, I would like to see a solution that means security information/configuration is kept apart from contributions themselves...
Re comment #29, we need a way to dynamically change the role or profile of the current user. I currently work on a larger banking system, where we use both profiles and roles: - a profile defines you primary job in the bank. You can have multiple profiles (teller, loan, pension, ...). - a profile is divided into a number of roles, that can be turned on and off by the current user on the fly. We just consider this two levels of the same security concept although one could consider roles = activities.
(In reply to comment #40) > Is it somehow possible to bind the right to: > - secondaryId of the view > - editorInput of the editor (i haven't worked with editors so I don't know > exaclty > what an editor gets passed in) AFAIK there is no way to define the secondaryId or the editorInput in the plugin.xml. The actual editorInput depends on the "class" attribute. And this class may do everything. It's intentionally designed this way to widen the flexibility of editors. And I don't know a way to define secondaryId's. (In reply to comment #35, comment #36, comment #37) > Do others on this bug agree that the visibleWhen expression should be nested in > the view(/editor/wizard...) declaration? This couples the view definition with > the access rights for those views, i.e. you couldn't restrict the visibility of > a third-party view. (comment #38) > I would certainly prefer this additional level of indirection. Controlling > third-party contributions seems impossible without it. There is already a standard extension point: "org.eclipse.core.expressions.definitions". I just gave it a quick test and it worked. If you look at the description of this extension point in the manifest editor, you can see examples how to use it (at the end of the document). To comment #41: I totally agree with you: additionally we need a central masterPlugin.xml which overrides all plugin.xml settings. I suggest to implement it in the same way I already did. I "just" have to extend/change the most basic plugin.xml reading code in the OSGi core. I must see how the OSGi guys think about this.
(In reply to comment #43) > ... I "just" have to extend/change the most basic plugin.xml reading > code in the OSGi core. I must see how the OSGi guys think about this. This two sentences were maybe too short: of course I know the Eclipse OSGi implementation is called Equinox, and don't just copy&paste the OSGi reference implementation with some Eclipse tweaks (since it's only a reference and no real implementation). Just to prevent unnecessary discussions :)
(In reply to comment #33) > Created an attachment (id=83410) [details] > Example implementation of user rights check with Expressions. Some comments on your zip: 1) please make it a patch ... that was painful to apply and as your line endings are different than linux, it shows up the entire files as changes. I'm assuming that was against HEAD? 2) as per other comments, providing a sepearate extension point where we can associated visibleWhen with element-types we support is probably a flexible approach we would want to look at. That will also save you a lot of grief w.r.t. changing all of those API interfaces :-) since it will be an "association" lookup, not a direct "owns" relationship. 3) if you are just evaluating the expressions when you need them, it's probably not a good idea to use the ResultCaches. They depend on something like the EvaluationAuthority to both cache results and clear results when the appropriate source changes. Your utility methods could just execute the code you have in your evaluate() method. 4) using the IEvaluationService global state is good, since it gives your expressions full access to the eclipse state and will also make security sources available to standard expressions (like menus visibleWhen and handlers enabledWhen, and in 3.4 handlers activeWhen). But you just need to modify your expressions with a "with" element. > <visibleWhen> > <!-- Test it with a property tester --> > <test property="org.eclipse.rcpauth.expressions.rightsProperty" > args="write"/> > </visibleWhen> <visibleWhen> <with variable="securityAuthority"> <test property="org.eclipse.rcpauth.expressions.rightsProperty" args="write"/> </with> </visibleWhen> > > I thought about it again and came to the conclusion that "sudo" and similar > behaviors are very uncommon and unlikely in almost every use-case. This kind of dynamic based permissions-roles has been available in JAAS type security setups (most notibly J2EE, Websphere, WebLogic, etc) for over 7 years ... while it might not be something we would want to support in the first pass, it is not that uncommon and unlikely :-) I'll just have a quick look at a helper, see if I can attach something. PW
(In reply to comment #45) > Some comments on your zip: > > 1) please make it a patch ... that was painful to apply and as your line > endings are different than linux, it shows up the entire files as changes. I'm > assuming that was against HEAD? Sorry. Next time I will. I thought it would be a nice solution since it's version independent and you don't have to use CVS to look at them. You just have to overwrite your files from Eclipse 3.4M3. Or don't download any source-code at all. And you can find my changes easily with all my comments. > 2) as per other comments, providing a sepearate extension point where we can > associated visibleWhen with element-types we support is probably a flexible > approach we would want to look at. Done - not by me :) . Look at comment 43. The advantage of using existing solutions. > 3) if you are just evaluating the expressions when you need them, it's probably > not a good idea to use the ResultCaches. They depend on something like the > EvaluationAuthority to both cache results and clear results when the > appropriate source changes. Your utility methods could just execute the code > you have in your evaluate() method. Using this special ResultCache has the advantage that you can easily insert your own ISourceProvider listeners which then update the EvaluationResultCaches and clear them. There are always people who want to connect an ISourceProvider to ... almost everything (OK a little bit exaggerated ;). And caching seems a very good idea to me, since I'm not sure how many calls the evaluation will get. And: think of more complex Expressions which consist of maybe six individual checks. > 4) using the IEvaluationService global state is good, since it gives your > expressions full access to the eclipse state and will also make security > sources available to standard expressions (like menus visibleWhen and handlers > enabledWhen, and in 3.4 handlers activeWhen). But you just need to modify your > expressions with a "with" element. > > > <visibleWhen> > > <!-- Test it with a property tester --> > > <test property="org.eclipse.rcpauth.expressions.rightsProperty" > > args="write"/> > > </visibleWhen> > > <visibleWhen> > <with variable="securityAuthority"> > <test property="org.eclipse.rcpauth.expressions.rightsProperty" > args="write"/> > </with> > </visibleWhen> You're right. But it would make only sense if you would extend the current solution to use ISourceProviders, wouldn't it? > > I thought about it again and came to the conclusion that "sudo" and similar > > behaviors are very uncommon and unlikely in almost every use-case. > > This kind of dynamic based permissions-roles has been available in JAAS type > security setups (most notibly J2EE, Websphere, WebLogic, etc) for over 7 years > ... while it might not be something we would want to support in the first pass, > it is not that uncommon and unlikely :-) Well, yes, I think this isn't something for the first pass. I think it should be later made API. The way I suggested with the answer of point (3) can be a part of the solution. Finally: Thanks. I can't say how happy I am that someone really finally looks at my source-code.
Created attachment 83543 [details] Security through associations This prototype uses the association approach (associate visibleWhen with whatever you want to remove). This is still following Jan's pattern (hook the registries for objects of interest) although I only targetted the EditorRegistry, and only enough to make the default text editor appear/disappear. <extension id="z.ex.dropdown.security" point="org.eclipse.ui.security"> <securityAssocation targetID="org.eclipse.ui.DefaultTextEditor"> <visibleWhen> <systemTest property="z.ex.dropdown.go" value="true"> </systemTest> </visibleWhen> </securityAssocation> </extension> This approach uses the expression/sources/caching already available through the IEvaluationService, and could be modified to be more dynamic in nature ... but it works fine for just adding hooks and querying. Tom's example that adds security as a source would make that available to use, satisfying our "static" case now and potentially dynamic cases in the future. PW
(In reply to comment #46) > Done - not by me :) . Look at comment 43. The advantage of using existing > solutions. I don't think org.eclipse.core.expressions.definitions works quite how we would want in this scenario. It supplies re-usable core expressions to be used by the core expression "reference" element, but its IDs are not relevant outside of an internal core expression reference manager. > Using this special ResultCache has the advantage that you can easily insert > your own ISourceProvider listeners which then update the EvaluationResultCaches > and clear them. There are always people who want to connect an ISourceProvider > to ... almost everything (OK a little bit exaggerated ;). And caching seems a > very good idea to me, since I'm not sure how many calls the evaluation will > get. And: think of more complex Expressions which consist of maybe six > individual checks. That's one advantage of using the IEvaluationService ... it is providing the caching and updating, even if you only every use it as a "hook" query (like the hooks in EditorRegistry in the above example). > > Finally: Thanks. I can't say how happy I am that someone really finally looks > at my source-code. > And thank you for driving this. The little prototypes advance these kind of enhancements far faster than just discussion. PW
(In reply to comment #47) > <extension > id="z.ex.dropdown.security" > point="org.eclipse.ui.security"> > <securityAssocation > targetID="org.eclipse.ui.DefaultTextEditor"> > <visibleWhen> > <systemTest > property="z.ex.dropdown.go" > value="true"> > </systemTest> > </visibleWhen> > </securityAssocation> > </extension> That's a nice solution. I could make a clear implementation easily. But there's one question left: should there be the additional possibility to add local nested <visibleWhen> Expressions? I say: no! Because I prefer your suggestion with externalized declarations and if there are two possibilities to define the <visibleWhen> Expressions the question comes up at a double-definitions: Who "wins"? I would bypass this question, by just implementing the last "external" solution. (I know that the masterPlugin.xml does exactly that: overwriting declarations. But that's another story: it's only _one_ file at an exact defined point, dedicated to overwriting and external declarations. And it's the simplest and most sensible solution for the problem it solves.)
Jan, Please note that there can still be more than one security definition for a single targetID, so you still have to detect the situation. In case of multiple definitions, just report the fact, but please include information about which bundles that was inwolved :-)
(In reply to comment #49) > nested <visibleWhen> Expressions? I say: no! Because I prefer your suggestion > with externalized declarations and if there are two possibilities to define the > <visibleWhen> Expressions the question comes up at a double-definitions: > Who "wins"? I would bypass this question, by just implementing the last > "external" solution. There are a couple of options for this kind of scenario: Some old standby's :-) 1) first/last one in wins (we can even log the losers) 2) sort by contributing plugin id (it adds a level of predictability) Or actually deal with it in the extension point: 3) Apply either "and" (visible when all contributions return true) or "or" (visible when any contribution returns true) semantics 4) Provide something "product level" that would arbitrate conflicts. The most inclusive is #3+or and the most exclusive is #3+and. The IDE would want #3+or, but I could see that RCP apps would most often want #3+and. #4 provides product level control, but we've found the products often have a hard time configuring all of the options they could care about. PW
(In reply to comment #51) > 3) Apply either "and" (visible when all contributions return true) or "or" > (visible when any contribution returns true) semantics > > The most inclusive is #3+or and the most exclusive is #3+and. The IDE would > want #3+or, but I could see that RCP apps would most often want #3+and. You can combine the two; for example, you could have showWhen and hideWhen expressions, and calculate (showWhen1 || showWhen2 ...) && !hideWhen1 && !hideWhen2 ... Use showWhen expressions if you care about showing things in certain situations (feels like the original purpose of activities), and use hideWhen if the current user's role does not allow access to a certain element.
(In reply to comment #51) - and comment #52 > 3) Apply either "and" (visible when all contributions return true) or "or" > (visible when any contribution returns true) semantics > > The most inclusive is #3+or and the most exclusive is #3+and. The IDE would > want #3+or, but I could see that RCP apps would most often want #3+and. Yes, I agree with that. I think the solution in #3 is the one which makes the most sense. With, and only with, an "and" connection. Of course I could add a hideWhen like suggested in comment #52. But all these answers don't answer my real question: Should the general design allow nested <visibleWhen> Expressions? Or should they all be "externally" applied (with a "targetId")? Isn't it much clearer if only one way is allowed? I really think so. That has nothing to do with multiple declarations with the same id's. That's a different story, and I think we already found an appropriate solution.
No more comments? Is this the final agreement for version 3.4? Well, I did the implementation based on Paul Websters patch. And I allowed reference declarations via "targetID" and also nested declarations. I did it for Editors, Views, Perspectives, PropertyPages and Wizards. I also added the possibility to use a masterPlugin.xml which overwrites every plugin xml-node which haves the same id (or class) identifier as defined in the masterPlugin.xml. I admit that the solution for the masterPlugin.xml is only a dirty hack compared to the other solutions, since it's based on Transformations. I'm searching through the equinox registry code and I think it's much more complex than the UI stuff. It won't be easy and is maybe really something that should be done in Eclipse 4.0. I just want to make it clear: The solution using transformations to implement the masterPlugin.xml needs no rights or any other data from the user. It just replaces nodes. I'm not sure if a "hideWhen" is really necessary, because you can already make inverted boolean checks in Expressions, look at the "<not>" tag. I'm thinking that complex concatenations of these (<and>...<or>...</or></and>) should be implemented via a better implementation of expressions. And I didn't post the code yet because I'm waiting for more input, because the code is, in principle, identical to Paul Webster's code. Except that it concatenates multiple definitions with the same ID with boolean "and"s (we agreed already on that). Plus: To implement Paul Webstars code changes was easy. Such changes aren't hard (if you don't have to fight w/ Eclipse Bugs (don't ask)). So: any more input? Any comments? Something that should be implemented? Or should my code already be _the_ _final_ patch.
(In reply to comment #54) > I did it > for Editors, Views, Perspectives, PropertyPages and Wizards. For wizards, did you cover Import, Export, and New wizards? What about preference pages? > So: any more input? Any comments? Something that should be implemented? Or > should my code already be _the_ _final_ patch. Please post your patch so that the interested parties can try to implement their use cases with it and then give more specific feedback.
(In reply to comment #54) > I did it > for Editors, Views, Perspectives, PropertyPages and Wizards. I would like to see the solution for commands, handles and menu contributions as well. We do have the possibility to add the visibility stuff to the present enabledWhen or visibleWhen expressions, but - what about 3rd party contributions? - also I would prefer to have all security related declarations concentrated in one place and not scattered all over the place, as this match the development organization for our current project. > And I didn't post the code yet because I'm waiting for more input, because the > code is, in principle, identical to Paul Webster's code. Except that it > concatenates multiple definitions with the same ID with boolean "and"s (we > agreed already on that). Don't we want to make the "and" configurable? As in "or", "and" and "error"? How difficult will it be to make your changes dynamic so it is possible to change the role assignment at run-time? Also, does the code act nicely if the new security extensions are updated dynamically?
(In reply to comment #56) > (In reply to comment #54) > > > I did it > > for Editors, Views, Perspectives, PropertyPages and Wizards. > > I would like to see the solution for commands, handles and menu contributions > as well. That is already implemented! Just add a few commands. Add the extension point "org.eclipse.ui.menus", add "menuContribution" items, add commands to them, add "visibleWhen" Expressions to them. Done. But this wasn't me! That has already been implemented since Eclipse 3.3, right from the beginning! And to the handlers: that is something we can think about. Whilst the thing we the commands is another story: the designers mentioned explicitly that they want to delegate the "visibleWhen" and different checks to the referers of the commands. > We do have the possibility to add the visibility stuff to the present > enabledWhen or visibleWhen expressions, but > - what about 3rd party contributions? The thing with the 3rd party contributions has already been discussed a few comments before. > - also I would prefer to have all security related declarations concentrated > in one place and not scattered all over the place, as this match the > development organization for our current project. So, should I count your voice against the possibility to add nested Expressions? Or did you mean that you want a central declaration point? To make it clear, as summary: We have now 3 places to declare "visibleWhen" Expressions: - Nested, in the node itself - At a special extension point, with targetIDs. The suggestion from Paul Webster. - With an external masterPlugin.xml which can overwrite all plugin definitions. I admit that I didn't made one thing really clear: the masterPlugin.xml mechanism has the possibility to overwrite almost arbitrary child nodes. Not only the "visibleWhen" nodes. This adds much more flexibility to the management of 3rd party plugins generally. > > And I didn't post the code yet because I'm waiting for more input, because the > > code is, in principle, identical to Paul Webster's code. Except that it > > concatenates multiple definitions with the same ID with boolean "and"s (we > > agreed already on that). > > Don't we want to make the "and" configurable? As in "or", "and" and "error"? As already mentioned: this is something that belongs to a more sophisticated Expressions implementation. And I gave it a quick test yesterday: it looks like they already implemented it. Although I couldn't see this in any of their examples, you can already define things like this: "(a && (b || c))". > How difficult will it be to make your changes dynamic so it is possible to > change the role assignment at run-time? Also, does the code act nicely if the > new security extensions are updated dynamically? Whilst this requirement is really rare it should be possible. The Systemwide EvaluationService should be made API for that. In the time between you can access it via the OSGi/Equinox Service Registry, like this: IEvaluationService eval = (IEvaluationService) PlatformUI.getWorkbench().getService(IEvaluationService.class); ISourceProvider provider = new OwnSourceProvider(); eval.addSourceProvider(provider); Look at the different ISourceProvider implementations in the Eclipse source to understand the concept (e.g. in org.eclipse.ui.workbench). I'm also not really sure who in this project is accountable to make this thing API. Should I do it? Paul Webster? I would prefer the last, because I wasn't the original designer of this service. And don't mix this rights-stuff with the uncluttering of jam-packed workbenchs. That's exactly what Activities were designed for. The Activites have also the great advantage that you can still access every removed element through some menu-points, if you are a professional user who is annoyed by the limited "beginners" workbench. Every professional who had to use crippled software knows how awful it can be if you're forced to use the "newbie" version of a program.
Created attachment 84215 [details] As described in last comments, after idea of Paul Webster. This is the example implementation I promised. Sorry for this delay. But I had to fight with broken Eclipse setups (due to many experiments). I have already an example Implementation I used for testing, but it isn't (as Microsoft would say) "housewive" ready. I want to make it to a more end-user compatible testing application.
Sorry, I forgot to mention it: I renamed "visibleWhen" to "accessibleWhen", because that's what it is: an UI element which failed the Expressions check isn't available at all! It's just not only invisible, you even won't find it with "Ctrl+3" in Eclipse. Also to access it programmatically, like "object.getId("org.bla")", is impossible if it fails the check. It's directly "locked" at the root. And to answer another question: with an implementation so deep at the roots, the one implementation for wizards handles them all: the import-, export- and new-wizards. I admit the solution for property pages isn't very nice (yet!): instead of a completely removed property you can still select the property page entry, but you will see a blank page if the Expressions check failed.
I am trying to use the patch for dynamic role support, but I'm running into some issues. Maybe someone with more (any) familiarity with the EvaluationService can point me in the right direction. It is my understanding that I can't force the reevaluation of a property without having a variable assigned to it. Is that correct? Unfortunately, when I run the SDK, I get an exception ("The variable userRole is not defined"). I have not found any way to create a variable at plugin.xml time; is there one? Any ideas would be appreciated. -Ernest I have a view defined with an accessibleWhen clause: <extension point="org.eclipse.ui.views"> <category id="RoleAwarePlugin" name="Sample Category"> </category> <view category="RoleAwarePlugin" class="roleawareplugin.views.SampleView" icon="icons/sample.gif" id="roleawareplugin.views.SampleView" name="Sample View"> <accessibleWhen> <with variable="userRole"> <test property="com.sas.ViewTester" args="" forcePluginActivation="true"/> </with> </accessibleWhen> </view> </extension> I have defined a property tester: <extension point="org.eclipse.core.expressions.propertyTesters"> <propertyTester class="com.sas.TestPropertyTester" id="com.sas.TestPropertyTester" namespace="com.sas" properties="ViewTester" type="java.util.Collections$EmptyList"> </propertyTester> </extension>
Sorry, I could have at least released some example plugin.xml declaration code. Here it is: <extension point="org.eclipse.core.expressions.propertyTesters"> <propertyTester id="expressioneclipsetst.tester.propertyTester" namespace="expressioneclipsetst.tester" properties="rightsProperty" type="java.lang.Object" class="expressioneclipsetst.tester.AuthentificationPropertyTester"/> </extension> <extension point="org.eclipse.ui.newWizards"> <wizard class="expressioneclipsetst.NewWizard" id="ExpressionEclipseTst.NewWizardID" name="New Wizard"> <accessibleWhen> <test property="expressioneclipsetst.tester.rightsProperty" args="newWizardRole"/> </accessibleWhen> </wizard> </extension> I have actually never used a variable. I just used this straight and direct way.
Do you know how to make the test property get re-evaluated? You hinted at a way in an earlier post (about using a source provider), but it was not obvious to me how to get it to work. It seemed to require a variable, which is how I got started down that path.
Created attachment 84340 [details] Role based source provider Here is an example source (a la 3.3) that provides "roles" ... it is a prototype, but should give you an idea of how you can use a source in the IEvaluationService. This source would work for org.eclipse.ui.handlers/enabledWhen and org.eclipse.ui.menus/visibleWhen This example won't really work for org.eclipse.ui.handlers/activeWhen (without some work). In 3.4 there should be a slightly better way of registering variable sources than tracking down the services and adding them manually. In 3.4 we're also considering API to tell the framework to re-evaluate property tester expressions. PW
(In reply to comment #62) > Do you know how to make the test property get re-evaluated? You hinted at a > way in an earlier post (about using a source provider), but it was not obvious > to me how to get it to work. It seemed to require a variable, which is how I > got started down that path. I will add the re-evaluation to the example I promised.
(In reply to comment #58) > Created an attachment (id=84215) [details] > As described in last comments, after idea of Paul Webster. > Some comments ... one of the reasons to go with association based security is to avoid making massive changes to API. Things like IEditorDescriptor, etc, those changes just don't look necessary. Same for changing all of those schema definitions, if you have an association based extension point then that's a lot of change for incremental benefit. Also, you don't appear to be using the IEvaluationService, but your AccessibleListener only evaluations the expression once (and then never again). In general, what would be a good practice for targetting the security? IDs for the different areas (views, editors, wizards, preference pages, commands, handlers, UI elements, etc) are not unique across the application. Each extension point specifies how it deals with duplicate IDs within its own extension point, but there is no restriction on IDs between, say, org.eclipse.ui.commands and org.eclipse.ui.menus. Should the securityAssociation include a way to target registered object? or maybe extension point readers? Maybe the helper isAccessible(*) would take a type of some kind that would help identify which security association it cares about (i.e. EditorRegistry could use isAccessible("org.eclipse.ui.IEditorDescriptor", "editor.id.in.question") ) PW
(In reply to comment #65) > Some comments ... one of the reasons to go with association based security is > to avoid making massive changes to API. Things like IEditorDescriptor, etc, > those changes just don't look necessary. Same for changing all of those schema > definitions, if you have an association based extension point then that's a lot > of change for incremental benefit. I really asked if the people would like nested Expressions, and I got the impression they liked it. And as I experimented with it I found it really convenient to have this additional feature. And it's the same as in the implementation for the menu commands with the "visibleWhen" Expression. But this is very important for me: I made _no_ "massive changes to the API". Look at the code: every UI element (except 1) I handled has already the getter "String getId()". I just moved this definition to a basic Interface and let them all inherit it. It's one "extends IBasicUIDescriptor". Nothing more, nothing less. The only one which hadn't it, had something identical but with a different name, so I could add my method to the class with the same return value. Additionally I edited some interfaces and added the hint to the javadoc that the result depends on the success/failure of the Expressions check. Something like: "Returns the Objects, except the objects which failed the Expressions check." > Also, you don't appear to be using the IEvaluationService, but your > AccessibleListener only evaluations the expression once (and then never again). Sorry, I admit I didn't pay full attention to the re-evaluation for the AccessibleListener. So I implemented the IPropertyChangeListener interface, but forgot to re-add this line in the constructor: "evalService.addEvaluationListener(expression, this, ACCESS_CHANGE, null);" (while ACCESS_CHANGE could be a String like "accessChange"). However it's still not really that what anybody of us would call "convenient" if the EvaluationService is still not API. This should maybe improved anyway if the EvaluationService API definition is final. > In general, what would be a good practice for targetting the security? IDs > for the different areas (views, editors, wizards, preference pages, commands, > handlers, UI elements, etc) are not unique across the application. Each > extension point specifies how it deals with duplicate IDs within its own > extension point, but there is no restriction on IDs between, say, > org.eclipse.ui.commands and org.eclipse.ui.menus. Should the > securityAssociation include a way to target registered object? or maybe > extension point readers? Maybe the helper isAccessible(*) would take a type of > some kind that would help identify which security association it cares about > (i.e. EditorRegistry could use isAccessible("org.eclipse.ui.IEditorDescriptor", > "editor.id.in.question") ) Yes, that makes especially sense if you look at stickyViews and normal Views. The stickyViews target with their id to Views, so they have the same id as the views, that's confusing. A 2nd id like "org.eclipse.ui.stickyView" could help.
I should have mentioned it earlier: To the patch and the included .exsd files: There is a small bug which makes the nested "accessibleWhen" tags non-optional for some ui elements. I already fixed it last Friday. I just wasn't sure if I should clog this extreme long bug entry for this, since the only result of this failure are some yellow exclamation marks in the manifest editor, in its xml-editor page.
Should AccessibleListener implement eclipse.jface.util.IPropertyChangeListener? Right now, it appears to implement org.eclipse.core.runtime.Preferences.IPropertyChangeListener. I don't know if it makes any significant difference, but the evalService.addEvaluationListener takes the jface.util version. At least in 3.4M3.
(In reply to comment #68) > Should AccessibleListener implement eclipse.jface.util.IPropertyChangeListener? > Right now, it appears to implement > org.eclipse.core.runtime.Preferences.IPropertyChangeListener. I don't know if > it makes any significant difference, but the evalService.addEvaluationListener > takes the jface.util version. You're right. I noticed the same as I added the line (addEvaluationListener). Use the jface.util imports only: import org.eclipse.jface.util.IPropertyChangeListener; import org.eclipse.jface.util.PropertyChangeEvent; I didn't expect that someone really would patch this himself after the comment. Nice to read that :)
(In reply to comment #66) > I really asked if the people would like nested Expressions, and I got the > impression they liked it. And as I experimented with it I found it really > convenient to have this additional feature. And it's the same as in the > implementation for the menu commands with the "visibleWhen" Expression. All I'm saying is that this is a lot of change in API that isn't necessary considering the associations nature of the main extension point (which is necessary because it needs to solve the 3rd party contribution problem). It's also important to different audiences of developers (some RCP developers might care a lot for it, but other RCP developers and IDE plugin developers wouldn't), so putting it all over the place might be overkill. Also, some of those extensions already have visibleWhen-type core expressions (which would have similar access to variable sources and property testers). > But this is very important for me: > > I made _no_ "massive changes to the API". If you updated those classes, that's a "massive change", but also unnecessary since it's simply to extract an ID. Changing the signature to isAccessible(String id) would accomplish the same thing without having to touch all of those classes. > Additionally I edited some interfaces and added the hint to the javadoc that > the result depends on the success/failure of the Expressions check. Something > like: "Returns the Objects, except the objects which failed the Expressions > check." That's OK, it would be important to update the public methods that can have their "results" changed based on evaluations. PW
(In reply to comment #70) > > But this is very important for me: > > > > I made _no_ "massive changes to the API". > > If you updated those classes, that's a "massive change". That's very important for me, I want to know this before everything else! Although I read the Eclipse coding guidelines, I didn't know that. I thought an API change would be the removal of methods, or changing their general behavior. I mean, I did nothing of this. Every IViewDescriptor, IEditorDescriptor... has still its getId() method, which does the same (IViewDescriptor view, view.getId() will still work the same way). Nobody runs into trouble, using a "getId()" on some element, it's the same as before, it's still in all interfaces. Only that I comes now from a super-interface, what makes no difference only that you have now the great advantage of compiler support for type safety. What makes really sense because these objects have all very much in common, I thought that this basic descriptor would be used in the future as accumulation-bin for more things which should be added to all ui elements.
I really hoped I had the time to play around with the patches today, but I seem to have run out of time now... Basically I have three requests, I hope is part of the patches (in priority order): - It should be possible to add all the security related stuff in separate plug-ins. - It should be possible to update plug-ins dynamically and get all the expressions re-evaluated. - It should be possible to change profiles/roles dynamically. We do this in some of our present applications, so we really hope this is possible in the new framework as well... Jan-Hendrik, do you know if this is present in the current patches or not?
(In reply to comment #72) > - It should be possible to add all the security related stuff in separate > plug-ins. Well, that should be possible. But it's hard to say what "security related stuff" means. Most plugin.xml files are processed after the Eclipse login screen. The authentification for that can be done by JAAS, which isn't ready yet. Or your own solution which has maybe a lower startuplevel, or whatever. At least something that is ready before the login screen. It's hard to say, since the possibilities seem endless. We could easily start extreme long discussions about this. > - It should be possible to update plug-ins dynamically and get all the > expressions re-evaluated. I added this function basically. But what I won't support is the automatic removal or addition of elements which got different rights. If you want to realize this you have to attach your ui elements them self as listener to the EvaluationService. PS: As discussed in comments #65, #66, #68 and #69, you have to add one line and change two imports to make the current code re-evaluation able. > - It should be possible to change profiles/roles dynamically. That is totally in your hand. Every evaluation of Expressions calls a handler you defined. You must make sure that the parameters (like Admin, Backup...) you get as an input are treated up-to-date.
(In reply to comment #70, #71) I searched again at Eclipse Coding Guidelines and found something related in the _Binary_ compability. There I found, for interfaces, the following two points: > Move API method up type hierarchy: > If method in supertype must be implemented by Client > Breaks compatibility (7) > (7) Moving methods [...], the net effect of moving an API method to a supertype > is an API method being added to the supertype; the subtype is not normally > affected because the API method would still be inherited. Source: http://wiki.eclipse.org/Evolving_Java-based_APIs_2 That's what I'm talking about: "not normally affected". I don't know what's so abnormal about this situation that this might not work? So, after reading this and my comment #71, do you still mean it's that bad?
(In reply to comment #74) > That's what I'm talking about: "not normally affected". I don't know what's so > abnormal about this situation that this might not work? > So, after reading this and my comment #71, do you still mean it's that bad? > Bad is not the word :-) But with only getId() to offer, it seems like a lot of churn for very little return. Plus in general we try and avoid making multiple unrelated objects tied together in a hierarchy, although sometimes it is necessary. PW
I've gotten Jan's code to work (mostly) and I have a couple of questions (some of them are more general than his patch). I am using 3.4M3. Thanks for any answers. 1. Is there an expression other than a property test that would be more convenient to use for expression evaluation? All I really need I think is a hook that I could use to return true/false. Or does using the Evaluation Service require a property to allow things to be dynamic? 2. I patched Jan's code to add the evaluation listener as below. However, I think it's odd for the property name to be "UserRoles" instead of "com.sas.ViewTester" (which is what I declared in my property tester). Does it matter what this property is called? Does it have to match the property name that I use to fireSourceChanged() in my SourceProvider implementation, or anything else? public AccessibleListener(Expression expression) { this.expression = expression; evalService.addEvaluationListener(expression, this, "UserRoles", null); } 3. My property tester test method seems to get called a lot. I don't anticipate this being a peformance problem, but it seems wrong.
Created attachment 84454 [details] Activities based prototype v01 An activities based prototype. It allows you to create an activity and tie it to a core expression ... if you include a security source (like attachement 84340 - Role based source provider) then you tie your activity to your security with an enabledWhen expression. The equinox security will be based on JAAS, so a source with variables that make sense for JAAS would probably also make a good prototype (I'll see about digging some out of the incubator). If we were considering this for 3.4, we would probably use activities with some enhancements, like if it's gone the user really can't get it back (no "show me all" checkboxes), using the activity manager filtering in more places to make sure that views and editors cannot be accessed, and an option to pick between ANDing activity enablement and ORing activity enablement. My example targetted an IDed menu contribution: <extension point="org.eclipse.ui.activities"> <activity id="org.eclipse.ui.examples.sources.activity" name="Menu X security"> <enabledWhen> <with variable="org.eclipse.ui.examlpes.roles"> <iterate operator="or"> <equals value="user"> </equals> </iterate> </with> </enabledWhen> </activity> <activityPatternBinding activityId="org.eclipse.ui.examples.sources.activity" pattern="org\.eclipse\.ui\.tests/org\.eclipse\.ui\.tests\.menus\.itemX5"> </activityPatternBinding> </extension> PW
(In reply to comment #77) There were different reasons I didn't want to use activities: they have a bunch of features which are absolute wrong for this. I posted them already above as "First thoughts". But to make it more clear I can repeat it in a variation: - It will become intransparent complicated if you want to mix different activity sets with access activities sets. Say you want independent activities for uncluttering desktops, and they should be context-sensitive too. And these activities should be mixed with activities removing elements role based. It could become a mess because they're so similar and different at the same time. - The pattern binding with regular expressions. Since you want security not to be bound vaguely, you always have to use this awkward "bla\. \. \." declaration. - The for activities reasonable double-definition (activityPatternBinding) is extreme annoying for user-rights. What sense makes this 2nd definition anyway? None! 95% of the users seem to want a declaration of needed roles/rights on each element and not a declaration of roles with 1000 times repeating declarations of rights at target elements. I tell you: if you go this way, it can easily end in an administration hell. Since you have to repeat all settings for each role you create. So, if this shouldn't be an armchair decision of a tiny group of people it had at least to look like this: <extension point="org.eclipse.ui.activities"> <securityActivity targetId="org.eclipse.ui.tests.menus.itemX5" name="Menu X security"> <enabledWhen> <with variable="org.eclipse.ui.examlpes.roles"> <iterate operator="or"> <equals value="user"> </equals> </iterate> </with> </enabledWhen> </activity> </extension> <!-- Removed this role-to-targets stuff --> Well, and for a tight security the acitivies should really be checked at a low level in the element registry classes (or similar classes). So that no programming failure, or someone who just re-uses this classes directly, can accidentally break the security. But if you take all this in consideration, tell me: where is the big difference to my solution? Remember that you would have to maintain two different types of activities, look at my example declaration. They are not that similar to the classic activities. And I think everyone agrees that a model should never be brutally forced to fit to old code. Especially in this case where we would need only small API-changes for a major new version; we are not talking about 3.3.X, we're talking about 3.4!
(In reply to comment #78) > - It will become intransparent complicated if you want to mix different > activity sets with access activities sets. Say you want independent activities > for uncluttering desktops, and they should be context-sensitive too. And these > activities should be mixed with activities removing elements role based. It > could become a mess because they're so similar and different at the same time. Providing role based security to target elements is already complicated ... but it is conceptually the same as activities. It's just activities so far have had one main usecase: progressive disclosure (hiding and showing) of complicated UIs to users. Now we're adding a second usecase. You can drive activity enablement using a core expression (which can then pick up a security source provider). > - The pattern binding with regular expressions. Since you want security not to > be bound vaguely, you always have to use this awkward "bla\. \. \." > declaration. flexibility == complexity. We could make it a much simpler syntax by making sure that you must specify every element you want to remove ... however it is common to restrict all contributions from a certain plugin (like the admin plugin), and that would mean tracking down and specifying all IDs. > - The for activities reasonable double-definition (activityPatternBinding) is > extreme annoying for user-rights. What sense makes this 2nd definition anyway? In order to support applying user-rights management to 3rd party contributions (and that is a requirement) you need this association-like syntax ... since it can meet all of the other requirements, why sprinkle this throught all of the existing extension points (it's just for syntactic sugar), and becomes a code maintenance nightmare. Also, that would mean the plugin that defines its security would not be easily re-used in a slightly different product (with different roles, etc). > So, if this shouldn't be an armchair decision of a tiny group of people it had > at least to look like this: > <extension > point="org.eclipse.ui.activities"> > <securityActivity > targetId="org.eclipse.ui.tests.menus.itemX5" > name="Menu X security"> > <enabledWhen> > <with > variable="org.eclipse.ui.examlpes.roles"> > <iterate > operator="or"> > <equals > value="user"> > </equals> > </iterate> > </with> > </enabledWhen> > </activity> > </extension> > <!-- Removed this role-to-targets stuff --> with minor enhancements to the target, that's a possibility. You wouldn't normally target one menu item, you would probably group several menu items, 2 views, and an editor into one activity and then control that with the admin role. That would be one activity element, 2 or 3 activityPatternBindings, and one core expression (which are verbose). The above syntax can't do that. > Well, and for a tight security the acitivies should really be checked at a low > level in the element registry classes (or similar classes). So that no > programming failure, or someone who just re-uses this classes directly, can > accidentally break the security. There are already some activity hooks in the same places you want (like EditorRegistry). This would just involve adding more hooks to make sure we catch the places that matter for the new use case (security). > > But if you take all this in consideration, tell me: where is the big difference > to my solution? Actually, your solution seems to be "security activities". Hooks that make stuff disappear when they should be disabled. RegistryExpressionHelper.isAccessibleStatic(desc) is the same as WorkbenchActivityHelper.filterItem(desc). IBasicUIDescriptor is the same as IPluginContribution. You had to add your hooks in the same places (and some more). > And I think everyone agrees that a model should never be brutally forced to fit > to old code. True ... but you are doing almost the exact same thing (controlling what the user can access), you need hooks in the same place, and your first pass implementation even had a similar API shape. This leads me to believe that this not a new-model/old code scenario, but a new usecase for an existing framework. We would definitely need to make sure that the security scenarios in activities would be respected above all else, and progressive disclosure activities were the "secondary viewing criteria". Also make sure that the activities hooks were in all the important places, not just UI access points, and that security activities cannot have their enablement changed by anything other than the core expression. It might also be worth our while to add a different element to distinguish security activities from progressive disclosure activities. PW
Okay, except for the notion of preferred enforcement (making the role-based activities paramount), what are Activities missing? Hooks for some type of UI elements? Will the UI elements disappear when the activity is disabled? I wouldn't want editors to close or views to disappear (I think apps have to handle that themselves), but I expect toolbar and menu items to disappear, views to disappear from the show view list, and perspective icons to go away.
Strange, so many interested parties and no votes, I gave it a least mine because RCP really lacks this.
I'm in agreement with Paul. The solution that you're outlining smells so much like activities that it makes no sense to re-implement it as something distinct. If it was offering some kind of true "security" I'd say go for it but as it stands I think Pauls idea of enhancing activities makes much more sense. Not only would it benefit those who want to use them for roles but also those who're using them in their more traditional capacity. Not only that, but the UI team is spread pretty thin. If we can avoid maintaining yet another API we'll be very grateful. On the topic of activities... back in 3.1 we decided to introduce the ITriggerPointAdvisor - a beast that could help products decide how activities could be utilized. It's rather shallow at the moment but much of the behavioral changes you're needing for this solution could be expressed as new abilities of the advisor. Then, RCP products that wish to use activities as roles would simply use the RoleTriggerPointAdvisor that would tweak activity behavior such that "show all" was removed, activities should be "and"'d, and whatnot.
Hello, I tried the JAAS equinox security sample from CVS. If you are successfully authenticated the user has a subject, which is available through the singleton SecurityPlatform. This subject contains different so called Principals. the principals will be created and set from the LoginModule. with equinox security you can add you own LoginModule via extensions. So each plugin could have it's own module if neccessary. You can say that each principal stands for role or a group. The principal can contain custom information. This principal represents a role. The interface is: package java.security; public interface Principal { public abstract boolean equals(Object obj); public abstract String toString(); public abstract int hashCode(); public abstract String getName(); } In my opinion, this should be considered. I use the equinox security stuff to authenticate a RCP agains a Geronimo2 JEE5 server, with my own LoginModule. The EJB's resp. each method in a bean can run in it's own context, which is represented by an principal. I can define the roles of the methods by myself in a descriptor-xml. So on deployment time the JEE server knows all the user-group-role mappings. I user "X" authenticates his subject will get each role, which is assigned to him. I think this is a good solution and should be considered in this discussion. Mark
Re comment 82 : I think we actually want "true" security. There are several related work areas under development: => Trusted / non-trusted bundles (see bug 211745) => Code-based authorization (Sword4J http://www.alphaworks.ibm.com/tech/sword4j ) => JAAS integration (see http://www.eclipse.org/equinox/incubator/security/ ) It is likely that those elements (once implemented) could be used to provide authentication information for the UI side. Probably we'll have some "EquinoxPrincipal" associated with the Subject to provide name of the user and roles associated with it. The SecurePlatform class from the Equinox security incubator can register listeners to be triggered on login / logout. If we combine OS integration, JAAS framework, and bundle signing we can end up with a very real solution.
(In reply to comment #79) > ... It's just activities so far have had one main usecase: progressive > disclosure (hiding and showing) of complicated UIs to users. > > Now we're adding a second usecase. You can drive activity enablement using a > core expression (which can then pick up a security source provider). I hope you mean _total_ disappearance with disablement!? > > - ... Since you want security not to > > be bound vaguely, you always have to use this awkward "bla\. \. \." > > declaration. > > flexibility == complexity. We could make it a much simpler syntax by making > sure that you must specify every element you want to remove ... however it is > common to restrict all contributions from a certain plugin (like the admin > plugin), and that would mean tracking down and specifying all IDs. We could at least add a second alternate attribute with a none regular-expression string. > > - The for activities reasonable double-definition (activityPatternBinding) > > is > > extreme annoying for user-rights. What sense makes this 2nd definition > > anyway? > In order to support applying user-rights management to 3rd party contributions > ... I can agree with that. I did a similar (but not identical) implementation with the "org.eclipse.ui.security" extension point. But this extended concept is OK. It just looked to me first as someone wanted the old "apply single role to multiple targets" stuff back. > There are already some activity hooks in the same places you want (like > EditorRegistry). This would just involve adding more hooks to make sure we > catch the places that matter for the new use case (security). I'm not totally convinced. Look at EditorRegistry and: findEditor, getDefaultEditor and getSortedEditorsFromPlugins. They all don't deny the "existence" of any EditorDescriptors. > > ... > Actually, Hooks that make > stuff disappear when they should be disabled. > > RegistryExpressionHelper.isAccessibleStatic(desc) is the same as > WorkbenchActivityHelper.filterItem(desc). IBasicUIDescriptor is the same as > IPluginContribution. You had to add your hooks in the same places (and some > more). I admit they are similar. But what I find really disturbing is this: "... your solution seems to be \"security activities\". Hooks that make stuff disappear when they should be disabled"! Didn't I say this often enough? The target isn't disabling, it's total removing! That's it! It shouldn't be just disabled in the toolbar, or removed from the toolbar and visible through something else like Ctrl+3 (where it is disabled but visible). It's a known experience that if people see something: they want it. Even if they have no use for it. They feel cramped. In worst cases coming always to the administrator and demanding more and more, I think many administrators experienced that already. And that's a big difference to the original target of activities. Elements which fail the check are denied completly. Even the _disabled_ existence of them must be denied. I'm happy that Ernest Pasour in comment #80 agrees with that. Sorry if I misunderstood you Paul, but I must make this absolute clear. > ... It might also be worth our while to add a > different element to distinguish security activities from progressive > disclosure activities. Yes. It should have definitely a second tag, like "securityActivity". Well, I will look at the activities and how I can implement a second Activities-API sub-branch for security with total removal of ui elements (this includes also the answer to Kim Hornes posting in comment #82). To JAAS: I think Paul (PW) and I are totally aware of it. But JAAS adds to my knowledge only security checks to big things like JARs, plugins and whatever. For us is only the Authentication/Authorization part interesting where we get the roles the currently logged on user has. And as soon as we implemented our UI stuff, we will add a possibility to hook into JAAS, or melt everything together -- we'll see.
(In reply to comment #85) > > flexibility == complexity. We could make it a much simpler syntax by making > > sure that you must specify every element you want to remove ... however it is > > common to restrict all contributions from a certain plugin (like the admin > > plugin), and that would mean tracking down and specifying all IDs. > We could at least add a second alternate attribute with a none > regular-expression string. org.eclipse.ui and org\.eclipse\.ui aren't different enough and painful enough to warrant another attribute with extra parsing ... although this is a minor point, and could be added afterwards if enough developers were interested. > > There are already some activity hooks in the same places you want (like > > EditorRegistry). This would just involve adding more hooks to make sure we > > catch the places that matter for the new use case (security). > I'm not totally convinced. Look at EditorRegistry and: findEditor, > getDefaultEditor and getSortedEditorsFromPlugins. They all don't deny the > "existence" of any EditorDescriptors. That's why I said "...involve adding more hooks to make sure we catch the places that matter for the new use case". It doesn't do it now, but it's just a case of adding the filterItem(*) hooks in more places. > I admit they are similar. But what I find really disturbing is this: "... your > solution seems to be \"security activities\". Hooks that make stuff disappear > when they should be disabled"! Didn't I say this often enough? The target isn't > disabling, it's total removing! This I think is just a communications glitch on my part (since disable has a very specific meaning in UI parlance). Yes, if the activity that matched an IEditorDescriptor was disabled then that IEditorDescriptor should never be returned from the system, anywhere. > To JAAS: > I think Paul (PW) and I are totally aware of it. But JAAS adds to my knowledge > only security checks to big things like JARs, plugins and whatever. JAAS does provide user authentication and authorization ... through its use of Subjects, Principals, and Permissions. In our case, though, the JAAS work becomes the driver for a Security Source Provider that activity core expressions (and most of the other core expression users) can access. But any given RCP could provide its own Security Source Provider as long as it provided the correct variable source with the correct format (a Collection of String roles, for example, although that will still need to be refined). PW
(In reply to comment #85) > To JAAS: > I think Paul (PW) and I are totally aware of it. But JAAS adds to my knowledge > only security checks to big things like JARs, plugins and whatever. For us is > only the Authentication/Authorization part interesting where we get the roles > the currently logged on user has. And as soon as we implemented our UI stuff, > we will add a possibility to hook into JAAS, or melt everything together -- > we'll see. Hello, I think Authorization is a very important part. It defines what I can do. The roles of EJB decide if I can use the method or not. In the RCP it is the same. Authorization decides wether I can use a view or not. Now you can go more in detail and say define a role for viewing, editing or showing only certain controls, viewers, sorters. Further I find it a good solution in EJB to have a so called sessioncontext. In a view this would mean, you get your SessionContext instance via getSite(). The session context allows me to run methods like isUserInRole(String roleName) or methods to get the Principal. If you won't change the existing API, make a framework. Maybe a framework is a better solution. For example databinding. You can use it or not. If I don't need it I have no dependencies to the databinding framework. Mark
Thanks for taking some action on this issue. I am currently struggling to hide certain functionality from the user using contexts. I have already thought about a source provider solution, as it could present the logged in subjects roles fairly easy. Currently I am rolling my own JAAS bases login system using the 3.3 Splashscreen extension. However, I am experimenting with on-the-fly logins, that can change your permissions. Lets say you are logged in as an Editor and quickly want to execute a administrative task. You could choose to log in as admin, your subject gets new roles and when you logoff as admin, those roles are revoked from your current subject. The UI should update appropriately. Of course using Activities all those "show all wizards" stuff has to be removed as suggested by Kim in #82 using the ITriggerPointAdvisor. Voted for the bug as this a crucial feature for mature RCP apps.
(In reply to comment #87) > If you won't change the existing API, make a framework. Maybe a framework is a > better solution. For example databinding. You can use it or not. If I don't > need it I have no dependencies to the databinding framework. Equinox is developing a framework that deals with both sides of the issue (is the code from a valid source, is the user who they say they are, is the user allowed to do that). Some of it will be finished for 3.4 and some more will be available afterwards. AFAIK it is JAAS based (and already has prototypes available in the equinox incubator). This bug is not addressing those issues. Instead it is dealing with "I have an authenticated user with some roles ... how do I use that information and have the UI respect my restrictions on what the user is allowed to do/see?" i.e. they can't access views, they can't see or execute certain commands, etc. PW
It would be interesting to have some Riena guys involved in this discussion
(In reply to comment #90) > It would be interesting to have some Riena guys involved in this discussion Actually I wrote already in their newsgroup about this bug. But they are likely still in an very decision early state and still don't know _themself_ what they want to do.
(In reply to comment #90) > It would be interesting to have some Riena guys involved in this discussion Actually I wrote already in their newsgroup about this bug. But they are likely still in a very early decision state and still don't know _themself_ what they want to do.
In terms of hiding UI elements, I am also interested in hiding help content associated with disabled activities. So regular help and cheat sheets at least.
(In reply to comment #77) While I'm trying different solutions (yours, mine), I'm asking myself: Shouldn't the activities have the ability to have 3 states? 1. Removed completly. 2. Disabled. That means: disappeared from the workbench, but accessible through some menus (like Ctrl+3, Show View, ...). For simple customization for uncluttered workspaces. 3. Enabled. Your solution is only able to have 2 states: 1 and 3. I prefer my 3-state solution because we're at this already and the wish will come up sooner or later anyway. Please don't argue again this will maybe not be easy to program/maintain. It's not that hard, and shouldn't be considered as limit at every single step.
(In reply to comment #94) > (In reply to comment #77) > While I'm trying different solutions (yours, mine), I'm asking myself: > Shouldn't the activities have the ability to have 3 states? > > 1. Removed completly. > 2. Disabled. That means: disappeared from the workbench, but accessible through > some menus (like Ctrl+3, Show View, ...). For simple customization for > uncluttered workspaces. > 3. Enabled. > > Your solution is only able to have 2 states: 1 and 3. I prefer my 3-state > solution because we're at this already and the wish will come up sooner or > later anyway. > > Please don't argue again this will maybe not be easy to program/maintain. It's > not that hard, and shouldn't be considered as limit at every single step. > I would argue that we should have all or nothing everywhere, pervasively. I dont see the value that three states brings to the table - it just seems like a bug that we're not disabling everywhere.
(In reply to comment #94) > 1. Removed completly. > 2. Disabled. That means: disappeared from the workbench, but accessible through > some menus (like Ctrl+3, Show View, ...). For simple customization for > uncluttered workspaces. > 3. Enabled. Assuming I understood your request correctly, we shouldn't use the term disabled for that. That means visible but not clickable in UI discussions. With the proposed solution we have 2 different levels of activities, "progressive disclosure" activities and "master" (used for role-base UI) activities. The "progressive disclosure" activities will continue to work the way they do now. i.e. certain parts of the UI will disappear and be replaced by the random assortment of "show all perspectives available" buttons. The "master" activities will go much deeper to meet the role-based access control usecase. i.e. We won't return the model object from the registry, there's not "show all" button to make it come back, etc. Does this not cover off your #2 request? PW
Created attachment 84896 [details] Activities based prototype v02 Works the same way, but movies the responsibility for being enabled from the Set of IDs to the Activity object itself. PW
(In reply to comment #97) > Created an attachment (id=84896) [details] > Activities based prototype v02 > > Works the same way, but movies the responsibility for being enabled from the > Set of IDs to the Activity object itself. > > PW > This seems to work pretty well for me for menus and views. A couple of things: 1) I haven't been able to hide cheatsheets. 2) Some things could be more dynamic. For instance, once a perspective is shown, the perspective bar doesn't update if an activity corresponding to a perspective is disabled (The drop down menu updates, but not the icons on the bar). Should the activity remove these items, or is there an update call I can make?
(In reply to comment #98) > This seems to work pretty well for me for menus and views. > > A couple of things: > 1) I haven't been able to hide cheatsheets. The new-style hooks (that allow normal activities to pass but restrict core-expression based activities) haven't been added yet. I'll try and update the prototype with the new hooks and maybe some cheatsheet restriction points. > > 2) Some things could be more dynamic. For instance, once a perspective is > shown, the perspective bar doesn't update if an activity corresponding to a > perspective is disabled (The drop down menu updates, but not the icons on the > bar). Should the activity remove these items, or is there an update call I can > make? > On the bar it's an open perspective. If it's on the perspective bar, think of it more like an open editor or view. We don't currently close editors or views if the user roles make them inaccessible. PW
Created attachment 85215 [details] Activities based prototype v03 This is the activities prototype v3. What this describes: 1) any activity that has a core expression cannot be enabled programmatically (it only accepts the core expression state) 2) IIdentifiers used to be enabled based on this pseudo expression: Any matching activity is enabled or there are no matching activities. Now that activities can have a core expression, IIdentifier enablement is the above ANDed with "no core expression activity is disabled" 3) we've moved some of the IIdentifier enablement logic into a new ITriggerPointAdvisor method, updateIdentifier(IIdentifier identifier, Set changedActivityIds); 4) The hook that would go into, say, the EditorRegistry in places we are not to return workbench model objects for core expression activities is part of WorkbenchActivityHelper: restrictUseOf(ITriggerPoint triggerPoint, Object object) And we've provide a trigger point: WorkbenchActivityHelper#TRIGGER_WORKBENCH_MODEL. The intent of this trigger point is "consider only activities with core expressions" The above method and trigger point have the property that they only restricts the Object based on the state of any core expression activities. This allows the current use-cases to continue to function, i.e. progressive disclosure activities tend not to be visible (WorkbenchActivityHelper#filterItem(*)) but are still available from the workbench model. It could be that this logic could be refactored into WorkbenchActivityHelper#allowUseOf(*), and just by virtue of using the TRIGGER_WORKBENCH_MODEL trigger point it would work as expected. 5) this also includes uses of the new (albeit verbose) hook in EditorRegistry and CheatSheetRegistryReader ... all of the SDK model objects that need to disappear will need to have the hooks applied in the correct place (this will only make it disappear, AFAIK the cheatsheets won't come back :-) 6) the setup for updateIdentifier(*) in our WorkbenchTriggerPointAdvisor is not ideal as is ... it needed access to package level methods in Identifier, plus 2 internal classes. This boundry would require much closer scrutiny. PW
Created attachment 85216 [details] Role based source provider v02 I've included an update to the role based source provider. 1) it includes 3 activity examples: an org.eclipse.ui.tests menu contribution, the default text editor, and a PDE cheatsheet 2) I spelled the "examples" variable correctly :-) PW
(In reply to comment #100) > Created an attachment (id=85215) [details] > Activities based prototype v03 > > This is the activities prototype v3. What this describes: > > 1) any activity that has a core expression cannot be enabled programmatically > (it only accepts the core expression state) Just to confirm, when you say "programmatically", you mean via the workbench activity manager (IActivityManager)? > > 2) IIdentifiers used to be enabled based on this pseudo expression: Any > matching activity is enabled or there are no matching activities. Now that > activities can have a core expression, IIdentifier enablement is the above > ANDed with "no core expression activity is disabled" > > 3) we've moved some of the IIdentifier enablement logic into a new > ITriggerPointAdvisor method, updateIdentifier(IIdentifier identifier, Set > changedActivityIds); > > 4) The hook that would go into, say, the EditorRegistry in places we are not to > return workbench model objects for core expression activities is part of > WorkbenchActivityHelper: > restrictUseOf(ITriggerPoint triggerPoint, Object object) > > And we've provide a trigger point: > WorkbenchActivityHelper#TRIGGER_WORKBENCH_MODEL. The intent of this trigger > point is "consider only activities with core expressions" > > The above method and trigger point have the property that they only restricts > the Object based on the state of any core expression activities. This allows > the current use-cases to continue to function, i.e. progressive disclosure > activities tend not to be visible (WorkbenchActivityHelper#filterItem(*)) but > are still available from the workbench model. > > It could be that this logic could be refactored into > WorkbenchActivityHelper#allowUseOf(*), and just by virtue of using the > TRIGGER_WORKBENCH_MODEL trigger point it would work as expected. > > 5) this also includes uses of the new (albeit verbose) hook in EditorRegistry > and CheatSheetRegistryReader ... all of the SDK model objects that need to > disappear will need to have the hooks applied in the correct place (this will > only make it disappear, AFAIK the cheatsheets won't come back :-) This seems to work as advertised. Thanks! Does this preclude making the cheatsheets dynamic in the future? (I hope not). > > 6) the setup for updateIdentifier(*) in our WorkbenchTriggerPointAdvisor is not > ideal as is ... it needed access to package level methods in Identifier, plus 2 > internal classes. This boundry would require much closer scrutiny. > > > PW >
(In reply to comment #102) > Just to confirm, when you say "programmatically", you mean via the workbench > activity manager (IActivityManager)? Correct. You can't drop another plugin in and use setEnabledActivityIds(*) to override the core expression. > > 5) this also includes uses of the new (albeit verbose) hook in EditorRegistry > > and CheatSheetRegistryReader ... all of the SDK model objects that need to > > disappear will need to have the hooks applied in the correct place (this will > > only make it disappear, AFAIK the cheatsheets won't come back :-) > > This seems to work as advertised. Thanks! Does this preclude making the > cheatsheets dynamic in the future? (I hope not). The activities and IIdentifiers themselves are dynamic in nature ... it should just be a case of finding the all of the cheatsheet hooks. i.e. if getting the cheatsheets filtered them out (as opposed to reading them, which is where I put the hook) then if the matching activity became enabled the next call to get the cheatsheets would see them (which is where we would put the editor registry hooks). We don't own the cheatsheets plugin, though, User Assistance does. PW
(In reply to comment #100) > Created an attachment (id=85215) [details] > Activities based prototype v03 Oh, there's a bug in the WorkbenchActivityHelper#restrict(*) method. It should only consider IIdentifier#isEnabled() if one or more of the activities in question has a core expression. Otherwise it will filter out objects with progressive disclosure activities, and we don't want that. PW
Created attachment 85294 [details] A rough-draft of Expressions security melted with Activities This is a rough-draft. At this patch it means that the code compiles, runs, and _seems_ to work flawless. But I gave it no real tests. I lost so many times to test code which was thrown away, I skip this step. Please, don't focus on "what works", but "_how_ the code works". What do you think about the concepts in the code? The conceptual difference to PWs code is, that every activity can have 4 (actual 3) states: - disabled via Expressions, deactivated via "classic ways" --> totally disabled. - disabled via Expressions, activated via "classic ways" --> totally disabled. - disabled via Expressions, deactivated via "classic ways" --> partially disabled/enabled. - disabled via Expressions, deactivated via "classic ways" --> totally enabled. So, even if it's made visible by Expressions, the end-user can turn it "off" for uncluttering of the desktop. Even if the activities have Expression definitions which have been turned "on". PS: CheatSheets support isn't build in yet.
(In reply to comment #105) > > The conceptual difference to PWs code is, that every activity can have 4 > (actual 3) states: > - disabled via Expressions, deactivated via "classic ways" > --> totally disabled. > - disabled via Expressions, activated via "classic ways" > --> totally disabled. > - disabled via Expressions, deactivated via "classic ways" > --> partially disabled/enabled. > - disabled via Expressions, deactivated via "classic ways" > --> totally enabled. Is this table a cut/paste error? > - disabled via Expressions, deactivated via "classic ways" > --> totally disabled. and > - disabled via Expressions, deactivated via "classic ways" > --> partially disabled/enabled. seem to be invalid. > So, even if it's made visible by Expressions, the end-user can turn it "off" > for uncluttering of the desktop. Even if the activities have Expression > definitions which have been turned "on". The activities v03 prototype supports this (with the fix to WorkbenchActivityHelper#restrict(*)). PW
(In reply to comment #106) > Is this table a cut/paste error? > > > - disabled via Expressions, deactivated via "classic ways" > > --> totally disabled. > and > > - disabled via Expressions, deactivated via "classic ways" > > --> partially disabled/enabled. > seem to be invalid. No, partially enabled/disabled means: disabled menu points, buttons, etc. but still enabled/reachable through, e.g. the "Ctrl+F3 Menu" or API calls with the correct id as parameter. That wasn't very clear, sorry.
(In reply to comment #107) > (In reply to comment #106) > > Is this table a cut/paste error? > > > > > - disabled via Expressions, deactivated via "classic ways" > > > --> totally disabled. > > and > > > - disabled via Expressions, deactivated via "classic ways" > > > --> partially disabled/enabled. > > seem to be invalid. > > No, partially enabled/disabled means: disabled menu points, buttons, etc. but > still enabled/reachable through, e.g. the "Ctrl+F3 Menu" or API calls with the > correct id as parameter. > That wasn't very clear, sorry. I mean saying this 'disabled via Expressions, deactivated via "classic ways"' There must be a 3rd piece of information to differentiate those 2 scenarios, and it wasn't clear from your table what information you are using. In the prototype v03 the API call (filterItem(object) vs restrictUseOf(workbenchModel triggerpoint, object)) distinguishes between "filter based on the activity state" vs "only filter core expression activities" PW
(In reply to comment #106) > (In reply to comment #105) > > So, even if it's made visible by Expressions, the end-user can turn it "off" > > for uncluttering of the desktop. Even if the activities have Expression > > definitions which have been turned "on". > > The activities v03 prototype supports this (with the fix to > WorkbenchActivityHelper#restrict(*)). Well, I admit I misunderstood one thing in your code, I overlooked it. I will think about a few other enhancements/improvements before I post another patch.
(In reply to comment #109) > Well, I admit I misunderstood one thing in your code, I overlooked it. I will > think about a few other enhancements/improvements before I post another patch. > Some comments on your patch: It's probably not a good idea to include the no-regex stuff ... if that is something you want, please open another bug and patch it there. Don't change the IEditorDescriptor (and other) interfaces to include IPluginContribution ... that should continue to come from the implementation class. Activities design have discussed (and rejected) multi-state activities in the past. If filterItem(*) supports (OR of progressive disclosure activities) ANDed with (AND of core expression activities) and restrictUseOf(*) supports (AND of core expression activities) you will get the desired behaviour. You're idea to provide a restrictUseOf(Object) is a good one, since we know exactly what trigger point we need to get. IActivity#hasExpression() is one way to go, but we prefer the getExpression() ... it could be used by clients outside the framework, and the check is simply "getExpression()!=null" Persistence#readActivityDefinition(*) probably shouldn't be changed to take an IConfigurationElement if all of the others take an IMemento ... but yes, that did lead to the problem where I had to read the core expression elsewhere (or I would need to turn it into a w3c Element). Moving the IEvaluationService out of initializeDefaultServices() might also be a good idea, at least it will be available (although not correct) while the activities are being read ... initializeDefaultServices() will cause it to fire evaluation events to the activities. We're not keen on the reflection stuff in WorkbenchActivityHelper ... processing lists is a good use of the helper, but let the return methods that need to instantiate new typed arrays. WorkbenchPreferenceExtensionNode has been completely reformatted, and that makes it hard to discover what has changed. Also, why can't the code WorkbenchPreferenceExpressionNode just be in the WorkbenchPreferenceExtensionNode, since it has a class cast anyway. And the extension file. We can't call it security (since we can't realistically provide it at our level) and accessible is not a good choice either, since that also has precise meaning when talking about UI. The activities can be enabled or disabled and the UI items themselves can be visible or not ... we should stick to that nomenclature. PW
(In reply to comment #110) > Some comments on your patch: Oh yes! Always nice to see someone reading and commenting it! > It's probably not a good idea to include the no-regex stuff ... if that is > something you want, please open another bug and patch it there. Well, I could open another bug entry for that, but it would be really convenient to have this working and usable already in this patch. The amount of necessary code-changes is very small. Especially since it's already so good programmed, I had to add only 2 lines of code to the patterns "isMatch" method. > Don't change the IEditorDescriptor (and other) interfaces ... > Activities design have discussed (and rejected) multi-state activities in the > past. ... OK. > IActivity#hasExpression() is one way to go, ... [but getExpression() is better] ... Well, I'm always open for _much_ more API extensions, but since you get always unhappy with too much code-changes I went the minimum way. I would prefer actually this: - void #setExpression(Expression expression) {this.expression = expression; hasExpression = (expression != null);} _and_ - boolean #hasExpression() {return hasExpression;} _and_ - Expression #getExpression() {return expression;} Well, if you have special wishes which make sense and are in the area of this enhancement: say it. For example cheat-sheets: I will of course include them, and other similar elements. BTW: I feared much more you would complain about the stuff I did in jface. I overloaded the constructor of the PreferenceManager, because I had to have the possibility to commit my own rootNode, from an child-class instance. > Persistence#readActivityDefinition(*) probably shouldn't be changed to take an > IConfigurationElement if all of the others take an IMemento ... but yes, that > did lead to the problem where I had to read the core expression elsewhere (or I > would need to turn it into a w3c Element). The problem lies somewhere else: there should exist a possibility to create the Expression through an IMemento variable. (But that can't be made in the Expressions core itself, since IMemento is an UI element.) > Moving the IEvaluationService out of initializeDefaultServices() might also be > a good idea, at least it will be available (although not correct) while the > activities are being read ... initializeDefaultServices() will cause it to fire > evaluation events to the activities. > We're not keen on the reflection stuff in WorkbenchActivityHelper ... > processing lists is a good use of the helper, but let the return methods that > need to instantiate new typed arrays. Well, I really dislike the amount of additional overhead. But that's a matter of taste, no need to argue. I will change this. But look at it: it's not _really_ reflection stuff, you will get nice ClassCast Exceptions immediately if you make it wrong! > WorkbenchPreferenceExtensionNode has been completely reformatted, and that > makes it hard to discover what has changed. Ctrl+Shift+F rules ;-) (whoops). > Also, why can't the code > WorkbenchPreferenceExpressionNode just be in the > WorkbenchPreferenceExtensionNode, since it has a class cast anyway. I could fit the whole Expressions logic into this node class; which is already enough, even in this basic state, to pass it over as root node for the PreferenceManager. And the rootNode doesn't need all the WorkbenchPreferenceExtensionNode "crap". > And the extension file. We can't call it security ... OK.
A question to the junit tests in org.eclipse.ui.tests.activities: Since many tests _may_ have Expressionss which change the outcome, should we inherit them and overwrite them? That we have two testing paths: One without Expressions (a.k.a. "classic" activities) and another which is mixed? That could make the outcome of the test results maybe clearer?! So that you can easier distinguish and track down the error source? Or re-write all tests, which makes the test themself likely easier to maintain?
Created attachment 85462 [details] Second draft patch for Expression Activities The second "Expressions-Activities" patch with the changes discussed with Paul Webster. As in the patch before it's a demonstration implementation. Only superficial tests were made. What I found irritating was the fact that Paul Webster moved one Expressions check in a private function in the CheatSheets registry which builds the internal CheatSheets tree (#finishCheatSheets). While I'm no CheatSheets expert it looks like you must re-read the registry if Expressions changed and rebuild the internal tree (or at least it's the only really clean solution in that situation). Instead of this method I suggest to move the Expressions check to the "CheatSheetCollectionElement" class and its methods, as I did it with the WizardCollectionElement class. And only implement Expressions checks in the registry methods which are public and make it possible to "bypass" the "CollectionElement" classes. Because these methods aren't involved in the registry data building process. So it's possible to reflect changes in Expressions without re-reading of the registry.
Created attachment 85488 [details] Activities based prototype v04 OK, the core support prototype v04, which includes: Added the trigger point, and its use will be through a helper method: WorkbenchActivityHelper.restrictUseOf(Object); The workbench simply creates the evaluation service first, and then it is available when initializing the activity support. The ITriggerPointAdvisor now takes the IIdentifier, and determines its enablement soley from public API (it can use the activity IDs to determine the correct enabled state). There is one test :-) there needs to be more. We've decided to separate out activity support and API from use of the API for filtering, so this patch doesn't include any use of restrictUseOf(*). This patch is also missing the arrayToArray(*) type methods from WorkbenchActivityHelper. I'm not too keen on them, but I could see them included in v05 of the activity support patch. I ran out of time. The next step is I guess to 1) decide if you want the list/array support back in WorkbenchActivityHelper and respin the activity support patch and 2) create another patch using restrictUseOf(*) to restrict the model objects. PW
Created attachment 85508 [details] Activities based prototype v05 The v04 activity support prototype with 4 helper methods added: Collection restrictCollection(Collection objects); Object[] restrictArray(Object[] array); Collection filterCollection(Collection objects); Object[] filterArray(Object[] array); The return an object with the same type that is passed in. PW
Created attachment 85517 [details] Registry and Model object v03 This is some of "attachement 85462 : Second draft patch for Expression Activities" converted to use the activities support prototype v05 This patch focuses on use of the new activity support. I didn't include any of the preference node changes as they didn't make any sense to me, I don't see why the restrictions cannot be put in the normal WorkbenchPreferenceExtensionNode if necessary. I've adapted this patch so it can be used as a starting point ... but I think instead of making whole-sale changes, for use of the new activity support we'll need to add the support to one thing at a time. (i.e. first editor registry. then view registry. then wizards. then cheatsheets. etc). For now, please use "attachement 85508 : Activities based prototype v05" and this patch as your starting point for investigations/further updates, and make sure to break up your any new patches into the 2 parts (activity API support or use of the new activity support). We should have some more information before the end of the week. PW
(In reply to comment #116) > I didn't include any of the preference node changes as they didn't make any > sense to me, I don't see why the restrictions cannot be put in the normal > WorkbenchPreferenceExtensionNode if necessary. I won't use your patch. Please read my postings before you throw things out! I explained it already: > BTW: I feared much more you would complain about the stuff I did in jface. I > overloaded the constructor of the PreferenceManager, because I had to have the > possibility to commit my own rootNode, from an child-class instance. ... > > Also, why can't the code > > WorkbenchPreferenceExpressionNode just be in the > > WorkbenchPreferenceExtensionNode, since it has a class cast anyway. > I could fit the whole Expressions logic into this node class; which is already > enough, even in this basic state, to pass it over as root node for the > PreferenceManager. And the rootNode doesn't need all the > WorkbenchPreferenceExtensionNode "crap". You throw the whole jface stuff out of the window?! Now everytime a "get sub child nodes of the root" is used, the damn primitive PreferenceNode is used, which has absolute no clue about activities. And the other way, to hammer Activities into jface just for this, is obviously nonsense. > I've adapted this patch so it can be used as a starting point ... but I think > instead of making whole-sale changes, for use of the new activity support we'll > need to add the support to one thing at a time. (i.e. first editor registry. > then view registry. then wizards. then cheatsheets. etc). Huh? Open new Bug Enhancements for every UI element? Or new patches for every single element? I think that's way to much overhead, there are a dozen UI Elements. What's so bad about including all changes into 2 patches, which change for each UI element just around 12 lines of code? > ... break up ... patches into the 2 parts (activity API support or > use of the new activity support). If I can find always a clear border between them, OK.
(In reply to comment #117) > I won't use your patch. Please read my postings before you throw things out! I did ... but your last patch included the prototype plus so much cruft that you didn't remove (and were asked to). From now on, the only patches we will look at are 1) applying the "activity based prototype v*" patch, followed by 2) applying the "Registry and Model object v*" patch. You can update both patches and add back some of the things that I missed when I updated your patch to Registry and Model v03. Also, the next time you update Registry and Model, you should add your contributor information to the copyright notice at the top of each file. Usually your name and email address, and maybe your company if appropriate. But try not to make gratuitus changes (any changes that are not directly in support of the activity based prototype). Patches need to be clean and as small as possible. re: preference nodes > > BTW: I feared much more you would complain about the stuff I did in jface. I > > overloaded the constructor of the PreferenceManager, because I had to have the > > possibility to commit my own rootNode, from an child-class instance. What I should have said was I don't understand what that has to do with "allowing activities with expressions to control workbench model objects". I see that you need to make the root node also respect activities. The JFace change can be added back. > > I could fit the whole Expressions logic into this node class; which is already > > enough, even in this basic state, to pass it over as root node for the > > PreferenceManager. And the rootNode doesn't need all the > > WorkbenchPreferenceExtensionNode "crap". It's always a good idea to avoid filling stuff up with crap, but this is a workbench level class and is the appropriate place to put them. There's not much to it except either WorkbenchActivityHelper.restrictUseOf(object) or WorkbenchActivityHelper.restrictCollection(collection)... that doesn't look like a lot of crap to me. > You throw the whole jface stuff out of the window?! Now everytime a "get sub > child nodes of the root" is used, the damn primitive PreferenceNode is used, > which has absolute no clue about activities. And the other way, to hammer > Activities into jface just for this, is obviously nonsense. Yes, you're correct, we wouldn't be able to push activities down to the JFace level. > Huh? Open new Bug Enhancements for every UI element? Or new patches for every > single element? I think that's way to much overhead, there are a dozen UI > Elements. What's so bad about including all changes into 2 patches, which > change for each UI element just around 12 lines of code? well, no ... 12 lines of code (maybe) for editors, plus maybe some commands to do with editors, plus tests to make sure that a restricted editor doesn't blow anything up ... Also, while it's sometimes necessary it's generally a bad idea to commit an uber-patch that change access in all of the eclipse registries at the same time. That's a debugging nightmare. Commit the changes to editors + tests. Still working? OK, commit the view changes and tests? Still working? etc. Sometimes we handle that with different bugs, sometimes with different patches on the same bug. For now, leaving just the 2 patches is fine, if we get the go ahead we can decide on the specifics to get everything ready to be committed. > If I can find always a clear border between them, OK. at least for the moment, it's clear. Anything in org.eclipse.ui.internal.activities* is activity support. Using restrictUseOf(*) or restrictArray(*) is use of the new activity support. PW
(In reply to comment #118) PreferenceNode: > It's always a good idea to avoid filling stuff up with crap, but this is a > workbench level class and is the appropriate place to put them. There's not > much to it except either WorkbenchActivityHelper.restrictUseOf(object) or > WorkbenchActivityHelper.restrictCollection(collection)... that doesn't look > like a lot of crap to me. Now it seems much clearer, we're talking at cross-purposes :-D ! I meant it the other way: not that the WorkbenchPreferenceExtensionNode gets overloaded, it's the jface rootNode, in this case the WorkbenchPreferenceExpressionNode, which would get too much overhead. So you would have a rootNode(!) whith the following: - An IConfigurationElement as _must_ constructor parameter. Which must be likely faked with an empty IConfigurationElement. And the IConfigurationElement must have an IObjectManager as constructor parameter, which might not be available at the time of construction, also it's somewhat unnecessary because the manager won't be used at all. - The following complete useless methods and fields: private static final String TAG_KEYWORD_REFERENCE = "keywordReference"; private Collection keywordReferences; private IConfigurationElement configurationElement; private ImageDescriptor imageDescriptor; private Image image; private Collection keywordLabelCache; public Collection getKeywordReferences() public Collection getKeywordLabels() { public void clearKeywords() { public void disposeResources() { public Image getLabelImage() { public String getLabelText() { public ImageDescriptor getImageDescriptor() { public IConfigurationElement getConfigurationElement() Patches: Well, I will split them. But that might often not be possible! Because of the nature of OO-Programming, where the most sensible point for a change is maybe the root class for 2 or more classes. For example: look at the WorkbenchPreferenceExpressionNode (or *ExtensionNode if you wish), the class is the patch for Properties _and_ Preferences _at once_. And it's also already the most elegant solution. There are things which must be sticked together.
I want to add that I don't understand why you removed the type restriction of the "restrictArray" methods in WorkbenchActivityHelper. The change from "IPluginContribution[]" back to "Object[]". I would really prefer the last solution, since we only want IPluginContribution arrays, and not arbitrary Object arrays. What else should these methods get as parameters? Of course it's easier if you save the need for a type conversion, but isn't this just easier in the "lazy" way, instead of the "cleaner programming" way? Remember that we don't handle arbitrary objects and you won't get any exceptions if you pass arbitrary objects which aren't IPluginContribution objects. If we wanted this, we could program maybe better in C, with "void *" and similar.
Another thing: Please don't tell me that you want a single patch for every example implementation (Demo RCP plugin). That's a border-line for me, where it becomes really chaotic. At first many basic patches: the activities, a dozen patches for each UI element, maybe inclusive their tests - which likely can't be extracted sensible enough out of the UI tests, so it's a separate patch for all UI tests anyway, and then also a dozen patches for example Implementations for the readers of this bug. That are like 1 + 12 + 1 + 12 = 26 patches! Which you must puzzle all together in their latest versions. I have no time for puzzling!
Created attachment 85589 [details] Activities based prototype v06 Same as v05 except that the boundary between the helper and the advisor is cleaner. There's also a bit more javadoc on ITriggerPointAdvisor.allow() that supports our code.
(In reply to comment #121) > Another thing: > Please don't tell me that you want a single patch for every example > implementation (Demo RCP plugin). It's only tests for the functionality we're changing in the workbench that matter ... that demo source plugin is just that, a demo. As I said, for now we'll just keep them separate in 2 patches, the activity prototype patch and the registry and model patch. PW
(In reply to comment #120) > I want to add that I don't understand why you removed the type restriction of > the "restrictArray" methods in WorkbenchActivityHelper. The change from > "IPluginContribution[]" back to "Object[]". I would really prefer the last > solution, since we only want IPluginContribution arrays, Actually, in the workbench we almost never want IPluginContribution[]. It's almost always an IEditorDescriptor[] or IViewDescriptor[], etc. Now, if you pass in a IEditorDescriptor[] you'll get that back out (yes, you have to cast but that's not a tragedy). If you pass in an ArrayList, you'll get one back out. As for caring about IPluginContribution, the current behaviour is that WorkbenchActivityHelper can only filter/restrict an instance of IPluginContribution ... something that is not an instance of IPluginContribution will never be filtered (it will always just pass through). Now since we supply the IEditorDescriptors, for example, they'll always also be IPluginContributions, but that won't always be the case for all callers of filterItem(*) or restrictUseOf(*) (and the matching array and collection methods are just helpers). PW
Created attachment 86201 [details] Activities based prototype v07 Expression enabled Activities implementation which has the possibility to allow _additional_ filtering of expression enabled activities. The necessary code-change for that is in WorkbenchTriggerPointAdvisor#updateIdentifier(IIdentifier). That means an element which has been enabled through Expressions can be additionally filtered. An example use of that feature is in the more sophisticated UnitTest "UtilTest#testExpressionEnablement()". While the extended test tests ~3 times more test-cases it must be still much more improved. There is still an non-reproducable bug with the Expression update, which seems to be never triggered by Unit tests.
Created attachment 86203 [details] Registry and Model object v04 See comment #125. Implementation of the UI elements using the enhanced activity abilities.
I'm on a team developing a very custom and not very extensible solution to the roles and permissions problem for a commercial RCP application. I thought I would just listen in to see what develops here. Thanks
Is there any interaction planed with OSGi's User Admin Service?
(In reply to comment #128) > Is there any interaction planed with OSGi's User Admin Service? At first we want to finish this. After that we will look further how we connect this to the user admin service of OSGi, to the Equinox/Eclipse/whatever JAAS implementation and what else makes sense. Even with much smaller patches it's not that easy to get them through. So please be a little patient, we're not ignoring anything generally, but we must finish this first, and I hope this will be the case in the near future.
Are these changes likely to make it into the 3.4 release?
(In reply to comment #130) > Are these changes likely to make it into the 3.4 release? Good question. I will look at the patch in its current form later today (hopefully). Ernest, seeing that you are interested in this - could you review the patch(es) as well? Are you happy with what we ended up with?
(In reply to comment #129) Thanks for the clarification. I did not want to put pressure on you, just asking if this is in the context of this bug. Thanks again
(In reply to comment #131) > (In reply to comment #130) > > Are these changes likely to make it into the 3.4 release? > > Good question. I will look at the patch in its current form later today > (hopefully). > > Ernest, seeing that you are interested in this - could you review the patch(es) > as well? Are you happy with what we ended up with? > The last patch I took was on maybe 12/13. It did pretty much what I needed it to do with the possible exception of dynamically affecting cheatsheets. I have not gotten a chance to download the latest patches. Which patch levels are the candidates that you're evaluating?
Jan-Hendrik: I was not able to apply your patch (Activities based prototype v07) to the current code in HEAD, even though Kim's patch (v06) still applies if you use a high enough fuzz factor. Could you please recreate that patch?
About "Registry and Model Object v04" - why did you change code in JFace?
(In reply to comment #135) > About "Registry and Model Object v04" - why did you change code in JFace? I needed a new overloaded constructor to pass my own extended root node which respects activities.
Created attachment 87862 [details] Activities based prototype v08 Activities based prototype v08. Fixed a lot of merge problems of v07 with the latest code from HEAD.
Created attachment 88412 [details] Activities based prototype v09 Fixed a few bugs in JUnit tests for activities. In the class org.eclipse.ui.tests.activities.DynamicTest. The tests had to be enhanced, because they don't use activities like the normal Eclipse, instead they use their own special testing classes and similar stuff.
First step: The patch "Activities based prototype v06" is released to HEAD, with a couple of changes to method names, Javadoc, and to the default behaviour of the WorkbenchTriggerPointAdvisor.
Jan - your contributions ("Registry and Model object v04" and "Activities based prototype v09") have been approved by the Eclipse Foundation legal staff today. My work so far was based on the last patch by Paul and Kim. Can you help me find the parts of your patch "Activities based prototype v09" that didn't make it in so far? I don't want you to attach a new patch to avoid having to go through the approval process again, but you could help me by listing the classes and methods that I should look at.
(In reply to comment #140) > Jan - your contributions ("Registry and Model object v04" and "Activities based > prototype v09") have been approved by the Eclipse Foundation legal staff today. > My work so far was based on the last patch by Paul and Kim. Can you help me > find the parts of your patch "Activities based prototype v09" that didn't make > it in so far? I don't want you to attach a new patch to avoid having to go > through the approval process again, but you could help me by listing the > classes and methods that I should look at. Could you post all your modified "almost-final" patches here? How should I else know what still needs a fix, or where some of my solutions won't work anymore because you changed or removed some unwanted stuff? Or fixed some bugs that I have fixed too, but in a complete different way?
It's OK. I misinterpreted your posted as: will be released in the next release-cycle, please let us improve it before that. But it's already there. I'm already on it.
Created attachment 90555 [details] Activities based prototype v10 Here are a few changes, and I widened the JUnit tests a little, although there could be more unit tests for this. I thoroughly checked everything you changed and have no problems with it. I want to add that I also widened the possible states of the identifiers. Now they can be enabled by an expression, but additionally be filtered. So that one activity can be: enabled by an expression, but still be disabled and filtered. You can see this easily in the JUnit test.
Comment on attachment 90555 [details] Activities based prototype v10 Jan, didn't you read what I wrote? I explicitly said that I didn't want you to create a new attachment because we would have to go through the IP process all over again.
I marked the attachment "Activities based prototype v10" as obsolete because I won't be looking at it. We cannot go through the IP process again if we want this feature in 3.4 - the next milestone (just after EclipseCon) is our API freeze, and it is approaching fast. Jan, if you still want to help, please do not produce more attachments. Comments are fine, or pointers to code in the patches that were already reviewed (Activities based prototype v09 and Registry and Model object v04).
(In reply to comment #145) > ... Comments or pointers to code in the patches that were already > reviewed (Activities based prototype v09 and Registry and Model object v04). What do you mean with the phrase "pointers to code"? Well I didn't know that every tiny change leads to another IPZilla go-through (even if it's so small). But that's OK. I will post this later. Should I wait for the milestone to be released or post this as a new enhancement?
> Can you help me find the parts of your patch "Activities based prototype v09" > that didn't make it in so far? ... but you could help me by listing the > classes and methods that I should look at. Did you mean this with "pointers to the code"? ?? But I posted this already! Look at the last patch. It's all in there. Every change to your code.
Well, it fell like scales fell from my eyes. There's a small bug! It's no security risk but it's extreme annoying! You should make the same "satori" experience if I tell you the problem: Make a default enabled activity! If you make a default enabled activity, and enable the expression for that activity, the MutableActivityManager will say: hey, this activity _is already in the enabled list_ (!) so, why should I update "activity.enabled"? While this is super-secure (no enablement for anyone), you can only "really" enable the activity if you (a) disable it, and then (b) enable it again. This is extreme useful for torturing users who may have menu entries like "Show View X" which depend on the expression state and only appear (or not being grayed out) if the expression is enabled. So they click on the menu entry and get only "not found" exceptions. Very frustrating.
Hi Boris, if I want to look at the current state of affairs should I just look at 3.4 HEAD? If not - which patches should I be looking at? I understand that you've released Kim's patch and may add some more based on Jan's patches "Registry and Model object v04" and "Activities based prototype v09". Sorry if I ask the obvious, but the large number of posts and patches make it a bit overwhelming for me :-P. Thanks, Elias.
Hi Elias, it is probably best to wait until I have released the remainder of the contributions. At this point, we only have the activities-based mechanism in place. After I have released an updated "registry and model object" patch it would be good if you could look at HEAD and give us feedback.
(In reply to comment #143) > I want to add that I also widened the possible > states of the identifiers. Now they can be enabled by an expression, but > additionally be filtered. So that one activity can be: enabled by an > expression, but still be disabled and filtered. You can see this easily in the > JUnit test. Jan, can you please file a new bug for this? (In reply to comment #148) > If you make a default enabled activity, and enable the expression for that > activity, the MutableActivityManager will say: hey, this activity _is already > in the enabled list_ (!) so, why should I update "activity.enabled"? While this > is super-secure (no enablement for anyone), you can only "really" enable the > activity if you (a) disable it, and then (b) enable it again. Again, could you file a new bug for this?
Created attachment 92003 [details] Registry and Model object v05 Updated patch (Javadoc, removed TODO comments, adapted to API changes, contribution commments).
The two issues listed in comment 151 still need to be addressed. Patch "Registry and Model object v05" released to HEAD >20080309. Should be in the next integration build available from download.eclipse.org/eclipse/downloads - probably called I20080311-0800. Everyone, please give this a try as soon as possible and give us feedback. Feedback should be given in new Bugzillas (but you should refer to them from a new comment on this bug if it is of interest to the cc list members).
Added a new bug about a stack overflow error against the integration build. It may really be a bug, or the code may have changed enough since the last patch I tested against that I'm doing something really wrong. https://bugs.eclipse.org/bugs/show_bug.cgi?id=222627
Verified in I20080327-2251
Marking as verified.
is there any description on how to use this new feature?
(In reply to comment #157) > is there any description on how to use this new feature? > Have a look at http://www.bredex.de/en/rcp-auth/paper.html where you will find some papers and examples. The examples are not maintained but worked last time I tried them (during EclipseCon).
Created attachment 97756 [details] Replace for the outdated RightsSourceProvider (In reply to comment #158) > Have a look at http://www.bredex.de/en/rcp-auth/paper.html where you will find > some papers and examples. The examples are not maintained but worked last time > I tried them (during EclipseCon). The examples are good, but at least one example is no longer up-to-date. It's the example implementation for the current implementation of authorization via expression enabled activities: "Example 3: Authorization using Activity". Replace the "RightsSourceProvider.java" with the one from the attached zip file. You can find the file in the folder: rcp.authorization.activities.example\src\rcp\authorization\activities\example\handlers
Well, I still wonder how it is used now? Are there any other samples available that actually run with the 3.4?
I also tried the example, however I was unable to run it as it was giving me a "Login failure, no correct data provided" when I ran the product configuration. Maybe somebody has an idea on the cause, and also a more detailed help section from A to Z would be useful.
(In reply to comment #161) > I also tried the example, however I was unable to run it as it was giving me a > "Login failure, no correct data provided" when I ran the product configuration. > Maybe somebody has an idea on the cause, and also a more detailed help section > from A to Z would be useful. I should mention that the examples were updated and improved. Maybe that helps. Please reply if that didn't help.
