Openbravo Issue Tracking System - Retail Modules
View Issue Details
IDProjectCategoryView StatusDate SubmittedLast Update
0034330Retail ModulesStoreServerpublic2016-10-28 10:282016-12-28 10:56
Reportermtaal 
Assigned Tomtaal 
PrioritynormalSeveritymajorReproducibilityhave not tried
StatusclosedResolutionfixed 
PlatformOS5OS Version
Product Version 
Target VersionRR17Q1Fixed in VersionRR17Q1 
Merge Request Status
Review Assigned ToSandrahuguet
OBNetwork customer
Support ticket
Regression level
Regression date
Regression introduced in release
Regression introduced by commit
Triggers an Emergency PackNo
Summary0034330: Support multi-server requests in a better more secure way
DescriptionCurrently we allow multi-server requests by setting very wide cross domain allowances [1]. This makes the system very flexible and lowers the configuration effort. However, this also means that there is a potential for cross-domain scripting [2].

Therefore the proposal is to improve this and work with a list of allowed domains, configurable in OB.

[1]
https://code.openbravo.com/erp/pmods/org.openbravo.mobile.core/file/8078590c67e8/src/org/openbravo/mobile/core/process/WebServiceAuthenticatedServlet.java#l48 [^]

[2]
https://en.wikipedia.org/wiki/Cross-site_scripting [^]

[3]
https://en.wikipedia.org/wiki/Cross-origin_resource_sharing [^]

[4]
https://www.owasp.org/index.php/Cross-Site_Request_Forgery_(CSRF) [^]
Steps To ReproduceInstall multi-server central and store
login with webpos
stop the store server
the requests move to central server, doing cross-domain request
Proposed SolutionAllow defining list of allowed domains in the mobile servers

Read this list of allowed domains and use it to compare against the referer/http origin

Return the allowed domain in the origin if it exists in the list allowed domain

This change also needs to be done in the BaseKernelServlet, so part of the solution will be implemented in the core erp. See the related issue.
Additional Information
TagsNo tags attached.
Relationships
related to feature request 0034298RR17Q1 closed mtaal Retail Modules Make use of common method to generate cors response headers 
related to feature request 0034063 closed mtaal Retail Modules [STORE SERVER 1791]Support offline and multi-server for Mobile Warehouse 
depends on feature request 00343313.0PR17Q1 closed mtaal Openbravo ERP Support pre-defined allowed domains for cross-domain requests in a multi-server environment 
depends on feature request 00342673.0PR17Q1 closed mtaal Openbravo ERP Let the basekernelservlet handle cors requests 
Attached Files
Issue History
Date ModifiedUsernameFieldChange
2016-10-28 10:28mtaalNew Issue
2016-10-28 10:28mtaalAssigned To => mtaal
2016-10-28 10:28mtaalTriggers an Emergency Pack => No
2016-10-28 10:32mtaalDescription Updatedbug_revision_view_page.php?rev_id=13505#r13505
2016-10-28 10:45mtaalRelationship addeddepends on 0034331
2016-10-28 10:45mtaalRelationship addedrelated to 0034298
2016-10-28 10:45mtaalRelationship addedrelated to 0034063
2016-10-28 10:46mtaalRelationship addeddepends on 0034267
2016-11-07 11:13mtaalNote Added: 0091194
2016-11-28 01:14mtaalNote Added: 0091873
2016-12-03 12:42hgbotCheckin
2016-12-03 12:42hgbotNote Added: 0092067
2016-12-03 12:42hgbotCheckin
2016-12-03 12:42hgbotNote Added: 0092068
2016-12-03 12:42hgbotCheckin
2016-12-03 12:42hgbotNote Added: 0092069
2016-12-03 12:42hgbotCheckin
2016-12-03 12:42hgbotNote Added: 0092070
2016-12-08 08:35hgbotCheckin
2016-12-08 08:35hgbotNote Added: 0092206
2016-12-08 16:52hgbotCheckin
2016-12-08 16:52hgbotNote Added: 0092222
2016-12-09 16:45hgbotCheckin
2016-12-09 16:45hgbotNote Added: 0092263
2016-12-09 16:45hgbotStatusnew => resolved
2016-12-09 16:45hgbotResolutionopen => fixed
2016-12-09 16:45hgbotFixed in SCM revision => http://code.openbravo.com/erp/pmods/org.openbravo.mobile.core/rev/0f47b673c3f5257d658a2bdee9a51ca36e78ef11 [^]
2016-12-10 21:52hgbotCheckin
2016-12-10 21:52hgbotNote Added: 0092266
2016-12-11 00:00hgbotCheckin
2016-12-11 00:00hgbotNote Added: 0092267
2016-12-19 11:52mtaalReview Assigned To => Sandrahuguet
2016-12-22 10:42mtaalNote Added: 0092936
2016-12-22 10:42mtaalStatusresolved => new
2016-12-22 10:42mtaalResolutionfixed => open
2016-12-27 07:10hgbotCheckin
2016-12-27 07:10hgbotNote Added: 0093003
2016-12-27 07:10hgbotStatusnew => resolved
2016-12-27 07:10hgbotResolutionopen => fixed
2016-12-27 07:10hgbotFixed in SCM revisionhttp://code.openbravo.com/erp/pmods/org.openbravo.mobile.core/rev/0f47b673c3f5257d658a2bdee9a51ca36e78ef11 [^] => http://code.openbravo.com/erp/pmods/org.openbravo.mobile.core/rev/0d19df38b16b69adf4a713e1bd22dfd794f7302f [^]
2016-12-28 10:56SandrahuguetNote Added: 0093037
2016-12-28 10:56SandrahuguetStatusresolved => closed
2016-12-28 10:56SandrahuguetFixed in Version => RR17Q1

