Secured URLs strategy

[Jacques Le Roux]

At
https://demo904.ofbiz.org/catalog/control/setUserPreference?userPrefGroupTypeId=GLOBAL_PREFERENCES&userPrefTypeId=COMPACT_HEADER&userPrefValue=Y

I get see this message.
Found URL parameter [userPrefTypeId] passed to secure (https) request-map with uri [setUserPreference] with an event that calls
service [setUserPreference]; this is not allowed for security reasons! The data should be encrypted by making it part of the request
body (a form field) instead of the request URL.

I thought we gave up with this message (or just have it only in log?). But I was just thinking about that yesterday and I think that
we should contunue to have it in trunk and not in 9.04. So we will be able to catch them (before having a tool to list them all, I
hope to work on that next week) without disturbing 9.04 users

WDYT ?

Jacques

[David E Jones-3]

On Apr 18, 2009, at 4:48 AM, Jacques Le Roux wrote:

> At https://demo904.ofbiz.org/catalog/control/setUserPreference?userPrefGroupTypeId=GLOBAL_PREFERENCES&userPrefTypeId=COMPACT_HEADER&userPrefValue=Y
>
> I get see this message.
> Found URL parameter [userPrefTypeId] passed to secure (https)  
> request-map with uri [setUserPreference] with an event that calls  
> service [setUserPreference]; this is not allowed for security  
> reasons! The data should be encrypted by making it part of the  
> request body (a form field) instead of the request URL.

That particular is already fixed, and just hasn't updated yet.

> I thought we gave up with this message (or just have it only in log?).

If a change had been made you would have seen it in the commit log...  
and hopefully explicitly called out as such (it is unfortunate that we  
get so many poorly written commit logs that don't even try to describe  
the changes made...).

> But I was just thinking about that yesterday and I think that we  
> should contunue to have it in trunk and not in 9.04. So we will be  
> able to catch them (before having a tool to list them all, I hope to  
> work on that next week) without disturbing 9.04 users

The main point of that error is to protect against XSRF attacks.  
Without that error and not allowing the condition it checks there is  
nothing keeping spoofed parameters from piggy-backing on a cloned  
encrypted request (or caught and modified through a man-in-the-middle  
sort of attack).

Personally I'd rather see these fixed in both the release branch and  
in the trunk, but if we get too many complaints about it in the  
release branch then I'm totally fine disabling that constraint  
temporarily.

-David

[Jacques Le Roux]

From: "David E Jones" <[hidden email]>
>> I thought we gave up with this message (or just have it only in log?).
>
> If a change had been made you would have seen it in the commit log...  
> and hopefully explicitly called out as such (it is unfortunate that we  
> get so many poorly written commit logs that don't even try to describe  
> the changes made...).

I was referring to r764286 and yes the parameter in url.properties is set to
    service.http.parameters.require.encrypted=Y
I thought it was =N, memory loss...

>> But I was just thinking about that yesterday and I think that we  
>> should contunue to have it in trunk and not in 9.04. So we will be  
>> able to catch them (before having a tool to list them all, I hope to  
>> work on that next week) without disturbing 9.04 users
>
> The main point of that error is to protect against XSRF attacks.  
> Without that error and not allowing the condition it checks there is  
> nothing keeping spoofed parameters from piggy-backing on a cloned  
> encrypted request (or caught and modified through a man-in-the-middle  
> sort of attack).
>
> Personally I'd rather see these fixed in both the release branch and  
> in the trunk, but if we get too many complaints about it in the  
> release branch then I'm totally fine disabling that constraint  
> temporarily.

Yes, let's see what happens...

Jacques
 
> -David
>

[Ashish Vijaywargiya]

Hello David,

Below is your comments from rev no. 757316.

>>Changed back to throw an exception when a non-body parameter is passed to
a secure request that calls a service as an event; now that >>we have the
Form Widget and form defs fixed up to handle these better it should be fine
for most things, but chances are there are some >>links in FTL files that
will still be broken and will need to be manually fixed; with this we can
look forward to more issues and >>questions/comments on the mailing list,
but this also makes it a lot more secure and pretty difficult to spoof one
of these requests (will >>have to hack HTTPS and encrypt the body to do so)

Here you are saying that we may need to explicitly set the form values in
the FTL files. But how should we handle the case when we have "Delete" &
"Update" case on the list form that is build up using FTL file.

For Ex :
https://localhost:8443/catalog/control/EditCategoryParties?productCategoryId=CATALOG1

I have created form something similar to the above link but I am confused
how should I handle this case in which we have "Update" & "Delete" button.

It can be a stupid question but I tried my best to solve the problem
although didn't get success.
Thanks in advance for any help !

--
Ashish Vijaywargiya


On Sun, Apr 19, 2009 at 4:27 AM, David E Jones
<[hidden email]>wrote:

