[jira] [Commented] (OFBIZ-11306) POC for CSRF Token
[ https://issues.apache.org/jira/browse/OFBIZ-11306?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17013652#comment-17013652 ] Jacques Le Roux commented on OFBIZ-11306: - Thanks James, I'll have a look at OFBIZ-11318. Also, in my lastcomment I forgot to mention "I still need to review RequestHandler closer". > POC for CSRF Token > -- > > Key: OFBIZ-11306 > URL: https://issues.apache.org/jira/browse/OFBIZ-11306 > Project: OFBiz > Issue Type: Improvement > Components: ALL APPLICATIONS >Affects Versions: Upcoming Branch >Reporter: James Yong >Assignee: Jacques Le Roux >Priority: Minor > Labels: CSRF > Fix For: Upcoming Branch > > Attachments: OFBIZ-11306-v2.patch, OFBIZ-11306.patch, > OFBIZ-11306.patch, OFBIZ-11306.patch, OFBIZ-11306.patch, OFBIZ-11306.patch, > OFBIZ-11306.patch, OFBIZ-11306.patch, OFBIZ-11306.patch, OFBIZ-11306.patch, > OFBIZ-11306.patch, OFBIZ-11306.patch, OFBIZ-11306_Plugins.patch, > OFBIZ-11306_Plugins.patch, OFBIZ-11306_Plugins.patch, > OFBIZ-11306_Plugins.patch > > > CRSF tokens are generated using CSRF Guard library and used in: > 1) In widget form where a hidden token field is auto-generated. > 2) In FTL form where a <@csrfTokenField> macro is used to generate the csrf > token field. > 3) In Ajax call where a <@csrfTokenAjax> macro is used to assign csrf token > to X-CSRF-Token in request header. > CSRF tokens are stored in the user sessions, and verified during POST request. > A new attribute i.e. csrf-token is added to the security tag to exempt CSRF > token check. > Certain request path, like LookupPartyName, can be exempt from CSRF token > check during Ajax POST call. -- This message was sent by Atlassian Jira (v8.3.4#803005)
[jira] [Comment Edited] (OFBIZ-11306) POC for CSRF Token
[
https://issues.apache.org/jira/browse/OFBIZ-11306?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17013563#comment-17013563
]
Jacques Le Roux edited comment on OFBIZ-11306 at 1/12/20 6:04 AM:
--
Hi James,
First I must apologize. Because I initially suggested that we should not
differentiate GET and POST methods. I undertstood I was wrong after reading
[OWASP Disclosure of Token in
URL|https://cheatsheetseries.owasp.org/cheatsheets/Cross-Site_Request_Forgery_Prevention_Cheat_Sheet.html#disclosure-of-token-in-url]
and thought about it. As explained in my comment above, it has an impact on
the work already done. Fortunately most is easy to remove. But we need to add
some code.
After reviewing Java code (still a WIP) here are my conclusions:
* OrderServices.xml should not be part of this patch, another Jira should be
created
* ConfigXMLReader: OK with me
* ControlEventListener: OK with me
* WidgetWorker: we don't want tokens in form ULRs, hidden fields only should
be used
* MacroFormRenderer: we don't want tokens in form ULRs, hidden fields only
should be used, as it's done in HtmlFormMacroLibrary.ftl. We don't need tokens
for pagination. It's idempotent, no harm possible.
* CsrfTokenTransform is useless we don't want tokens in form ULRs, as
mentioned for WidgetWorker and MacroFormRenderer
* UrlRegexpTransform, CatalogAltUrlSeoTransform: we don't want tokens in
navigation ULRs. We though need to check that all URLs are idempotent. They
should be since those are only for navigation, and forms are used for changes.
* CsrfTokenAjaxTransform: OK with me
Other misc. stuff:
* We need to adds crsftoken hidden fields in forms in ftl files. There at 706
of them. Only non idempotent forms are concerned. I guess most of them if, not
all. This sounds like a boring task. Maybe we can find a smart way to do that
globally. I did not think about it already.
* I don't think adding {{if (!options.crossDomain)}} in ajaxPrefilter will
work with CORS. I have to check that once we will commit, not a big deal.
* I like the idea of controlPath in ofbizUrl macro. But it's unrelated to
CSRF. I suggest to remove it from this issue and create a specific issue for
that.
After a preliminary preliminary review, I still need to review CsrfUtil and
RequestHandler closer..
Again thanks for the good work!
was (Author: jacques.le.roux):
Hi James,
First I must apologize. Because I initially suggested that we should not
differentiate GET and POST methods. I undertstood I was wrong after reading
[OWASP Disclosure of Token in
URL|https://cheatsheetseries.owasp.org/cheatsheets/Cross-Site_Request_Forgery_Prevention_Cheat_Sheet.html#disclosure-of-token-in-url]
and thought about it. As explained in my comment above, it has an impact on
the work already done. Fortunately most is easy to remove. But we need to add
some code.
After reviewing Java code (still a WIP) here are my conclusions:
* OrderServices.xml should not be part of this patch, another Jira should be
created
* ConfigXMLReader: OK with me
* ControlEventListener: OK with me
* WidgetWorker: we don't want tokens in form ULRs, hidden fields only should
be used
* MacroFormRenderer: we don't want tokens in form ULRs, hidden fields only
should be used, as it's done in HtmlFormMacroLibrary.ftl. We don't need tokens
for pagination. It's idempotent, no harm possible.
* CsrfTokenTransform is useless we don't want tokens in form ULRs, as
mentioned for WidgetWorker and MacroFormRenderer
* UrlRegexpTransform, CatalogAltUrlSeoTransform: we don't want tokens in
navigation ULRs. We though need to check that all URLs are idempotent. They
should be since those are only for navigation, and forms are used for changes.
* CsrfTokenAjaxTransform: OK with me
Other misc. stuff:
* We need to adds crsftoken hidden fields in forms in ftl files. There at 706
of them. Only non idempotent forms are concerned. I guess most of them if, not
all. This sounds like a boring task. Maybe we can find a smart way to do that
globally. I did not think about it already.
* I don't think adding {{if (!options.crossDomain)}} in ajaxPrefilter will
work with CORS. I have to check that once we will commit, not a big deal.
* I like the idea of controlPath in ofbizUrl macro. But it's unrelated to
CSRF. I suggest to remove it from this issue and create a specific issue for
that.
After a preliminary preliminary review, I still need to review CsrfUtil
closer...
Again thanks for the good work!
> POC for CSRF Token
> --
>
> Key: OFBIZ-11306
> URL: https://issues.apache.org/jira/browse/OFBIZ-11306
> Project: OFBiz
> Issue Type: Improvement
> Components: ALL APPLICATIONS
>Affects Versions: Upcoming Branch
>Reporter: James Yong
>Assignee: Jacques Le Roux
>
[jira] [Commented] (OFBIZ-11306) POC for CSRF Token
[ https://issues.apache.org/jira/browse/OFBIZ-11306?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17013641#comment-17013641 ] James Yong commented on OFBIZ-11306: Created 3 separate JIRA from this issue, OFBIZ-11317, OFBIZ-11318, OFBIZ-11319. > POC for CSRF Token > -- > > Key: OFBIZ-11306 > URL: https://issues.apache.org/jira/browse/OFBIZ-11306 > Project: OFBiz > Issue Type: Improvement > Components: ALL APPLICATIONS >Affects Versions: Upcoming Branch >Reporter: James Yong >Assignee: Jacques Le Roux >Priority: Minor > Labels: CSRF > Fix For: Upcoming Branch > > Attachments: OFBIZ-11306-v2.patch, OFBIZ-11306.patch, > OFBIZ-11306.patch, OFBIZ-11306.patch, OFBIZ-11306.patch, OFBIZ-11306.patch, > OFBIZ-11306.patch, OFBIZ-11306.patch, OFBIZ-11306.patch, OFBIZ-11306.patch, > OFBIZ-11306.patch, OFBIZ-11306.patch, OFBIZ-11306_Plugins.patch, > OFBIZ-11306_Plugins.patch, OFBIZ-11306_Plugins.patch, > OFBIZ-11306_Plugins.patch > > > CRSF tokens are generated using CSRF Guard library and used in: > 1) In widget form where a hidden token field is auto-generated. > 2) In FTL form where a <@csrfTokenField> macro is used to generate the csrf > token field. > 3) In Ajax call where a <@csrfTokenAjax> macro is used to assign csrf token > to X-CSRF-Token in request header. > CSRF tokens are stored in the user sessions, and verified during POST request. > A new attribute i.e. csrf-token is added to the security tag to exempt CSRF > token check. > Certain request path, like LookupPartyName, can be exempt from CSRF token > check during Ajax POST call. -- This message was sent by Atlassian Jira (v8.3.4#803005)
[jira] [Updated] (OFBIZ-11317) Add 'controlPath' attribute to 'ofbizUrl' freemarker macro
[
https://issues.apache.org/jira/browse/OFBIZ-11317?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]
James Yong updated OFBIZ-11317:
---
Attachment: OFBIZ-11317_Plugins.patch
> Add 'controlPath' attribute to 'ofbizUrl' freemarker macro
> --
>
> Key: OFBIZ-11317
> URL: https://issues.apache.org/jira/browse/OFBIZ-11317
> Project: OFBiz
> Issue Type: Improvement
> Components: ALL APPLICATIONS
>Affects Versions: Trunk
>Reporter: James Yong
>Priority: Minor
> Fix For: Upcoming Branch
>
> Attachments: OFBIZ-11317.patch, OFBIZ-11317_Plugins.patch
>
>
> Currently the ofbizUrl freemarker macro doesn't support uri for other OFBiz
> web application.
> Adding the controlPath attribute to allow the controls by ofbizUrl to support
> uri to other OFBiz web application.
> +*Sample Example*+
> Before:
> class="buttontext">${requestItem.productId!}
> After:
> EditProduct?productId=${requestItem.productId!}"
> class="buttontext">${requestItem.productId!}
--
This message was sent by Atlassian Jira
(v8.3.4#803005)
[jira] [Updated] (OFBIZ-11317) Add 'controlPath' attribute to 'ofbizUrl' freemarker macro
[
https://issues.apache.org/jira/browse/OFBIZ-11317?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]
James Yong updated OFBIZ-11317:
---
Attachment: OFBIZ-11317.patch
> Add 'controlPath' attribute to 'ofbizUrl' freemarker macro
> --
>
> Key: OFBIZ-11317
> URL: https://issues.apache.org/jira/browse/OFBIZ-11317
> Project: OFBiz
> Issue Type: Improvement
> Components: ALL APPLICATIONS
>Affects Versions: Trunk
>Reporter: James Yong
>Priority: Minor
> Fix For: Upcoming Branch
>
> Attachments: OFBIZ-11317.patch, OFBIZ-11317_Plugins.patch
>
>
> Currently the ofbizUrl freemarker macro doesn't support uri for other OFBiz
> web application.
> Adding the controlPath attribute to allow the controls by ofbizUrl to support
> uri to other OFBiz web application.
> +*Sample Example*+
> Before:
> class="buttontext">${requestItem.productId!}
> After:
> EditProduct?productId=${requestItem.productId!}"
> class="buttontext">${requestItem.productId!}
--
This message was sent by Atlassian Jira
(v8.3.4#803005)
[jira] [Assigned] (OFBIZ-11319) processorder is submitted as GET instead of POST
[ https://issues.apache.org/jira/browse/OFBIZ-11319?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ] James Yong reassigned OFBIZ-11319: -- Component/s: ecommerce Fix Version/s: Upcoming Branch Affects Version/s: Trunk Assignee: James Yong > processorder is submitted as GET instead of POST > > > Key: OFBIZ-11319 > URL: https://issues.apache.org/jira/browse/OFBIZ-11319 > Project: OFBiz > Issue Type: Bug > Components: ecommerce >Affects Versions: Trunk >Reporter: James Yong >Assignee: James Yong >Priority: Minor > Fix For: Upcoming Branch > > Attachments: OFBIZ-11319_Plugins.patch > > > During checkout of shopping cart, it is observed that the processorder of the > form action is submitted as GET instead of POST -- This message was sent by Atlassian Jira (v8.3.4#803005)
[jira] [Updated] (OFBIZ-11319) processorder is submitted as GET instead of POST
[ https://issues.apache.org/jira/browse/OFBIZ-11319?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ] James Yong updated OFBIZ-11319: --- Attachment: OFBIZ-11319_Plugins.patch > processorder is submitted as GET instead of POST > > > Key: OFBIZ-11319 > URL: https://issues.apache.org/jira/browse/OFBIZ-11319 > Project: OFBiz > Issue Type: Bug >Reporter: James Yong >Priority: Minor > Attachments: OFBIZ-11319_Plugins.patch > > > During checkout of shopping cart, it is observed that the processorder of the > form action is submitted as GET instead of POST -- This message was sent by Atlassian Jira (v8.3.4#803005)
[jira] [Created] (OFBIZ-11319) processorder is submitted as GET instead of POST
James Yong created OFBIZ-11319: -- Summary: processorder is submitted as GET instead of POST Key: OFBIZ-11319 URL: https://issues.apache.org/jira/browse/OFBIZ-11319 Project: OFBiz Issue Type: Bug Reporter: James Yong During checkout of shopping cart, it is observed that the processorder of the form action is submitted as GET instead of POST -- This message was sent by Atlassian Jira (v8.3.4#803005)
[jira] [Updated] (OFBIZ-11318) Unable to continue to Step 3 in One Page Checkout
[ https://issues.apache.org/jira/browse/OFBIZ-11318?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ] James Yong updated OFBIZ-11318: --- Issue Type: Bug (was: Improvement) > Unable to continue to Step 3 in One Page Checkout > -- > > Key: OFBIZ-11318 > URL: https://issues.apache.org/jira/browse/OFBIZ-11318 > Project: OFBiz > Issue Type: Bug > Components: ecommerce >Affects Versions: Trunk >Reporter: James Yong >Priority: Critical > Fix For: Upcoming Branch > > Attachments: OFBIZ-11318.patch > > > The following error is returned when trying to continue to Step3 when > checking out using One Page Checkout. > Could not create new purpose, a purpose with that type already exists.: > [GenericEntity:PartyContactWithPurpose][contactFromDate,2020-01-05 > 19:48:18.29(java.sql.Timestamp)][contactMechId,10020(java.lang.String)][contactMechPurposeTypeId,SHIPPING_LOCATION(java.lang.String)][partyId,admin(java.lang.String)][purposeFromDate,2020-01-05 > 19:48:18.332(java.sql.Timestamp) -- This message was sent by Atlassian Jira (v8.3.4#803005)
[jira] [Updated] (OFBIZ-11318) Unable to continue to Step 3 in One Page Checkout
[ https://issues.apache.org/jira/browse/OFBIZ-11318?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ] James Yong updated OFBIZ-11318: --- Attachment: OFBIZ-11318.patch > Unable to continue to Step 3 in One Page Checkout > -- > > Key: OFBIZ-11318 > URL: https://issues.apache.org/jira/browse/OFBIZ-11318 > Project: OFBiz > Issue Type: Improvement > Components: ecommerce >Affects Versions: Trunk >Reporter: James Yong >Priority: Critical > Fix For: Upcoming Branch > > Attachments: OFBIZ-11318.patch > > > The following error is returned when trying to continue to Step3 when > checking out using One Page Checkout. > Could not create new purpose, a purpose with that type already exists.: > [GenericEntity:PartyContactWithPurpose][contactFromDate,2020-01-05 > 19:48:18.29(java.sql.Timestamp)][contactMechId,10020(java.lang.String)][contactMechPurposeTypeId,SHIPPING_LOCATION(java.lang.String)][partyId,admin(java.lang.String)][purposeFromDate,2020-01-05 > 19:48:18.332(java.sql.Timestamp) -- This message was sent by Atlassian Jira (v8.3.4#803005)
[jira] [Created] (OFBIZ-11318) Unable to continue to Step 3 in One Page Checkout
James Yong created OFBIZ-11318: -- Summary: Unable to continue to Step 3 in One Page Checkout Key: OFBIZ-11318 URL: https://issues.apache.org/jira/browse/OFBIZ-11318 Project: OFBiz Issue Type: Improvement Components: ecommerce Affects Versions: Trunk Reporter: James Yong Fix For: Upcoming Branch The following error is returned when trying to continue to Step3 when checking out using One Page Checkout. Could not create new purpose, a purpose with that type already exists.: [GenericEntity:PartyContactWithPurpose][contactFromDate,2020-01-05 19:48:18.29(java.sql.Timestamp)][contactMechId,10020(java.lang.String)][contactMechPurposeTypeId,SHIPPING_LOCATION(java.lang.String)][partyId,admin(java.lang.String)][purposeFromDate,2020-01-05 19:48:18.332(java.sql.Timestamp) -- This message was sent by Atlassian Jira (v8.3.4#803005)
[jira] [Created] (OFBIZ-11317) Add 'controlPath' attribute to 'ofbizUrl' freemarker macro
James Yong created OFBIZ-11317:
--
Summary: Add 'controlPath' attribute to 'ofbizUrl' freemarker macro
Key: OFBIZ-11317
URL: https://issues.apache.org/jira/browse/OFBIZ-11317
Project: OFBiz
Issue Type: Improvement
Components: ALL APPLICATIONS
Affects Versions: Trunk
Reporter: James Yong
Fix For: Upcoming Branch
Currently the ofbizUrl freemarker macro doesn't support uri for other OFBiz web
application.
Adding the controlPath attribute to allow the controls by ofbizUrl to support
uri to other OFBiz web application.
+*Sample Example*+
Before:
${requestItem.productId!}
After:
EditProduct?productId=${requestItem.productId!}"
class="buttontext">${requestItem.productId!}
--
This message was sent by Atlassian Jira
(v8.3.4#803005)
[jira] [Commented] (OFBIZ-11310) JSON renderer for screen/menu/form
[ https://issues.apache.org/jira/browse/OFBIZ-11310?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17013595#comment-17013595 ] Mathieu Lirzin commented on OFBIZ-11310: Sorry Olivier for the late reply I did not notice that you actually opened a ticket for the JSON screen renderer. To avoid this kind of issue next time, feel free to either send a message on the mailing list on the associated thread linking the discussion with the ticket, or you can ping people with a [~mthl] mention. In order to start the review, it will help me if you could provide a patch combining the newly added files and defining the location inside the framework where those file will be put. Thanks > JSON renderer for screen/menu/form > --- > > Key: OFBIZ-11310 > URL: https://issues.apache.org/jira/browse/OFBIZ-11310 > Project: OFBiz > Issue Type: Improvement > Components: framework >Affects Versions: Trunk >Reporter: Olivier Heintz >Assignee: Mathieu Lirzin >Priority: Minor > Attachments: FrontJsFormRenderer.java, FrontJsMenuRenderer.java, > FrontJsOutput.java, FrontJsScreenRenderer.java, > FrontJsScreenViewHandler.java, FrontJsTreeRenderer.java > > > In a POC approach for using a SPA framework for GUI in OFBiz, json renderer > have been developed. > This Jira could be used to do code review on these renderer. > Currently, these renderer are on a V0 situation, it's a work in process > situation. After mailing-list discussion with mathieu we decide to go to a > V0.1 by 2-3 tasks > * add some javadoc each time it's necessary or needed > * remove all comment code or explain the decision waiting to activate or > remove the code > * do checkstyle > On first step, code will be with only what is used in a use case (no code for > tag not currently manage). > Json renderer are part of vuejs-portal plugin, in ofbizextra gitlab > (https://gitlab.ofbizextra.org/ofbizextra/ofbizplugins/vuejsPortal), so > commit are done on it, and mailing-list with these commit [are > ofbizextra-com...@lists.sourceforge.net.|mailto:are%c2%a0ofbizextra-com...@lists.sourceforge.net.] > But we will publish an update on this Jira each week there are some > modifications on renderer files > This task will take some time because it will used to do a complete code > review, not only in renderer but in vuejs component too, and so complete > documentation. -- This message was sent by Atlassian Jira (v8.3.4#803005)
[jira] [Assigned] (OFBIZ-11310) JSON renderer for screen/menu/form
[ https://issues.apache.org/jira/browse/OFBIZ-11310?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ] Mathieu Lirzin reassigned OFBIZ-11310: -- Assignee: Mathieu Lirzin > JSON renderer for screen/menu/form > --- > > Key: OFBIZ-11310 > URL: https://issues.apache.org/jira/browse/OFBIZ-11310 > Project: OFBiz > Issue Type: Improvement > Components: framework >Affects Versions: Trunk >Reporter: Olivier Heintz >Assignee: Mathieu Lirzin >Priority: Minor > Attachments: FrontJsFormRenderer.java, FrontJsMenuRenderer.java, > FrontJsOutput.java, FrontJsScreenRenderer.java, > FrontJsScreenViewHandler.java, FrontJsTreeRenderer.java > > > In a POC approach for using a SPA framework for GUI in OFBiz, json renderer > have been developed. > This Jira could be used to do code review on these renderer. > Currently, these renderer are on a V0 situation, it's a work in process > situation. After mailing-list discussion with mathieu we decide to go to a > V0.1 by 2-3 tasks > * add some javadoc each time it's necessary or needed > * remove all comment code or explain the decision waiting to activate or > remove the code > * do checkstyle > On first step, code will be with only what is used in a use case (no code for > tag not currently manage). > Json renderer are part of vuejs-portal plugin, in ofbizextra gitlab > (https://gitlab.ofbizextra.org/ofbizextra/ofbizplugins/vuejsPortal), so > commit are done on it, and mailing-list with these commit [are > ofbizextra-com...@lists.sourceforge.net.|mailto:are%c2%a0ofbizextra-com...@lists.sourceforge.net.] > But we will publish an update on this Jira each week there are some > modifications on renderer files > This task will take some time because it will used to do a complete code > review, not only in renderer but in vuejs component too, and so complete > documentation. -- This message was sent by Atlassian Jira (v8.3.4#803005)
[jira] [Commented] (OFBIZ-11296) Use 'depends-on' everywhere
[ https://issues.apache.org/jira/browse/OFBIZ-11296?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17013587#comment-17013587 ] Mathieu Lirzin commented on OFBIZ-11296: You are right that your anwser is containing points specific to this issue however they are based on general principles (stability for production, backward compatibility, relation to community approval). Those principles are too far from my perspective/requirements as a developer trying to make OFBiz better, that I cannot see a way to resolve this specific issue without discussing the principles first. > Use 'depends-on' everywhere > --- > > Key: OFBIZ-11296 > URL: https://issues.apache.org/jira/browse/OFBIZ-11296 > Project: OFBiz > Issue Type: Improvement > Components: framework >Affects Versions: Trunk >Reporter: Mathieu Lirzin >Assignee: Mathieu Lirzin >Priority: Minor > Fix For: Upcoming Branch > > Attachments: > OFBIZ-11296_0001-Improved-Use-depends-on-attribute-instead-of-compone.patch, > OFBIZ-11296_ignore-depends-on-when-a-component-load.xml-is-prese.patch > > > We currently have two ways to define component loading order. Either > by using ‘depends-on’ attribute in “component-config.xml” or by adding > a “component-load.xml” file at the root of a component directory. > “depends-on” is more flexible because it handles partial ordering when > “component-load.xml” defines a total order which is not necessarily > meaningful, so it is better to rely only “depends-on”. -- This message was sent by Atlassian Jira (v8.3.4#803005)
[jira] [Comment Edited] (OFBIZ-11306) POC for CSRF Token
[
https://issues.apache.org/jira/browse/OFBIZ-11306?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17013563#comment-17013563
]
Jacques Le Roux edited comment on OFBIZ-11306 at 1/11/20 7:43 PM:
--
Hi James,
First I must apologize. Because I initially suggested that we should not
differentiate GET and POST methods. I undertstood I was wrong after reading
[OWASP Disclosure of Token in
URL|https://cheatsheetseries.owasp.org/cheatsheets/Cross-Site_Request_Forgery_Prevention_Cheat_Sheet.html#disclosure-of-token-in-url]
and thought about it. As explained in my comment above, it has an impact on
the work already done. Fortunately most is easy to remove. But we need to add
some code.
After reviewing Java code (still a WIP) here are my conclusions:
* OrderServices.xml should not be part of this patch, another Jira should be
created
* ConfigXMLReader: OK with me
* ControlEventListener: OK with me
* WidgetWorker: we don't want tokens in form ULRs, hidden fields only should
be used
* MacroFormRenderer: we don't want tokens in form ULRs, hidden fields only
should be used, as it's done in HtmlFormMacroLibrary.ftl. We don't need tokens
for pagination. It's idempotent, no harm possible.
* CsrfTokenTransform is useless we don't want tokens in form ULRs, as
mentioned for WidgetWorker and MacroFormRenderer
* UrlRegexpTransform, CatalogAltUrlSeoTransform: we don't want tokens in
navigation ULRs. We though need to check that all URLs are idempotent. They
should be since those are only for navigation, and forms are used for changes.
* CsrfTokenAjaxTransform: OK with me
Other misc. stuff:
* We need to adds crsftoken hidden fields in forms in ftl files. There at 706
of them. Only non idempotent forms are concerned. I guess most of them if, not
all. This sounds like a boring task. Maybe we can find a smart way to do that
globally. I did not think about it already.
* I don't think adding {{if (!options.crossDomain)}} in ajaxPrefilter will
work with CORS. I have to check that once we will commit, not a big deal.
* I like the idea of controlPath in ofbizUrl macro. But it's unrelated to
CSRF. I suggest to remove it from this issue and create a specific issue for
that.
After a preliminary preliminary review, I still need to review CsrfUtil
closer...
Again thanks for the good work!
was (Author: jacques.le.roux):
Hi James,
First I must apologize. Because I initially suggested that we should not
differentiate GET and POST methods. I undertstood I was wrong after reading
[OWASP Disclosure of Token in
URL|https://cheatsheetseries.owasp.org/cheatsheets/Cross-Site_Request_Forgery_Prevention_Cheat_Sheet.html#disclosure-of-token-in-url]
and thought about it. As explained in my comment abovet, it has an impact on
the work already done. Fortunately most is easy to remove. But we need to add
some code.
After reviewing Java code (still a WIP) here are my conclusions:
* OrderServices.xml should not be part of this patch, another Jira should be
created
* ConfigXMLReader: OK with me
* ControlEventListener: OK with me
* WidgetWorker: we don't want tokens in form ULRs, hidden fields only should
be used
* MacroFormRenderer: we don't want tokens in form ULRs, hidden fields only
should be used, as it's done in HtmlFormMacroLibrary.ftl. We don't need tokens
for pagination. It's idempotent, no harm possible.
* CsrfTokenTransform is useless we don't want tokens in form ULRs, as
mentioned for WidgetWorker and MacroFormRenderer
* UrlRegexpTransform, CatalogAltUrlSeoTransform: we don't want tokens in
navigation ULRs. We though need to check that all URLs are idempotent. They
should be since those are only for navigation, and forms are used for changes.
* CsrfTokenAjaxTransform: OK with me
Other misc. stuff:
* We need to adds crsftoken hidden fields in forms in ftl files. There at 706
of them. Only non idempotent forms are concerned. I guess most of them if, not
all. This ssound like a boring task. Maybe we can find a smart way to do that
globally. I did not think about it already.
* I don't think adding {{if (!options.crossDomain)}} in ajaxPrefilter will
work with CORS. I have to check that once we will commit, not a big deal.
* I like the idea of controlPath in ofbizUrl macro. But it's unrelated to
CSRF. I suggest to remove it from this issue and create a specific issue for
that.
After a preliminary preliminary review, I still need to review CsrfUtil
closer...
Again thanks for the good work!
> POC for CSRF Token
> --
>
> Key: OFBIZ-11306
> URL: https://issues.apache.org/jira/browse/OFBIZ-11306
> Project: OFBiz
> Issue Type: Improvement
> Components: ALL APPLICATIONS
>Affects Versions: Upcoming Branch
>Reporter: James Yong
>Assignee: Jacques Le Roux
>Priority:
[jira] [Commented] (OFBIZ-11306) POC for CSRF Token
[
https://issues.apache.org/jira/browse/OFBIZ-11306?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17013563#comment-17013563
]
Jacques Le Roux commented on OFBIZ-11306:
-
Hi James,
First I must apologize. Because I initially suggested that we should not
differentiate GET and POST methods. I undertstood I was wrong after reading
[OWASP Disclosure of Token in
URL|https://cheatsheetseries.owasp.org/cheatsheets/Cross-Site_Request_Forgery_Prevention_Cheat_Sheet.html#disclosure-of-token-in-url]
and thought about it. As explained in my comment abovet, it has an impact on
the work already done. Fortunately most is easy to remove. But we need to add
some code.
After reviewing Java code (still a WIP) here are my conclusions:
* OrderServices.xml should not be part of this patch, another Jira should be
created
* ConfigXMLReader: OK with me
* ControlEventListener: OK with me
* WidgetWorker: we don't want tokens in form ULRs, hidden fields only should
be used
* MacroFormRenderer: we don't want tokens in form ULRs, hidden fields only
should be used, as it's done in HtmlFormMacroLibrary.ftl. We don't need tokens
for pagination. It's idempotent, no harm possible.
* CsrfTokenTransform is useless we don't want tokens in form ULRs, as
mentioned for WidgetWorker and MacroFormRenderer
* UrlRegexpTransform, CatalogAltUrlSeoTransform: we don't want tokens in
navigation ULRs. We though need to check that all URLs are idempotent. They
should be since those are only for navigation, and forms are used for changes.
* CsrfTokenAjaxTransform: OK with me
Other misc. stuff:
* We need to adds crsftoken hidden fields in forms in ftl files. There at 706
of them. Only non idempotent forms are concerned. I guess most of them if, not
all. This ssound like a boring task. Maybe we can find a smart way to do that
globally. I did not think about it already.
* I don't think adding {{if (!options.crossDomain)}} in ajaxPrefilter will
work with CORS. I have to check that once we will commit, not a big deal.
* I like the idea of controlPath in ofbizUrl macro. But it's unrelated to
CSRF. I suggest to remove it from this issue and create a specific issue for
that.
After a preliminary preliminary review, I still need to review CsrfUtil
closer...
Again thanks for the good work!
> POC for CSRF Token
> --
>
> Key: OFBIZ-11306
> URL: https://issues.apache.org/jira/browse/OFBIZ-11306
> Project: OFBiz
> Issue Type: Improvement
> Components: ALL APPLICATIONS
>Affects Versions: Upcoming Branch
>Reporter: James Yong
>Assignee: Jacques Le Roux
>Priority: Minor
> Labels: CSRF
> Fix For: Upcoming Branch
>
> Attachments: OFBIZ-11306-v2.patch, OFBIZ-11306.patch,
> OFBIZ-11306.patch, OFBIZ-11306.patch, OFBIZ-11306.patch, OFBIZ-11306.patch,
> OFBIZ-11306.patch, OFBIZ-11306.patch, OFBIZ-11306.patch, OFBIZ-11306.patch,
> OFBIZ-11306.patch, OFBIZ-11306.patch, OFBIZ-11306_Plugins.patch,
> OFBIZ-11306_Plugins.patch, OFBIZ-11306_Plugins.patch,
> OFBIZ-11306_Plugins.patch
>
>
> CRSF tokens are generated using CSRF Guard library and used in:
> 1) In widget form where a hidden token field is auto-generated.
> 2) In FTL form where a <@csrfTokenField> macro is used to generate the csrf
> token field.
> 3) In Ajax call where a <@csrfTokenAjax> macro is used to assign csrf token
> to X-CSRF-Token in request header.
> CSRF tokens are stored in the user sessions, and verified during POST request.
> A new attribute i.e. csrf-token is added to the security tag to exempt CSRF
> token check.
> Certain request path, like LookupPartyName, can be exempt from CSRF token
> check during Ajax POST call.
--
This message was sent by Atlassian Jira
(v8.3.4#803005)
[jira] [Closed] (OFBIZ-11316) Bug when order contains adjustments with NULL amount
[ https://issues.apache.org/jira/browse/OFBIZ-11316?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ] Jacques Le Roux closed OFBIZ-11316. --- Fix Version/s: 18.12.01 17.12.01 Resolution: Fixed Thanks Amine, Your patch is in. I have changed the description to make it clear. > Bug when order contains adjustments with NULL amount > > > Key: OFBIZ-11316 > URL: https://issues.apache.org/jira/browse/OFBIZ-11316 > Project: OFBiz > Issue Type: Bug > Components: accounting >Affects Versions: Trunk >Reporter: Amine Azzi >Assignee: Jacques Le Roux >Priority: Major > Fix For: 17.12.01, 18.12.01 > > Attachments: OFBIZ-11316.patch > > > The issue was introduced by OFBIZ-7012 where adding isTaxIncludedInPrice was > missing at line 565. Because the compared amount in this line (in the > adjustment) is different from 0 and equals to amountAlreadyIncluded only if > the tax is included in price -- This message was sent by Atlassian Jira (v8.3.4#803005)
[jira] [Commented] (OFBIZ-11316) Bug when order contains adjustments with NULL amount
[ https://issues.apache.org/jira/browse/OFBIZ-11316?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17013438#comment-17013438 ] ASF subversion and git services commented on OFBIZ-11316: - Commit 2c727bed212019e154bf50071257e84c0c3dd52d in ofbiz-framework's branch refs/heads/release17.12 from Jacques Le Roux [ https://gitbox.apache.org/repos/asf?p=ofbiz-framework.git;h=2c727be ] Fixed: Bug when order contains adjustments with NULL amount (OFBIZ-11316) The issue was introduced by OFBIZ-7012 where adding isTaxIncludedInPrice was missing at line 565. Because the compared amount in this line (in the adjustment) is different from 0 and equals to amountAlreadyIncluded only if the tax is included in price Thanks: Amine Azzi > Bug when order contains adjustments with NULL amount > > > Key: OFBIZ-11316 > URL: https://issues.apache.org/jira/browse/OFBIZ-11316 > Project: OFBiz > Issue Type: Bug > Components: accounting >Affects Versions: Trunk >Reporter: Amine Azzi >Assignee: Jacques Le Roux >Priority: Major > Attachments: OFBIZ-11316.patch > > > The issue was introduced by OFBIZ-7012 where adding isTaxIncludedInPrice was > missing at line 565. Because the compared amount in this line (in the > adjustment) is different from 0 and equals to amountAlreadyIncluded only if > the tax is included in price -- This message was sent by Atlassian Jira (v8.3.4#803005)
[jira] [Commented] (OFBIZ-7012) VAT tax are not recorded as separate line items in Invoice for products with VAT tax included in their price
[
https://issues.apache.org/jira/browse/OFBIZ-7012?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17013439#comment-17013439
]
ASF subversion and git services commented on OFBIZ-7012:
Commit 2c727bed212019e154bf50071257e84c0c3dd52d in ofbiz-framework's branch
refs/heads/release17.12 from Jacques Le Roux
[ https://gitbox.apache.org/repos/asf?p=ofbiz-framework.git;h=2c727be ]
Fixed: Bug when order contains adjustments with NULL amount
(OFBIZ-11316)
The issue was introduced by OFBIZ-7012 where adding isTaxIncludedInPrice was
missing at line 565. Because the compared amount in this line (in the
adjustment)
is different from 0 and equals to amountAlreadyIncluded only if
the tax is included in price
Thanks: Amine Azzi
> VAT tax are not recorded as separate line items in Invoice for products with
> VAT tax included in their price
>
>
> Key: OFBIZ-7012
> URL: https://issues.apache.org/jira/browse/OFBIZ-7012
> Project: OFBiz
> Issue Type: New Feature
> Components: accounting, order
>Reporter: Divesh Dutta
>Assignee: Divesh Dutta
>Priority: Major
> Labels: VAT, tax
> Attachments: OFBIZ-7012.patch
>
>
> In countries like UK VAT system works. So in VAT system, VAT taxes are
> included in product's price. So OFBiz have support of VAT tax where you can
> include VAT in product's price itself. Here is the example data we create to
> enable VAT calculations:
> {code}
>
>
>
>
>
>
> taxAuthGeoId="GBR" taxAuthPartyId="GB_TA"/>
>
> productStoreId="9000" taxAuthGeoId="GBR" taxAuthPartyId="GB_TA"
> taxAuthorityRateTypeId="VAT_TAX" taxPercentage="20.00" />
> productPricePurposeId="PURCHASE" productPriceTypeId="DEFAULT_PRICE"
> productStoreGroupId="_NA_" taxAuthGeoId="GBR" taxAuthPartyId="GB_TA"
> taxInPrice="Y" />
> {code}
> When invoices are created, tax amount included in product's price is not
> separated in new invoice line item. And when general ledger postings are
> done, then tax amount is not posted in Liability account. So due to this
> organizations have no way to figure out how much tax is to be paid to tax
> authority.
> Ideally, product's actual price (without VAT) should be recorded in revenue
> account and tax should be recorded in liability account.
> For doing this, when invoice is created, VAT amount should be separated and
> created as new line item.
> Recently we have worked for one of our client where we have fixed this issue
> and enabled invoicing in OFBiz to record vat tax amount as separate line
> items. [~lektran] has actively worked with us to finalize the solution design
> and completing the implementation. [~ankush.upadhyay] is the developer who
> worked on solution design. [~ankush.upadhyay] please provide the patch for
> this issue.
--
This message was sent by Atlassian Jira
(v8.3.4#803005)
[jira] [Commented] (OFBIZ-11316) Bug when order contains adjustments with NULL amount
[ https://issues.apache.org/jira/browse/OFBIZ-11316?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17013436#comment-17013436 ] ASF subversion and git services commented on OFBIZ-11316: - Commit 23aa779df5ba2a6761af67b67ac8d402a5bd9608 in ofbiz-framework's branch refs/heads/release18.12 from Jacques Le Roux [ https://gitbox.apache.org/repos/asf?p=ofbiz-framework.git;h=23aa779 ] Fixed: Bug when order contains adjustments with NULL amount (OFBIZ-11316) The issue was introduced by OFBIZ-7012 where adding isTaxIncludedInPrice was missing at line 565. Because the compared amount in this line (in the adjustment) is different from 0 and equals to amountAlreadyIncluded only if the tax is included in price Thanks: Amine Azzi > Bug when order contains adjustments with NULL amount > > > Key: OFBIZ-11316 > URL: https://issues.apache.org/jira/browse/OFBIZ-11316 > Project: OFBiz > Issue Type: Bug > Components: accounting >Affects Versions: Trunk >Reporter: Amine Azzi >Assignee: Jacques Le Roux >Priority: Major > Attachments: OFBIZ-11316.patch > > > The issue was introduced by OFBIZ-7012 where adding isTaxIncludedInPrice was > missing at line 565. Because the compared amount in this line (in the > adjustment) is different from 0 and equals to amountAlreadyIncluded only if > the tax is included in price -- This message was sent by Atlassian Jira (v8.3.4#803005)
[jira] [Commented] (OFBIZ-7012) VAT tax are not recorded as separate line items in Invoice for products with VAT tax included in their price
[
https://issues.apache.org/jira/browse/OFBIZ-7012?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17013437#comment-17013437
]
ASF subversion and git services commented on OFBIZ-7012:
Commit 23aa779df5ba2a6761af67b67ac8d402a5bd9608 in ofbiz-framework's branch
refs/heads/release18.12 from Jacques Le Roux
[ https://gitbox.apache.org/repos/asf?p=ofbiz-framework.git;h=23aa779 ]
Fixed: Bug when order contains adjustments with NULL amount
(OFBIZ-11316)
The issue was introduced by OFBIZ-7012 where adding isTaxIncludedInPrice was
missing at line 565. Because the compared amount in this line (in the
adjustment)
is different from 0 and equals to amountAlreadyIncluded only if
the tax is included in price
Thanks: Amine Azzi
> VAT tax are not recorded as separate line items in Invoice for products with
> VAT tax included in their price
>
>
> Key: OFBIZ-7012
> URL: https://issues.apache.org/jira/browse/OFBIZ-7012
> Project: OFBiz
> Issue Type: New Feature
> Components: accounting, order
>Reporter: Divesh Dutta
>Assignee: Divesh Dutta
>Priority: Major
> Labels: VAT, tax
> Attachments: OFBIZ-7012.patch
>
>
> In countries like UK VAT system works. So in VAT system, VAT taxes are
> included in product's price. So OFBiz have support of VAT tax where you can
> include VAT in product's price itself. Here is the example data we create to
> enable VAT calculations:
> {code}
>
>
>
>
>
>
> taxAuthGeoId="GBR" taxAuthPartyId="GB_TA"/>
>
> productStoreId="9000" taxAuthGeoId="GBR" taxAuthPartyId="GB_TA"
> taxAuthorityRateTypeId="VAT_TAX" taxPercentage="20.00" />
> productPricePurposeId="PURCHASE" productPriceTypeId="DEFAULT_PRICE"
> productStoreGroupId="_NA_" taxAuthGeoId="GBR" taxAuthPartyId="GB_TA"
> taxInPrice="Y" />
> {code}
> When invoices are created, tax amount included in product's price is not
> separated in new invoice line item. And when general ledger postings are
> done, then tax amount is not posted in Liability account. So due to this
> organizations have no way to figure out how much tax is to be paid to tax
> authority.
> Ideally, product's actual price (without VAT) should be recorded in revenue
> account and tax should be recorded in liability account.
> For doing this, when invoice is created, VAT amount should be separated and
> created as new line item.
> Recently we have worked for one of our client where we have fixed this issue
> and enabled invoicing in OFBiz to record vat tax amount as separate line
> items. [~lektran] has actively worked with us to finalize the solution design
> and completing the implementation. [~ankush.upadhyay] is the developer who
> worked on solution design. [~ankush.upadhyay] please provide the patch for
> this issue.
--
This message was sent by Atlassian Jira
(v8.3.4#803005)
[jira] [Commented] (OFBIZ-7012) VAT tax are not recorded as separate line items in Invoice for products with VAT tax included in their price
[
https://issues.apache.org/jira/browse/OFBIZ-7012?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17013434#comment-17013434
]
ASF subversion and git services commented on OFBIZ-7012:
Commit 8d3d2149fcfb1902357086696505de674d98bb8d in ofbiz-framework's branch
refs/heads/trunk from Jacques Le Roux
[ https://gitbox.apache.org/repos/asf?p=ofbiz-framework.git;h=8d3d214 ]
Fixed: Bug when order contains adjustments with NULL amount
(OFBIZ-11316)
The issue was introduced by OFBIZ-7012 where adding isTaxIncludedInPrice was
missing at line 565. Because the compared amount in this line (in the
adjustment)
is different from 0 and equals to amountAlreadyIncluded only if
the tax is included in price
Thanks: Amine Azzi
> VAT tax are not recorded as separate line items in Invoice for products with
> VAT tax included in their price
>
>
> Key: OFBIZ-7012
> URL: https://issues.apache.org/jira/browse/OFBIZ-7012
> Project: OFBiz
> Issue Type: New Feature
> Components: accounting, order
>Reporter: Divesh Dutta
>Assignee: Divesh Dutta
>Priority: Major
> Labels: VAT, tax
> Attachments: OFBIZ-7012.patch
>
>
> In countries like UK VAT system works. So in VAT system, VAT taxes are
> included in product's price. So OFBiz have support of VAT tax where you can
> include VAT in product's price itself. Here is the example data we create to
> enable VAT calculations:
> {code}
>
>
>
>
>
>
> taxAuthGeoId="GBR" taxAuthPartyId="GB_TA"/>
>
> productStoreId="9000" taxAuthGeoId="GBR" taxAuthPartyId="GB_TA"
> taxAuthorityRateTypeId="VAT_TAX" taxPercentage="20.00" />
> productPricePurposeId="PURCHASE" productPriceTypeId="DEFAULT_PRICE"
> productStoreGroupId="_NA_" taxAuthGeoId="GBR" taxAuthPartyId="GB_TA"
> taxInPrice="Y" />
> {code}
> When invoices are created, tax amount included in product's price is not
> separated in new invoice line item. And when general ledger postings are
> done, then tax amount is not posted in Liability account. So due to this
> organizations have no way to figure out how much tax is to be paid to tax
> authority.
> Ideally, product's actual price (without VAT) should be recorded in revenue
> account and tax should be recorded in liability account.
> For doing this, when invoice is created, VAT amount should be separated and
> created as new line item.
> Recently we have worked for one of our client where we have fixed this issue
> and enabled invoicing in OFBiz to record vat tax amount as separate line
> items. [~lektran] has actively worked with us to finalize the solution design
> and completing the implementation. [~ankush.upadhyay] is the developer who
> worked on solution design. [~ankush.upadhyay] please provide the patch for
> this issue.
--
This message was sent by Atlassian Jira
(v8.3.4#803005)
[jira] [Commented] (OFBIZ-11316) Bug when order contains adjustments with NULL amount
[ https://issues.apache.org/jira/browse/OFBIZ-11316?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17013433#comment-17013433 ] ASF subversion and git services commented on OFBIZ-11316: - Commit 8d3d2149fcfb1902357086696505de674d98bb8d in ofbiz-framework's branch refs/heads/trunk from Jacques Le Roux [ https://gitbox.apache.org/repos/asf?p=ofbiz-framework.git;h=8d3d214 ] Fixed: Bug when order contains adjustments with NULL amount (OFBIZ-11316) The issue was introduced by OFBIZ-7012 where adding isTaxIncludedInPrice was missing at line 565. Because the compared amount in this line (in the adjustment) is different from 0 and equals to amountAlreadyIncluded only if the tax is included in price Thanks: Amine Azzi > Bug when order contains adjustments with NULL amount > > > Key: OFBIZ-11316 > URL: https://issues.apache.org/jira/browse/OFBIZ-11316 > Project: OFBiz > Issue Type: Bug > Components: accounting >Affects Versions: Trunk >Reporter: Amine Azzi >Assignee: Jacques Le Roux >Priority: Major > Attachments: OFBIZ-11316.patch > > > The issue was introduced by OFBIZ-7012 where adding isTaxIncludedInPrice was > missing at line 565. Because the compared amount in this line (in the > adjustment) is different from 0 and equals to amountAlreadyIncluded only if > the tax is included in price -- This message was sent by Atlassian Jira (v8.3.4#803005)
[jira] [Updated] (OFBIZ-11316) Bug when order contains adjustments with NULL amount
[
https://issues.apache.org/jira/browse/OFBIZ-11316?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]
Jacques Le Roux updated OFBIZ-11316:
Description:
The issue was introduced by OFBIZ-7012 where adding isTaxIncludedInPrice was
missing at line 565. Because the compared amount in this line (in the
adjustment) is different from 0 and equals to amountAlreadyIncluded only if
the tax is included in price
was:
On org/apache/ofbiz/accounting/invoice/InvoiceServices.java:561.
The condition evaluation gives an error when the adjustment has a null amount
even when isTaxIncludedInPrice is FALSE.
The condition should be re-written to
if (isTaxIncludedInPrice && (adj.getBigDecimal("amount").signum() == 0))
> Bug when order contains adjustments with NULL amount
>
>
> Key: OFBIZ-11316
> URL: https://issues.apache.org/jira/browse/OFBIZ-11316
> Project: OFBiz
> Issue Type: Bug
> Components: accounting
>Affects Versions: Trunk
>Reporter: Amine Azzi
>Assignee: Jacques Le Roux
>Priority: Major
> Attachments: OFBIZ-11316.patch
>
>
> The issue was introduced by OFBIZ-7012 where adding isTaxIncludedInPrice was
> missing at line 565. Because the compared amount in this line (in the
> adjustment) is different from 0 and equals to amountAlreadyIncluded only if
> the tax is included in price
--
This message was sent by Atlassian Jira
(v8.3.4#803005)
