You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@ofbiz.apache.org by Ashish Vijaywargiya <vi...@gmail.com> on 2015/02/09 12:24:23 UTC
Re: Review Multi-Tenancy Setup regarding OFBIZ-5898, commit r1645310
and OFBIZ-5986
Hello Pierre,
Thanks for providing all the details for tenant functionality.
Today Arun and I had a good discussion on all the points that you shared
here. Arun will provide the concluded details here.
--
Kind Regards,
Ashish Vijaywargiya
On Thu, Jan 29, 2015 at 7:19 AM, Pierre Smits <pi...@gmail.com>
wrote:
> This is a review regarding the effect of recent changes committed through
> r1645310 (patch provided through issue OFBIZ-5898) and the patch provided
> through OFBIZ-5986.
>
> *References:*
>
> - OFBIZ-5898 Tenant should run with specified domain name. Front store
> should support mulit tenancy feature (see:
> https://issues.apache.org/jira/browse/OFBIZ-5898)
> - OFBIZ commit r1643510 (see http://svn.apache.org/r1643510)
> - OFBIZ-5986 Error on accessing new tenant (see
> https://issues.apache.org/jira/browse/OFBIZ-5986)
>
>
>
> *Background*
> OFBiz facilitates the single tenant and multiple tenants setups. These
> setups are:
>
> 1. Single tenancy setup, supporting the master having a single internal
> organisation or multiple internal organisations
> 2. Multi tenancy setup, supporting each tenant (including the master)
> having a single internal organisation or multiple
>
> Ad 1.
> This is enforced through following property in
> /framework/common/config/general.properties and utilises the datasets in
> databases ofbiz, ofbizolap and ofbiztenant (the master databases) in the
> RDBMS:
>
> mulititenant=N
>
> Multiple internal organisations is facilitated through the OFBiz Setup
> application/component.
>
> Ad 2.
> This is enforced through following property in
> /framework/common/config/general.properties:
>
> mulititenant=Y
>
> Multiple internal organisations per tenant is facilitated through the same
> application as mentioned in ad 1.
>
> The effect of 'mulitenant=Y' is that the tenantId field is shown on the
> login screen and
>
> 1. that the tenantId can be valid (checked against records in the Tenant
> entity in master database ofbiztenant of the RDBMS)
> 2. that the user can be authenticated against the permission settings in
> the tenant database (ofbiz-<tenantId>) and can utilise the functions in
> an
> applications to create, augment and delete records in all the tenant
> databases in the RDBMS (ofbiz-<tenantId> and ofbizolap-<tenantId).
>
> A user logging in at a tenant (the tenantId field has a value in the login
> screen) can never access the data of the master databases (ofbiz, ofbizolap
> and ofbiztenant).
>
>
> *Re: issue OFBIZ-5898 Tenant should run with specified domain name. Front
> store should support mulit tenancy feature*
> This subject of the issue suggest two things, namely:
>
> - a perceived bug: in OFBiz it isn't possible to run a tenant within a
> specific domain, and
> - an improvement: store front should support multi tenant feature
>
> Subsequently, following aspects are lined up in the description to address
> the subject of the issue:
>
> 1. Enable multi tenancy for a store front application
> 2. Tenant should run with specified domain name
> 3. User should be able to provide the domain name during tenant creation
> 4. Configuration details should be tenant specific
>
>
> *Re: perceptions and findings regarding the issue, the patch provided and
> subsequent commit r1645310*
> This issue is about having the functionality to be able to associate
> multiple domains (sites) to a tenant in stead of just one domain, as I
> understand it, as well as being about having per tenant configuration
> settings.
>
> I will address the issue, the patch (the second - revised - one submitted
> on Dec 6th at 07.47) and subsequent commit per aspect outlined in the
> description of the issue.
>
> *aspect 1. Enable multi tenancy for a store front application*
> As I see this, it is about having an application (the store front) work for
> both anonymous users and authenticated users given the existence of
> multiple tenants. The creator of the issue perceives that OFBiz does not
> have the capability to deliver functionalities in an application to support
> anonymous users. The applications in the feature portfolio intended for
> anonymous users are:
>
> 1. The E-commerce application - application in the Special Purpose stack
> 2. The MyPortal application - application in the Special Purpose stack.
>
> This is about the first, the E-commerce application, whereby the creator of
> issue believes (as I see it) that OFBiz doesn't have the capabilities to
> deliver functionalities to anonymous users given the multi tenancy setup
> and thus having tenant specific data being delivered to a anonymous user of
> a particular tenant. Because after all, how does OFBiz retrieve and deliver
> tenant specific data to an anonymous user when the tenant is not
> identified?
>
> Hence the introduction of aspect 2 of the issue which I will address later.
>
> First a little explanation of how the delegator works regarding single and
> multi tenancy setups.
> In a single tenancy setup (multitenant=N) the delegator is always
> 'default'. This 'default' comes from the setting in the web.xml
> configuration file in any and all applications in OFBiz. See following
> example:
>
> *<context-param>*
>
> *<param-name>entityDelegatorName</param-name>*
>
> *<param-value>default</param-value>*
>
> *<description>The Name of the Entity Delegator to use, defined in
> entityengine.xml</description>*
>
> *</context-param>*
>
> This ensures that OFBiz can work with the application without regard of
> tenantId and whether any user is logged in or not. It is more about OFBiz
> being able to work with the application than a user, namely being able to
> work with data from the master databases (ofbiz, ofbizolap and
> ofbiztenant).
>
> In a multi tenancy setup (multitenant=Y) the delegator gets, when a user
> logs in at the login screen and provides the tenantId, extended with a
> separator (the # sign) and the tenantId, resulting in 'default#<tenantId'
> as can be seen in the logs at various registrations to have the user to be
> able to work with functions of applications in relation to data of the
> tenant.
>
> So, how does OFBiz enable access to the application, retrieve and deliver
> tenant specific data for the anonymous users of an application given a
> specific tenant?
>
> Well, this is done by changing the configuration setting for the delegator
> in the web.xml of the application that should deliver the tenant specific
> data.
>
> When the setting in web.xml of an application is changed to
>
> *<context-param>*
>
> *<param-name>entityDelegatorName</param-name>*
>
> *<param-value>default#<tenantId></param-value>*
>
> *<description>The Name of the Entity Delegator to use, defined in
> entityengine.xml</description>*
>
> *</context-param>*
>
> the application/component will deliver data of the tenant again to
> authenticated/authorised users when they provide the tenantId in the login
> screen, but also to anonymous users (who don't provide the tenantId). The
> specific entityDelegatorName tells OFBiz from which associated database it
> should retrieve data from and write data to. It essentially overrides the
> generic function regarding the 'default' way of working. This also ensures
> application can't be accessed and data can't be retrieved from or written
> to by authenticated users of a different tenant (different tenantId) or
> even users authenticated against the master.
>
> So regarding aspect 1 of the issue, OFbiz is covered and working as
> intended and no change should be needed.
>
> *Aspect 2: Tenant should run with specified domain name*
> Prior to the issue, only 1 domain name could be registered in a record in
> the Tenant entity through the interfaces of WEBTools.
>
> Apart from that OFBiz has the capabilities to define site URIs in the
> content application, so that ofbiz can work with not only multiple domains
> (
> www.example.com vs www.rexample.org), but also with multiple sites within
> one domain (site1.example.com vs site2.example.com). Given the nature of
> OFBiz this works for both the single tenancy setup and the multi tenancy
> setup. Again, for anonymous and authenticated users. This I mentioned in a
> comment in the issue. See:
>
> https://issues.apache.org/jira/browse/OFBIZ-5898?focusedCommentId=14235398&page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#comment-14235398
>
> But, as I surmise, the creator of the issue was not aware of this
> functionality in the content application (and apparently chose to ignore
> the comment in the issue about content functionality visavis domains/web
> sites in the issue) and thus meant the aspect to be: have multi domain
> functionality in the tenant configuration (at master level).
>
> This aspect seems to be the main reason that the patch addresses.
>
> *Aspect 3: User should be able to provide domain name at tenant creation.*
> Prior to the issue, the OFBiz administrator (the user) could not set the
> domain name at creation of the tenant. Also, the OFBiz administrator could
> only set the one domain name through functionalities in the WEBTOOLS
> application by editing existing tenant records in the Tenant entity.
>
> Again I surmise, as no additional explanation is given within the context
> of the issue by the creator, that he sought to improve the process by
> providing the ability/option for setting the domain name in the tenant
> creation functionality (ant target).
>
> This aspect seems to be the second reason that the patch addresses.
>
> Aspect 4. Configuration details should be tenant specific.
> In order to have a tenant specific aspect to work properly, configurations
> settings should be applicable per tenant, and if a configuration setting is
> not provided for a specific tenant the default configuration settings (in
> the properties files of various applications) should/must be used.
>
> This aspect is not addressed by the patch. In fact, in a comment in the
> issue, the creator stated that a new issue were to be created (and has been
> created) to address this specifically). See
> https://issues.apache.org/jira/browse/OFBIZ-5902
>
> *Effects of the patch on how OFBiz works after the commit*
>
>
> The commit has the following effect on how OFBiz is working:
>
> - Creating a tenant - the new method of creating a tenant (./ant
> create-tenant) now included setting the domain name of the tenant. This
> results in setting the tenant id and the domain name in entity
> TenantDomainName.
> As setting the domain name is optional when executing ./ant
> create-tenant, this should not be executed when the domain name is left
> blank. However, setting the domain name is always executed, and as the
> domainName is (part of) the primary key of the entity TenantDomainName
> it
> will be automatically generated (gets sequenced Id) when left blank.
>
> - Changing how urlServletHelper is working. Prior to the change the
> domain name was checked against the records in entity Tenant (single
> domain
> setup). With the change the domain name is now checked against the
> records
> in entity TenantDomainName (multiple domain setup).
>
> - Changing how ContextFilter is working. Prior to the change the domain
> name was checked against the records in entity Tenant (single domain
> setup). With the change the domain name is now checked against the
> records
> in entity TenantDomainName (multiple domain setup).
>
> - Changing how LoginWorker is working. Prior to the change the login was
> checked against availability set through the login screen or (when not
> available) the setting in the param-name 'entityDelegatorName' in the
> web.xml of the component. With the change an additional check is
> included
> and the cleanup of the delegator (from '*default**#<tenantId>'* back to
> 'default') is removed.
>
> Re: domainName setting (aspect 3)
> Having this setting of the domain name on the moment of the creation of the
> tenant via ./ant create-tenant (the commit) seems to be a regression in
> functionality,as it always creates a tenant domain (using a sequenced Id
> for the domainName field in entity TenantDomainName when left empty), in
> stead of working with the optionality (the choice not to set the domain
> name)
>
> Re: context-param param-name='entityDelegatorName' setting
> It seems that the commit has led to a more relaxed level of security
> regarding access to tenant specific applications/components.
>
> By setting the context-param for the entityDelegatorName in the web.xml
> file of the component, there is the extra security layer that the
> application/component is made available only to users within the defined
> delegator (*default**#<tenantId>*).
> This ensures that it can't be accessed by users of other tenants (apart
> from accidentally having the combination (userId-passWord-tenantId) right
> on login. That also excludes users of the master setup to be able to access
> the application/component, inherent to the expectation that userIds in the
> master database (ofbiz) don't exist in the db of the tenant
> (ofbiz_<tenantId>), irrespectively of having the SecurityPermissionSeedData
> of the tenant specific application and associations between userId and the
> security permissions of that application in the tables of the master
> database. Now it doesn't work that way anymore.
>
> Having tested against trunk and having security permissions of a tenant
> specific database in the master database combined with a userId associated
> to those security permissions, a user accessing the master environment
> (meaning: without providing the tenantId) sees the name of the tenant
> specific application in the top menu and can access that application. This
> is wrong.
>
>
> I did not test the functionality against another domain (the intended happy
> flow) than the default (localhost).
>
> I surmise that when the patch appeared in the issue only the code and the
> happy flow was reviewed and tested by the committer, no more thoughts were
> given regarding non-happy flows, how the changes affects how OFBiz works
> with respect to backend functionality or the comment regarding OFBiz
> content application and domains/web sites.
>
> As an example, the use-case regarding the removal of the delegator cleanup
> was neither motivated in the description of the issue as a need, nor
> questioned when the patch was committed.
>
>
> *Re: OFBIZ-5986 Error on accessing new tenant*
> This issue appeared after incorporation of commit r1643510 and seems
> similar to issue OFBIZ-5447 (see
> https://issues.apache.org/jira/browse/OFBIZ-5447). Strangely, OFBIZ-5447
> was closed by the reviewing committer approximately an hour before he
> created OFBIZ-5986.But that is another issue. Shortly thereafter a patch
> was provided, which has not been reviewed till today.
>
>
>
> *The patch*
>
>
> - Changes how ContextFilter is working by adding additional checks for
> when a tenantId is not provided, like in the case of logging in at the
> master.
> - Removing changes introduced at r1643510 regarding, about the check
> when the delegatorName isn't equal to the baseDelegatorName (in effect:
> *default**#<tenantId>* !- default ) and the actions for when they aren't
> the same.
>
> Testing the patch against a backend applications in domain localhost
> yielded the following results:
>
> 1. While logging in at the master a tenant specific application (with
> context-param param-name='entityDelegatorName' having value *default*
> *#<tenantId>*) still shows the tenant specific application when the seed
> and demo data of that app is in the master database and the user can
> access
> the application. Effectively, the contex-param isn't factored in
> security-wise.
> 2. While logging in at the tenant (by providing the tenantId at the
> login screen) at the url for the tenant specific application (
> https://localhost:8443/tb5896/control/login) led to an NPE.
> 3. Logging in at the tenant at the url for a application that is not
> tenant specific context-param param-name='entityDelegatorName' having
> value
> *default*) yields success.
> 4. Switching from the non tenant specific application to the tenant
> specific application led to the presentation of the login screen. This
> should not have happened as the user is already logged in at the tenant.
> After having provided the credentials of the tenant user again the
> application can be accessed.
>
>
>
> *Suggestions:*
> Given that the commit has led to unwanted effects (as indicated in the user
> ml, see
>
> http://ofbiz.markmail.org/message/22bmphzfhkprhrcm?q=Multiple+root+apps+%28tenant%29
> and
>
> http://ofbiz.markmail.org/message/7pdfttecmqx6ygio?q=Unable+to+connect+tenant+component+with+tenant+database
> )
> and the creation of the second issue (OFBIZ-5986), I suggest that we reopen
> both issues (5898 and 5986) and redo the process and rework the solution.
>
> The reason for this is the following: Issues and their subjects/titles are
> incorporated in release notes, and when a user needs to evaluate a release
> (for upgrade or new implementation) he will look first at those release
> notes to be able to assess the applicability of the release regarding his
> requirements. If such an issue subject is unclear to him, he will look at
> the description of such issue. At first he will disregard the comments.
>
> OFBIZ-5898 has both an ambiguous title and aspects in the description that
> aren't addressed by the patch, but are in one or more other issues. E.g.
> aspect 4. And the commit breaks existing functionality.
> In stead of leaving the commit in and resolve the effects in other issues
> (like OFBIZ-5896) and creating more confusion for evaluators of the next
> release, we can reworking title and description of OFBIZ-5898 to we improve
> the clarity and applicability of the issue and the focus of the solution
> and redo the solution. That way we can determine issue OFBIZ-5986 is still
> warranted and can address the comment regarding OFBIZ content application
> visavis domains/web sites and the comments and findings stated above.
>
> Best regards,
>
> Pierre Smits
>
> *ORRTIZ.COM <http://www.orrtiz.com>*
> Services & Solutions for Cloud-
> Based Manufacturing, Professional
> Services and Retail & Trade
> http://www.orrtiz.com
>
Re: Review Multi-Tenancy Setup regarding OFBIZ-5898, commit
r1645310 and OFBIZ-5986
Posted by Jacques Le Roux <ja...@les7arts.com>.
Thanks for the feedback Arun.
Jacques
Le 14/04/2015 08:34, Arun Patidar a écrit :
> Hi Jacques,
>
> I have discussed these issues with Pierre on Skype chat. It seems that these issues needs more discussions to come up with a proper fix. Currently I
> am not working on these issues so any one who wants to work and have proper fix can start on same.
>
> Thanks & Regards
> ---
> Arun Patidar
> Manager,Enterprise Software Development
> HotWax Media
> www.hotwaxsystems.com
>
> On Friday 10 April 2015 07:37 PM, Jacques Le Roux wrote:
>> Hi Arun,
>>
>> What is the situation here please? Should other people work on it or are you still expecting to fix it? Did I miss a Jira with the fix?
>>
>> Thanks
>>
>> Jacques
>>
>> Le 19/03/2015 23:43, Jacques Le Roux a écrit :
>>> Le 19/03/2015 23:35, Jacques Le Roux a écrit :
>>>> Hi Arun,
>>>>
>>>> Sorry it was hard to follow, are there Jiras?
>>>
>>> Ah, Pierre created them:
>>> https://issues.apache.org/jira/browse/OFBIZ-6065
>>> https://issues.apache.org/jira/browse/OFBIZ-6066
>>>
>>> BTW from Pierre message I guess the committer is also committed...
>>>
>>> Jacques
>>>
>>>>
>>>> For 2. Pierre Smits does not seem to agree: http://markmail.org/message/pjqjw4s4mzwkczvz
>>>>
>>>> Did you get a chance to look at it, is it done?
>>>>
>>>> Thanks
>>>>
>>>> Jacques
>>>>
>>>> Le 09/02/2015 21:44, Jacques Le Roux a écrit :
>>>>> Thanks for the update Arun!
>>>>>
>>>>> Le 09/02/2015 13:54, Arun Patidar a écrit :
>>>>>> A). Below are the two issues on which I will work:
>>>>>>
>>>>>> 1. Domain name should not create for tenant if its left empty during tenant creation.
>>>>>> 2. Tenant component should not visible to other tenant or default.
>>>>>> -- Its not related to my patch and changes but I will give a try to fix this issue as this should not be the case.
>>>>>
>>>>> That's what I have remembered from the discussion
>>>>>
>>>>> Looking forward
>>>>>
>>>>> Jacques
>>>>>
>>>>
>>>
>
>
Re: Review Multi-Tenancy Setup regarding OFBIZ-5898, commit r1645310
and OFBIZ-5986
Posted by Arun Patidar <ar...@hotwaxsystems.com>.
Hi Jacques,
I have discussed these issues with Pierre on Skype chat. It seems that
these issues needs more discussions to come up with a proper fix.
Currently I am not working on these issues so any one who wants to work
and have proper fix can start on same.
Thanks & Regards
---
Arun Patidar
Manager,Enterprise Software Development
HotWax Media
www.hotwaxsystems.com
On Friday 10 April 2015 07:37 PM, Jacques Le Roux wrote:
> Hi Arun,
>
> What is the situation here please? Should other people work on it or
> are you still expecting to fix it? Did I miss a Jira with the fix?
>
> Thanks
>
> Jacques
>
> Le 19/03/2015 23:43, Jacques Le Roux a écrit :
>> Le 19/03/2015 23:35, Jacques Le Roux a écrit :
>>> Hi Arun,
>>>
>>> Sorry it was hard to follow, are there Jiras?
>>
>> Ah, Pierre created them:
>> https://issues.apache.org/jira/browse/OFBIZ-6065
>> https://issues.apache.org/jira/browse/OFBIZ-6066
>>
>> BTW from Pierre message I guess the committer is also committed...
>>
>> Jacques
>>
>>>
>>> For 2. Pierre Smits does not seem to agree:
>>> http://markmail.org/message/pjqjw4s4mzwkczvz
>>>
>>> Did you get a chance to look at it, is it done?
>>>
>>> Thanks
>>>
>>> Jacques
>>>
>>> Le 09/02/2015 21:44, Jacques Le Roux a écrit :
>>>> Thanks for the update Arun!
>>>>
>>>> Le 09/02/2015 13:54, Arun Patidar a écrit :
>>>>> A). Below are the two issues on which I will work:
>>>>>
>>>>> 1. Domain name should not create for tenant if its left empty
>>>>> during tenant creation.
>>>>> 2. Tenant component should not visible to other tenant or default.
>>>>> -- Its not related to my patch and changes but I will give a
>>>>> try to fix this issue as this should not be the case.
>>>>
>>>> That's what I have remembered from the discussion
>>>>
>>>> Looking forward
>>>>
>>>> Jacques
>>>>
>>>
>>
Re: Review Multi-Tenancy Setup regarding OFBIZ-5898, commit
r1645310 and OFBIZ-5986
Posted by Jacques Le Roux <ja...@les7arts.com>.
Hi Arun,
What is the situation here please? Should other people work on it or are you still expecting to fix it? Did I miss a Jira with the fix?
Thanks
Jacques
Le 19/03/2015 23:43, Jacques Le Roux a écrit :
> Le 19/03/2015 23:35, Jacques Le Roux a écrit :
>> Hi Arun,
>>
>> Sorry it was hard to follow, are there Jiras?
>
> Ah, Pierre created them:
> https://issues.apache.org/jira/browse/OFBIZ-6065
> https://issues.apache.org/jira/browse/OFBIZ-6066
>
> BTW from Pierre message I guess the committer is also committed...
>
> Jacques
>
>>
>> For 2. Pierre Smits does not seem to agree: http://markmail.org/message/pjqjw4s4mzwkczvz
>>
>> Did you get a chance to look at it, is it done?
>>
>> Thanks
>>
>> Jacques
>>
>> Le 09/02/2015 21:44, Jacques Le Roux a écrit :
>>> Thanks for the update Arun!
>>>
>>> Le 09/02/2015 13:54, Arun Patidar a écrit :
>>>> A). Below are the two issues on which I will work:
>>>>
>>>> 1. Domain name should not create for tenant if its left empty during tenant creation.
>>>> 2. Tenant component should not visible to other tenant or default.
>>>> -- Its not related to my patch and changes but I will give a try to fix this issue as this should not be the case.
>>>
>>> That's what I have remembered from the discussion
>>>
>>> Looking forward
>>>
>>> Jacques
>>>
>>
>
Re: Review Multi-Tenancy Setup regarding OFBIZ-5898, commit
r1645310 and OFBIZ-5986
Posted by Jacques Le Roux <ja...@les7arts.com>.
Le 19/03/2015 23:35, Jacques Le Roux a écrit :
> Hi Arun,
>
> Sorry it was hard to follow, are there Jiras?
Ah, Pierre created them:
https://issues.apache.org/jira/browse/OFBIZ-6065
https://issues.apache.org/jira/browse/OFBIZ-6066
BTW from Pierre message I guess the committer is also committed...
Jacques
>
> For 2. Pierre Smits does not seem to agree: http://markmail.org/message/pjqjw4s4mzwkczvz
>
> Did you get a chance to look at it, is it done?
>
> Thanks
>
> Jacques
>
> Le 09/02/2015 21:44, Jacques Le Roux a écrit :
>> Thanks for the update Arun!
>>
>> Le 09/02/2015 13:54, Arun Patidar a écrit :
>>> A). Below are the two issues on which I will work:
>>>
>>> 1. Domain name should not create for tenant if its left empty during tenant creation.
>>> 2. Tenant component should not visible to other tenant or default.
>>> -- Its not related to my patch and changes but I will give a try to fix this issue as this should not be the case.
>>
>> That's what I have remembered from the discussion
>>
>> Looking forward
>>
>> Jacques
>>
>
Re: Review Multi-Tenancy Setup regarding OFBIZ-5898, commit
r1645310 and OFBIZ-5986
Posted by Jacques Le Roux <ja...@les7arts.com>.
Hi Arun,
Sorry it was hard to follow, are there Jiras?
For 2. Pierre Smits does not seem to agree: http://markmail.org/message/pjqjw4s4mzwkczvz
Did you get a chance to look at it, is it done?
Thanks
Jacques
Le 09/02/2015 21:44, Jacques Le Roux a écrit :
> Thanks for the update Arun!
>
> Le 09/02/2015 13:54, Arun Patidar a écrit :
>> A). Below are the two issues on which I will work:
>>
>> 1. Domain name should not create for tenant if its left empty during tenant creation.
>> 2. Tenant component should not visible to other tenant or default.
>> -- Its not related to my patch and changes but I will give a try to fix this issue as this should not be the case.
>
> That's what I have remembered from the discussion
>
> Looking forward
>
> Jacques
>
Re: Review Multi-Tenancy Setup regarding OFBIZ-5898, commit
r1645310 and OFBIZ-5986
Posted by Jacques Le Roux <ja...@les7arts.com>.
Thanks for the update Arun!
Le 09/02/2015 13:54, Arun Patidar a écrit :
> A). Below are the two issues on which I will work:
>
> 1. Domain name should not create for tenant if its left empty during tenant creation.
> 2. Tenant component should not visible to other tenant or default.
> -- Its not related to my patch and changes but I will give a try to fix this issue as this should not be the case.
That's what I have remembered from the discussion
Looking forward
Jacques
Re: Review Multi-Tenancy Setup regarding OFBIZ-5898, commit r1645310
and OFBIZ-5986
Posted by Arun Patidar <ar...@hotwaxmedia.com>.
Hi Pierre,
As I am the creator of issue and patch so trying to provide details on
each concern you mentioned. Here is a document 'Extension in
Multitenancy support <https://cwiki.apache.org/confluence/x/9oT0Ag>' in
which I explained the extended multi tenancy functionality of OFBiz.
Please see below details for each concern:
*Aspect 1. Enable multi tenancy for a store front application*
-- There is a support to enable multi tenancy for front store by setting
entityDelegator in web.xml file as you mentioned. Here my code works
along with existing functionalities. My fix only added an additional
support to run a common webapp(front-store) in multi-tenant environment
by hitting specific domain name.
There is a discussion on task OFBIZ-5898 which shows use case for this.
I have also verified the both the functionalities(with entityDelegator
and domain name) on trunk and its working as expected(after applying
patch at OFBIZ-5986)
*Aspect 2: Tenant should run with specified domain name*
-- I have enough idea about capabilities to define site URIs in the
content application. This can be use to provide specific host name to
websites. But before digging into content data model we have to identify
the delegator based on given domain name. So here, we used domain name
to identify the tenant delegator first. After switching to correct
delegator, we can use content data model for getting website.
*Aspect 3: User should be able to provide domain name at tenant creation.*
-- You are correct, Its only to improve the process by providing the
ability/option for setting the domain name in the tenant creation
functionality (ant target) instead of setting it from webtools after
tenant creation.
Domain name is optional here. so I will fix the issue of auto domain
name creation when user left it empty.
*Aspect 4: Configuration details should be tenant specific.*
-- In multi-tenancy environment, it requires to have configuration
setting specific to tenant. We just modified the method to get it from
data first and then from files. This was the large amount of work so we
created a separate task OFBIZ-5902 for tracking progress.
Summary:
A). Below are the two issues on which I will work:
1. Domain name should not create for tenant if its left empty during
tenant creation.
2. Tenant component should not visible to other tenant or default.
-- Its not related to my patch and changes but I will give a try
to fix this issue as this should not be the case.
B). I need some more details about below issues you reported:
1. Switching from the non tenant specific application to the tenant
specific application led to the presentation of the login screen. This
should not have happened as the user is already logged in at the tenant.
After having provided the credentials of the tenant user again the
application can be accessed.
2. While logging in at the tenant (by providing the tenantId at the
login screen) at the url for the tenant specific application (
https://localhost:8443/tb5896/control/login) led to an NPE.
My changes only the enhancements to the existing multi-tenancy
functionality and there is a backward compatibility as well. Its does
not seems to break any previous functionality. If my changes introduces
any issue then please let me know with details to regenerate it, I will
look into it.
Thanks & Regards
---
Arun Patidar
Manager,Enterprise Software Development
HotWax Media
www.hotwaxsystems.com
On Monday 09 February 2015 04:54 PM, Ashish Vijaywargiya wrote:
> Hello Pierre,
>
> Thanks for providing all the details for tenant functionality.
> Today Arun and I had a good discussion on all the points that you shared
> here. Arun will provide the concluded details here.
>
> --
> Kind Regards,
> Ashish Vijaywargiya
>
> On Thu, Jan 29, 2015 at 7:19 AM, Pierre Smits <pi...@gmail.com>
> wrote:
>
>> This is a review regarding the effect of recent changes committed through
>> r1645310 (patch provided through issue OFBIZ-5898) and the patch provided
>> through OFBIZ-5986.
>>
>> *References:*
>>
>> - OFBIZ-5898 Tenant should run with specified domain name. Front store
>> should support mulit tenancy feature (see:
>> https://issues.apache.org/jira/browse/OFBIZ-5898)
>> - OFBIZ commit r1643510 (see http://svn.apache.org/r1643510)
>> - OFBIZ-5986 Error on accessing new tenant (see
>> https://issues.apache.org/jira/browse/OFBIZ-5986)
>>
>>
>>
>> *Background*
>> OFBiz facilitates the single tenant and multiple tenants setups. These
>> setups are:
>>
>> 1. Single tenancy setup, supporting the master having a single internal
>> organisation or multiple internal organisations
>> 2. Multi tenancy setup, supporting each tenant (including the master)
>> having a single internal organisation or multiple
>>
>> Ad 1.
>> This is enforced through following property in
>> /framework/common/config/general.properties and utilises the datasets in
>> databases ofbiz, ofbizolap and ofbiztenant (the master databases) in the
>> RDBMS:
>>
>> mulititenant=N
>>
>> Multiple internal organisations is facilitated through the OFBiz Setup
>> application/component.
>>
>> Ad 2.
>> This is enforced through following property in
>> /framework/common/config/general.properties:
>>
>> mulititenant=Y
>>
>> Multiple internal organisations per tenant is facilitated through the same
>> application as mentioned in ad 1.
>>
>> The effect of 'mulitenant=Y' is that the tenantId field is shown on the
>> login screen and
>>
>> 1. that the tenantId can be valid (checked against records in the Tenant
>> entity in master database ofbiztenant of the RDBMS)
>> 2. that the user can be authenticated against the permission settings in
>> the tenant database (ofbiz-<tenantId>) and can utilise the functions in
>> an
>> applications to create, augment and delete records in all the tenant
>> databases in the RDBMS (ofbiz-<tenantId> and ofbizolap-<tenantId).
>>
>> A user logging in at a tenant (the tenantId field has a value in the login
>> screen) can never access the data of the master databases (ofbiz, ofbizolap
>> and ofbiztenant).
>>
>>
>> *Re: issue OFBIZ-5898 Tenant should run with specified domain name. Front
>> store should support mulit tenancy feature*
>> This subject of the issue suggest two things, namely:
>>
>> - a perceived bug: in OFBiz it isn't possible to run a tenant within a
>> specific domain, and
>> - an improvement: store front should support multi tenant feature
>>
>> Subsequently, following aspects are lined up in the description to address
>> the subject of the issue:
>>
>> 1. Enable multi tenancy for a store front application
>> 2. Tenant should run with specified domain name
>> 3. User should be able to provide the domain name during tenant creation
>> 4. Configuration details should be tenant specific
>>
>>
>> *Re: perceptions and findings regarding the issue, the patch provided and
>> subsequent commit r1645310*
>> This issue is about having the functionality to be able to associate
>> multiple domains (sites) to a tenant in stead of just one domain, as I
>> understand it, as well as being about having per tenant configuration
>> settings.
>>
>> I will address the issue, the patch (the second - revised - one submitted
>> on Dec 6th at 07.47) and subsequent commit per aspect outlined in the
>> description of the issue.
>>
>> *aspect 1. Enable multi tenancy for a store front application*
>> As I see this, it is about having an application (the store front) work for
>> both anonymous users and authenticated users given the existence of
>> multiple tenants. The creator of the issue perceives that OFBiz does not
>> have the capability to deliver functionalities in an application to support
>> anonymous users. The applications in the feature portfolio intended for
>> anonymous users are:
>>
>> 1. The E-commerce application - application in the Special Purpose stack
>> 2. The MyPortal application - application in the Special Purpose stack.
>>
>> This is about the first, the E-commerce application, whereby the creator of
>> issue believes (as I see it) that OFBiz doesn't have the capabilities to
>> deliver functionalities to anonymous users given the multi tenancy setup
>> and thus having tenant specific data being delivered to a anonymous user of
>> a particular tenant. Because after all, how does OFBiz retrieve and deliver
>> tenant specific data to an anonymous user when the tenant is not
>> identified?
>>
>> Hence the introduction of aspect 2 of the issue which I will address later.
>>
>> First a little explanation of how the delegator works regarding single and
>> multi tenancy setups.
>> In a single tenancy setup (multitenant=N) the delegator is always
>> 'default'. This 'default' comes from the setting in the web.xml
>> configuration file in any and all applications in OFBiz. See following
>> example:
>>
>> *<context-param>*
>>
>> *<param-name>entityDelegatorName</param-name>*
>>
>> *<param-value>default</param-value>*
>>
>> *<description>The Name of the Entity Delegator to use, defined in
>> entityengine.xml</description>*
>>
>> *</context-param>*
>>
>> This ensures that OFBiz can work with the application without regard of
>> tenantId and whether any user is logged in or not. It is more about OFBiz
>> being able to work with the application than a user, namely being able to
>> work with data from the master databases (ofbiz, ofbizolap and
>> ofbiztenant).
>>
>> In a multi tenancy setup (multitenant=Y) the delegator gets, when a user
>> logs in at the login screen and provides the tenantId, extended with a
>> separator (the # sign) and the tenantId, resulting in 'default#<tenantId'
>> as can be seen in the logs at various registrations to have the user to be
>> able to work with functions of applications in relation to data of the
>> tenant.
>>
>> So, how does OFBiz enable access to the application, retrieve and deliver
>> tenant specific data for the anonymous users of an application given a
>> specific tenant?
>>
>> Well, this is done by changing the configuration setting for the delegator
>> in the web.xml of the application that should deliver the tenant specific
>> data.
>>
>> When the setting in web.xml of an application is changed to
>>
>> *<context-param>*
>>
>> *<param-name>entityDelegatorName</param-name>*
>>
>> *<param-value>default#<tenantId></param-value>*
>>
>> *<description>The Name of the Entity Delegator to use, defined in
>> entityengine.xml</description>*
>>
>> *</context-param>*
>>
>> the application/component will deliver data of the tenant again to
>> authenticated/authorised users when they provide the tenantId in the login
>> screen, but also to anonymous users (who don't provide the tenantId). The
>> specific entityDelegatorName tells OFBiz from which associated database it
>> should retrieve data from and write data to. It essentially overrides the
>> generic function regarding the 'default' way of working. This also ensures
>> application can't be accessed and data can't be retrieved from or written
>> to by authenticated users of a different tenant (different tenantId) or
>> even users authenticated against the master.
>>
>> So regarding aspect 1 of the issue, OFbiz is covered and working as
>> intended and no change should be needed.
>>
>> *Aspect 2: Tenant should run with specified domain name*
>> Prior to the issue, only 1 domain name could be registered in a record in
>> the Tenant entity through the interfaces of WEBTools.
>>
>> Apart from that OFBiz has the capabilities to define site URIs in the
>> content application, so that ofbiz can work with not only multiple domains
>> (
>> www.example.com vs www.rexample.org), but also with multiple sites within
>> one domain (site1.example.com vs site2.example.com). Given the nature of
>> OFBiz this works for both the single tenancy setup and the multi tenancy
>> setup. Again, for anonymous and authenticated users. This I mentioned in a
>> comment in the issue. See:
>>
>> https://issues.apache.org/jira/browse/OFBIZ-5898?focusedCommentId=14235398&page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#comment-14235398
>>
>> But, as I surmise, the creator of the issue was not aware of this
>> functionality in the content application (and apparently chose to ignore
>> the comment in the issue about content functionality visavis domains/web
>> sites in the issue) and thus meant the aspect to be: have multi domain
>> functionality in the tenant configuration (at master level).
>>
>> This aspect seems to be the main reason that the patch addresses.
>>
>> *Aspect 3: User should be able to provide domain name at tenant creation.*
>> Prior to the issue, the OFBiz administrator (the user) could not set the
>> domain name at creation of the tenant. Also, the OFBiz administrator could
>> only set the one domain name through functionalities in the WEBTOOLS
>> application by editing existing tenant records in the Tenant entity.
>>
>> Again I surmise, as no additional explanation is given within the context
>> of the issue by the creator, that he sought to improve the process by
>> providing the ability/option for setting the domain name in the tenant
>> creation functionality (ant target).
>>
>> This aspect seems to be the second reason that the patch addresses.
>>
>> Aspect 4. Configuration details should be tenant specific.
>> In order to have a tenant specific aspect to work properly, configurations
>> settings should be applicable per tenant, and if a configuration setting is
>> not provided for a specific tenant the default configuration settings (in
>> the properties files of various applications) should/must be used.
>>
>> This aspect is not addressed by the patch. In fact, in a comment in the
>> issue, the creator stated that a new issue were to be created (and has been
>> created) to address this specifically). See
>> https://issues.apache.org/jira/browse/OFBIZ-5902
>>
>> *Effects of the patch on how OFBiz works after the commit*
>>
>>
>> The commit has the following effect on how OFBiz is working:
>>
>> - Creating a tenant - the new method of creating a tenant (./ant
>> create-tenant) now included setting the domain name of the tenant. This
>> results in setting the tenant id and the domain name in entity
>> TenantDomainName.
>> As setting the domain name is optional when executing ./ant
>> create-tenant, this should not be executed when the domain name is left
>> blank. However, setting the domain name is always executed, and as the
>> domainName is (part of) the primary key of the entity TenantDomainName
>> it
>> will be automatically generated (gets sequenced Id) when left blank.
>>
>> - Changing how urlServletHelper is working. Prior to the change the
>> domain name was checked against the records in entity Tenant (single
>> domain
>> setup). With the change the domain name is now checked against the
>> records
>> in entity TenantDomainName (multiple domain setup).
>>
>> - Changing how ContextFilter is working. Prior to the change the domain
>> name was checked against the records in entity Tenant (single domain
>> setup). With the change the domain name is now checked against the
>> records
>> in entity TenantDomainName (multiple domain setup).
>>
>> - Changing how LoginWorker is working. Prior to the change the login was
>> checked against availability set through the login screen or (when not
>> available) the setting in the param-name 'entityDelegatorName' in the
>> web.xml of the component. With the change an additional check is
>> included
>> and the cleanup of the delegator (from '*default**#<tenantId>'* back to
>> 'default') is removed.
>>
>> Re: domainName setting (aspect 3)
>> Having this setting of the domain name on the moment of the creation of the
>> tenant via ./ant create-tenant (the commit) seems to be a regression in
>> functionality,as it always creates a tenant domain (using a sequenced Id
>> for the domainName field in entity TenantDomainName when left empty), in
>> stead of working with the optionality (the choice not to set the domain
>> name)
>>
>> Re: context-param param-name='entityDelegatorName' setting
>> It seems that the commit has led to a more relaxed level of security
>> regarding access to tenant specific applications/components.
>>
>> By setting the context-param for the entityDelegatorName in the web.xml
>> file of the component, there is the extra security layer that the
>> application/component is made available only to users within the defined
>> delegator (*default**#<tenantId>*).
>> This ensures that it can't be accessed by users of other tenants (apart
>> from accidentally having the combination (userId-passWord-tenantId) right
>> on login. That also excludes users of the master setup to be able to access
>> the application/component, inherent to the expectation that userIds in the
>> master database (ofbiz) don't exist in the db of the tenant
>> (ofbiz_<tenantId>), irrespectively of having the SecurityPermissionSeedData
>> of the tenant specific application and associations between userId and the
>> security permissions of that application in the tables of the master
>> database. Now it doesn't work that way anymore.
>>
>> Having tested against trunk and having security permissions of a tenant
>> specific database in the master database combined with a userId associated
>> to those security permissions, a user accessing the master environment
>> (meaning: without providing the tenantId) sees the name of the tenant
>> specific application in the top menu and can access that application. This
>> is wrong.
>>
>>
>> I did not test the functionality against another domain (the intended happy
>> flow) than the default (localhost).
>>
>> I surmise that when the patch appeared in the issue only the code and the
>> happy flow was reviewed and tested by the committer, no more thoughts were
>> given regarding non-happy flows, how the changes affects how OFBiz works
>> with respect to backend functionality or the comment regarding OFBiz
>> content application and domains/web sites.
>>
>> As an example, the use-case regarding the removal of the delegator cleanup
>> was neither motivated in the description of the issue as a need, nor
>> questioned when the patch was committed.
>>
>>
>> *Re: OFBIZ-5986 Error on accessing new tenant*
>> This issue appeared after incorporation of commit r1643510 and seems
>> similar to issue OFBIZ-5447 (see
>> https://issues.apache.org/jira/browse/OFBIZ-5447). Strangely, OFBIZ-5447
>> was closed by the reviewing committer approximately an hour before he
>> created OFBIZ-5986.But that is another issue. Shortly thereafter a patch
>> was provided, which has not been reviewed till today.
>>
>>
>>
>> *The patch*
>>
>>
>> - Changes how ContextFilter is working by adding additional checks for
>> when a tenantId is not provided, like in the case of logging in at the
>> master.
>> - Removing changes introduced at r1643510 regarding, about the check
>> when the delegatorName isn't equal to the baseDelegatorName (in effect:
>> *default**#<tenantId>* !- default ) and the actions for when they aren't
>> the same.
>>
>> Testing the patch against a backend applications in domain localhost
>> yielded the following results:
>>
>> 1. While logging in at the master a tenant specific application (with
>> context-param param-name='entityDelegatorName' having value *default*
>> *#<tenantId>*) still shows the tenant specific application when the seed
>> and demo data of that app is in the master database and the user can
>> access
>> the application. Effectively, the contex-param isn't factored in
>> security-wise.
>> 2. While logging in at the tenant (by providing the tenantId at the
>> login screen) at the url for the tenant specific application (
>> https://localhost:8443/tb5896/control/login) led to an NPE.
>> 3. Logging in at the tenant at the url for a application that is not
>> tenant specific context-param param-name='entityDelegatorName' having
>> value
>> *default*) yields success.
>> 4. Switching from the non tenant specific application to the tenant
>> specific application led to the presentation of the login screen. This
>> should not have happened as the user is already logged in at the tenant.
>> After having provided the credentials of the tenant user again the
>> application can be accessed.
>>
>>
>>
>> *Suggestions:*
>> Given that the commit has led to unwanted effects (as indicated in the user
>> ml, see
>>
>> http://ofbiz.markmail.org/message/22bmphzfhkprhrcm?q=Multiple+root+apps+%28tenant%29
>> and
>>
>> http://ofbiz.markmail.org/message/7pdfttecmqx6ygio?q=Unable+to+connect+tenant+component+with+tenant+database
>> )
>> and the creation of the second issue (OFBIZ-5986), I suggest that we reopen
>> both issues (5898 and 5986) and redo the process and rework the solution.
>>
>> The reason for this is the following: Issues and their subjects/titles are
>> incorporated in release notes, and when a user needs to evaluate a release
>> (for upgrade or new implementation) he will look first at those release
>> notes to be able to assess the applicability of the release regarding his
>> requirements. If such an issue subject is unclear to him, he will look at
>> the description of such issue. At first he will disregard the comments.
>>
>> OFBIZ-5898 has both an ambiguous title and aspects in the description that
>> aren't addressed by the patch, but are in one or more other issues. E.g.
>> aspect 4. And the commit breaks existing functionality.
>> In stead of leaving the commit in and resolve the effects in other issues
>> (like OFBIZ-5896) and creating more confusion for evaluators of the next
>> release, we can reworking title and description of OFBIZ-5898 to we improve
>> the clarity and applicability of the issue and the focus of the solution
>> and redo the solution. That way we can determine issue OFBIZ-5986 is still
>> warranted and can address the comment regarding OFBIZ content application
>> visavis domains/web sites and the comments and findings stated above.
>>
>> Best regards,
>>
>> Pierre Smits
>>
>> *ORRTIZ.COM <http://www.orrtiz.com>*
>> Services & Solutions for Cloud-
>> Based Manufacturing, Professional
>> Services and Retail & Trade
>> http://www.orrtiz.com
>>