>
> On Apr 18, 2009, at 4:48 AM, Jacques Le Roux wrote:
>
>  At
>> https://demo904.ofbiz.org/catalog/control/setUserPreference?userPrefGroupTypeId=GLOBAL_PREFERENCES&userPrefTypeId=COMPACT_HEADER&userPrefValue=Y
>>
>> I get see this message.
>> Found URL parameter [userPrefTypeId] passed to secure (https) request-map
>> with uri [setUserPreference] with an event that calls service
>> [setUserPreference]; this is not allowed for security reasons! The data
>> should be encrypted by making it part of the request body (a form field)
>> instead of the request URL.
>>
>
> That particular is already fixed, and just hasn't updated yet.
>
>  I thought we gave up with this message (or just have it only in log?).
>>
>
> If a change had been made you would have seen it in the commit log... and
> hopefully explicitly called out as such (it is unfortunate that we get so
> many poorly written commit logs that don't even try to describe the changes
> made...).
>
>  But I was just thinking about that yesterday and I think that we should
>> contunue to have it in trunk and not in 9.04. So we will be able to catch
>> them (before having a tool to list them all, I hope to work on that next
>> week) without disturbing 9.04 users
>>
>
> The main point of that error is to protect against XSRF attacks. Without
> that error and not allowing the condition it checks there is nothing keeping
> spoofed parameters from piggy-backing on a cloned encrypted request (or
> caught and modified through a man-in-the-middle sort of attack).
>
> Personally I'd rather see these fixed in both the release branch and in the
> trunk, but if we get too many complaints about it in the release branch then
> I'm totally fine disabling that constraint temporarily.
>
> -David
>
>

[David E Jones-3]

I'm not sure I followed all of that... are you worried about having a  
form nested within another form?

Yes, the forms will have to be outside the main form, but the links to  
those forms can be inside the main form.

-David


On Apr 20, 2009, at 2:25 AM, Ashish Vijaywargiya wrote:

> Hello David,
>
> Below is your comments from rev no. 757316.
>
>>> Changed back to throw an exception when a non-body parameter is  
>>> passed to
> a secure request that calls a service as an event; now that >>we  
> have the
> Form Widget and form defs fixed up to handle these better it should  
> be fine
> for most things, but chances are there are some >>links in FTL files  
> that
> will still be broken and will need to be manually fixed; with this  
> we can
> look forward to more issues and >>questions/comments on the mailing  
> list,
> but this also makes it a lot more secure and pretty difficult to  
> spoof one
> of these requests (will >>have to hack HTTPS and encrypt the body to  
> do so)
>
> Here you are saying that we may need to explicitly set the form  
> values in
> the FTL files. But how should we handle the case when we have  
> "Delete" &
> "Update" case on the list form that is build up using FTL file.
>
> For Ex :
> https://localhost:8443/catalog/control/EditCategoryParties?productCategoryId=CATALOG1
>
> I have created form something similar to the above link but I am  
> confused
> how should I handle this case in which we have "Update" & "Delete"  
> button.
>
> It can be a stupid question but I tried my best to solve the problem
> although didn't get success.
> Thanks in advance for any help !
>
> --
> Ashish Vijaywargiya
>
>
> On Sun, Apr 19, 2009 at 4:27 AM, David E Jones
> <[hidden email]>wrote:
>
>>
>> On Apr 18, 2009, at 4:48 AM, Jacques Le Roux wrote:
>>
>> At
>>> https://demo904.ofbiz.org/catalog/control/setUserPreference?userPrefGroupTypeId=GLOBAL_PREFERENCES&userPrefTypeId=COMPACT_HEADER&userPrefValue=Y
>>>
>>> I get see this message.
>>> Found URL parameter [userPrefTypeId] passed to secure (https)  
>>> request-map
>>> with uri [setUserPreference] with an event that calls service
>>> [setUserPreference]; this is not allowed for security reasons! The  
>>> data
>>> should be encrypted by making it part of the request body (a form  
>>> field)
>>> instead of the request URL.
>>>
>>
>> That particular is already fixed, and just hasn't updated yet.
>>
>> I thought we gave up with this message (or just have it only in  
>> log?).
>>>
>>
>> If a change had been made you would have seen it in the commit  
>> log... and
>> hopefully explicitly called out as such (it is unfortunate that we  
>> get so
>> many poorly written commit logs that don't even try to describe the  
>> changes
>> made...).
>>
>> But I was just thinking about that yesterday and I think that we  
>> should
>>> contunue to have it in trunk and not in 9.04. So we will be able  
>>> to catch
>>> them (before having a tool to list them all, I hope to work on  
>>> that next
>>> week) without disturbing 9.04 users
>>>
>>
>> The main point of that error is to protect against XSRF attacks.  
>> Without
>> that error and not allowing the condition it checks there is  
>> nothing keeping
>> spoofed parameters from piggy-backing on a cloned encrypted request  
>> (or
>> caught and modified through a man-in-the-middle sort of attack).
>>
>> Personally I'd rather see these fixed in both the release branch  
>> and in the
>> trunk, but if we get too many complaints about it in the release  
>> branch then
>> I'm totally fine disabling that constraint temporarily.
>>
>> -David
>>
>>

