[xwiki-devs] [VOTE] Merge experimental xwiki-platform-security on master
Hi all, As you have probably notice, I have recently committed an feature-security-authorization branch on platform. I am working on this for a while now and it was the first step to share the outcome of this large refactoring of the initial work done early last year by Andreas. Since the code was not quality compliant with platform but the general structure Andreas has build seems to me well appropriate, I have progressively refactor its code to better fit our real needs. Here is what I have been done: 1) Split in to module api and bridge to allow breaking the currently unavoidable dependency on oldcore. Now only bridge depends on oldcore, and the api does not depends on bridge. As mush as possible has been written in the api (still some code to migrate), and some temporary internal bridge are used to access oldcore stuffs since augmenting the existing document-bridge does not seems appropriate IMO. 2) The initial enumeration of rights as been replaced by a Right class, which could be seen has a pseudo enum, but could be augmented with new rights. To register a new Right, you have to provide a RightDescription to the AuthorizationManager. The description will define the default state, the tie resolution policy, the inheritance policy, the list of entity types for which the right is applicable, the implied rights and if the right could be allowed in read-only mode. So new defined Right will benefit the whole logic of the AuthorizationManager and currently existing one could be declaratively defined. 3) Large renaming to better distinguish stuffs, clarify comments and prepare for future. I have voluntarily not taken existing names to clearly split the old and the new api. In brief, the new right service is now named AuthorizationManager. Internally, it manipulates SecurityReference (as well as UserSecurityReference and GroupSecurityReference, to represent entities, user and group), SecurityRule (representing a right object) and SecurityAccess (representing an access level in the old nomenclature), which are store in a SecurityCache using SecurityRuleEntry (a set of rules) and SecurityAccessEntry (the access of a given user). The AuthorizationManager delegate cache management to a SecurityCacheLoader which loads rules using a SecurityRuleLoader ; and delegate itself the computing of the access for a given user and a set of rules to an AuthorizationSettler. This last one could be overridden to provide specific decision that could not be done in declarative mode. 4) Refactoring was necessary to improve consistency and reduce complexity, and simplify as much as possible; while extending the limitations to allow more rights to be registered. This work has been a little bit opposed to the optimization done by Andreas, in particular on memory usage. But optimization is often the enemy of clean code. 5) Improvement were necessary to better mimic the existing implementation in some peculiar but necessary rules to stay compatible with current working wiki. I tend to reduce as much as possible what is not done declaratively, but there are still some special cases, like delete for creators, deny for other user on explicit allow and admin for wiki owner that are settle by the authorization settler. My implementation should be almost compatible with the old one, except for groups that are currently not checked from the entity wiki, but only from the user wiki. This needs some more refactoring for which I feel inconfortable with, some I'd like to share first. 6) The AuthorizationManager interface has been simplified, providing 2 methods for either checking or verifying an access right (the checking methods throws while the verifying one return a boolean), and one to register a new right. The existing RightService could be bridged on the new implementation using the XWikiCachingRightService class in xwiki.cfg and the new API could be used side-by-side with the old implementation as well. What should still really need to be improved is the unit testing, currently some tests are still awful and incomplete. I already refactor some of them, to provide a better coverage of essential part of the code: the security cache and the default authorization settler. Obviously, any help is welcomed. Since I already have an existing wiki using this implementation successfully and using it for creating new rights for extensions, I would like to merge this new implementation as experimental in platform to have it available for anyone who need it or want to test it, and for you to use in your new experimental development as well. Providing it in platform will encourage it to be finalized and replace the existing implementation. Here is my +1 for the merge on 4.x, WDYT ? -- Denis Gervalle SOFTEC sa - CEO eGuilde sarl - CTO
On Wed, Feb 15, 2012 at 11:17, Denis Gervalle <[email protected]> wrote:
Hi all,
As you have probably notice, I have recently committed an feature-security-authorization branch on platform. I am working on this for a while now and it was the first step to share the outcome of this large refactoring of the initial work done early last year by Andreas. Since the code was not quality compliant with platform but the general structure Andreas has build seems to me well appropriate, I have progressively refactor its code to better fit our real needs. Here is what I have been done:
1) Split in to module api and bridge to allow breaking the currently unavoidable dependency on oldcore. Now only bridge depends on oldcore, and the api does not depends on bridge. As mush as possible has been written in the api (still some code to migrate), and some temporary internal bridge are used to access oldcore stuffs since augmenting the existing document-bridge does not seems appropriate IMO.
2) The initial enumeration of rights as been replaced by a Right class, which could be seen has a pseudo enum, but could be augmented with new rights. To register a new Right, you have to provide a RightDescription to the AuthorizationManager. The description will define the default state, the tie resolution policy, the inheritance policy, the list of entity types for which the right is applicable, the implied rights and if the right could be allowed in read-only mode. So new defined Right will benefit the whole logic of the AuthorizationManager and currently existing one could be declaratively defined.
3) Large renaming to better distinguish stuffs, clarify comments and prepare for future. I have voluntarily not taken existing names to clearly split the old and the new api. In brief, the new right service is now named AuthorizationManager. Internally, it manipulates SecurityReference (as well as UserSecurityReference and GroupSecurityReference, to represent entities, user and group), SecurityRule (representing a right object) and SecurityAccess (representing an access level in the old nomenclature), which are store in a SecurityCache using SecurityRuleEntry (a set of rules) and SecurityAccessEntry (the access of a given user). The AuthorizationManager delegate cache management to a SecurityCacheLoader which loads rules using a SecurityRuleLoader ; and delegate itself the computing of the access for a given user and a set of rules to an AuthorizationSettler. This last one could be overridden to provide specific decision that could not be done in declarative mode.
4) Refactoring was necessary to improve consistency and reduce complexity, and simplify as much as possible; while extending the limitations to allow more rights to be registered. This work has been a little bit opposed to the optimization done by Andreas, in particular on memory usage. But optimization is often the enemy of clean code.
5) Improvement were necessary to better mimic the existing implementation in some peculiar but necessary rules to stay compatible with current working wiki. I tend to reduce as much as possible what is not done declaratively, but there are still some special cases, like delete for creators, deny for other user on explicit allow and admin for wiki owner that are settle by the authorization settler. My implementation should be almost compatible with the old one, except for groups that are currently not checked from the entity wiki, but only from the user wiki. This needs some more refactoring for which I feel inconfortable with, some I'd like to share first.
Forgot to mention that for the same reason, using this new security on an empty database does not allow importing the initial wiki. You need to enable superadmin and log with it. This is another part I d'like to see discussed since fixing this could cause limitation in the evolution we may want to see introduced later.
6) The AuthorizationManager interface has been simplified, providing 2 methods for either checking or verifying an access right (the checking methods throws while the verifying one return a boolean), and one to register a new right.
The existing RightService could be bridged on the new implementation using the XWikiCachingRightService class in xwiki.cfg and the new API could be used side-by-side with the old implementation as well. What should still really need to be improved is the unit testing, currently some tests are still awful and incomplete. I already refactor some of them, to provide a better coverage of essential part of the code: the security cache and the default authorization settler. Obviously, any help is welcomed.
Since I already have an existing wiki using this implementation successfully and using it for creating new rights for extensions, I would like to merge this new implementation as experimental in platform to have it available for anyone who need it or want to test it, and for you to use in your new experimental development as well. Providing it in platform will encourage it to be finalized and replace the existing implementation.
Here is my +1 for the merge on 4.x,
WDYT ?
-- Denis Gervalle SOFTEC sa - CEO eGuilde sarl - CTO
-- Denis Gervalle SOFTEC sa - CEO eGuilde sarl - CTO
This is not very cool. How come ? It's because there is not default edit right to guest ? Ludovic 2012/2/16 Denis Gervalle <[email protected]>
On Wed, Feb 15, 2012 at 11:17, Denis Gervalle <[email protected]> wrote:
Hi all,
As you have probably notice, I have recently committed an feature-security-authorization branch on platform. I am working on this for a while now and it was the first step to share the outcome of this large refactoring of the initial work done early last year by Andreas. Since the code was not quality compliant with platform but the general structure Andreas has build seems to me well appropriate, I have progressively refactor its code to better fit our real needs. Here is what I have been done:
1) Split in to module api and bridge to allow breaking the currently unavoidable dependency on oldcore. Now only bridge depends on oldcore, and the api does not depends on bridge. As mush as possible has been written in the api (still some code to migrate), and some temporary internal bridge are used to access oldcore stuffs since augmenting the existing document-bridge does not seems appropriate IMO.
2) The initial enumeration of rights as been replaced by a Right class, which could be seen has a pseudo enum, but could be augmented with new rights. To register a new Right, you have to provide a RightDescription to the AuthorizationManager. The description will define the default state, the tie resolution policy, the inheritance policy, the list of entity types for which the right is applicable, the implied rights and if the right could be allowed in read-only mode. So new defined Right will benefit the whole logic of the AuthorizationManager and currently existing one could be declaratively defined.
3) Large renaming to better distinguish stuffs, clarify comments and prepare for future. I have voluntarily not taken existing names to clearly split the old and the new api. In brief, the new right service is now named AuthorizationManager. Internally, it manipulates SecurityReference (as well as UserSecurityReference and GroupSecurityReference, to represent entities, user and group), SecurityRule (representing a right object) and SecurityAccess (representing an access level in the old nomenclature), which are store in a SecurityCache using SecurityRuleEntry (a set of rules) and SecurityAccessEntry (the access of a given user). The AuthorizationManager delegate cache management to a SecurityCacheLoader which loads rules using a SecurityRuleLoader ; and delegate itself the computing of the access for a given user and a set of rules to an AuthorizationSettler. This last one could be overridden to provide specific decision that could not be done in declarative mode.
4) Refactoring was necessary to improve consistency and reduce complexity, and simplify as much as possible; while extending the limitations to allow more rights to be registered. This work has been a little bit opposed to the optimization done by Andreas, in particular on memory usage. But optimization is often the enemy of clean code.
5) Improvement were necessary to better mimic the existing implementation in some peculiar but necessary rules to stay compatible with current working wiki. I tend to reduce as much as possible what is not done declaratively, but there are still some special cases, like delete for creators, deny for other user on explicit allow and admin for wiki owner that are settle by the authorization settler. My implementation should be almost compatible with the old one, except for groups that are currently not checked from the entity wiki, but only from the user wiki. This needs some more refactoring for which I feel inconfortable with, some I'd like to share first.
Forgot to mention that for the same reason, using this new security on an empty database does not allow importing the initial wiki. You need to enable superadmin and log with it. This is another part I d'like to see discussed since fixing this could cause limitation in the evolution we may want to see introduced later.
6) The AuthorizationManager interface has been simplified, providing 2 methods for either checking or verifying an access right (the checking methods throws while the verifying one return a boolean), and one to register a new right.
The existing RightService could be bridged on the new implementation
using
the XWikiCachingRightService class in xwiki.cfg and the new API could be used side-by-side with the old implementation as well. What should still really need to be improved is the unit testing, currently some tests are still awful and incomplete. I already refactor some of them, to provide a better coverage of essential part of the code: the security cache and the default authorization settler. Obviously, any help is welcomed.
Since I already have an existing wiki using this implementation successfully and using it for creating new rights for extensions, I would like to merge this new implementation as experimental in platform to have it available for anyone who need it or want to test it, and for you to use in your new experimental development as well. Providing it in platform will encourage it to be finalized and replace the existing implementation.
Here is my +1 for the merge on 4.x,
WDYT ?
-- Denis Gervalle SOFTEC sa - CEO eGuilde sarl - CTO
-- Denis Gervalle SOFTEC sa - CEO eGuilde sarl - CTO _______________________________________________ devs mailing list [email protected] http://lists.xwiki.org/mailman/listinfo/devs
-- Ludovic Dubost Founder and CEO Blog: http://blog.ludovic.org/ XWiki: http://www.xwiki.com Skype: ldubost GTalk: ldubost
On Thu, Feb 16, 2012 at 11:48, Ludovic Dubost <[email protected]> wrote:
This is not very cool. How come ? It's because there is not default edit right to guest ?
This is not only edit right that is required, but admin and programming. What I do not like mostly, is that having default allow. This is not a safe way to see right algorithm IMO. If the only reason is the bootstrap, this not a good reason. This also prevent providing alternate settler that would rely on default right (ie: do not deny others when one is allowed). And, moreover, this is not about guest, but also null user. I have not fully dive into the issue, but even by allowing all rights by default, I do not get it working due to some null user request which I currently consider denied upfront. I have some ideas about other ways to do it better IMO. But I think it is early for a discussion around them: - use a special settler until de wiki has some users - use a special evaluation of isSuperadmin(), which made you superadmin until there is some users Do not think of it has so important, we just need to fix it before using this implementation as a default. Denis
Ludovic
2012/2/16 Denis Gervalle <[email protected]>
On Wed, Feb 15, 2012 at 11:17, Denis Gervalle <[email protected]> wrote:
Hi all,
As you have probably notice, I have recently committed an feature-security-authorization branch on platform. I am working on this for a while now and it was the first step to share the outcome of this large refactoring of the initial work done early last year by Andreas. Since the code was not quality compliant with platform but the general structure Andreas has build seems to me well appropriate, I have progressively refactor its code to better fit our real needs. Here is what I have been done:
1) Split in to module api and bridge to allow breaking the currently unavoidable dependency on oldcore. Now only bridge depends on oldcore, and the api does not depends on bridge. As mush as possible has been written in the api (still some code to migrate), and some temporary internal bridge are used to access oldcore stuffs since augmenting the existing document-bridge does not seems appropriate IMO.
2) The initial enumeration of rights as been replaced by a Right class, which could be seen has a pseudo enum, but could be augmented with new rights. To register a new Right, you have to provide a RightDescription to the AuthorizationManager. The description will define the default state, the tie resolution policy, the inheritance policy, the list of entity types for which the right is applicable, the implied rights and if the right could be allowed in read-only mode. So new defined Right will benefit the whole logic of the AuthorizationManager and currently existing one could be declaratively defined.
3) Large renaming to better distinguish stuffs, clarify comments and prepare for future. I have voluntarily not taken existing names to clearly split the old and the new api. In brief, the new right service is now named AuthorizationManager. Internally, it manipulates SecurityReference (as well as UserSecurityReference and GroupSecurityReference, to represent entities, user and group), SecurityRule (representing a right object) and SecurityAccess (representing an access level in the old nomenclature), which are store in a SecurityCache using SecurityRuleEntry (a set of rules) and SecurityAccessEntry (the access of a given user). The AuthorizationManager delegate cache management to a SecurityCacheLoader which loads rules using a SecurityRuleLoader ; and delegate itself the computing of the access for a given user and a set of rules to an AuthorizationSettler. This last one could be overridden to provide specific decision that could not be done in declarative mode.
4) Refactoring was necessary to improve consistency and reduce complexity, and simplify as much as possible; while extending the limitations to allow more rights to be registered. This work has been a little bit opposed to the optimization done by Andreas, in particular on memory usage. But optimization is often the enemy of clean code.
5) Improvement were necessary to better mimic the existing implementation in some peculiar but necessary rules to stay compatible with current working wiki. I tend to reduce as much as possible what is not done declaratively, but there are still some special cases, like delete for creators, deny for other user on explicit allow and admin for wiki owner that are settle by the authorization settler. My implementation should be almost compatible with the old one, except for groups that are currently not checked from the entity wiki, but only from the user wiki. This needs some more refactoring for which I feel inconfortable with, some I'd like to share first.
Forgot to mention that for the same reason, using this new security on an empty database does not allow importing the initial wiki. You need to enable superadmin and log with it. This is another part I d'like to see discussed since fixing this could cause limitation in the evolution we may want to see introduced later.
6) The AuthorizationManager interface has been simplified, providing 2 methods for either checking or verifying an access right (the checking methods throws while the verifying one return a boolean), and one to register a new right.
The existing RightService could be bridged on the new implementation
using
the XWikiCachingRightService class in xwiki.cfg and the new API could be used side-by-side with the old implementation as well. What should still really need to be improved is the unit testing, currently some tests are still awful and incomplete. I already refactor some of them, to provide a better coverage of essential part of the code: the security cache and the default authorization settler. Obviously, any help is welcomed.
Since I already have an existing wiki using this implementation successfully and using it for creating new rights for extensions, I would like to merge this new implementation as experimental in platform to have it available for anyone who need it or want to test it, and for you to use in your new experimental development as well. Providing it in platform will encourage it to be finalized and replace the existing implementation.
Here is my +1 for the merge on 4.x,
WDYT ?
-- Denis Gervalle SOFTEC sa - CEO eGuilde sarl - CTO
-- Denis Gervalle SOFTEC sa - CEO eGuilde sarl - CTO _______________________________________________ devs mailing list [email protected] http://lists.xwiki.org/mailman/listinfo/devs
-- Ludovic Dubost Founder and CEO Blog: http://blog.ludovic.org/ XWiki: http://www.xwiki.com Skype: ldubost GTalk: ldubost _______________________________________________ devs mailing list [email protected] http://lists.xwiki.org/mailman/listinfo/devs
-- Denis Gervalle SOFTEC sa - CEO eGuilde sarl - CTO
On Thu, Feb 16, 2012 at 2:37 PM, Denis Gervalle <[email protected]> wrote:
On Thu, Feb 16, 2012 at 11:48, Ludovic Dubost <[email protected]> wrote:
This is not very cool. How come ? It's because there is not default edit right to guest ?
This is not only edit right that is required, but admin and programming.
What I do not like mostly, is that having default allow. This is not a safe way to see right algorithm IMO. If the only reason is the bootstrap, this not a good reason. This also prevent providing alternate settler that would rely on default right (ie: do not deny others when one is allowed). And, moreover, this is not about guest, but also null user. I have not fully dive into the issue, but even by allowing all rights by default, I do not get it working due to some null user request which I currently consider denied upfront.
I have some ideas about other ways to do it better IMO. But I think it is early for a discussion around them: - use a special settler until de wiki has some users - use a special evaluation of isSuperadmin(), which made you superadmin until there is some users
Do not think of it has so important, we just need to fix it before using this implementation as a default.
Yep it's OK if this implementation still have some limitations since it will still be experimental and not used by default (except some components maybe). What's important is that it's easy to enable and test it to report issue and improve it. It's also a good occasion to fix/improve or at least discuss some behaviors of the current implementation.
Denis
Ludovic
2012/2/16 Denis Gervalle <[email protected]>
On Wed, Feb 15, 2012 at 11:17, Denis Gervalle <[email protected]> wrote:
Hi all,
As you have probably notice, I have recently committed an feature-security-authorization branch on platform. I am working on this for a while now and it was the first step to share the outcome of this large refactoring of the initial work done early last year by Andreas. Since the code was not quality compliant with platform but the general structure Andreas has build seems to me well appropriate, I have progressively refactor its code to better fit our real needs. Here is what I have been done:
1) Split in to module api and bridge to allow breaking the currently unavoidable dependency on oldcore. Now only bridge depends on oldcore, and the api does not depends on bridge. As mush as possible has been written in the api (still some code to migrate), and some temporary internal bridge are used to access oldcore stuffs since augmenting the existing document-bridge does not seems appropriate IMO.
2) The initial enumeration of rights as been replaced by a Right class, which could be seen has a pseudo enum, but could be augmented with new rights. To register a new Right, you have to provide a RightDescription to the AuthorizationManager. The description will define the default state, the tie resolution policy, the inheritance policy, the list of entity types for which the right is applicable, the implied rights and if the right could be allowed in read-only mode. So new defined Right will benefit the whole logic of the AuthorizationManager and currently existing one could be declaratively defined.
3) Large renaming to better distinguish stuffs, clarify comments and prepare for future. I have voluntarily not taken existing names to clearly split the old and the new api. In brief, the new right service is now named AuthorizationManager. Internally, it manipulates SecurityReference (as well as UserSecurityReference and GroupSecurityReference, to represent entities, user and group), SecurityRule (representing a right object) and SecurityAccess (representing an access level in the old nomenclature), which are store in a SecurityCache using SecurityRuleEntry (a set of rules) and SecurityAccessEntry (the access of a given user). The AuthorizationManager delegate cache management to a SecurityCacheLoader which loads rules using a SecurityRuleLoader ; and delegate itself the computing of the access for a given user and a set of rules to an AuthorizationSettler. This last one could be overridden to provide specific decision that could not be done in declarative mode.
4) Refactoring was necessary to improve consistency and reduce complexity, and simplify as much as possible; while extending the limitations to allow more rights to be registered. This work has been a little bit opposed to the optimization done by Andreas, in particular on memory usage. But optimization is often the enemy of clean code.
5) Improvement were necessary to better mimic the existing implementation in some peculiar but necessary rules to stay compatible with current working wiki. I tend to reduce as much as possible what is not done declaratively, but there are still some special cases, like delete for creators, deny for other user on explicit allow and admin for wiki owner that are settle by the authorization settler. My implementation should be almost compatible with the old one, except for groups that are currently not checked from the entity wiki, but only from the user wiki. This needs some more refactoring for which I feel inconfortable with, some I'd like to share first.
Forgot to mention that for the same reason, using this new security on an empty database does not allow importing the initial wiki. You need to enable superadmin and log with it. This is another part I d'like to see discussed since fixing this could cause limitation in the evolution we may want to see introduced later.
6) The AuthorizationManager interface has been simplified, providing 2 methods for either checking or verifying an access right (the checking methods throws while the verifying one return a boolean), and one to register a new right.
The existing RightService could be bridged on the new implementation
using
the XWikiCachingRightService class in xwiki.cfg and the new API could be used side-by-side with the old implementation as well. What should still really need to be improved is the unit testing, currently some tests are still awful and incomplete. I already refactor some of them, to provide a better coverage of essential part of the code: the security cache and the default authorization settler. Obviously, any help is welcomed.
Since I already have an existing wiki using this implementation successfully and using it for creating new rights for extensions, I would like to merge this new implementation as experimental in platform to have it available for anyone who need it or want to test it, and for you to use in your new experimental development as well. Providing it in platform will encourage it to be finalized and replace the existing implementation.
Here is my +1 for the merge on 4.x,
WDYT ?
-- Denis Gervalle SOFTEC sa - CEO eGuilde sarl - CTO
-- Denis Gervalle SOFTEC sa - CEO eGuilde sarl - CTO _______________________________________________ devs mailing list [email protected] http://lists.xwiki.org/mailman/listinfo/devs
-- Ludovic Dubost Founder and CEO Blog: http://blog.ludovic.org/ XWiki: http://www.xwiki.com Skype: ldubost GTalk: ldubost _______________________________________________ devs mailing list [email protected] http://lists.xwiki.org/mailman/listinfo/devs
-- Denis Gervalle SOFTEC sa - CEO eGuilde sarl - CTO _______________________________________________ devs mailing list [email protected] http://lists.xwiki.org/mailman/listinfo/devs
-- Thomas Mortagne
+1 On Wed, Feb 15, 2012 at 11:17 AM, Denis Gervalle <[email protected]> wrote:
Hi all,
As you have probably notice, I have recently committed an feature-security-authorization branch on platform. I am working on this for a while now and it was the first step to share the outcome of this large refactoring of the initial work done early last year by Andreas. Since the code was not quality compliant with platform but the general structure Andreas has build seems to me well appropriate, I have progressively refactor its code to better fit our real needs. Here is what I have been done:
1) Split in to module api and bridge to allow breaking the currently unavoidable dependency on oldcore. Now only bridge depends on oldcore, and the api does not depends on bridge. As mush as possible has been written in the api (still some code to migrate), and some temporary internal bridge are used to access oldcore stuffs since augmenting the existing document-bridge does not seems appropriate IMO.
2) The initial enumeration of rights as been replaced by a Right class, which could be seen has a pseudo enum, but could be augmented with new rights. To register a new Right, you have to provide a RightDescription to the AuthorizationManager. The description will define the default state, the tie resolution policy, the inheritance policy, the list of entity types for which the right is applicable, the implied rights and if the right could be allowed in read-only mode. So new defined Right will benefit the whole logic of the AuthorizationManager and currently existing one could be declaratively defined.
3) Large renaming to better distinguish stuffs, clarify comments and prepare for future. I have voluntarily not taken existing names to clearly split the old and the new api. In brief, the new right service is now named AuthorizationManager. Internally, it manipulates SecurityReference (as well as UserSecurityReference and GroupSecurityReference, to represent entities, user and group), SecurityRule (representing a right object) and SecurityAccess (representing an access level in the old nomenclature), which are store in a SecurityCache using SecurityRuleEntry (a set of rules) and SecurityAccessEntry (the access of a given user). The AuthorizationManager delegate cache management to a SecurityCacheLoader which loads rules using a SecurityRuleLoader ; and delegate itself the computing of the access for a given user and a set of rules to an AuthorizationSettler. This last one could be overridden to provide specific decision that could not be done in declarative mode.
4) Refactoring was necessary to improve consistency and reduce complexity, and simplify as much as possible; while extending the limitations to allow more rights to be registered. This work has been a little bit opposed to the optimization done by Andreas, in particular on memory usage. But optimization is often the enemy of clean code.
5) Improvement were necessary to better mimic the existing implementation in some peculiar but necessary rules to stay compatible with current working wiki. I tend to reduce as much as possible what is not done declaratively, but there are still some special cases, like delete for creators, deny for other user on explicit allow and admin for wiki owner that are settle by the authorization settler. My implementation should be almost compatible with the old one, except for groups that are currently not checked from the entity wiki, but only from the user wiki. This needs some more refactoring for which I feel inconfortable with, some I'd like to share first.
6) The AuthorizationManager interface has been simplified, providing 2 methods for either checking or verifying an access right (the checking methods throws while the verifying one return a boolean), and one to register a new right.
The existing RightService could be bridged on the new implementation using the XWikiCachingRightService class in xwiki.cfg and the new API could be used side-by-side with the old implementation as well. What should still really need to be improved is the unit testing, currently some tests are still awful and incomplete. I already refactor some of them, to provide a better coverage of essential part of the code: the security cache and the default authorization settler. Obviously, any help is welcomed.
Since I already have an existing wiki using this implementation successfully and using it for creating new rights for extensions, I would like to merge this new implementation as experimental in platform to have it available for anyone who need it or want to test it, and for you to use in your new experimental development as well. Providing it in platform will encourage it to be finalized and replace the existing implementation.
Here is my +1 for the merge on 4.x,
WDYT ?
-- Denis Gervalle SOFTEC sa - CEO eGuilde sarl - CTO _______________________________________________ devs mailing list [email protected] http://lists.xwiki.org/mailman/listinfo/devs
-- Thomas Mortagne
+0 Thanks, Marius On Wed, Feb 15, 2012 at 12:17 PM, Denis Gervalle <[email protected]> wrote:
Hi all,
As you have probably notice, I have recently committed an feature-security-authorization branch on platform. I am working on this for a while now and it was the first step to share the outcome of this large refactoring of the initial work done early last year by Andreas. Since the code was not quality compliant with platform but the general structure Andreas has build seems to me well appropriate, I have progressively refactor its code to better fit our real needs. Here is what I have been done:
1) Split in to module api and bridge to allow breaking the currently unavoidable dependency on oldcore. Now only bridge depends on oldcore, and the api does not depends on bridge. As mush as possible has been written in the api (still some code to migrate), and some temporary internal bridge are used to access oldcore stuffs since augmenting the existing document-bridge does not seems appropriate IMO.
2) The initial enumeration of rights as been replaced by a Right class, which could be seen has a pseudo enum, but could be augmented with new rights. To register a new Right, you have to provide a RightDescription to the AuthorizationManager. The description will define the default state, the tie resolution policy, the inheritance policy, the list of entity types for which the right is applicable, the implied rights and if the right could be allowed in read-only mode. So new defined Right will benefit the whole logic of the AuthorizationManager and currently existing one could be declaratively defined.
3) Large renaming to better distinguish stuffs, clarify comments and prepare for future. I have voluntarily not taken existing names to clearly split the old and the new api. In brief, the new right service is now named AuthorizationManager. Internally, it manipulates SecurityReference (as well as UserSecurityReference and GroupSecurityReference, to represent entities, user and group), SecurityRule (representing a right object) and SecurityAccess (representing an access level in the old nomenclature), which are store in a SecurityCache using SecurityRuleEntry (a set of rules) and SecurityAccessEntry (the access of a given user). The AuthorizationManager delegate cache management to a SecurityCacheLoader which loads rules using a SecurityRuleLoader ; and delegate itself the computing of the access for a given user and a set of rules to an AuthorizationSettler. This last one could be overridden to provide specific decision that could not be done in declarative mode.
4) Refactoring was necessary to improve consistency and reduce complexity, and simplify as much as possible; while extending the limitations to allow more rights to be registered. This work has been a little bit opposed to the optimization done by Andreas, in particular on memory usage. But optimization is often the enemy of clean code.
5) Improvement were necessary to better mimic the existing implementation in some peculiar but necessary rules to stay compatible with current working wiki. I tend to reduce as much as possible what is not done declaratively, but there are still some special cases, like delete for creators, deny for other user on explicit allow and admin for wiki owner that are settle by the authorization settler. My implementation should be almost compatible with the old one, except for groups that are currently not checked from the entity wiki, but only from the user wiki. This needs some more refactoring for which I feel inconfortable with, some I'd like to share first.
6) The AuthorizationManager interface has been simplified, providing 2 methods for either checking or verifying an access right (the checking methods throws while the verifying one return a boolean), and one to register a new right.
The existing RightService could be bridged on the new implementation using the XWikiCachingRightService class in xwiki.cfg and the new API could be used side-by-side with the old implementation as well. What should still really need to be improved is the unit testing, currently some tests are still awful and incomplete. I already refactor some of them, to provide a better coverage of essential part of the code: the security cache and the default authorization settler. Obviously, any help is welcomed.
Since I already have an existing wiki using this implementation successfully and using it for creating new rights for extensions, I would like to merge this new implementation as experimental in platform to have it available for anyone who need it or want to test it, and for you to use in your new experimental development as well. Providing it in platform will encourage it to be finalized and replace the existing implementation.
Here is my +1 for the merge on 4.x,
WDYT ?
-- Denis Gervalle SOFTEC sa - CEO eGuilde sarl - CTO _______________________________________________ devs mailing list [email protected] http://lists.xwiki.org/mailman/listinfo/devs
+1 Best Regards, /Andreas 2012-02-15 11:17, Denis Gervalle skrev:
Hi all,
As you have probably notice, I have recently committed an feature-security-authorization branch on platform. I am working on this for a while now and it was the first step to share the outcome of this large refactoring of the initial work done early last year by Andreas. Since the code was not quality compliant with platform but the general structure Andreas has build seems to me well appropriate, I have progressively refactor its code to better fit our real needs. Here is what I have been done:
1) Split in to module api and bridge to allow breaking the currently unavoidable dependency on oldcore. Now only bridge depends on oldcore, and the api does not depends on bridge. As mush as possible has been written in the api (still some code to migrate), and some temporary internal bridge are used to access oldcore stuffs since augmenting the existing document-bridge does not seems appropriate IMO.
2) The initial enumeration of rights as been replaced by a Right class, which could be seen has a pseudo enum, but could be augmented with new rights. To register a new Right, you have to provide a RightDescription to the AuthorizationManager. The description will define the default state, the tie resolution policy, the inheritance policy, the list of entity types for which the right is applicable, the implied rights and if the right could be allowed in read-only mode. So new defined Right will benefit the whole logic of the AuthorizationManager and currently existing one could be declaratively defined.
3) Large renaming to better distinguish stuffs, clarify comments and prepare for future. I have voluntarily not taken existing names to clearly split the old and the new api. In brief, the new right service is now named AuthorizationManager. Internally, it manipulates SecurityReference (as well as UserSecurityReference and GroupSecurityReference, to represent entities, user and group), SecurityRule (representing a right object) and SecurityAccess (representing an access level in the old nomenclature), which are store in a SecurityCache using SecurityRuleEntry (a set of rules) and SecurityAccessEntry (the access of a given user). The AuthorizationManager delegate cache management to a SecurityCacheLoader which loads rules using a SecurityRuleLoader ; and delegate itself the computing of the access for a given user and a set of rules to an AuthorizationSettler. This last one could be overridden to provide specific decision that could not be done in declarative mode.
4) Refactoring was necessary to improve consistency and reduce complexity, and simplify as much as possible; while extending the limitations to allow more rights to be registered. This work has been a little bit opposed to the optimization done by Andreas, in particular on memory usage. But optimization is often the enemy of clean code.
5) Improvement were necessary to better mimic the existing implementation in some peculiar but necessary rules to stay compatible with current working wiki. I tend to reduce as much as possible what is not done declaratively, but there are still some special cases, like delete for creators, deny for other user on explicit allow and admin for wiki owner that are settle by the authorization settler. My implementation should be almost compatible with the old one, except for groups that are currently not checked from the entity wiki, but only from the user wiki. This needs some more refactoring for which I feel inconfortable with, some I'd like to share first.
6) The AuthorizationManager interface has been simplified, providing 2 methods for either checking or verifying an access right (the checking methods throws while the verifying one return a boolean), and one to register a new right.
The existing RightService could be bridged on the new implementation using the XWikiCachingRightService class in xwiki.cfg and the new API could be used side-by-side with the old implementation as well. What should still really need to be improved is the unit testing, currently some tests are still awful and incomplete. I already refactor some of them, to provide a better coverage of essential part of the code: the security cache and the default authorization settler. Obviously, any help is welcomed.
Since I already have an existing wiki using this implementation successfully and using it for creating new rights for extensions, I would like to merge this new implementation as experimental in platform to have it available for anyone who need it or want to test it, and for you to use in your new experimental development as well. Providing it in platform will encourage it to be finalized and replace the existing implementation.
Here is my +1 for the merge on 4.x,
WDYT ?
3 +1, 1 +0, no -1, merged On Fri, Feb 17, 2012 at 09:42, Andreas Jonsson <[email protected]> wrote:
+1
Best Regards,
/Andreas
2012-02-15 11:17, Denis Gervalle skrev:
Hi all,
As you have probably notice, I have recently committed an feature-security-authorization branch on platform. I am working on this for a while now and it was the first step to share the outcome of this large refactoring of the initial work done early last year by Andreas. Since the code was not quality compliant with platform but the general structure Andreas has build seems to me well appropriate, I have progressively refactor its code to better fit our real needs. Here is what I have been done:
1) Split in to module api and bridge to allow breaking the currently unavoidable dependency on oldcore. Now only bridge depends on oldcore, and the api does not depends on bridge. As mush as possible has been written in the api (still some code to migrate), and some temporary internal bridge are used to access oldcore stuffs since augmenting the existing document-bridge does not seems appropriate IMO.
2) The initial enumeration of rights as been replaced by a Right class, which could be seen has a pseudo enum, but could be augmented with new rights. To register a new Right, you have to provide a RightDescription to the AuthorizationManager. The description will define the default state, the tie resolution policy, the inheritance policy, the list of entity types for which the right is applicable, the implied rights and if the right could be allowed in read-only mode. So new defined Right will benefit the whole logic of the AuthorizationManager and currently existing one could be declaratively defined.
3) Large renaming to better distinguish stuffs, clarify comments and prepare for future. I have voluntarily not taken existing names to clearly split the old and the new api. In brief, the new right service is now named AuthorizationManager. Internally, it manipulates SecurityReference (as well as UserSecurityReference and GroupSecurityReference, to represent entities, user and group), SecurityRule (representing a right object) and SecurityAccess (representing an access level in the old nomenclature), which are store in a SecurityCache using SecurityRuleEntry (a set of rules) and SecurityAccessEntry (the access of a given user). The AuthorizationManager delegate cache management to a SecurityCacheLoader which loads rules using a SecurityRuleLoader ; and delegate itself the computing of the access for a given user and a set of rules to an AuthorizationSettler. This last one could be overridden to provide specific decision that could not be done in declarative mode.
4) Refactoring was necessary to improve consistency and reduce complexity, and simplify as much as possible; while extending the limitations to allow more rights to be registered. This work has been a little bit opposed to the optimization done by Andreas, in particular on memory usage. But optimization is often the enemy of clean code.
5) Improvement were necessary to better mimic the existing implementation in some peculiar but necessary rules to stay compatible with current working wiki. I tend to reduce as much as possible what is not done declaratively, but there are still some special cases, like delete for creators, deny for other user on explicit allow and admin for wiki owner that are settle by the authorization settler. My implementation should be almost compatible with the old one, except for groups that are currently not checked from the entity wiki, but only from the user wiki. This needs some more refactoring for which I feel inconfortable with, some I'd like to share first.
6) The AuthorizationManager interface has been simplified, providing 2 methods for either checking or verifying an access right (the checking methods throws while the verifying one return a boolean), and one to register a new right.
The existing RightService could be bridged on the new implementation using the XWikiCachingRightService class in xwiki.cfg and the new API could be used side-by-side with the old implementation as well. What should still really need to be improved is the unit testing, currently some tests are still awful and incomplete. I already refactor some of them, to provide a better coverage of essential part of the code: the security cache and the default authorization settler. Obviously, any help is welcomed.
Since I already have an existing wiki using this implementation successfully and using it for creating new rights for extensions, I would like to merge this new implementation as experimental in platform to have it available for anyone who need it or want to test it, and for you to use in your new experimental development as well. Providing it in platform will encourage it to be finalized and replace the existing implementation.
Here is my +1 for the merge on 4.x,
WDYT ?
_______________________________________________ devs mailing list [email protected] http://lists.xwiki.org/mailman/listinfo/devs
-- Denis Gervalle SOFTEC sa - CEO eGuilde sarl - CTO
+1 if you lead the effort to making this implementation the default in the future. I'd like to know what are the next steps though and their timeframe. Denis, do you plan to refactor some existing code to use this new API? IMO this would be the best next step to prove that it works. Fo you have any idea about what code could be migrated? We need to be careful not to keep this new security module not used for too long as otherwise it could "rot". So IMO we need to start using it ASAP and then have a plan for switching to it. Thanks -Vincent On Feb 15, 2012, at 11:17 AM, Denis Gervalle wrote:
Hi all,
As you have probably notice, I have recently committed an feature-security-authorization branch on platform. I am working on this for a while now and it was the first step to share the outcome of this large refactoring of the initial work done early last year by Andreas. Since the code was not quality compliant with platform but the general structure Andreas has build seems to me well appropriate, I have progressively refactor its code to better fit our real needs. Here is what I have been done:
1) Split in to module api and bridge to allow breaking the currently unavoidable dependency on oldcore. Now only bridge depends on oldcore, and the api does not depends on bridge. As mush as possible has been written in the api (still some code to migrate), and some temporary internal bridge are used to access oldcore stuffs since augmenting the existing document-bridge does not seems appropriate IMO.
2) The initial enumeration of rights as been replaced by a Right class, which could be seen has a pseudo enum, but could be augmented with new rights. To register a new Right, you have to provide a RightDescription to the AuthorizationManager. The description will define the default state, the tie resolution policy, the inheritance policy, the list of entity types for which the right is applicable, the implied rights and if the right could be allowed in read-only mode. So new defined Right will benefit the whole logic of the AuthorizationManager and currently existing one could be declaratively defined.
3) Large renaming to better distinguish stuffs, clarify comments and prepare for future. I have voluntarily not taken existing names to clearly split the old and the new api. In brief, the new right service is now named AuthorizationManager. Internally, it manipulates SecurityReference (as well as UserSecurityReference and GroupSecurityReference, to represent entities, user and group), SecurityRule (representing a right object) and SecurityAccess (representing an access level in the old nomenclature), which are store in a SecurityCache using SecurityRuleEntry (a set of rules) and SecurityAccessEntry (the access of a given user). The AuthorizationManager delegate cache management to a SecurityCacheLoader which loads rules using a SecurityRuleLoader ; and delegate itself the computing of the access for a given user and a set of rules to an AuthorizationSettler. This last one could be overridden to provide specific decision that could not be done in declarative mode.
4) Refactoring was necessary to improve consistency and reduce complexity, and simplify as much as possible; while extending the limitations to allow more rights to be registered. This work has been a little bit opposed to the optimization done by Andreas, in particular on memory usage. But optimization is often the enemy of clean code.
5) Improvement were necessary to better mimic the existing implementation in some peculiar but necessary rules to stay compatible with current working wiki. I tend to reduce as much as possible what is not done declaratively, but there are still some special cases, like delete for creators, deny for other user on explicit allow and admin for wiki owner that are settle by the authorization settler. My implementation should be almost compatible with the old one, except for groups that are currently not checked from the entity wiki, but only from the user wiki. This needs some more refactoring for which I feel inconfortable with, some I'd like to share first.
6) The AuthorizationManager interface has been simplified, providing 2 methods for either checking or verifying an access right (the checking methods throws while the verifying one return a boolean), and one to register a new right.
The existing RightService could be bridged on the new implementation using the XWikiCachingRightService class in xwiki.cfg and the new API could be used side-by-side with the old implementation as well. What should still really need to be improved is the unit testing, currently some tests are still awful and incomplete. I already refactor some of them, to provide a better coverage of essential part of the code: the security cache and the default authorization settler. Obviously, any help is welcomed.
Since I already have an existing wiki using this implementation successfully and using it for creating new rights for extensions, I would like to merge this new implementation as experimental in platform to have it available for anyone who need it or want to test it, and for you to use in your new experimental development as well. Providing it in platform will encourage it to be finalized and replace the existing implementation.
Here is my +1 for the merge on 4.x,
WDYT ?
On Wed, Mar 28, 2012 at 10:33, Vincent Massol <[email protected]> wrote:
+1 if you lead the effort to making this implementation the default in the future.
Sure, with the help of the community... this was an initial effort of Andreas, and it was ignored for too long.
I'd like to know what are the next steps though and their timeframe.
Since this module has some slight differences with the original security module, and since it also provide new possibilities, we first need to document it, and start some discussion about the peculiarities of our current implementation we want to see changed. This is going to be long discussion....
Denis, do you plan to refactor some existing code to use this new API? IMO this would be the best next step to prove that it works.
It works, I use it everyday ! Their minor differences only impact really special usage.
Fo you have any idea about what code could be migrated?
IMO, we may start using it for new components, or components that we are refactoring, since it will allow being independent of the old core for these component. This module may works for a while in parallel with the existing service. By merging it, and providing it as an option, it will allows more user to try it. There is a bridge between the old service and this new one that you may simply activate in xwiki.cfg. This new service improve performance, allow adding more rights on the fly and allow customizing the way it works, so it is an answer to those wanting these improvements.
We need to be careful not to keep this new security module not used for too long as otherwise it could "rot".
It will not stay unused on my side, so be sure I will stay close to it, since I have written it because I need those improvements. I really encourage anyone to try it !
So IMO we need to start using it ASAP and then have a plan for switching to it.
IMO, the switch will be welcome when we have discussed the small differences it has, and fix the few we do not tolerate. In particular, the startup of an empty wiki require login has superadmin with this new module. I know this require more documentation to be written, I am really aware of that, this just require time to be done... hopes Andreas and others could helps as well.
Thanks -Vincent
On Feb 15, 2012, at 11:17 AM, Denis Gervalle wrote:
Hi all,
As you have probably notice, I have recently committed an feature-security-authorization branch on platform. I am working on this for a while now and it was the first step to share the outcome of this large refactoring of the initial work done early last year by Andreas. Since the code was not quality compliant with platform but the general structure Andreas has build seems to me well appropriate, I have progressively refactor its code to better fit our real needs. Here is what I have been done:
1) Split in to module api and bridge to allow breaking the currently unavoidable dependency on oldcore. Now only bridge depends on oldcore, and the api does not depends on bridge. As mush as possible has been written in the api (still some code to migrate), and some temporary internal bridge are used to access oldcore stuffs since augmenting the existing document-bridge does not seems appropriate IMO.
2) The initial enumeration of rights as been replaced by a Right class, which could be seen has a pseudo enum, but could be augmented with new rights. To register a new Right, you have to provide a RightDescription to the AuthorizationManager. The description will define the default state, the tie resolution policy, the inheritance policy, the list of entity types for which the right is applicable, the implied rights and if the right could be allowed in read-only mode. So new defined Right will benefit the whole logic of the AuthorizationManager and currently existing one could be declaratively defined.
3) Large renaming to better distinguish stuffs, clarify comments and prepare for future. I have voluntarily not taken existing names to clearly split the old and the new api. In brief, the new right service is now named AuthorizationManager. Internally, it manipulates SecurityReference (as well as UserSecurityReference and GroupSecurityReference, to represent entities, user and group), SecurityRule (representing a right object) and SecurityAccess (representing an access level in the old nomenclature), which are store in a SecurityCache using SecurityRuleEntry (a set of rules) and SecurityAccessEntry (the access of a given user). The AuthorizationManager delegate cache management to a SecurityCacheLoader which loads rules using a SecurityRuleLoader ; and delegate itself the computing of the access for a given user and a set of rules to an AuthorizationSettler. This last one could be overridden to provide specific decision that could not be done in declarative mode.
4) Refactoring was necessary to improve consistency and reduce complexity, and simplify as much as possible; while extending the limitations to allow more rights to be registered. This work has been a little bit opposed to the optimization done by Andreas, in particular on memory usage. But optimization is often the enemy of clean code.
5) Improvement were necessary to better mimic the existing implementation in some peculiar but necessary rules to stay compatible with current working wiki. I tend to reduce as much as possible what is not done declaratively, but there are still some special cases, like delete for creators, deny for other user on explicit allow and admin for wiki owner that are settle by the authorization settler. My implementation should be almost compatible with the old one, except for groups that are currently not checked from the entity wiki, but only from the user wiki. This needs some more refactoring for which I feel inconfortable with, some I'd like to share first.
6) The AuthorizationManager interface has been simplified, providing 2 methods for either checking or verifying an access right (the checking methods throws while the verifying one return a boolean), and one to register a new right.
The existing RightService could be bridged on the new implementation using the XWikiCachingRightService class in xwiki.cfg and the new API could be used side-by-side with the old implementation as well. What should still really need to be improved is the unit testing, currently some tests are still awful and incomplete. I already refactor some of them, to provide a better coverage of essential part of the code: the security cache and the default authorization settler. Obviously, any help is welcomed.
Since I already have an existing wiki using this implementation successfully and using it for creating new rights for extensions, I would like to merge this new implementation as experimental in platform to have it available for anyone who need it or want to test it, and for you to use in your new experimental development as well. Providing it in platform will encourage it to be finalized and replace the existing implementation.
Here is my +1 for the merge on 4.x,
WDYT ?
devs mailing list [email protected] http://lists.xwiki.org/mailman/listinfo/devs
-- Denis Gervalle SOFTEC sa - CEO eGuilde sarl - CTO
Hi Denis, On Mar 28, 2012, at 11:04 AM, Denis Gervalle wrote: [snip]
Denis, do you plan to refactor some existing code to use this new API? IMO this would be the best next step to prove that it works.
It works, I use it everyday ! Their minor differences only impact really special usage.
We all know the "it works on my machine" syndrome but that doesn't prove anything… :) As I said in another mail we really need a very code unit test coverage.
Fo you have any idea about what code could be migrated?
IMO, we may start using it for new components, or components that we are refactoring, since it will allow being independent of the old core for these component. This module may works for a while in parallel with the existing service.
I was asking for an example of what we could start with. Do you have any idea?
By merging it, and providing it as an option, it will allows more user to try it. There is a bridge between the old service and this new one that you may simply activate in xwiki.cfg. This new service improve performance, allow adding more rights on the fly and allow customizing the way it works, so it is an answer to those wanting these improvements.
We need to be careful not to keep this new security module not used for too long as otherwise it could "rot".
It will not stay unused on my side, so be sure I will stay close to it, since I have written it because I need those improvements. I really encourage anyone to try it !
So IMO we need to start using it ASAP and then have a plan for switching to it.
IMO, the switch will be welcome when we have discussed the small differences it has, and fix the few we do not tolerate. In particular, the startup of an empty wiki require login has superadmin with this new module. I know this require more documentation to be written, I am really aware of that, this just require time to be done... hopes Andreas and others could helps as well.
For me you are now responsible for this part (even though technically we're all responsible of course since we use no code ownership ;) - but for example Thomas and I know more the Rendering and are fixing stuff there, while others have more knowledge in other parts). I understand you wanted this in the platform because you're using it already. I just want to be sure we're going to maintain it since I can see it'll require a lot more work before being to full switch to this implementation. Thanks -Vincent
Thanks -Vincent
On Feb 15, 2012, at 11:17 AM, Denis Gervalle wrote:
Hi all,
As you have probably notice, I have recently committed an feature-security-authorization branch on platform. I am working on this for a while now and it was the first step to share the outcome of this large refactoring of the initial work done early last year by Andreas. Since the code was not quality compliant with platform but the general structure Andreas has build seems to me well appropriate, I have progressively refactor its code to better fit our real needs. Here is what I have been done:
1) Split in to module api and bridge to allow breaking the currently unavoidable dependency on oldcore. Now only bridge depends on oldcore, and the api does not depends on bridge. As mush as possible has been written in the api (still some code to migrate), and some temporary internal bridge are used to access oldcore stuffs since augmenting the existing document-bridge does not seems appropriate IMO.
2) The initial enumeration of rights as been replaced by a Right class, which could be seen has a pseudo enum, but could be augmented with new rights. To register a new Right, you have to provide a RightDescription to the AuthorizationManager. The description will define the default state, the tie resolution policy, the inheritance policy, the list of entity types for which the right is applicable, the implied rights and if the right could be allowed in read-only mode. So new defined Right will benefit the whole logic of the AuthorizationManager and currently existing one could be declaratively defined.
3) Large renaming to better distinguish stuffs, clarify comments and prepare for future. I have voluntarily not taken existing names to clearly split the old and the new api. In brief, the new right service is now named AuthorizationManager. Internally, it manipulates SecurityReference (as well as UserSecurityReference and GroupSecurityReference, to represent entities, user and group), SecurityRule (representing a right object) and SecurityAccess (representing an access level in the old nomenclature), which are store in a SecurityCache using SecurityRuleEntry (a set of rules) and SecurityAccessEntry (the access of a given user). The AuthorizationManager delegate cache management to a SecurityCacheLoader which loads rules using a SecurityRuleLoader ; and delegate itself the computing of the access for a given user and a set of rules to an AuthorizationSettler. This last one could be overridden to provide specific decision that could not be done in declarative mode.
4) Refactoring was necessary to improve consistency and reduce complexity, and simplify as much as possible; while extending the limitations to allow more rights to be registered. This work has been a little bit opposed to the optimization done by Andreas, in particular on memory usage. But optimization is often the enemy of clean code.
5) Improvement were necessary to better mimic the existing implementation in some peculiar but necessary rules to stay compatible with current working wiki. I tend to reduce as much as possible what is not done declaratively, but there are still some special cases, like delete for creators, deny for other user on explicit allow and admin for wiki owner that are settle by the authorization settler. My implementation should be almost compatible with the old one, except for groups that are currently not checked from the entity wiki, but only from the user wiki. This needs some more refactoring for which I feel inconfortable with, some I'd like to share first.
6) The AuthorizationManager interface has been simplified, providing 2 methods for either checking or verifying an access right (the checking methods throws while the verifying one return a boolean), and one to register a new right.
The existing RightService could be bridged on the new implementation using the XWikiCachingRightService class in xwiki.cfg and the new API could be used side-by-side with the old implementation as well. What should still really need to be improved is the unit testing, currently some tests are still awful and incomplete. I already refactor some of them, to provide a better coverage of essential part of the code: the security cache and the default authorization settler. Obviously, any help is welcomed.
Since I already have an existing wiki using this implementation successfully and using it for creating new rights for extensions, I would like to merge this new implementation as experimental in platform to have it available for anyone who need it or want to test it, and for you to use in your new experimental development as well. Providing it in platform will encourage it to be finalized and replace the existing implementation.
Here is my +1 for the merge on 4.x,
WDYT ?
On Wed, Mar 28, 2012 at 11:24, Vincent Massol <[email protected]> wrote:
Hi Denis,
On Mar 28, 2012, at 11:04 AM, Denis Gervalle wrote:
[snip]
Denis, do you plan to refactor some existing code to use this new API? IMO this would be the best next step to prove that it works.
It works, I use it everyday ! Their minor differences only impact really special usage.
We all know the "it works on my machine" syndrome but that doesn't prove anything… :)
As I said in another mail we really need a very code unit test coverage.
Fo you have any idea about what code could be migrated?
IMO, we may start using it for new components, or components that we are refactoring, since it will allow being independent of the old core for these component. This module may works for a while in parallel with the existing service.
I was asking for an example of what we could start with. Do you have any idea?
By merging it, and providing it as an option, it will allows more user to try it. There is a bridge between the old service and this new one that you may simply activate in xwiki.cfg. This new service improve performance, allow adding more rights on the fly and allow customizing the way it works, so it is an answer to those wanting these improvements.
We need to be careful not to keep this new security module not used for too long as otherwise it could "rot".
It will not stay unused on my side, so be sure I will stay close to it, since I have written it because I need those improvements. I really encourage anyone to try it !
So IMO we need to start using it ASAP and then have a plan for switching to it.
IMO, the switch will be welcome when we have discussed the small differences it has, and fix the few we do not tolerate. In particular, the startup of an empty wiki require login has superadmin with this new module. I know this require more documentation to be written, I am really aware of that, this just require time to be done... hopes Andreas and others could helps as well.
For me you are now responsible for this part (even though technically we're all responsible of course since we use no code ownership ;) - but for example Thomas and I know more the Rendering and are fixing stuff there, while others have more knowledge in other parts).
Have I said the contrary ? All I said is that I will do my best supporting it, but this is really a large part, and some help is also welcome. I have taken the time to have it at the current state in the hope we will be able to move out of the crappy service we use currently more that 20 times per requests. It is now mature enough to start using it, since we do, and...
I understand you wanted this in the platform because you're using it already.
...no, I am not so concerned about having it in platform for using it. I now trust my module more than the existing implementation. I would like to have it in platform to encourage others to look at it, use it, adopt it and contribute to it, since I am really confident it is really better than the current implementation.
I just want to be sure we're going to maintain it since I can see it'll require a lot more work before being to full switch to this implementation.
IMO, there is even more work to do to use the current right service, since its own test cover only 52% of the right service itself and the documentation is really sparse and almost unwritable ! (BTW, these tests cannot be used on our bridge since these use implementation method that are not published in the interface.)
Thanks -Vincent
Thanks -Vincent
On Feb 15, 2012, at 11:17 AM, Denis Gervalle wrote:
Hi all,
As you have probably notice, I have recently committed an feature-security-authorization branch on platform. I am working on this for a while now and it was the first step to share the outcome of this large refactoring of the initial work done early last year by Andreas. Since the code was not quality compliant with platform but the general structure Andreas has build seems to me well appropriate, I have progressively refactor its code to better fit our real needs. Here is what I have been done:
1) Split in to module api and bridge to allow breaking the currently unavoidable dependency on oldcore. Now only bridge depends on oldcore, and the api does not depends on bridge. As mush as possible has been written in the api (still some code to migrate), and some temporary internal bridge are used to access oldcore stuffs since augmenting the existing document-bridge does not seems appropriate IMO.
2) The initial enumeration of rights as been replaced by a Right class, which could be seen has a pseudo enum, but could be augmented with new rights. To register a new Right, you have to provide a RightDescription to the AuthorizationManager. The description will define the default state, the tie resolution policy, the inheritance policy, the list of entity types for which the right is applicable, the implied rights and if the right could be allowed in read-only mode. So new defined Right will benefit the whole logic of the AuthorizationManager and currently existing one could be declaratively defined.
3) Large renaming to better distinguish stuffs, clarify comments and prepare for future. I have voluntarily not taken existing names to clearly split the old and the new api. In brief, the new right service is now named AuthorizationManager. Internally, it manipulates SecurityReference (as well as UserSecurityReference and GroupSecurityReference, to represent entities, user and group), SecurityRule (representing a right object) and SecurityAccess (representing an access level in the old nomenclature), which are store in a SecurityCache using SecurityRuleEntry (a set of rules) and SecurityAccessEntry (the access of a given user). The AuthorizationManager delegate cache management to a SecurityCacheLoader which loads rules using a SecurityRuleLoader ; and delegate itself the computing of the access for a given user and a set of rules to an AuthorizationSettler. This last one could be overridden to provide specific decision that could not be done in declarative mode.
4) Refactoring was necessary to improve consistency and reduce complexity, and simplify as much as possible; while extending the limitations to allow more rights to be registered. This work has been a little bit opposed to the optimization done by Andreas, in particular on memory usage. But optimization is often the enemy of clean code.
5) Improvement were necessary to better mimic the existing implementation in some peculiar but necessary rules to stay compatible with current working wiki. I tend to reduce as much as possible what is not done declaratively, but there are still some special cases, like delete for creators, deny for other user on explicit allow and admin for wiki owner that are settle by the authorization settler. My implementation should be almost compatible with the old one, except for groups that are currently not checked from the entity wiki, but only from the user wiki. This needs some more refactoring for which I feel inconfortable with, some I'd like to share first.
6) The AuthorizationManager interface has been simplified, providing 2 methods for either checking or verifying an access right (the checking methods throws while the verifying one return a boolean), and one to register a new right.
The existing RightService could be bridged on the new implementation using the XWikiCachingRightService class in xwiki.cfg and the new API could be used side-by-side with the old implementation as well. What should still really need to be improved is the unit testing, currently some tests are still awful and incomplete. I already refactor some of them, to provide a better coverage of essential part of the code: the security cache and the default authorization settler. Obviously, any help is welcomed.
Since I already have an existing wiki using this implementation successfully and using it for creating new rights for extensions, I would like to merge this new implementation as experimental in platform to have it available for anyone who need it or want to test it, and for you to use in your new experimental development as well. Providing it in platform will encourage it to be finalized and replace the existing implementation.
Here is my +1 for the merge on 4.x,
WDYT ?
devs mailing list [email protected] http://lists.xwiki.org/mailman/listinfo/devs
-- Denis Gervalle SOFTEC sa - CEO eGuilde sarl - CTO
2012-03-28 19:08, Denis Gervalle skrev:
On Wed, Mar 28, 2012 at 11:24, Vincent Massol <[email protected]> wrote:
Hi Denis,
On Mar 28, 2012, at 11:04 AM, Denis Gervalle wrote:
[snip]
Denis, do you plan to refactor some existing code to use this new API? IMO this would be the best next step to prove that it works.
It works, I use it everyday ! Their minor differences only impact really special usage.
We all know the "it works on my machine" syndrome but that doesn't prove anything… :)
As I said in another mail we really need a very code unit test coverage.
Fo you have any idea about what code could be migrated?
IMO, we may start using it for new components, or components that we are refactoring, since it will allow being independent of the old core for these component. This module may works for a while in parallel with the existing service.
I was asking for an example of what we could start with. Do you have any idea?
By merging it, and providing it as an option, it will allows more user to try it. There is a bridge between the old service and this new one that you may simply activate in xwiki.cfg. This new service improve performance, allow adding more rights on the fly and allow customizing the way it works, so it is an answer to those wanting these improvements.
We need to be careful not to keep this new security module not used for too long as otherwise it could "rot".
It will not stay unused on my side, so be sure I will stay close to it, since I have written it because I need those improvements. I really encourage anyone to try it !
So IMO we need to start using it ASAP and then have a plan for switching to it.
IMO, the switch will be welcome when we have discussed the small differences it has, and fix the few we do not tolerate. In particular, the startup of an empty wiki require login has superadmin with this new module. I know this require more documentation to be written, I am really aware of that, this just require time to be done... hopes Andreas and others could helps as well.
For me you are now responsible for this part (even though technically we're all responsible of course since we use no code ownership ;) - but for example Thomas and I know more the Rendering and are fixing stuff there, while others have more knowledge in other parts).
Have I said the contrary ? All I said is that I will do my best supporting it, but this is really a large part, and some help is also welcome. I have taken the time to have it at the current state in the hope we will be able to move out of the crappy service we use currently more that 20 times per requests. It is now mature enough to start using it, since we do, and...
I will also support this module, of course, and help out fixing issues with it, and add more unit tests. I feel much more motivated to work on it when it is on track to replace the old implementation. Best Regards, /Andreas
I understand you wanted this in the platform because you're using it already.
...no, I am not so concerned about having it in platform for using it. I now trust my module more than the existing implementation. I would like to have it in platform to encourage others to look at it, use it, adopt it and contribute to it, since I am really confident it is really better than the current implementation.
I just want to be sure we're going to maintain it since I can see it'll require a lot more work before being to full switch to this implementation.
IMO, there is even more work to do to use the current right service, since its own test cover only 52% of the right service itself and the documentation is really sparse and almost unwritable ! (BTW, these tests cannot be used on our bridge since these use implementation method that are not published in the interface.)
Thanks -Vincent
Thanks -Vincent
On Feb 15, 2012, at 11:17 AM, Denis Gervalle wrote:
Hi all,
As you have probably notice, I have recently committed an feature-security-authorization branch on platform. I am working on this for a while now and it was the first step to share the outcome of this large refactoring of the initial work done early last year by Andreas. Since the code was not quality compliant with platform but the general structure Andreas has build seems to me well appropriate, I have progressively refactor its code to better fit our real needs. Here is what I have been done:
1) Split in to module api and bridge to allow breaking the currently unavoidable dependency on oldcore. Now only bridge depends on oldcore, and the api does not depends on bridge. As mush as possible has been written in the api (still some code to migrate), and some temporary internal bridge are used to access oldcore stuffs since augmenting the existing document-bridge does not seems appropriate IMO.
2) The initial enumeration of rights as been replaced by a Right class, which could be seen has a pseudo enum, but could be augmented with new rights. To register a new Right, you have to provide a RightDescription to the AuthorizationManager. The description will define the default state, the tie resolution policy, the inheritance policy, the list of entity types for which the right is applicable, the implied rights and if the right could be allowed in read-only mode. So new defined Right will benefit the whole logic of the AuthorizationManager and currently existing one could be declaratively defined.
3) Large renaming to better distinguish stuffs, clarify comments and prepare for future. I have voluntarily not taken existing names to clearly split the old and the new api. In brief, the new right service is now named AuthorizationManager. Internally, it manipulates SecurityReference (as well as UserSecurityReference and GroupSecurityReference, to represent entities, user and group), SecurityRule (representing a right object) and SecurityAccess (representing an access level in the old nomenclature), which are store in a SecurityCache using SecurityRuleEntry (a set of rules) and SecurityAccessEntry (the access of a given user). The AuthorizationManager delegate cache management to a SecurityCacheLoader which loads rules using a SecurityRuleLoader ; and delegate itself the computing of the access for a given user and a set of rules to an AuthorizationSettler. This last one could be overridden to provide specific decision that could not be done in declarative mode.
4) Refactoring was necessary to improve consistency and reduce complexity, and simplify as much as possible; while extending the limitations to allow more rights to be registered. This work has been a little bit opposed to the optimization done by Andreas, in particular on memory usage. But optimization is often the enemy of clean code.
5) Improvement were necessary to better mimic the existing implementation in some peculiar but necessary rules to stay compatible with current working wiki. I tend to reduce as much as possible what is not done declaratively, but there are still some special cases, like delete for creators, deny for other user on explicit allow and admin for wiki owner that are settle by the authorization settler. My implementation should be almost compatible with the old one, except for groups that are currently not checked from the entity wiki, but only from the user wiki. This needs some more refactoring for which I feel inconfortable with, some I'd like to share first.
6) The AuthorizationManager interface has been simplified, providing 2 methods for either checking or verifying an access right (the checking methods throws while the verifying one return a boolean), and one to register a new right.
The existing RightService could be bridged on the new implementation using the XWikiCachingRightService class in xwiki.cfg and the new API could be used side-by-side with the old implementation as well. What should still really need to be improved is the unit testing, currently some tests are still awful and incomplete. I already refactor some of them, to provide a better coverage of essential part of the code: the security cache and the default authorization settler. Obviously, any help is welcomed.
Since I already have an existing wiki using this implementation successfully and using it for creating new rights for extensions, I would like to merge this new implementation as experimental in platform to have it available for anyone who need it or want to test it, and for you to use in your new experimental development as well. Providing it in platform will encourage it to be finalized and replace the existing implementation.
Here is my +1 for the merge on 4.x,
WDYT ?
devs mailing list [email protected] http://lists.xwiki.org/mailman/listinfo/devs
On Mar 28, 2012, at 9:47 PM, Andreas Jonsson wrote:
2012-03-28 19:08, Denis Gervalle skrev:
On Wed, Mar 28, 2012 at 11:24, Vincent Massol <[email protected]> wrote:
Hi Denis,
On Mar 28, 2012, at 11:04 AM, Denis Gervalle wrote:
[snip]
Denis, do you plan to refactor some existing code to use this new API? IMO this would be the best next step to prove that it works.
It works, I use it everyday ! Their minor differences only impact really special usage.
We all know the "it works on my machine" syndrome but that doesn't prove anything… :)
As I said in another mail we really need a very code unit test coverage.
Fo you have any idea about what code could be migrated?
IMO, we may start using it for new components, or components that we are refactoring, since it will allow being independent of the old core for these component. This module may works for a while in parallel with the existing service.
I was asking for an example of what we could start with. Do you have any idea?
By merging it, and providing it as an option, it will allows more user to try it. There is a bridge between the old service and this new one that you may simply activate in xwiki.cfg. This new service improve performance, allow adding more rights on the fly and allow customizing the way it works, so it is an answer to those wanting these improvements.
We need to be careful not to keep this new security module not used for too long as otherwise it could "rot".
It will not stay unused on my side, so be sure I will stay close to it, since I have written it because I need those improvements. I really encourage anyone to try it !
So IMO we need to start using it ASAP and then have a plan for switching to it.
IMO, the switch will be welcome when we have discussed the small differences it has, and fix the few we do not tolerate. In particular, the startup of an empty wiki require login has superadmin with this new module. I know this require more documentation to be written, I am really aware of that, this just require time to be done... hopes Andreas and others could helps as well.
For me you are now responsible for this part (even though technically we're all responsible of course since we use no code ownership ;) - but for example Thomas and I know more the Rendering and are fixing stuff there, while others have more knowledge in other parts).
Have I said the contrary ? All I said is that I will do my best supporting it, but this is really a large part, and some help is also welcome. I have taken the time to have it at the current state in the hope we will be able to move out of the crappy service we use currently more that 20 times per requests. It is now mature enough to start using it, since we do, and...
I will also support this module, of course, and help out fixing issues with it, and add more unit tests. I feel much more motivated to work on it when it is on track to replace the old implementation.
That's great to hear! :) Thanks -Vincent [snip]
Actually I've now looked at the merge and I've made some quick review in GitHub (I've only done this very quickly nothing thorough). It's a good start but there are quite a few things to improve: * Test are a bit of a mess: very long methods (should have a lot more test methods), some bad practices in several places, creating static mocks instead of dynamic mocks. * Missing javadocs in lots of places * Missing @since tags * We also need to stabilize/comment on the API method names themselves. I've commented on a few. I'd like to know what's the test coverage percentage you have? We need at least 80 to 90% (if not more) for this type of code IMO since it's very sensitive. Thanks -Vincent On Mar 28, 2012, at 10:33 AM, Vincent Massol wrote:
+1 if you lead the effort to making this implementation the default in the future.
I'd like to know what are the next steps though and their timeframe.
Denis, do you plan to refactor some existing code to use this new API? IMO this would be the best next step to prove that it works. Fo you have any idea about what code could be migrated?
We need to be careful not to keep this new security module not used for too long as otherwise it could "rot". So IMO we need to start using it ASAP and then have a plan for switching to it.
Thanks -Vincent
On Feb 15, 2012, at 11:17 AM, Denis Gervalle wrote:
Hi all,
As you have probably notice, I have recently committed an feature-security-authorization branch on platform. I am working on this for a while now and it was the first step to share the outcome of this large refactoring of the initial work done early last year by Andreas. Since the code was not quality compliant with platform but the general structure Andreas has build seems to me well appropriate, I have progressively refactor its code to better fit our real needs. Here is what I have been done:
1) Split in to module api and bridge to allow breaking the currently unavoidable dependency on oldcore. Now only bridge depends on oldcore, and the api does not depends on bridge. As mush as possible has been written in the api (still some code to migrate), and some temporary internal bridge are used to access oldcore stuffs since augmenting the existing document-bridge does not seems appropriate IMO.
2) The initial enumeration of rights as been replaced by a Right class, which could be seen has a pseudo enum, but could be augmented with new rights. To register a new Right, you have to provide a RightDescription to the AuthorizationManager. The description will define the default state, the tie resolution policy, the inheritance policy, the list of entity types for which the right is applicable, the implied rights and if the right could be allowed in read-only mode. So new defined Right will benefit the whole logic of the AuthorizationManager and currently existing one could be declaratively defined.
3) Large renaming to better distinguish stuffs, clarify comments and prepare for future. I have voluntarily not taken existing names to clearly split the old and the new api. In brief, the new right service is now named AuthorizationManager. Internally, it manipulates SecurityReference (as well as UserSecurityReference and GroupSecurityReference, to represent entities, user and group), SecurityRule (representing a right object) and SecurityAccess (representing an access level in the old nomenclature), which are store in a SecurityCache using SecurityRuleEntry (a set of rules) and SecurityAccessEntry (the access of a given user). The AuthorizationManager delegate cache management to a SecurityCacheLoader which loads rules using a SecurityRuleLoader ; and delegate itself the computing of the access for a given user and a set of rules to an AuthorizationSettler. This last one could be overridden to provide specific decision that could not be done in declarative mode.
4) Refactoring was necessary to improve consistency and reduce complexity, and simplify as much as possible; while extending the limitations to allow more rights to be registered. This work has been a little bit opposed to the optimization done by Andreas, in particular on memory usage. But optimization is often the enemy of clean code.
5) Improvement were necessary to better mimic the existing implementation in some peculiar but necessary rules to stay compatible with current working wiki. I tend to reduce as much as possible what is not done declaratively, but there are still some special cases, like delete for creators, deny for other user on explicit allow and admin for wiki owner that are settle by the authorization settler. My implementation should be almost compatible with the old one, except for groups that are currently not checked from the entity wiki, but only from the user wiki. This needs some more refactoring for which I feel inconfortable with, some I'd like to share first.
6) The AuthorizationManager interface has been simplified, providing 2 methods for either checking or verifying an access right (the checking methods throws while the verifying one return a boolean), and one to register a new right.
The existing RightService could be bridged on the new implementation using the XWikiCachingRightService class in xwiki.cfg and the new API could be used side-by-side with the old implementation as well. What should still really need to be improved is the unit testing, currently some tests are still awful and incomplete. I already refactor some of them, to provide a better coverage of essential part of the code: the security cache and the default authorization settler. Obviously, any help is welcomed.
Since I already have an existing wiki using this implementation successfully and using it for creating new rights for extensions, I would like to merge this new implementation as experimental in platform to have it available for anyone who need it or want to test it, and for you to use in your new experimental development as well. Providing it in platform will encourage it to be finalized and replace the existing implementation.
Here is my +1 for the merge on 4.x,
WDYT ?
On Wed, Mar 28, 2012 at 11:18, Vincent Massol <[email protected]> wrote:
Actually I've now looked at the merge and I've made some quick review in GitHub (I've only done this very quickly nothing thorough).
Have you read my initial mail about this merge proposal ? The branch was there for a while, and I see that merging is the best way to have you look at it :) It's a good start but there are quite a few things to improve:
* Test are a bit of a mess: very long methods (should have a lot more test methods), some bad practices in several places, creating static mocks instead of dynamic mocks.
The tests in the bridge are bad, those came directly from the initial contribution of Andreas, these are not properly mocked, neither properly structured. I have kept them until we have better.
* Missing javadocs in lots of places
This is unfair, this happen mostly in tests. I will try to improve it anyway.
* Missing @since tags
Not easy to put @since until we know when we merge, will be improved. Would be nice not have to do that manually :)
* We also need to stabilize/comment on the API method names themselves. I've commented on a few.
I'd like to know what's the test coverage percentage you have? We need at least 80 to 90% (if not more) for this type of code IMO since it's very sensitive.
I have written the test in the api module. Those in the api module are properly mocked and covers the most critical part (right, settler, data structure, cache) at almost 100%, and the whole api module at 60% currently. I see the api module as the most solid one, and the bridge as a temporary one, until we go further, but I fully agree that we need to improve tests in both. Your help is welcome :)
Thanks -Vincent
On Mar 28, 2012, at 10:33 AM, Vincent Massol wrote:
+1 if you lead the effort to making this implementation the default in the future.
I'd like to know what are the next steps though and their timeframe.
Denis, do you plan to refactor some existing code to use this new API? IMO this would be the best next step to prove that it works. Fo you have any idea about what code could be migrated?
We need to be careful not to keep this new security module not used for too long as otherwise it could "rot". So IMO we need to start using it ASAP and then have a plan for switching to it.
Thanks -Vincent
On Feb 15, 2012, at 11:17 AM, Denis Gervalle wrote:
Hi all,
As you have probably notice, I have recently committed an feature-security-authorization branch on platform. I am working on this for a while now and it was the first step to share the outcome of this large refactoring of the initial work done early last year by Andreas. Since the code was not quality compliant with platform but the general structure Andreas has build seems to me well appropriate, I have progressively refactor its code to better fit our real needs. Here is what I have been done:
1) Split in to module api and bridge to allow breaking the currently unavoidable dependency on oldcore. Now only bridge depends on oldcore, and the api does not depends on bridge. As mush as possible has been written in the api (still some code to migrate), and some temporary internal bridge are used to access oldcore stuffs since augmenting the existing document-bridge does not seems appropriate IMO.
2) The initial enumeration of rights as been replaced by a Right class, which could be seen has a pseudo enum, but could be augmented with new rights. To register a new Right, you have to provide a RightDescription to the AuthorizationManager. The description will define the default state, the tie resolution policy, the inheritance policy, the list of entity types for which the right is applicable, the implied rights and if the right could be allowed in read-only mode. So new defined Right will benefit the whole logic of the AuthorizationManager and currently existing one could be declaratively defined.
3) Large renaming to better distinguish stuffs, clarify comments and prepare for future. I have voluntarily not taken existing names to clearly split the old and the new api. In brief, the new right service is now named AuthorizationManager. Internally, it manipulates SecurityReference (as well as UserSecurityReference and GroupSecurityReference, to represent entities, user and group), SecurityRule (representing a right object) and SecurityAccess (representing an access level in the old nomenclature), which are store in a SecurityCache using SecurityRuleEntry (a set of rules) and SecurityAccessEntry (the access of a given user). The AuthorizationManager delegate cache management to a SecurityCacheLoader which loads rules using a SecurityRuleLoader ; and delegate itself the computing of the access for a given user and a set of rules to an AuthorizationSettler. This last one could be overridden to provide specific decision that could not be done in declarative mode.
4) Refactoring was necessary to improve consistency and reduce complexity, and simplify as much as possible; while extending the limitations to allow more rights to be registered. This work has been a little bit opposed to the optimization done by Andreas, in particular on memory usage. But optimization is often the enemy of clean code.
5) Improvement were necessary to better mimic the existing implementation in some peculiar but necessary rules to stay compatible with current working wiki. I tend to reduce as much as possible what is not done declaratively, but there are still some special cases, like delete for creators, deny for other user on explicit allow and admin for wiki owner that are settle by the authorization settler. My implementation should be almost compatible with the old one, except for groups that are currently not checked from the entity wiki, but only from the user wiki. This needs some more refactoring for which I feel inconfortable with, some I'd like to share first.
6) The AuthorizationManager interface has been simplified, providing 2 methods for either checking or verifying an access right (the checking methods throws while the verifying one return a boolean), and one to register a new right.
The existing RightService could be bridged on the new implementation using the XWikiCachingRightService class in xwiki.cfg and the new API could be used side-by-side with the old implementation as well. What should still really need to be improved is the unit testing, currently some tests are still awful and incomplete. I already refactor some of them, to provide a better coverage of essential part of the code: the security cache and the default authorization settler. Obviously, any help is welcomed.
Since I already have an existing wiki using this implementation successfully and using it for creating new rights for extensions, I would like to merge this new implementation as experimental in platform to have it available for anyone who need it or want to test it, and for you to use in your new experimental development as well. Providing it in platform will encourage it to be finalized and replace the existing implementation.
Here is my +1 for the merge on 4.x,
WDYT ?
_______________________________________________ devs mailing list [email protected] http://lists.xwiki.org/mailman/listinfo/devs
-- Denis Gervalle SOFTEC sa - CEO eGuilde sarl - CTO
2012-03-28 18:29, Denis Gervalle skrev:
On Wed, Mar 28, 2012 at 11:18, Vincent Massol <[email protected]> wrote:
Actually I've now looked at the merge and I've made some quick review in GitHub (I've only done this very quickly nothing thorough).
Have you read my initial mail about this merge proposal ? The branch was there for a while, and I see that merging is the best way to have you look at it :)
It's a good start but there are quite a few things to improve:
* Test are a bit of a mess: very long methods (should have a lot more test methods), some bad practices in several places, creating static mocks instead of dynamic mocks.
The tests in the bridge are bad, those came directly from the initial contribution of Andreas, these are not properly mocked, neither properly structured. I have kept them until we have better.
Writing proper tests have been on my TODO-list since june 2010. Now when it is finally on-track to be included as the default implementation, I'll be happy to write some tests. Best Regards, /Andreas
* Missing javadocs in lots of places
This is unfair, this happen mostly in tests. I will try to improve it anyway.
* Missing @since tags
Not easy to put @since until we know when we merge, will be improved. Would be nice not have to do that manually :)
* We also need to stabilize/comment on the API method names themselves. I've commented on a few.
I'd like to know what's the test coverage percentage you have? We need at least 80 to 90% (if not more) for this type of code IMO since it's very sensitive.
I have written the test in the api module. Those in the api module are properly mocked and covers the most critical part (right, settler, data structure, cache) at almost 100%, and the whole api module at 60% currently. I see the api module as the most solid one, and the bridge as a temporary one, until we go further, but I fully agree that we need to improve tests in both. Your help is welcome :)
Thanks -Vincent
On Mar 28, 2012, at 10:33 AM, Vincent Massol wrote:
+1 if you lead the effort to making this implementation the default in the future.
I'd like to know what are the next steps though and their timeframe.
Denis, do you plan to refactor some existing code to use this new API? IMO this would be the best next step to prove that it works. Fo you have any idea about what code could be migrated?
We need to be careful not to keep this new security module not used for too long as otherwise it could "rot". So IMO we need to start using it ASAP and then have a plan for switching to it.
Thanks -Vincent
On Feb 15, 2012, at 11:17 AM, Denis Gervalle wrote:
Hi all,
As you have probably notice, I have recently committed an feature-security-authorization branch on platform. I am working on this for a while now and it was the first step to share the outcome of this large refactoring of the initial work done early last year by Andreas. Since the code was not quality compliant with platform but the general structure Andreas has build seems to me well appropriate, I have progressively refactor its code to better fit our real needs. Here is what I have been done:
1) Split in to module api and bridge to allow breaking the currently unavoidable dependency on oldcore. Now only bridge depends on oldcore, and the api does not depends on bridge. As mush as possible has been written in the api (still some code to migrate), and some temporary internal bridge are used to access oldcore stuffs since augmenting the existing document-bridge does not seems appropriate IMO.
2) The initial enumeration of rights as been replaced by a Right class, which could be seen has a pseudo enum, but could be augmented with new rights. To register a new Right, you have to provide a RightDescription to the AuthorizationManager. The description will define the default state, the tie resolution policy, the inheritance policy, the list of entity types for which the right is applicable, the implied rights and if the right could be allowed in read-only mode. So new defined Right will benefit the whole logic of the AuthorizationManager and currently existing one could be declaratively defined.
3) Large renaming to better distinguish stuffs, clarify comments and prepare for future. I have voluntarily not taken existing names to clearly split the old and the new api. In brief, the new right service is now named AuthorizationManager. Internally, it manipulates SecurityReference (as well as UserSecurityReference and GroupSecurityReference, to represent entities, user and group), SecurityRule (representing a right object) and SecurityAccess (representing an access level in the old nomenclature), which are store in a SecurityCache using SecurityRuleEntry (a set of rules) and SecurityAccessEntry (the access of a given user). The AuthorizationManager delegate cache management to a SecurityCacheLoader which loads rules using a SecurityRuleLoader ; and delegate itself the computing of the access for a given user and a set of rules to an AuthorizationSettler. This last one could be overridden to provide specific decision that could not be done in declarative mode.
4) Refactoring was necessary to improve consistency and reduce complexity, and simplify as much as possible; while extending the limitations to allow more rights to be registered. This work has been a little bit opposed to the optimization done by Andreas, in particular on memory usage. But optimization is often the enemy of clean code.
5) Improvement were necessary to better mimic the existing implementation in some peculiar but necessary rules to stay compatible with current working wiki. I tend to reduce as much as possible what is not done declaratively, but there are still some special cases, like delete for creators, deny for other user on explicit allow and admin for wiki owner that are settle by the authorization settler. My implementation should be almost compatible with the old one, except for groups that are currently not checked from the entity wiki, but only from the user wiki. This needs some more refactoring for which I feel inconfortable with, some I'd like to share first.
6) The AuthorizationManager interface has been simplified, providing 2 methods for either checking or verifying an access right (the checking methods throws while the verifying one return a boolean), and one to register a new right.
The existing RightService could be bridged on the new implementation using the XWikiCachingRightService class in xwiki.cfg and the new API could be used side-by-side with the old implementation as well. What should still really need to be improved is the unit testing, currently some tests are still awful and incomplete. I already refactor some of them, to provide a better coverage of essential part of the code: the security cache and the default authorization settler. Obviously, any help is welcomed.
Since I already have an existing wiki using this implementation successfully and using it for creating new rights for extensions, I would like to merge this new implementation as experimental in platform to have it available for anyone who need it or want to test it, and for you to use in your new experimental development as well. Providing it in platform will encourage it to be finalized and replace the existing implementation.
Here is my +1 for the merge on 4.x,
WDYT ?
_______________________________________________ devs mailing list [email protected] http://lists.xwiki.org/mailman/listinfo/devs
2012-03-28 10:33, Vincent Massol skrev:
+1 if you lead the effort to making this implementation the default in the future.
I'd like to know what are the next steps though and their timeframe.
Regarding the short term plans, I will work to make this implementation the default as soon as possible. It is, of course, up to the community to decide when it is ready to make the switch, but I think it's possible to replace it within the next development cycle. My thoughts about the long term plans: * Stabilize the API and refactor old code to use the new right service directly. * Move redirection-to-login-page and action-to-right mapping to other modules. * Introduce a new API for setting and accessing rights, and deprecate accessing the rights objects through the object API. * Implement XWIKI-2184. * Implement a new rights storage. In particular, I would like to be able to support filtering on access level directly in database queries with reasonable performance. * Change the settler policy (i.e. the decision on which rights should be granted, given a set of partially conflicting rights that have been configured at wiki, space and document level) to something more intuitive. Best regards, /Andreas
Denis, do you plan to refactor some existing code to use this new API? IMO this would be the best next step to prove that it works. Fo you have any idea about what code could be migrated?
We need to be careful not to keep this new security module not used for too long as otherwise it could "rot". So IMO we need to start using it ASAP and then have a plan for switching to it.
Thanks -Vincent
On Feb 15, 2012, at 11:17 AM, Denis Gervalle wrote:
Hi all,
As you have probably notice, I have recently committed an feature-security-authorization branch on platform. I am working on this for a while now and it was the first step to share the outcome of this large refactoring of the initial work done early last year by Andreas. Since the code was not quality compliant with platform but the general structure Andreas has build seems to me well appropriate, I have progressively refactor its code to better fit our real needs. Here is what I have been done:
1) Split in to module api and bridge to allow breaking the currently unavoidable dependency on oldcore. Now only bridge depends on oldcore, and the api does not depends on bridge. As mush as possible has been written in the api (still some code to migrate), and some temporary internal bridge are used to access oldcore stuffs since augmenting the existing document-bridge does not seems appropriate IMO.
2) The initial enumeration of rights as been replaced by a Right class, which could be seen has a pseudo enum, but could be augmented with new rights. To register a new Right, you have to provide a RightDescription to the AuthorizationManager. The description will define the default state, the tie resolution policy, the inheritance policy, the list of entity types for which the right is applicable, the implied rights and if the right could be allowed in read-only mode. So new defined Right will benefit the whole logic of the AuthorizationManager and currently existing one could be declaratively defined.
3) Large renaming to better distinguish stuffs, clarify comments and prepare for future. I have voluntarily not taken existing names to clearly split the old and the new api. In brief, the new right service is now named AuthorizationManager. Internally, it manipulates SecurityReference (as well as UserSecurityReference and GroupSecurityReference, to represent entities, user and group), SecurityRule (representing a right object) and SecurityAccess (representing an access level in the old nomenclature), which are store in a SecurityCache using SecurityRuleEntry (a set of rules) and SecurityAccessEntry (the access of a given user). The AuthorizationManager delegate cache management to a SecurityCacheLoader which loads rules using a SecurityRuleLoader ; and delegate itself the computing of the access for a given user and a set of rules to an AuthorizationSettler. This last one could be overridden to provide specific decision that could not be done in declarative mode.
4) Refactoring was necessary to improve consistency and reduce complexity, and simplify as much as possible; while extending the limitations to allow more rights to be registered. This work has been a little bit opposed to the optimization done by Andreas, in particular on memory usage. But optimization is often the enemy of clean code.
5) Improvement were necessary to better mimic the existing implementation in some peculiar but necessary rules to stay compatible with current working wiki. I tend to reduce as much as possible what is not done declaratively, but there are still some special cases, like delete for creators, deny for other user on explicit allow and admin for wiki owner that are settle by the authorization settler. My implementation should be almost compatible with the old one, except for groups that are currently not checked from the entity wiki, but only from the user wiki. This needs some more refactoring for which I feel inconfortable with, some I'd like to share first.
6) The AuthorizationManager interface has been simplified, providing 2 methods for either checking or verifying an access right (the checking methods throws while the verifying one return a boolean), and one to register a new right.
The existing RightService could be bridged on the new implementation using the XWikiCachingRightService class in xwiki.cfg and the new API could be used side-by-side with the old implementation as well. What should still really need to be improved is the unit testing, currently some tests are still awful and incomplete. I already refactor some of them, to provide a better coverage of essential part of the code: the security cache and the default authorization settler. Obviously, any help is welcomed.
Since I already have an existing wiki using this implementation successfully and using it for creating new rights for extensions, I would like to merge this new implementation as experimental in platform to have it available for anyone who need it or want to test it, and for you to use in your new experimental development as well. Providing it in platform will encourage it to be finalized and replace the existing implementation.
Here is my +1 for the merge on 4.x,
WDYT ?
devs mailing list [email protected] http://lists.xwiki.org/mailman/listinfo/devs .
participants (6)
-
Andreas Jonsson -
Denis Gervalle -
Ludovic Dubost -
Marius Dumitru Florea -
Thomas Mortagne -
Vincent Massol