Notes
(0091194)
mtaal   
2016-11-07 11:13   
As an extra note: the system should also give a warning to the user when he/she logs into the store or central server using a wrong url with a hostname/domain/port which is not defined as an allowed domain in a multi-server environment.
(0091873)
mtaal   
2016-11-28 01:14   
Documentation:
http://wiki.openbravo.com/wiki/How_to_create_a_Store_Server#Define_the_Store_Server_as_a_mobile_server_in_the_Central_Server [^]

http://wiki.openbravo.com/wiki/Retail:Configuration_Guide#Allowed_Origin_Domains_Field_-_Cross_Domain_Requests [^]
(0092067)
hgbot   
2016-12-03 12:42   
Repository: erp/pmods/org.openbravo.mobile.core
Changeset: 6de3dea52e76389530e88e940362884eb0c1d81c
Author: Martin Taal <martin.taal <at> openbravo.com>
Date: Sat Dec 03 12:24:03 2016 +0100
URL: http://code.openbravo.com/erp/pmods/org.openbravo.mobile.core/rev/6de3dea52e76389530e88e940362884eb0c1d81c [^]

Related to issue 34330: Support multi-server requests in a better more secure way
Added new column/field allowed domains to mobile server table and window

---
M src-db/database/model/tables/OBMOBC_SERVER_DEFINITION.xml
M src-db/database/sourcedata/AD_COLUMN.xml
M src-db/database/sourcedata/AD_ELEMENT.xml
M src-db/database/sourcedata/AD_FIELD.xml
---
(0092068)
hgbot   
2016-12-03 12:42   
Repository: erp/pmods/org.openbravo.mobile.core
Changeset: c283f57219fd93ff93bb216c2b403a185281967a
Author: Martin Taal <martin.taal <at> openbravo.com>
Date: Sat Dec 03 12:25:19 2016 +0100
URL: http://code.openbravo.com/erp/pmods/org.openbravo.mobile.core/rev/c283f57219fd93ff93bb216c2b403a185281967a [^]

Related to issue 34330: Support multi-server requests in a better more secure way
Use new AllowedCrossDomainsHandler.setCORSHeaders instead of OBMOBCUtils.setCORSHeader