[Ashish Vijaywargiya-5]

Thanks David for your reply.
I am not worried about the nesting of one form with another.

I am thinking about the case when a form created in FTL has "Update" &
"Delete" link (As I have both the option in my custom form).
In your comment what do you mean by "but chances are there are some
links in FTL files that will still be broken and will need to be
manually fixed;" ?
Does it mean that we may need to pass the values as hidden using <input>
tag ?
If we do so then hidden values passed can be either used by "Update" or
"Delete".

Please take on the link I have referred in my previous email.

https://localhost:8443/catalog/control/EditCategoryParties?productCategoryId=CATALOG1

Here is the code snippet :
            <td align="center">
                <FORM method="post"
action="<@ofbizUrl>updatePartyToCategory</@ofbizUrl>"
name="lineForm${line}">
                    <#assign hasExpired = false>
                    <#if
(productCategoryRole.getTimestamp("thruDate"))?exists &&
(Static["org.ofbiz.base.util.UtilDateTime"].nowTimestamp().after(productCategoryRole.getTimestamp("thruDate")))>
<#assign hasExpired = true></#if>
                    <input type="hidden" name="productCategoryId"
value="${(productCategoryRole.productCategoryId)?if_exists}">
                    <input type="hidden" name="partyId"
value="${(productCategoryRole.partyId)?if_exists}">
                    <input type="hidden" name="roleTypeId"
value="${(productCategoryRole.roleTypeId)?if_exists}">
                    <input type="hidden" name="fromDate"
value="${(productCategoryRole.getTimestamp("fromDate"))?if_exists}">
                    <input type="text" size="25" name="thruDate"
value="${(productCategoryRole. getTimestamp("thruDate"))?if_exists}"
<#if hasExpired> style="color: red;"</#if>>
                    <a
href="javascript:call_cal(document.lineForm${line}.thruDate,
'${(productCategoryRole.getTimestamp("thruDate"))?default(nowTimestamp?string)}');"><img
src="<@ofbizContentUrl>/images/cal.gif</@ofbizContentUrl>" width="16"
height="16" border="0" alt="Calendar"></a>
                    <INPUT type="submit"
value="${uiLabelMap.CommonUpdate}" style="font-size: x-small;">
                </FORM>
            </td>
            <td align="center">
                <a
href="<@ofbizUrl>removePartyFromCategory?productCategoryId=${(productCategoryRole.productCategoryId)?if_exists}&partyId=${(productCategoryRole.partyId)?if_exists}&roleTypeId=${(productCategoryRole.roleTypeId)?if_exists}&fromDate=${productCategoryRole.getString("fromDate")}</@ofbizUrl>"
class="buttontext">${uiLabelMap.CommonDelete}</a>
            </td>
            </tr>


When I clicks on the Delete link then it is giving following error :

The Following Errors Occurred:

Error calling event: org.ofbiz.webapp.event.EventHandlerException: Found
URL parameter [productCategoryId] passed to secure (https) request-map
with uri [removePartyFromCategory] with an event that calls service
[removePartyFromCategory]; this is not allowed for security reasons! The
data should be encrypted by making it part of the request body (a form
field) instead of the request URL.


What should I do to solve this error ?
Thanks for your help in advance !

--
Ashish
 


David E Jones wrote:

