Anonymous | Login
Project:
RSS
  
News | My View | View Issues | Roadmap | Summary

View Issue Details[ Jump to Notes ] [ Issue History ] [ Print ]
ID
0034330
TypeCategorySeverityReproducibilityDate SubmittedLast Update
design defect[Retail Modules] StoreServermajorhave not tried2016-10-28 10:282016-12-28 10:56
ReportermtaalView Statuspublic 
Assigned Tomtaal 
PrioritynormalResolutionfixedFixed in VersionRR17Q1
StatusclosedFix in branchFixed in SCM revision0d19df38b16b
ProjectionnoneETAnoneTarget VersionRR17Q1
OSAnyDatabaseAnyJava version
OS VersionDatabase versionAnt version
Product VersionSCM revision 
Review Assigned ToSandrahuguet
Regression level
Regression date
Regression introduced in release
Regression introduced by commit
Triggers an Emergency PackNo
Summary

0034330: 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.
TagsNo tags attached.
Attached Files

- Relationships [ Relation Graph ] [ Dependency Graph ]
related to feature request 0034298RR17Q1 closedmtaal Retail Modules Make use of common method to generate cors response headers 
related to feature request 0034063 closedmtaal Retail Modules [STORE SERVER 1791]Support offline and multi-server for Mobile Warehouse 
depends on feature request 00343313.0PR17Q1 closedmtaal Openbravo ERP Support pre-defined allowed domains for cross-domain requests in a multi-server environment 
depends on feature request 00342673.0PR17Q1 closedmtaal Openbravo ERP Let the basekernelservlet handle cors requests 

-  Notes
(0091194)
mtaal (manager)
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 (manager)
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 (developer)
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 (developer)
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 (developer)
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 (developer)
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 (developer)
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 (developer)
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 (developer)
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 (developer)
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 (developer)
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 (manager)
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 (developer)
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 (developer)
2016-12-28 10:56

 reviewed and tested

- Issue History
Date Modified Username Field Change
2016-10-28 10:28 mtaal New Issue
2016-10-28 10:28 mtaal Assigned To => mtaal
2016-10-28 10:28 mtaal Triggers an Emergency Pack => No
2016-10-28 10:32 mtaal Description Updated View Revisions
2016-10-28 10:45 mtaal Relationship added depends on 0034331
2016-10-28 10:45 mtaal Relationship added related to 0034298
2016-10-28 10:45 mtaal Relationship added related to 0034063
2016-10-28 10:46 mtaal Relationship added depends on 0034267
2016-11-07 11:13 mtaal Note Added: 0091194
2016-11-28 01:14 mtaal Note Added: 0091873
2016-12-03 12:42 hgbot Checkin
2016-12-03 12:42 hgbot Note Added: 0092067
2016-12-03 12:42 hgbot Checkin
2016-12-03 12:42 hgbot Note Added: 0092068
2016-12-03 12:42 hgbot Checkin
2016-12-03 12:42 hgbot Note Added: 0092069
2016-12-03 12:42 hgbot Checkin
2016-12-03 12:42 hgbot Note Added: 0092070
2016-12-08 08:35 hgbot Checkin
2016-12-08 08:35 hgbot Note Added: 0092206
2016-12-08 16:52 hgbot Checkin
2016-12-08 16:52 hgbot Note Added: 0092222
2016-12-09 16:45 hgbot Checkin
2016-12-09 16:45 hgbot Note Added: 0092263
2016-12-09 16:45 hgbot Status new => resolved
2016-12-09 16:45 hgbot Resolution open => fixed
2016-12-09 16:45 hgbot Fixed in SCM revision => http://code.openbravo.com/erp/pmods/org.openbravo.mobile.core/rev/0f47b673c3f5257d658a2bdee9a51ca36e78ef11 [^]
2016-12-10 21:52 hgbot Checkin
2016-12-10 21:52 hgbot Note Added: 0092266
2016-12-11 00:00 hgbot Checkin
2016-12-11 00:00 hgbot Note Added: 0092267
2016-12-19 11:52 mtaal Review Assigned To => Sandrahuguet
2016-12-22 10:42 mtaal Note Added: 0092936
2016-12-22 10:42 mtaal Status resolved => new
2016-12-22 10:42 mtaal Resolution fixed => open
2016-12-27 07:10 hgbot Checkin
2016-12-27 07:10 hgbot Note Added: 0093003
2016-12-27 07:10 hgbot Status new => resolved
2016-12-27 07:10 hgbot Resolution open => fixed
2016-12-27 07:10 hgbot Fixed in SCM revision http://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:56 Sandrahuguet Note Added: 0093037
2016-12-28 10:56 Sandrahuguet Status resolved => closed
2016-12-28 10:56 Sandrahuguet Fixed in Version => RR17Q1

Copyright © 2000 - 2009 MantisBT Group
Powered by Mantis Bugtracker