---
M src/org/openbravo/mobile/core/MobileCoreComponentServlet.java
M src/org/openbravo/mobile/core/login/MobileCoreLoginHandler.java
M src/org/openbravo/mobile/core/login/MobileSessionActive.java
M src/org/openbravo/mobile/core/process/WebServiceAbstractServlet.java
M src/org/openbravo/mobile/core/process/WebServiceAuthenticatedServlet.java
M src/org/openbravo/mobile/core/utils/OBMOBCUtils.java
---
(0092069)
hgbot   
2016-12-03 12:42   
Repository: erp/pmods/org.openbravo.mobile.core
Changeset: 98b6c4597c8c9e525d29be4bacf0f33da4f559b3
Author: Martin Taal <martin.taal <at> openbravo.com>
Date: Sat Dec 03 12:26:25 2016 +0100
URL: http://code.openbravo.com/erp/pmods/org.openbravo.mobile.core/rev/98b6c4597c8c9e525d29be4bacf0f33da4f559b3 [^]

Related to issue 34330: Support multi-server requests in a better more secure way
Implement mobile-server domain checker using the allowed domains and urls set in the
mobile server definitions. For now, initial commit always allows, will be set to working
in next commit.

---
A src/org/openbravo/mobile/core/authenticate/MobileAllowedCrossDomainsChecker.java
---
(0092070)
hgbot   
2016-12-03 12:42   
Repository: erp/pmods/org.openbravo.mobile.core
Changeset: e2a5ae10332618905be323ffb7c91df3588679e5
Author: Martin Taal <martin.taal <at> openbravo.com>
Date: Sat Dec 03 12:27:55 2016 +0100
URL: http://code.openbravo.com/erp/pmods/org.openbravo.mobile.core/rev/e2a5ae10332618905be323ffb7c91df3588679e5 [^]

Related to issue 34330: Support multi-server requests in a better more secure way
Return error message flag if url is not from an allowed domain

---
M src/org/openbravo/mobile/core/login/MobileCoreLoginUtilsServlet.java
---
(0092206)
hgbot   
2016-12-08 08:35   
Repository: erp/pmods/org.openbravo.mobile.core
Changeset: fbf5c7bb6602f2bd3920ff85760e2c9de8e219ee
Author: Martin Taal <martin.taal <at> openbravo.com>
Date: Thu Dec 08 08:30:18 2016 +0100
URL: http://code.openbravo.com/erp/pmods/org.openbravo.mobile.core/rev/fbf5c7bb6602f2bd3920ff85760e2c9de8e219ee [^]

Related to issue 34330: Support multi-server requests in a better more secure way
Enabled cross domain checker

---
M src/org/openbravo/mobile/core/authenticate/MobileAllowedCrossDomainsChecker.java
---
(0092222)
hgbot   
2016-12-08 16:52   
Repository: erp/pmods/org.openbravo.retail.sampledata.multiserver
Changeset: 675176591178f40e3f414a55878482e737aa1636
Author: Martin Taal <martin.taal <at> openbravo.com>
Date: Thu Dec 08 16:52:43 2016 +0100
URL: http://code.openbravo.com/erp/pmods/org.openbravo.retail.sampledata.multiserver/rev/675176591178f40e3f414a55878482e737aa1636 [^]

Related to issue 34330: Support multi-server requests in a better more secure way
Set allowed domains in multi-server test definition

---
M referencedata/sampledata/The_White_Valley_Group/OBMOBC_SERVER_DEFINITION.xml
---
(0092263)
hgbot   
2016-12-09 16:45   
Repository: erp/pmods/org.openbravo.mobile.core
Changeset: 0f47b673c3f5257d658a2bdee9a51ca36e78ef11
Author: Martin Taal <martin.taal <at> openbravo.com>
Date: Fri Dec 09 16:44:48 2016 +0100
URL: http://code.openbravo.com/erp/pmods/org.openbravo.mobile.core/rev/0f47b673c3f5257d658a2bdee9a51ca36e78ef11 [^]

Fixes issue 34330: Support multi-server requests in a better more secure way
Add popup message if the origin is not part of the allowed servers