>
> I'm not sure I followed all of that... are you worried about having a
> form nested within another form?
>
> Yes, the forms will have to be outside the main form, but the links to
> those forms can be inside the main form.
>
> -David
>
>
> On Apr 20, 2009, at 2:25 AM, Ashish Vijaywargiya wrote:
>
>> Hello David,
>>
>> Below is your comments from rev no. 757316.
>>
>>>> Changed back to throw an exception when a non-body parameter is
>>>> passed to
>> a secure request that calls a service as an event; now that >>we have
>> the
>> Form Widget and form defs fixed up to handle these better it should
>> be fine
>> for most things, but chances are there are some >>links in FTL files
>> that
>> will still be broken and will need to be manually fixed; with this we
>> can
>> look forward to more issues and >>questions/comments on the mailing
>> list,
>> but this also makes it a lot more secure and pretty difficult to
>> spoof one
>> of these requests (will >>have to hack HTTPS and encrypt the body to
>> do so)
>>
>> Here you are saying that we may need to explicitly set the form
>> values in
>> the FTL files. But how should we handle the case when we have "Delete" &
>> "Update" case on the list form that is build up using FTL file.
>>
>> For Ex :
>> https://localhost:8443/catalog/control/EditCategoryParties?productCategoryId=CATALOG1 
>>
>>
>> I have created form something similar to the above link but I am
>> confused
>> how should I handle this case in which we have "Update" & "Delete"
>> button.
>>
>> It can be a stupid question but I tried my best to solve the problem
>> although didn't get success.
>> Thanks in advance for any help !
>>
>> --
>> Ashish Vijaywargiya
>>
>>
>> On Sun, Apr 19, 2009 at 4:27 AM, David E Jones
>> <[hidden email]>wrote:
>>
>>>
>>> On Apr 18, 2009, at 4:48 AM, Jacques Le Roux wrote:
>>>
>>> At
>>>> https://demo904.ofbiz.org/catalog/control/setUserPreference?userPrefGroupTypeId=GLOBAL_PREFERENCES&userPrefTypeId=COMPACT_HEADER&userPrefValue=Y 
>>>>
>>>>
>>>> I get see this message.
>>>> Found URL parameter [userPrefTypeId] passed to secure (https)
>>>> request-map
>>>> with uri [setUserPreference] with an event that calls service
>>>> [setUserPreference]; this is not allowed for security reasons! The
>>>> data
>>>> should be encrypted by making it part of the request body (a form
>>>> field)
>>>> instead of the request URL.
>>>>
>>>
>>> That particular is already fixed, and just hasn't updated yet.
>>>
>>> I thought we gave up with this message (or just have it only in log?).
>>>>
>>>
>>> If a change had been made you would have seen it in the commit
>>> log... and
>>> hopefully explicitly called out as such (it is unfortunate that we
>>> get so
>>> many poorly written commit logs that don't even try to describe the
>>> changes
>>> made...).
>>>
>>> But I was just thinking about that yesterday and I think that we should
>>>> contunue to have it in trunk and not in 9.04. So we will be able to
>>>> catch
>>>> them (before having a tool to list them all, I hope to work on that
>>>> next
>>>> week) without disturbing 9.04 users
>>>>
>>>
>>> The main point of that error is to protect against XSRF attacks.
>>> Without
>>> that error and not allowing the condition it checks there is nothing
>>> keeping
>>> spoofed parameters from piggy-backing on a cloned encrypted request (or
>>> caught and modified through a man-in-the-middle sort of attack).
>>>
>>> Personally I'd rather see these fixed in both the release branch and
>>> in the
>>> trunk, but if we get too many complaints about it in the release
>>> branch then
>>> I'm totally fine disabling that constraint temporarily.
>>>
>>> -David
>>>
>>>
>

[Ashish Vijaywargiya-5]

Another reference that is throwing error while clicking Delete link.
https://localhost:8443/catalog/control/EditCategoryProdCatalogs?productCategoryId=CATALOG1

Error details :

The Following Errors Occurred:

Error calling event: org.ofbiz.webapp.event.EventHandlerException: Found
URL parameter [prodCatalogId] passed to secure (https) request-map with
uri [category_removeProductCategoryFromProdCatalog] with an event that
calls service [removeProductCategoryFromProdCatalog]; this is not
allowed for security reasons! The data should be encrypted by making it
part of the request body (a form field) instead of the request URL.

--
Ashish


David E Jones wrote:

>
> I'm not sure I followed all of that... are you worried about having a
> form nested within another form?
>
> Yes, the forms will have to be outside the main form, but the links to
> those forms can be inside the main form.
>
> -David
>
>
> On Apr 20, 2009, at 2:25 AM, Ashish Vijaywargiya wrote:
>
>> Hello David,
>>
>

[David E Jones-3]

In this case I don't see any relationship between the  
updatePartyToCategory form and the removePartyFromCategory link.

The simple fact is that all links that call services must be changed  
to forms so they can pass parameters as hidden input fields so they  
are encrypted and therefore difficult to spoof.

Basically, the removePartyFromCategory link needs to be changed into a  
form, and that's all the error message is about.

-David


On Apr 20, 2009, at 11:15 PM, Ashish Vijaywargiya wrote:

> Thanks David for your reply.
> I am not worried about the nesting of one form with another.
>
> I am thinking about the case when a form created in FTL has "Update"  
> & "Delete" link (As I have both the option in my custom form).
> In your comment what do you mean by "but chances are there are some  
> links in FTL files that will still be broken and will need to be  
> manually fixed;" ?
> Does it mean that we may need to pass the values as hidden using  
> <input> tag ?
> If we do so then hidden values passed can be either used by "Update"  
> or "Delete".
>
> Please take on the link I have referred in my previous email.
>
> https://localhost:8443/catalog/control/EditCategoryParties?productCategoryId=CATALOG1
>
> Here is the code snippet :
>           <td align="center">
>               <FORM method="post"  
> action="<@ofbizUrl>updatePartyToCategory</@ofbizUrl>" name="lineForm$
> {line}">
>                   <#assign hasExpired = false>
>                   <#if  
> (productCategoryRole.getTimestamp("thruDate"))?exists &&  
> (Static
> ["org
> .ofbiz
> .base
> .util
> .UtilDateTime
> "].nowTimestamp
> ().after(productCategoryRole.getTimestamp("thruDate")))> <#assign  
> hasExpired = true></#if>
>                   <input type="hidden" name="productCategoryId"  
> value="${(productCategoryRole.productCategoryId)?if_exists}">
>                   <input type="hidden" name="partyId" value="$
> {(productCategoryRole.partyId)?if_exists}">
>                   <input type="hidden" name="roleTypeId" value="$
> {(productCategoryRole.roleTypeId)?if_exists}">
>                   <input type="hidden" name="fromDate" value="$
> {(productCategoryRole.getTimestamp("fromDate"))?if_exists}">
>                   <input type="text" size="25" name="thruDate"  
> value="${(productCategoryRole. getTimestamp("thruDate"))?if_exists}"  
> <#if hasExpired> style="color: red;"</#if>>
>                   <a href="javascript:call_cal(document.lineForm$
> {line}.thruDate, '${(productCategoryRole.getTimestamp("thruDate"))?
> default(nowTimestamp?string)}');"><img src="<@ofbizContentUrl>/
> images/cal.gif</@ofbizContentUrl>" width="16" height="16" border="0"  
> alt="Calendar"></a>
>                   <INPUT type="submit" value="$
> {uiLabelMap.CommonUpdate}" style="font-size: x-small;">
>               </FORM>
>           </td>
>           <td align="center">
>               <a href="<@ofbizUrl>removePartyFromCategory?
> productCategoryId=${(productCategoryRole.productCategoryId)?
> if_exists}&partyId=${(productCategoryRole.partyId)?
> if_exists}&roleTypeId=${(productCategoryRole.roleTypeId)?
> if_exists}&fromDate=${productCategoryRole.getString("fromDate")}</
> @ofbizUrl>" class="buttontext">${uiLabelMap.CommonDelete}</a>
>           </td>
>           </tr>
>
>
> When I clicks on the Delete link then it is giving following error :
>
> The Following Errors Occurred:
>
> Error calling event: org.ofbiz.webapp.event.EventHandlerException:  
> Found URL parameter [productCategoryId] passed to secure (https)  
> request-map with uri [removePartyFromCategory] with an event that  
> calls service [removePartyFromCategory]; this is not allowed for  
> security reasons! The data should be encrypted by making it part of  
> the request body (a form field) instead of the request URL.
>
>
> What should I do to solve this error ?
> Thanks for your help in advance !
>
> --
> Ashish
>
>
> David E Jones wrote:
>>
>> I'm not sure I followed all of that... are you worried about having  
>> a form nested within another form?
>>
>> Yes, the forms will have to be outside the main form, but the links  
>> to those forms can be inside the main form.
>>
>> -David
>>
>>
>> On Apr 20, 2009, at 2:25 AM, Ashish Vijaywargiya wrote:
>>
>>> Hello David,
>>>
>>> Below is your comments from rev no. 757316.
>>>
>>>>> Changed back to throw an exception when a non-body parameter is  
>>>>> passed to
>>> a secure request that calls a service as an event; now that >>we  
>>> have the
>>> Form Widget and form defs fixed up to handle these better it  
>>> should be fine
>>> for most things, but chances are there are some >>links in FTL  
>>> files that
>>> will still be broken and will need to be manually fixed; with this  
>>> we can
>>> look forward to more issues and >>questions/comments on the  
>>> mailing list,
>>> but this also makes it a lot more secure and pretty difficult to  
>>> spoof one
>>> of these requests (will >>have to hack HTTPS and encrypt the body  
>>> to do so)
>>>
>>> Here you are saying that we may need to explicitly set the form  
>>> values in
>>> the FTL files. But how should we handle the case when we have  
>>> "Delete" &
>>> "Update" case on the list form that is build up using FTL file.
>>>
>>> For Ex :
>>> https://localhost:8443/catalog/control/EditCategoryParties?productCategoryId=CATALOG1
>>>
>>> I have created form something similar to the above link but I am  
>>> confused
>>> how should I handle this case in which we have "Update" & "Delete"  
>>> button.
>>>
>>> It can be a stupid question but I tried my best to solve the problem
>>> although didn't get success.
>>> Thanks in advance for any help !
>>>
>>> --
>>> Ashish Vijaywargiya
>>>
>>>
>>> On Sun, Apr 19, 2009 at 4:27 AM, David E Jones
>>> <[hidden email]>wrote:
>>>
>>>>
>>>> On Apr 18, 2009, at 4:48 AM, Jacques Le Roux wrote:
>>>>
>>>> At
>>>>> https://demo904.ofbiz.org/catalog/control/setUserPreference?userPrefGroupTypeId=GLOBAL_PREFERENCES&userPrefTypeId=COMPACT_HEADER&userPrefValue=Y
>>>>>
>>>>> I get see this message.
>>>>> Found URL parameter [userPrefTypeId] passed to secure (https)  
>>>>> request-map
>>>>> with uri [setUserPreference] with an event that calls service
>>>>> [setUserPreference]; this is not allowed for security reasons!  
>>>>> The data
>>>>> should be encrypted by making it part of the request body (a  
>>>>> form field)
>>>>> instead of the request URL.
>>>>>
>>>>
>>>> That particular is already fixed, and just hasn't updated yet.
>>>>
>>>> I thought we gave up with this message (or just have it only in  
>>>> log?).
>>>>>
>>>>
>>>> If a change had been made you would have seen it in the commit  
>>>> log... and
>>>> hopefully explicitly called out as such (it is unfortunate that  
>>>> we get so
>>>> many poorly written commit logs that don't even try to describe  
>>>> the changes
>>>> made...).
>>>>
>>>> But I was just thinking about that yesterday and I think that we  
>>>> should
>>>>> contunue to have it in trunk and not in 9.04. So we will be able  
>>>>> to catch
>>>>> them (before having a tool to list them all, I hope to work on  
>>>>> that next
>>>>> week) without disturbing 9.04 users
>>>>>
>>>>
>>>> The main point of that error is to protect against XSRF attacks.  
>>>> Without
>>>> that error and not allowing the condition it checks there is  
>>>> nothing keeping
>>>> spoofed parameters from piggy-backing on a cloned encrypted  
>>>> request (or
>>>> caught and modified through a man-in-the-middle sort of attack).
>>>>
>>>> Personally I'd rather see these fixed in both the release branch  
>>>> and in the
>>>> trunk, but if we get too many complaints about it in the release  
>>>> branch then
>>>> I'm totally fine disabling that constraint temporarily.
>>>>
>>>> -David
>>>>
>>>>
>>