---
M src-db/database/sourcedata/AD_MESSAGE.xml
M src/org/openbravo/mobile/core/login/MobileCoreLoginUtilsServlet.java
M web/org.openbravo.mobile.core/source/model/ob-terminal-model.js
---
(0092266)
hgbot   
2016-12-10 21:52   
Repository: erp/pmods/org.openbravo.retail.sampledata.multiserver
Changeset: cffdde7ff85f0b67cf3229a9d4709709ebdcc61a
Author: Martin Taal <martin.taal <at> openbravo.com>
Date: Sat Dec 10 21:52:17 2016 +0100
URL: http://code.openbravo.com/erp/pmods/org.openbravo.retail.sampledata.multiserver/rev/cffdde7ff85f0b67cf3229a9d4709709ebdcc61a [^]

Related to issue 34330: Support multi-server requests in a better more secure way
Use comma as separator

---
M referencedata/sampledata/The_White_Valley_Group/OBMOBC_SERVER_DEFINITION.xml
---
(0092267)
hgbot   
2016-12-11 00:00   
Repository: erp/pmods/org.openbravo.retail.sampledata.multiserver
Changeset: 62fb6651a796f55184365d0629e68b50f255339e
Author: Martin Taal <martin.taal <at> openbravo.com>
Date: Sun Dec 11 00:00:05 2016 +0100
URL: http://code.openbravo.com/erp/pmods/org.openbravo.retail.sampledata.multiserver/rev/62fb6651a796f55184365d0629e68b50f255339e [^]

Related to issue 34330: Support multi-server requests in a better more secure way
Repair regular expression

---
M referencedata/sampledata/The_White_Valley_Group/OBMOBC_SERVER_DEFINITION.xml
---
(0092936)
mtaal   
2016-12-22 10:42   
I'm working on testing and reviewing issue 34330 and I have some questions:
1. You added a popup with this message "The WebPOS client has been loaded using an URL which does not support multi-server requests, contact your system administrator" [1], but when I try to log with a url that is not allowed I get the error that you can see in the attached screenshot.
MT>> Yes I found that the error message I added was never displayed because retail already gets into an error earlier. It was not possible (afaics) to override this behavior from retail as it fails at a very basic level (loading code). But I re-opened the issue to recheck this.


2. My second question is about the sampledata [2]. You only set allowed domains in the central server, and my question is that we need to do the same in stores or is not needed?
And now the url from the central server in the sampledata is http://central.openbravo.com:9080/openbravo/ [^] and you added in allowed domains: http://localhost:8080,http://10(.+) [^] . Are this correct or we need to replace by http://localhost:9080,http://10(.+) [^] ?
MT>> I guess you are right, but I also guess that for the tests it makes no difference (they pass) as they seem then to use the http://10 [^] ip-addresses. I will change however the sampledata.
(0093003)
hgbot   
2016-12-27 07:10   
Repository: erp/pmods/org.openbravo.mobile.core
Changeset: 0d19df38b16b69adf4a713e1bd22dfd794f7302f
Author: Martin Taal <martin.taal <at> openbravo.com>
Date: Tue Dec 27 07:10:36 2016 +0100
URL: http://code.openbravo.com/erp/pmods/org.openbravo.mobile.core/rev/0d19df38b16b69adf4a713e1bd22dfd794f7302f [^]

Fixes issue 34330: Support multi-server requests in a better more secure way
Show the correct (invalid url) message now also when logging in.

Changes:
ob-terminal-component.js, ob-terminal-model.js: show the message when the exception is returned from the server
MobileServerUtils.java: create common methods to set cors header and check origin, also includes formatting changes
MobileCoreLoginUtilsServlet.java: allow temporary cors to return origin error to the client, only do this for prerender action
Context.java: also return invalid origin error here, also includes formatting change

---
M src/org/openbravo/mobile/core/login/Context.java
M src/org/openbravo/mobile/core/login/MobileCoreLoginUtilsServlet.java
M src/org/openbravo/mobile/core/servercontroller/MobileServerUtils.java
M web/org.openbravo.mobile.core/source/component/ob-terminal-component.js
M web/org.openbravo.mobile.core/source/model/ob-terminal-model.js
---
(0093037)
Sandrahuguet   
2016-12-28 10:56   
reviewed and tested