[Ashish Vijaywargiya]

Thanks David for your comment.
I have committed a fix in trunk rev. 767072.

Please review it and comment on it if I did something wrong.
--
Ashish

On Tue, Apr 21, 2009 at 1:28 PM, David E Jones
<[hidden email]>wrote:

>
> In this case I don't see any relationship between the updatePartyToCategory
> form and the removePartyFromCategory link.
>
> The simple fact is that all links that call services must be changed to
> forms so they can pass parameters as hidden input fields so they are
> encrypted and therefore difficult to spoof.
>
> Basically, the removePartyFromCategory link needs to be changed into a
> form, and that's all the error message is about.
>
> -David
>
>

[Jacques Le Roux]

Thanks Ashish

I made a small change at r767093 since there may be several lines
Please refer to https://issues.apache.org/jira/browse/OFBIZ-2260 if you find others
I will have a look at the other one uio reported right now

Jacques

From: "Ashish Vijaywargiya" <[hidden email]>

> Thanks David for your comment.
> I have committed a fix in trunk rev. 767072.
>
> Please review it and comment on it if I did something wrong.
> --
> Ashish
>
> On Tue, Apr 21, 2009 at 1:28 PM, David E Jones
> <[hidden email]>wrote:
>
>>
>> In this case I don't see any relationship between the updatePartyToCategory
>> form and the removePartyFromCategory link.
>>
>> The simple fact is that all links that call services must be changed to
>> forms so they can pass parameters as hidden input fields so they are
>> encrypted and therefore difficult to spoof.
>>
>> Basically, the removePartyFromCategory link needs to be changed into a
>> form, and that's all the error message is about.
>>
>> -David
>>
>>
>

[Jacques Le Roux]

Oops, fixed a typo at r767099

Jacques

From: "Jacques Le Roux" <[hidden email]>

> Thanks Ashish
>
> I made a small change at r767093 since there may be several lines
> Please refer to https://issues.apache.org/jira/browse/OFBIZ-2260 if you find others
> I will have a look at the other one uio reported right now
>
> Jacques
>
> From: "Ashish Vijaywargiya" <[hidden email]>
>> Thanks David for your comment.
>> I have committed a fix in trunk rev. 767072.
>>
>> Please review it and comment on it if I did something wrong.
>> --
>> Ashish
>>
>> On Tue, Apr 21, 2009 at 1:28 PM, David E Jones
>> <[hidden email]>wrote:
>>
>>>
>>> In this case I don't see any relationship between the updatePartyToCategory
>>> form and the removePartyFromCategory link.
>>>
>>> The simple fact is that all links that call services must be changed to
>>> forms so they can pass parameters as hidden input fields so they are
>>> encrypted and therefore difficult to spoof.
>>>
>>> Basically, the removePartyFromCategory link needs to be changed into a
>>> form, and that's all the error message is about.
>>>
>>> -David
>>>
>>>
>>
>

[Ashish Vijaywargiya]

>> Oops, fixed a typo at r767099
Example of Super Fast Working style. ;o)
Thanks Jacques for your quick turn around on this.

--
Ashish


On Tue, Apr 21, 2009 at 3:25 PM, Jacques Le Roux <
[hidden email]> wrote:

> Oops, fixed a typo at r767099
>
> Jacques
>
>

[Jacques Le Roux]

Fiexd at r767102

Jacques

From: "Ashish Vijaywargiya" <[hidden email]>

> Thanks David for your reply.
> I am not worried about the nesting of one form with another.
>
> I am thinking about the case when a form created in FTL has "Update" & "Delete" link (As I have both the option in my custom
> form).
> In your comment what do you mean by "but chances are there are some links in FTL files that will still be broken and will need to
> be manually fixed;" ?
> Does it mean that we may need to pass the values as hidden using <input> tag ?
> If we do so then hidden values passed can be either used by "Update" or "Delete".
>
> Please take on the link I have referred in my previous email.
>
> https://localhost:8443/catalog/control/EditCategoryParties?productCategoryId=CATALOG1
>
> Here is the code snippet :
>            <td align="center">
>                <FORM method="post" action="<@ofbizUrl>updatePartyToCategory</@ofbizUrl>" name="lineForm${line}">
>                    <#assign hasExpired = false>
>                    <#if (productCategoryRole.getTimestamp("thruDate"))?exists &&
> (Static["org.ofbiz.base.util.UtilDateTime"].nowTimestamp().after(productCategoryRole.getTimestamp("thruDate")))> <#assign
> hasExpired = true></#if>
>                    <input type="hidden" name="productCategoryId" value="${(productCategoryRole.productCategoryId)?if_exists}">
>                    <input type="hidden" name="partyId" value="${(productCategoryRole.partyId)?if_exists}">
>                    <input type="hidden" name="roleTypeId" value="${(productCategoryRole.roleTypeId)?if_exists}">
>                    <input type="hidden" name="fromDate" value="${(productCategoryRole.getTimestamp("fromDate"))?if_exists}">
>                    <input type="text" size="25" name="thruDate" value="${(productCategoryRole.
> getTimestamp("thruDate"))?if_exists}" <#if hasExpired> style="color: red;"</#if>>
>                    <a href="javascript:call_cal(document.lineForm${line}.thruDate,
> '${(productCategoryRole.getTimestamp("thruDate"))?default(nowTimestamp?string)}');"><img
> src="<@ofbizContentUrl>/images/cal.gif</@ofbizContentUrl>" width="16" height="16" border="0" alt="Calendar"></a>
>                    <INPUT type="submit" value="${uiLabelMap.CommonUpdate}" style="font-size: x-small;">
>                </FORM>
>            </td>
>            <td align="center">
>                <a
> href="<@ofbizUrl>removePartyFromCategory?productCategoryId=${(productCategoryRole.productCategoryId)?if_exists}&partyId=${(productCategoryRole.partyId)?if_exists}&roleTypeId=${(productCategoryRole.roleTypeId)?if_exists}&fromDate=${productCategoryRole.getString("fromDate")}</@ofbizUrl>"
> class="buttontext">${uiLabelMap.CommonDelete}</a>
>            </td>
>            </tr>
>
>
> When I clicks on the Delete link then it is giving following error :
>
> The Following Errors Occurred:
>
> Error calling event: org.ofbiz.webapp.event.EventHandlerException: Found URL parameter [productCategoryId] passed to secure
> (https) request-map with uri [removePartyFromCategory] with an event that calls service [removePartyFromCategory]; this is not
> allowed for security reasons! The data should be encrypted by making it part of the request body (a form field) instead of the
> request URL.
>
>
> What should I do to solve this error ?
> Thanks for your help in advance !
>
> --
> Ashish
>
>
>
> David E Jones wrote:
>>
>> I'm not sure I followed all of that... are you worried about having a form nested within another form?
>>
>> Yes, the forms will have to be outside the main form, but the links to those forms can be inside the main form.
>>
>> -David
>>
>>
>> On Apr 20, 2009, at 2:25 AM, Ashish Vijaywargiya wrote:
>>
>>> Hello David,
>>>
>>> Below is your comments from rev no. 757316.
>>>
>>>>> Changed back to throw an exception when a non-body parameter is passed to
>>> a secure request that calls a service as an event; now that >>we have the
>>> Form Widget and form defs fixed up to handle these better it should be fine
>>> for most things, but chances are there are some >>links in FTL files that
>>> will still be broken and will need to be manually fixed; with this we can
>>> look forward to more issues and >>questions/comments on the mailing list,
>>> but this also makes it a lot more secure and pretty difficult to spoof one
>>> of these requests (will >>have to hack HTTPS and encrypt the body to do so)
>>>
>>> Here you are saying that we may need to explicitly set the form values in
>>> the FTL files. But how should we handle the case when we have "Delete" &
>>> "Update" case on the list form that is build up using FTL file.
>>>
>>> For Ex :
>>> https://localhost:8443/catalog/control/EditCategoryParties?productCategoryId=CATALOG1
>>>
>>> I have created form something similar to the above link but I am confused
>>> how should I handle this case in which we have "Update" & "Delete" button.
>>>
>>> It can be a stupid question but I tried my best to solve the problem
>>> although didn't get success.
>>> Thanks in advance for any help !
>>>
>>> --
>>> Ashish Vijaywargiya
>>>
>>>
>>> On Sun, Apr 19, 2009 at 4:27 AM, David E Jones
>>> <[hidden email]>wrote:
>>>
>>>>
>>>> On Apr 18, 2009, at 4:48 AM, Jacques Le Roux wrote:
>>>>
>>>> At
>>>>> https://demo904.ofbiz.org/catalog/control/setUserPreference?userPrefGroupTypeId=GLOBAL_PREFERENCES&userPrefTypeId=COMPACT_HEADER&userPrefValue=Y
>>>>>
>>>>> I get see this message.
>>>>> Found URL parameter [userPrefTypeId] passed to secure (https) request-map
>>>>> with uri [setUserPreference] with an event that calls service
>>>>> [setUserPreference]; this is not allowed for security reasons! The data
>>>>> should be encrypted by making it part of the request body (a form field)
>>>>> instead of the request URL.
>>>>>
>>>>
>>>> That particular is already fixed, and just hasn't updated yet.
>>>>
>>>> I thought we gave up with this message (or just have it only in log?).
>>>>>
>>>>
>>>> If a change had been made you would have seen it in the commit log... and
>>>> hopefully explicitly called out as such (it is unfortunate that we get so
>>>> many poorly written commit logs that don't even try to describe the changes
>>>> made...).
>>>>
>>>> But I was just thinking about that yesterday and I think that we should
>>>>> contunue to have it in trunk and not in 9.04. So we will be able to catch
>>>>> them (before having a tool to list them all, I hope to work on that next
>>>>> week) without disturbing 9.04 users
>>>>>
>>>>
>>>> The main point of that error is to protect against XSRF attacks. Without
>>>> that error and not allowing the condition it checks there is nothing keeping
>>>> spoofed parameters from piggy-backing on a cloned encrypted request (or
>>>> caught and modified through a man-in-the-middle sort of attack).
>>>>
>>>> Personally I'd rather see these fixed in both the release branch and in the
>>>> trunk, but if we get too many complaints about it in the release branch then
>>>> I'm totally fine disabling that constraint temporarily.
>>>>
>>>> -David
>>>>
>>>>
>>
>

[Jacques Le Roux]

Actually too fast, as ever  ;o)

Jacques

From: "Ashish Vijaywargiya" <[hidden email]>

>>> Oops, fixed a typo at r767099
> Example of Super Fast Working style. ;o)
> Thanks Jacques for your quick turn around on this.
>
> --
> Ashish
>
>
> On Tue, Apr 21, 2009 at 3:25 PM, Jacques Le Roux <
> [hidden email]> wrote:
>
>> Oops, fixed a typo at r767099
>>
>> Jacques
>>
>>
>

[Ashish Vijaywargiya]

r767102 - Thanks for fixing this up Jacques.

>> Actually too fast, as ever  ;o)

;o) - I remembered that you always commits first by saying "French
Translation" and after few minutes(5 or 10) again you commit the code by
saying "Better French Translation".  ;o) . This is one of the common
practice that you do and I noticed this in your commits :)

Thanks !

--
Ashish


On Tue, Apr 21, 2009 at 3:49 PM, Jacques Le Roux <
[hidden email]> wrote:

> Actually too fast, as ever  ;o)
>
> Jacques
>
> From: "Ashish Vijaywargiya" <[hidden email]>
>
>> Oops, fixed a typo at r767099
>>>>
>>> Example of Super Fast Working style. ;o)
>> Thanks Jacques for your quick turn around on this.
>>
>> --
>> Ashish
>>
>>
>>

[Jacques Le Roux]

Missed one other thing, fixed at 762757

Jacques

From: "Ashish Vijaywargiya" <[hidden email]>

> r767102 - Thanks for fixing this up Jacques.
>
>>> Actually too fast, as ever  ;o)
>
> ;o) - I remembered that you always commits first by saying "French
> Translation" and after few minutes(5 or 10) again you commit the code by
> saying "Better French Translation".  ;o) . This is one of the common
> practice that you do and I noticed this in your commits :)
>
> Thanks !
>
> --
> Ashish
>
>
> On Tue, Apr 21, 2009 at 3:49 PM, Jacques Le Roux <
> [hidden email]> wrote:
>
>> Actually too fast, as ever  ;o)
>>
>> Jacques
>>
>> From: "Ashish Vijaywargiya" <[hidden email]>
>>
>>> Oops, fixed a typo at r767099
>>>>>
>>>> Example of Super Fast Working style. ;o)
>>> Thanks Jacques for your quick turn around on this.
>>>
>>> --
>>> Ashish
>>>
>>>
>>>
>