You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@ofbiz.apache.org by Jacopo Cappellato <ti...@sastau.it> on 2007/01/25 15:41:51 UTC
Possible auth issue in the createPartyAndUserLogin service
Hi all,
could you have a look at the attached simple patch? It fixes an
authorization problem under some special situations (switching from
anonymous user to authenticated one...); however the issue is that, when
the "createUserLogin" is called, if the attribute include-user-login is
not set to false the manually passed in "system" user is overwritten by
the user in the context.
Should I commit this patch?
Or, in general, would be better, even if include-user-login is true, to
set the user login only if one is not already there in the service in map?
Jacopo
Re: Possible auth issue in the createPartyAndUserLogin service
Posted by Jacques Le Roux <ja...@les7arts.com>.
Hi Jacopo,
Does not this may depend of context ? Or otherwise said is this functionnality (overidding the curent user-login) not wanted in some
circumstances ?
Sorry only questions :o/
Jacques
----- Original Message -----
From: "Jacopo Cappellato" <ti...@sastau.it>
To: <de...@ofbiz.apache.org>
Sent: Thursday, January 25, 2007 3:41 PM
Subject: Possible auth issue in the createPartyAndUserLogin service
> Hi all,
>
> could you have a look at the attached simple patch? It fixes an
> authorization problem under some special situations (switching from
> anonymous user to authenticated one...); however the issue is that, when
> the "createUserLogin" is called, if the attribute include-user-login is
> not set to false the manually passed in "system" user is overwritten by
> the user in the context.
> Should I commit this patch?
> Or, in general, would be better, even if include-user-login is true, to
> set the user login only if one is not already there in the service in map?
>
> Jacopo
>
--------------------------------------------------------------------------------
> Index: applications/party/script/org/ofbiz/party/party/PartySimpleMethods.xml
> ===================================================================
> --- applications/party/script/org/ofbiz/party/party/PartySimpleMethods.xml (revisione 499802)
> +++ applications/party/script/org/ofbiz/party/party/PartySimpleMethods.xml (copia locale)
> @@ -85,7 +85,7 @@
> <field-map field-name="userLoginId" value="system"/>
> </entity-one>
>
> - <call-service service-name="createUserLogin" in-map-name="createUlInMap"/>
> + <call-service service-name="createUserLogin" in-map-name="createUlInMap" include-user-login="false"/>
> <entity-one entity-name="UserLogin" value-name="newUserLogin"/>
>
> <field-to-result field-name="newUserLogin"/>
>
Re: Possible auth issue in the createPartyAndUserLogin service
Posted by Jacopo Cappellato <ti...@sastau.it>.
Great,
I've committed it in rev. 505482
Thanks,
Jacopo
David E. Jones wrote:
>
> I don't think there will be any problem from such a side effect. It's
> good that you brought it up, but I'd be really surprised if there was
> code anywhere that depended on this quirk, as it is a bit funny in the
> first place, and most services (well, simple-methods anyway) don't do a
> lot with explicit user login passing around.
>
> -David
>
>
> On Feb 7, 2007, at 8:47 PM, Jacopo Cappellato wrote:
>
>> David,
>>
>> I can actually commit the patch #2 without problem; I'm just worried
>> about the side effects (if any) that this could cause on the existing
>> code. For example, I don't know if the set-service-fields operation is
>> copying also the user login from one map to another one; if this is
>> happening, then (with the existing code) the user login is overriden
>> by the one of the calling service; if I apply my patch this will be no
>> more true... could this cause problems in your opinion? Or am I just
>> too anxious?
>>
>> Thanks for your feedback,
>>
>> Jacopo
>>
>>
>>
>> David E. Jones wrote:
>>> Sorry for the delay on this Jacopo.
>>> Looking at this I actually like the fix in #2, ie changing the
>>> simple-method operations because what they do doesn't make sense. In
>>> other words, if you add a userLogin yourself you wouldn't expect the
>>> system to blow it away.
>>> If you agree feel free to commit that, your patch of the CallService
>>> and CallServiceAsynch classes looks just fine. If I don't see your
>>> response on it I'll throw that in later this evening or tomorrow.
>>> -David
>>> On Feb 7, 2007, at 10:38 AM, Jacopo Cappellato wrote:
>>>> Ah,
>>>>
>>>> forgot to mention that I'd vote for solution #1.
>>>>
>>>> Jacopo
>>>>
>>>> Jacopo Cappellato wrote:
>>>>> Should we take some action about this subject?
>>>>> I know that probably my explanations were not clear enough, so I'm
>>>>> proposing the two ways of fixing what I think it's an outstanding
>>>>> issue; I hope that, reviewing the (extremely simple) fixes I'm
>>>>> proposing will be easier to understand the issue I'm trying to report.
>>>>> Solution #1: just fix the service call to "createUserLogin" inside
>>>>> of createPersonAndUserLogin by adding the
>>>>> include-user-login="false"; in this way the manually "system" user,
>>>>> manually put in the input context will be not overriden by the user
>>>>> available in the context of the calling service
>>>>> ("createPersonAndUserLogin"); see fix1.patch
>>>>> Solution #2: fix for the minilang service call methods; if
>>>>> include-user-login is not set or it is true, the user login is put
>>>>> in the context but *only* if in the context there is not already a
>>>>> user login; see fix2.patch
>>>>> I really think that one of the two solutions should be committed.
>>>>> What do you think?
>>>>> Jacopo
>>>>> Jacopo Cappellato wrote:
>>>>>> David,
>>>>>>
>>>>>> I really did not describe it very well, sorry.
>>>>>> I don't think there is a problem with the createUserLogin service
>>>>>> definition or implementation.
>>>>>> The issue (maybe) could be the way the framework handles the
>>>>>> minilang attribute include-user-login="true": it just gets the
>>>>>> "userLogin" object from the context of the calling service and put
>>>>>> it in the context of the called service, even if in the context of
>>>>>> the called service there is already a "userLogin" object.
>>>>>> So for example the code in the createPersonAndUserLogin service
>>>>>> (for which I have provided the patch) is:
>>>>>>
>>>>>> <!-- call the service with the system account to get around
>>>>>> security constraints for this special create -->
>>>>>> <entity-one entity-name="UserLogin"
>>>>>> value-name="createUlInMap.userLogin">
>>>>>> <field-map field-name="userLoginId" value="system"/>
>>>>>> </entity-one>
>>>>>>
>>>>>> <call-service service-name="createUserLogin"
>>>>>> in-map-name="createUlInMap" include-user-login="false"/>
>>>>>>
>>>>>> Unfotunately the "system" userLogin object that is manually put in
>>>>>> the input context for the createUserLogin service is overriden (by
>>>>>> the framework), if one is available in the context of the
>>>>>> createPersonAndUserLogin service because the include-user-login
>>>>>> attribute defaults to true.
>>>>>>
>>>>>> Just adding the include-user-login="false" attribute to the
>>>>>> service call fixed the issue.
>>>>>>
>>>>>> Does it make sense?
>>>>>>
>>>>>> Jacopo
>>>>>>
>>>>>>
>>>>>>
>>>>>> David E. Jones wrote:
>>>>>>>
>>>>>>> On Jan 25, 2007, at 7:41 AM, Jacopo Cappellato wrote:
>>>>>>>
>>>>>>>> Hi all,
>>>>>>>>
>>>>>>>> could you have a look at the attached simple patch? It fixes an
>>>>>>>> authorization problem under some special situations (switching
>>>>>>>> from anonymous user to authenticated one...); however the issue
>>>>>>>> is that, when the "createUserLogin" is called, if the attribute
>>>>>>>> include-user-login is not set to false the manually passed in
>>>>>>>> "system" user is overwritten by the user in the context.
>>>>>>>
>>>>>>> I just traced through the code to try to figure out why this
>>>>>>> might be happening, and either I don't understand what you're
>>>>>>> describing, or I just can't find why this would happen.
>>>>>>>
>>>>>>> In the implementation of the service
>>>>>>> (LoginServices.java:createUserLogin) I don't see anything that
>>>>>>> would do this. In the service definition I don't see it either
>>>>>>> because the service definition does not have a "userLogin"
>>>>>>> parameter going out.
>>>>>>>
>>>>>>> Have you traced down the code that would be putting the user in
>>>>>>> the context? Also, do you mean by this the context of the current
>>>>>>> service call (ie for other services called by ECA rules, etc), or
>>>>>>> in the result (returned) Map of the service, or somewhere else?
>>>>>>>
>>>>>>> I apologize for pushing on the detail, I may just be too
>>>>>>> brain-dead to see it right now.
>>>>>>>
>>>>>>> -David
>>>>>>>
>>>>>>>
>>>>>>>> Should I commit this patch?
>>>>>>>> Or, in general, would be better, even if include-user-login is
>>>>>>>> true, to set the user login only if one is not already there in
>>>>>>>> the service in map?
>>>>>>>>
>>>>>>>> Jacopo
>>>>>>>> Index:
>>>>>>>> applications/party/script/org/ofbiz/party/party/PartySimpleMethods.xml
>>>>>>>>
>>>>>>>> ===================================================================
>>>>>>>> ---
>>>>>>>> applications/party/script/org/ofbiz/party/party/PartySimpleMethods.xml
>>>>>>>> (revisione 499802)
>>>>>>>> +++
>>>>>>>> applications/party/script/org/ofbiz/party/party/PartySimpleMethods.xml
>>>>>>>> (copia locale)
>>>>>>>> @@ -85,7 +85,7 @@
>>>>>>>> <field-map field-name="userLoginId" value="system"/>
>>>>>>>> </entity-one>
>>>>>>>>
>>>>>>>> - <call-service service-name="createUserLogin"
>>>>>>>> in-map-name="createUlInMap"/>
>>>>>>>> + <call-service service-name="createUserLogin"
>>>>>>>> in-map-name="createUlInMap" include-user-login="false"/>
>>>>>>>> <entity-one entity-name="UserLogin"
>>>>>>>> value-name="newUserLogin"/>
>>>>>>>>
>>>>>>>> <field-to-result field-name="newUserLogin"/>
>>>>>>>
>>>>>>
>>>>> ------------------------------------------------------------------------
>>>>>
>>>>> Index:
>>>>> applications/party/script/org/ofbiz/party/party/PartySimpleMethods.xml
>>>>> ===================================================================
>>>>> ---
>>>>> applications/party/script/org/ofbiz/party/party/PartySimpleMethods.xml
>>>>> (revisione 503593)
>>>>> +++
>>>>> applications/party/script/org/ofbiz/party/party/PartySimpleMethods.xml
>>>>> (copia locale)
>>>>> @@ -85,7 +85,7 @@
>>>>> <field-map field-name="userLoginId" value="system"/>
>>>>> </entity-one>
>>>>> - <call-service service-name="createUserLogin"
>>>>> in-map-name="createUlInMap"/>
>>>>> + <call-service service-name="createUserLogin"
>>>>> in-map-name="createUlInMap" include-user-login="false"/>
>>>>> <entity-one entity-name="UserLogin"
>>>>> value-name="newUserLogin"/>
>>>>> <field-to-result field-name="newUserLogin"/>
>>>>> ------------------------------------------------------------------------
>>>>>
>>>>> Index:
>>>>> framework/minilang/src/org/ofbiz/minilang/method/callops/CallServiceAsynch.java
>>>>>
>>>>> ===================================================================
>>>>> ---
>>>>> framework/minilang/src/org/ofbiz/minilang/method/callops/CallServiceAsynch.java
>>>>> (revisione 503593)
>>>>> +++
>>>>> framework/minilang/src/org/ofbiz/minilang/method/callops/CallServiceAsynch.java
>>>>> (copia locale)
>>>>> @@ -68,8 +68,9 @@
>>>>> if (includeUserLogin) {
>>>>> GenericValue userLogin = methodContext.getUserLogin();
>>>>> - if (userLogin != null)
>>>>> + if (userLogin != null && inMap.get("userLogin") ==
>>>>> null) {
>>>>> inMap.put("userLogin", userLogin);
>>>>> + }
>>>>> }
>>>>> // always add Locale to context unless null
>>>>> Index:
>>>>> framework/minilang/src/org/ofbiz/minilang/method/callops/CallService.java
>>>>>
>>>>> ===================================================================
>>>>> ---
>>>>> framework/minilang/src/org/ofbiz/minilang/method/callops/CallService.java
>>>>> (revisione 503593)
>>>>> +++
>>>>> framework/minilang/src/org/ofbiz/minilang/method/callops/CallService.java
>>>>> (copia locale)
>>>>> @@ -213,7 +213,7 @@
>>>>> if (includeUserLogin) {
>>>>> GenericValue userLogin = methodContext.getUserLogin();
>>>>> - if (userLogin != null) {
>>>>> + if (userLogin != null && inMap.get("userLogin") ==
>>>>> null) {
>>>>> inMap.put("userLogin", userLogin);
>>>>> }
>>>>> }
>>>>
>>>>
>>
>>
>
Re: Possible auth issue in the createPartyAndUserLogin service
Posted by "David E. Jones" <jo...@hotwaxmedia.com>.
I don't think there will be any problem from such a side effect. It's
good that you brought it up, but I'd be really surprised if there was
code anywhere that depended on this quirk, as it is a bit funny in
the first place, and most services (well, simple-methods anyway)
don't do a lot with explicit user login passing around.
-David
On Feb 7, 2007, at 8:47 PM, Jacopo Cappellato wrote:
> David,
>
> I can actually commit the patch #2 without problem; I'm just
> worried about the side effects (if any) that this could cause on
> the existing code. For example, I don't know if the set-service-
> fields operation is copying also the user login from one map to
> another one; if this is happening, then (with the existing code)
> the user login is overriden by the one of the calling service; if I
> apply my patch this will be no more true... could this cause
> problems in your opinion? Or am I just too anxious?
>
> Thanks for your feedback,
>
> Jacopo
>
>
>
> David E. Jones wrote:
>> Sorry for the delay on this Jacopo.
>> Looking at this I actually like the fix in #2, ie changing the
>> simple-method operations because what they do doesn't make sense.
>> In other words, if you add a userLogin yourself you wouldn't
>> expect the system to blow it away.
>> If you agree feel free to commit that, your patch of the
>> CallService and CallServiceAsynch classes looks just fine. If I
>> don't see your response on it I'll throw that in later this
>> evening or tomorrow.
>> -David
>> On Feb 7, 2007, at 10:38 AM, Jacopo Cappellato wrote:
>>> Ah,
>>>
>>> forgot to mention that I'd vote for solution #1.
>>>
>>> Jacopo
>>>
>>> Jacopo Cappellato wrote:
>>>> Should we take some action about this subject?
>>>> I know that probably my explanations were not clear enough, so
>>>> I'm proposing the two ways of fixing what I think it's an
>>>> outstanding issue; I hope that, reviewing the (extremely simple)
>>>> fixes I'm proposing will be easier to understand the issue I'm
>>>> trying to report.
>>>> Solution #1: just fix the service call to "createUserLogin"
>>>> inside of createPersonAndUserLogin by adding the include-user-
>>>> login="false"; in this way the manually "system" user, manually
>>>> put in the input context will be not overriden by the user
>>>> available in the context of the calling service
>>>> ("createPersonAndUserLogin"); see fix1.patch
>>>> Solution #2: fix for the minilang service call methods; if
>>>> include-user-login is not set or it is true, the user login is
>>>> put in the context but *only* if in the context there is not
>>>> already a user login; see fix2.patch
>>>> I really think that one of the two solutions should be committed.
>>>> What do you think?
>>>> Jacopo
>>>> Jacopo Cappellato wrote:
>>>>> David,
>>>>>
>>>>> I really did not describe it very well, sorry.
>>>>> I don't think there is a problem with the createUserLogin
>>>>> service definition or implementation.
>>>>> The issue (maybe) could be the way the framework handles the
>>>>> minilang attribute include-user-login="true": it just gets the
>>>>> "userLogin" object from the context of the calling service and
>>>>> put it in the context of the called service, even if in the
>>>>> context of the called service there is already a "userLogin"
>>>>> object.
>>>>> So for example the code in the createPersonAndUserLogin service
>>>>> (for which I have provided the patch) is:
>>>>>
>>>>> <!-- call the service with the system account to get around
>>>>> security constraints for this special create -->
>>>>> <entity-one entity-name="UserLogin" value-
>>>>> name="createUlInMap.userLogin">
>>>>> <field-map field-name="userLoginId" value="system"/>
>>>>> </entity-one>
>>>>>
>>>>> <call-service service-name="createUserLogin" in-map-
>>>>> name="createUlInMap" include-user-login="false"/>
>>>>>
>>>>> Unfotunately the "system" userLogin object that is manually put
>>>>> in the input context for the createUserLogin service is
>>>>> overriden (by the framework), if one is available in the
>>>>> context of the createPersonAndUserLogin service because the
>>>>> include-user-login attribute defaults to true.
>>>>>
>>>>> Just adding the include-user-login="false" attribute to the
>>>>> service call fixed the issue.
>>>>>
>>>>> Does it make sense?
>>>>>
>>>>> Jacopo
>>>>>
>>>>>
>>>>>
>>>>> David E. Jones wrote:
>>>>>>
>>>>>> On Jan 25, 2007, at 7:41 AM, Jacopo Cappellato wrote:
>>>>>>
>>>>>>> Hi all,
>>>>>>>
>>>>>>> could you have a look at the attached simple patch? It fixes
>>>>>>> an authorization problem under some special situations
>>>>>>> (switching from anonymous user to authenticated one...);
>>>>>>> however the issue is that, when the "createUserLogin" is
>>>>>>> called, if the attribute include-user-login is not set to
>>>>>>> false the manually passed in "system" user is overwritten by
>>>>>>> the user in the context.
>>>>>>
>>>>>> I just traced through the code to try to figure out why this
>>>>>> might be happening, and either I don't understand what you're
>>>>>> describing, or I just can't find why this would happen.
>>>>>>
>>>>>> In the implementation of the service
>>>>>> (LoginServices.java:createUserLogin) I don't see anything that
>>>>>> would do this. In the service definition I don't see it either
>>>>>> because the service definition does not have a "userLogin"
>>>>>> parameter going out.
>>>>>>
>>>>>> Have you traced down the code that would be putting the user
>>>>>> in the context? Also, do you mean by this the context of the
>>>>>> current service call (ie for other services called by ECA
>>>>>> rules, etc), or in the result (returned) Map of the service,
>>>>>> or somewhere else?
>>>>>>
>>>>>> I apologize for pushing on the detail, I may just be too brain-
>>>>>> dead to see it right now.
>>>>>>
>>>>>> -David
>>>>>>
>>>>>>
>>>>>>> Should I commit this patch?
>>>>>>> Or, in general, would be better, even if include-user-login
>>>>>>> is true, to set the user login only if one is not already
>>>>>>> there in the service in map?
>>>>>>>
>>>>>>> Jacopo
>>>>>>> Index: applications/party/script/org/ofbiz/party/party/
>>>>>>> PartySimpleMethods.xml
>>>>>>> ================================================================
>>>>>>> ===
>>>>>>> --- applications/party/script/org/ofbiz/party/party/
>>>>>>> PartySimpleMethods.xml (revisione 499802)
>>>>>>> +++ applications/party/script/org/ofbiz/party/party/
>>>>>>> PartySimpleMethods.xml (copia locale)
>>>>>>> @@ -85,7 +85,7 @@
>>>>>>> <field-map field-name="userLoginId"
>>>>>>> value="system"/>
>>>>>>> </entity-one>
>>>>>>>
>>>>>>> - <call-service service-name="createUserLogin" in-map-
>>>>>>> name="createUlInMap"/>
>>>>>>> + <call-service service-name="createUserLogin" in-map-
>>>>>>> name="createUlInMap" include-user-login="false"/>
>>>>>>> <entity-one entity-name="UserLogin" value-
>>>>>>> name="newUserLogin"/>
>>>>>>>
>>>>>>> <field-to-result field-name="newUserLogin"/>
>>>>>>
>>>>>
>>>> -------------------------------------------------------------------
>>>> -----
>>>> Index: applications/party/script/org/ofbiz/party/party/
>>>> PartySimpleMethods.xml
>>>> ===================================================================
>>>> --- applications/party/script/org/ofbiz/party/party/
>>>> PartySimpleMethods.xml (revisione 503593)
>>>> +++ applications/party/script/org/ofbiz/party/party/
>>>> PartySimpleMethods.xml (copia locale)
>>>> @@ -85,7 +85,7 @@
>>>> <field-map field-name="userLoginId" value="system"/>
>>>> </entity-one>
>>>> - <call-service service-name="createUserLogin"
>>>> in-map-name="createUlInMap"/>
>>>> + <call-service service-name="createUserLogin" in-map-
>>>> name="createUlInMap" include-user-login="false"/>
>>>> <entity-one entity-name="UserLogin" value-
>>>> name="newUserLogin"/>
>>>> <field-to-result field-name="newUserLogin"/>
>>>> -------------------------------------------------------------------
>>>> -----
>>>> Index: framework/minilang/src/org/ofbiz/minilang/method/callops/
>>>> CallServiceAsynch.java
>>>> ===================================================================
>>>> --- framework/minilang/src/org/ofbiz/minilang/method/callops/
>>>> CallServiceAsynch.java (revisione 503593)
>>>> +++ framework/minilang/src/org/ofbiz/minilang/method/callops/
>>>> CallServiceAsynch.java (copia locale)
>>>> @@ -68,8 +68,9 @@
>>>> if (includeUserLogin) {
>>>> GenericValue userLogin = methodContext.getUserLogin();
>>>> - if (userLogin != null)
>>>> + if (userLogin != null && inMap.get("userLogin") ==
>>>> null) {
>>>> inMap.put("userLogin", userLogin);
>>>> + }
>>>> }
>>>> // always add Locale to context unless null
>>>> Index: framework/minilang/src/org/ofbiz/minilang/method/callops/
>>>> CallService.java
>>>> ===================================================================
>>>> --- framework/minilang/src/org/ofbiz/minilang/method/callops/
>>>> CallService.java (revisione 503593)
>>>> +++ framework/minilang/src/org/ofbiz/minilang/method/callops/
>>>> CallService.java (copia locale)
>>>> @@ -213,7 +213,7 @@
>>>> if (includeUserLogin) {
>>>> GenericValue userLogin = methodContext.getUserLogin();
>>>> - if (userLogin != null) {
>>>> + if (userLogin != null && inMap.get("userLogin") ==
>>>> null) {
>>>> inMap.put("userLogin", userLogin);
>>>> }
>>>> }
>>>
>>>
>
>
Re: Possible auth issue in the createPartyAndUserLogin service
Posted by Jacopo Cappellato <ti...@sastau.it>.
David,
I can actually commit the patch #2 without problem; I'm just worried
about the side effects (if any) that this could cause on the existing
code. For example, I don't know if the set-service-fields operation is
copying also the user login from one map to another one; if this is
happening, then (with the existing code) the user login is overriden by
the one of the calling service; if I apply my patch this will be no more
true... could this cause problems in your opinion? Or am I just too anxious?
Thanks for your feedback,
Jacopo
David E. Jones wrote:
>
> Sorry for the delay on this Jacopo.
>
> Looking at this I actually like the fix in #2, ie changing the
> simple-method operations because what they do doesn't make sense. In
> other words, if you add a userLogin yourself you wouldn't expect the
> system to blow it away.
>
> If you agree feel free to commit that, your patch of the CallService and
> CallServiceAsynch classes looks just fine. If I don't see your response
> on it I'll throw that in later this evening or tomorrow.
>
> -David
>
>
> On Feb 7, 2007, at 10:38 AM, Jacopo Cappellato wrote:
>
>> Ah,
>>
>> forgot to mention that I'd vote for solution #1.
>>
>> Jacopo
>>
>> Jacopo Cappellato wrote:
>>> Should we take some action about this subject?
>>> I know that probably my explanations were not clear enough, so I'm
>>> proposing the two ways of fixing what I think it's an outstanding
>>> issue; I hope that, reviewing the (extremely simple) fixes I'm
>>> proposing will be easier to understand the issue I'm trying to report.
>>> Solution #1: just fix the service call to "createUserLogin" inside of
>>> createPersonAndUserLogin by adding the include-user-login="false"; in
>>> this way the manually "system" user, manually put in the input
>>> context will be not overriden by the user available in the context of
>>> the calling service ("createPersonAndUserLogin"); see fix1.patch
>>> Solution #2: fix for the minilang service call methods; if
>>> include-user-login is not set or it is true, the user login is put in
>>> the context but *only* if in the context there is not already a user
>>> login; see fix2.patch
>>> I really think that one of the two solutions should be committed.
>>> What do you think?
>>> Jacopo
>>> Jacopo Cappellato wrote:
>>>> David,
>>>>
>>>> I really did not describe it very well, sorry.
>>>> I don't think there is a problem with the createUserLogin service
>>>> definition or implementation.
>>>> The issue (maybe) could be the way the framework handles the
>>>> minilang attribute include-user-login="true": it just gets the
>>>> "userLogin" object from the context of the calling service and put
>>>> it in the context of the called service, even if in the context of
>>>> the called service there is already a "userLogin" object.
>>>> So for example the code in the createPersonAndUserLogin service (for
>>>> which I have provided the patch) is:
>>>>
>>>> <!-- call the service with the system account to get around security
>>>> constraints for this special create -->
>>>> <entity-one entity-name="UserLogin"
>>>> value-name="createUlInMap.userLogin">
>>>> <field-map field-name="userLoginId" value="system"/>
>>>> </entity-one>
>>>>
>>>> <call-service service-name="createUserLogin"
>>>> in-map-name="createUlInMap" include-user-login="false"/>
>>>>
>>>> Unfotunately the "system" userLogin object that is manually put in
>>>> the input context for the createUserLogin service is overriden (by
>>>> the framework), if one is available in the context of the
>>>> createPersonAndUserLogin service because the include-user-login
>>>> attribute defaults to true.
>>>>
>>>> Just adding the include-user-login="false" attribute to the service
>>>> call fixed the issue.
>>>>
>>>> Does it make sense?
>>>>
>>>> Jacopo
>>>>
>>>>
>>>>
>>>> David E. Jones wrote:
>>>>>
>>>>> On Jan 25, 2007, at 7:41 AM, Jacopo Cappellato wrote:
>>>>>
>>>>>> Hi all,
>>>>>>
>>>>>> could you have a look at the attached simple patch? It fixes an
>>>>>> authorization problem under some special situations (switching
>>>>>> from anonymous user to authenticated one...); however the issue is
>>>>>> that, when the "createUserLogin" is called, if the attribute
>>>>>> include-user-login is not set to false the manually passed in
>>>>>> "system" user is overwritten by the user in the context.
>>>>>
>>>>> I just traced through the code to try to figure out why this might
>>>>> be happening, and either I don't understand what you're describing,
>>>>> or I just can't find why this would happen.
>>>>>
>>>>> In the implementation of the service
>>>>> (LoginServices.java:createUserLogin) I don't see anything that
>>>>> would do this. In the service definition I don't see it either
>>>>> because the service definition does not have a "userLogin"
>>>>> parameter going out.
>>>>>
>>>>> Have you traced down the code that would be putting the user in the
>>>>> context? Also, do you mean by this the context of the current
>>>>> service call (ie for other services called by ECA rules, etc), or
>>>>> in the result (returned) Map of the service, or somewhere else?
>>>>>
>>>>> I apologize for pushing on the detail, I may just be too brain-dead
>>>>> to see it right now.
>>>>>
>>>>> -David
>>>>>
>>>>>
>>>>>> Should I commit this patch?
>>>>>> Or, in general, would be better, even if include-user-login is
>>>>>> true, to set the user login only if one is not already there in
>>>>>> the service in map?
>>>>>>
>>>>>> Jacopo
>>>>>> Index:
>>>>>> applications/party/script/org/ofbiz/party/party/PartySimpleMethods.xml
>>>>>>
>>>>>> ===================================================================
>>>>>> ---
>>>>>> applications/party/script/org/ofbiz/party/party/PartySimpleMethods.xml
>>>>>> (revisione 499802)
>>>>>> +++
>>>>>> applications/party/script/org/ofbiz/party/party/PartySimpleMethods.xml
>>>>>> (copia locale)
>>>>>> @@ -85,7 +85,7 @@
>>>>>> <field-map field-name="userLoginId" value="system"/>
>>>>>> </entity-one>
>>>>>>
>>>>>> - <call-service service-name="createUserLogin"
>>>>>> in-map-name="createUlInMap"/>
>>>>>> + <call-service service-name="createUserLogin"
>>>>>> in-map-name="createUlInMap" include-user-login="false"/>
>>>>>> <entity-one entity-name="UserLogin"
>>>>>> value-name="newUserLogin"/>
>>>>>>
>>>>>> <field-to-result field-name="newUserLogin"/>
>>>>>
>>>>
>>> ------------------------------------------------------------------------
>>> Index:
>>> applications/party/script/org/ofbiz/party/party/PartySimpleMethods.xml
>>> ===================================================================
>>> ---
>>> applications/party/script/org/ofbiz/party/party/PartySimpleMethods.xml
>>> (revisione 503593)
>>> +++
>>> applications/party/script/org/ofbiz/party/party/PartySimpleMethods.xml
>>> (copia locale)
>>> @@ -85,7 +85,7 @@
>>> <field-map field-name="userLoginId" value="system"/>
>>> </entity-one>
>>> - <call-service service-name="createUserLogin"
>>> in-map-name="createUlInMap"/>
>>> + <call-service service-name="createUserLogin"
>>> in-map-name="createUlInMap" include-user-login="false"/>
>>> <entity-one entity-name="UserLogin" value-name="newUserLogin"/>
>>> <field-to-result field-name="newUserLogin"/>
>>> ------------------------------------------------------------------------
>>> Index:
>>> framework/minilang/src/org/ofbiz/minilang/method/callops/CallServiceAsynch.java
>>>
>>> ===================================================================
>>> ---
>>> framework/minilang/src/org/ofbiz/minilang/method/callops/CallServiceAsynch.java
>>> (revisione 503593)
>>> +++
>>> framework/minilang/src/org/ofbiz/minilang/method/callops/CallServiceAsynch.java
>>> (copia locale)
>>> @@ -68,8 +68,9 @@
>>> if (includeUserLogin) {
>>> GenericValue userLogin = methodContext.getUserLogin();
>>> - if (userLogin != null)
>>> + if (userLogin != null && inMap.get("userLogin") == null) {
>>> inMap.put("userLogin", userLogin);
>>> + }
>>> }
>>> // always add Locale to context unless null
>>> Index:
>>> framework/minilang/src/org/ofbiz/minilang/method/callops/CallService.java
>>>
>>> ===================================================================
>>> ---
>>> framework/minilang/src/org/ofbiz/minilang/method/callops/CallService.java
>>> (revisione 503593)
>>> +++
>>> framework/minilang/src/org/ofbiz/minilang/method/callops/CallService.java
>>> (copia locale)
>>> @@ -213,7 +213,7 @@
>>> if (includeUserLogin) {
>>> GenericValue userLogin = methodContext.getUserLogin();
>>> - if (userLogin != null) {
>>> + if (userLogin != null && inMap.get("userLogin") == null) {
>>> inMap.put("userLogin", userLogin);
>>> }
>>> }
>>
>>
>
Re: Possible auth issue in the createPartyAndUserLogin service
Posted by "David E. Jones" <jo...@hotwaxmedia.com>.
Sorry for the delay on this Jacopo.
Looking at this I actually like the fix in #2, ie changing the simple-
method operations because what they do doesn't make sense. In other
words, if you add a userLogin yourself you wouldn't expect the system
to blow it away.
If you agree feel free to commit that, your patch of the CallService
and CallServiceAsynch classes looks just fine. If I don't see your
response on it I'll throw that in later this evening or tomorrow.
-David
On Feb 7, 2007, at 10:38 AM, Jacopo Cappellato wrote:
> Ah,
>
> forgot to mention that I'd vote for solution #1.
>
> Jacopo
>
> Jacopo Cappellato wrote:
>> Should we take some action about this subject?
>> I know that probably my explanations were not clear enough, so I'm
>> proposing the two ways of fixing what I think it's an outstanding
>> issue; I hope that, reviewing the (extremely simple) fixes I'm
>> proposing will be easier to understand the issue I'm trying to
>> report.
>> Solution #1: just fix the service call to "createUserLogin" inside
>> of createPersonAndUserLogin by adding the include-user-
>> login="false"; in this way the manually "system" user, manually
>> put in the input context will be not overriden by the user
>> available in the context of the calling service
>> ("createPersonAndUserLogin"); see fix1.patch
>> Solution #2: fix for the minilang service call methods; if include-
>> user-login is not set or it is true, the user login is put in the
>> context but *only* if in the context there is not already a user
>> login; see fix2.patch
>> I really think that one of the two solutions should be committed.
>> What do you think?
>> Jacopo
>> Jacopo Cappellato wrote:
>>> David,
>>>
>>> I really did not describe it very well, sorry.
>>> I don't think there is a problem with the createUserLogin service
>>> definition or implementation.
>>> The issue (maybe) could be the way the framework handles the
>>> minilang attribute include-user-login="true": it just gets the
>>> "userLogin" object from the context of the calling service and
>>> put it in the context of the called service, even if in the
>>> context of the called service there is already a "userLogin" object.
>>> So for example the code in the createPersonAndUserLogin service
>>> (for which I have provided the patch) is:
>>>
>>> <!-- call the service with the system account to get around
>>> security constraints for this special create -->
>>> <entity-one entity-name="UserLogin" value-
>>> name="createUlInMap.userLogin">
>>> <field-map field-name="userLoginId" value="system"/>
>>> </entity-one>
>>>
>>> <call-service service-name="createUserLogin" in-map-
>>> name="createUlInMap" include-user-login="false"/>
>>>
>>> Unfotunately the "system" userLogin object that is manually put
>>> in the input context for the createUserLogin service is overriden
>>> (by the framework), if one is available in the context of the
>>> createPersonAndUserLogin service because the include-user-login
>>> attribute defaults to true.
>>>
>>> Just adding the include-user-login="false" attribute to the
>>> service call fixed the issue.
>>>
>>> Does it make sense?
>>>
>>> Jacopo
>>>
>>>
>>>
>>> David E. Jones wrote:
>>>>
>>>> On Jan 25, 2007, at 7:41 AM, Jacopo Cappellato wrote:
>>>>
>>>>> Hi all,
>>>>>
>>>>> could you have a look at the attached simple patch? It fixes an
>>>>> authorization problem under some special situations (switching
>>>>> from anonymous user to authenticated one...); however the issue
>>>>> is that, when the "createUserLogin" is called, if the attribute
>>>>> include-user-login is not set to false the manually passed in
>>>>> "system" user is overwritten by the user in the context.
>>>>
>>>> I just traced through the code to try to figure out why this
>>>> might be happening, and either I don't understand what you're
>>>> describing, or I just can't find why this would happen.
>>>>
>>>> In the implementation of the service
>>>> (LoginServices.java:createUserLogin) I don't see anything that
>>>> would do this. In the service definition I don't see it either
>>>> because the service definition does not have a "userLogin"
>>>> parameter going out.
>>>>
>>>> Have you traced down the code that would be putting the user in
>>>> the context? Also, do you mean by this the context of the
>>>> current service call (ie for other services called by ECA rules,
>>>> etc), or in the result (returned) Map of the service, or
>>>> somewhere else?
>>>>
>>>> I apologize for pushing on the detail, I may just be too brain-
>>>> dead to see it right now.
>>>>
>>>> -David
>>>>
>>>>
>>>>> Should I commit this patch?
>>>>> Or, in general, would be better, even if include-user-login is
>>>>> true, to set the user login only if one is not already there in
>>>>> the service in map?
>>>>>
>>>>> Jacopo
>>>>> Index: applications/party/script/org/ofbiz/party/party/
>>>>> PartySimpleMethods.xml
>>>>> ==================================================================
>>>>> =
>>>>> --- applications/party/script/org/ofbiz/party/party/
>>>>> PartySimpleMethods.xml (revisione 499802)
>>>>> +++ applications/party/script/org/ofbiz/party/party/
>>>>> PartySimpleMethods.xml (copia locale)
>>>>> @@ -85,7 +85,7 @@
>>>>> <field-map field-name="userLoginId" value="system"/>
>>>>> </entity-one>
>>>>>
>>>>> - <call-service service-name="createUserLogin" in-map-
>>>>> name="createUlInMap"/>
>>>>> + <call-service service-name="createUserLogin" in-map-
>>>>> name="createUlInMap" include-user-login="false"/>
>>>>> <entity-one entity-name="UserLogin" value-
>>>>> name="newUserLogin"/>
>>>>>
>>>>> <field-to-result field-name="newUserLogin"/>
>>>>
>>>
>> ---------------------------------------------------------------------
>> ---
>> Index: applications/party/script/org/ofbiz/party/party/
>> PartySimpleMethods.xml
>> ===================================================================
>> --- applications/party/script/org/ofbiz/party/party/
>> PartySimpleMethods.xml (revisione 503593)
>> +++ applications/party/script/org/ofbiz/party/party/
>> PartySimpleMethods.xml (copia locale)
>> @@ -85,7 +85,7 @@
>> <field-map field-name="userLoginId" value="system"/>
>> </entity-one>
>> - <call-service service-name="createUserLogin" in-
>> map-name="createUlInMap"/>
>> + <call-service service-name="createUserLogin" in-map-
>> name="createUlInMap" include-user-login="false"/>
>> <entity-one entity-name="UserLogin" value-
>> name="newUserLogin"/>
>> <field-to-result field-name="newUserLogin"/>
>> ---------------------------------------------------------------------
>> ---
>> Index: framework/minilang/src/org/ofbiz/minilang/method/callops/
>> CallServiceAsynch.java
>> ===================================================================
>> --- framework/minilang/src/org/ofbiz/minilang/method/callops/
>> CallServiceAsynch.java (revisione 503593)
>> +++ framework/minilang/src/org/ofbiz/minilang/method/callops/
>> CallServiceAsynch.java (copia locale)
>> @@ -68,8 +68,9 @@
>> if (includeUserLogin) {
>> GenericValue userLogin = methodContext.getUserLogin();
>> - if (userLogin != null)
>> + if (userLogin != null && inMap.get("userLogin") ==
>> null) {
>> inMap.put("userLogin", userLogin);
>> + }
>> }
>> // always add Locale to context unless null
>> Index: framework/minilang/src/org/ofbiz/minilang/method/callops/
>> CallService.java
>> ===================================================================
>> --- framework/minilang/src/org/ofbiz/minilang/method/callops/
>> CallService.java (revisione 503593)
>> +++ framework/minilang/src/org/ofbiz/minilang/method/callops/
>> CallService.java (copia locale)
>> @@ -213,7 +213,7 @@
>> if (includeUserLogin) {
>> GenericValue userLogin = methodContext.getUserLogin();
>> - if (userLogin != null) {
>> + if (userLogin != null && inMap.get("userLogin") ==
>> null) {
>> inMap.put("userLogin", userLogin);
>> }
>> }
>
>
Re: Possible auth issue in the createPartyAndUserLogin service
Posted by Jacopo Cappellato <ti...@sastau.it>.
Ah,
forgot to mention that I'd vote for solution #1.
Jacopo
Jacopo Cappellato wrote:
> Should we take some action about this subject?
>
> I know that probably my explanations were not clear enough, so I'm
> proposing the two ways of fixing what I think it's an outstanding issue;
> I hope that, reviewing the (extremely simple) fixes I'm proposing will
> be easier to understand the issue I'm trying to report.
>
> Solution #1: just fix the service call to "createUserLogin" inside of
> createPersonAndUserLogin by adding the include-user-login="false"; in
> this way the manually "system" user, manually put in the input context
> will be not overriden by the user available in the context of the
> calling service ("createPersonAndUserLogin"); see fix1.patch
>
> Solution #2: fix for the minilang service call methods; if
> include-user-login is not set or it is true, the user login is put in
> the context but *only* if in the context there is not already a user
> login; see fix2.patch
>
> I really think that one of the two solutions should be committed.
>
> What do you think?
>
> Jacopo
>
>
>
>
> Jacopo Cappellato wrote:
>> David,
>>
>> I really did not describe it very well, sorry.
>> I don't think there is a problem with the createUserLogin service
>> definition or implementation.
>> The issue (maybe) could be the way the framework handles the minilang
>> attribute include-user-login="true": it just gets the "userLogin"
>> object from the context of the calling service and put it in the
>> context of the called service, even if in the context of the called
>> service there is already a "userLogin" object.
>> So for example the code in the createPersonAndUserLogin service (for
>> which I have provided the patch) is:
>>
>> <!-- call the service with the system account to get around security
>> constraints for this special create -->
>> <entity-one entity-name="UserLogin" value-name="createUlInMap.userLogin">
>> <field-map field-name="userLoginId" value="system"/>
>> </entity-one>
>>
>> <call-service service-name="createUserLogin"
>> in-map-name="createUlInMap" include-user-login="false"/>
>>
>> Unfotunately the "system" userLogin object that is manually put in the
>> input context for the createUserLogin service is overriden (by the
>> framework), if one is available in the context of the
>> createPersonAndUserLogin service because the include-user-login
>> attribute defaults to true.
>>
>> Just adding the include-user-login="false" attribute to the service
>> call fixed the issue.
>>
>> Does it make sense?
>>
>> Jacopo
>>
>>
>>
>> David E. Jones wrote:
>>>
>>> On Jan 25, 2007, at 7:41 AM, Jacopo Cappellato wrote:
>>>
>>>> Hi all,
>>>>
>>>> could you have a look at the attached simple patch? It fixes an
>>>> authorization problem under some special situations (switching from
>>>> anonymous user to authenticated one...); however the issue is that,
>>>> when the "createUserLogin" is called, if the attribute
>>>> include-user-login is not set to false the manually passed in
>>>> "system" user is overwritten by the user in the context.
>>>
>>> I just traced through the code to try to figure out why this might be
>>> happening, and either I don't understand what you're describing, or I
>>> just can't find why this would happen.
>>>
>>> In the implementation of the service
>>> (LoginServices.java:createUserLogin) I don't see anything that would
>>> do this. In the service definition I don't see it either because the
>>> service definition does not have a "userLogin" parameter going out.
>>>
>>> Have you traced down the code that would be putting the user in the
>>> context? Also, do you mean by this the context of the current service
>>> call (ie for other services called by ECA rules, etc), or in the
>>> result (returned) Map of the service, or somewhere else?
>>>
>>> I apologize for pushing on the detail, I may just be too brain-dead
>>> to see it right now.
>>>
>>> -David
>>>
>>>
>>>> Should I commit this patch?
>>>> Or, in general, would be better, even if include-user-login is true,
>>>> to set the user login only if one is not already there in the
>>>> service in map?
>>>>
>>>> Jacopo
>>>> Index:
>>>> applications/party/script/org/ofbiz/party/party/PartySimpleMethods.xml
>>>> ===================================================================
>>>> ---
>>>> applications/party/script/org/ofbiz/party/party/PartySimpleMethods.xml
>>>> (revisione 499802)
>>>> +++
>>>> applications/party/script/org/ofbiz/party/party/PartySimpleMethods.xml
>>>> (copia locale)
>>>> @@ -85,7 +85,7 @@
>>>> <field-map field-name="userLoginId" value="system"/>
>>>> </entity-one>
>>>>
>>>> - <call-service service-name="createUserLogin"
>>>> in-map-name="createUlInMap"/>
>>>> + <call-service service-name="createUserLogin"
>>>> in-map-name="createUlInMap" include-user-login="false"/>
>>>> <entity-one entity-name="UserLogin"
>>>> value-name="newUserLogin"/>
>>>>
>>>> <field-to-result field-name="newUserLogin"/>
>>>
>>
>
>
> ------------------------------------------------------------------------
>
> Index: applications/party/script/org/ofbiz/party/party/PartySimpleMethods.xml
> ===================================================================
> --- applications/party/script/org/ofbiz/party/party/PartySimpleMethods.xml (revisione 503593)
> +++ applications/party/script/org/ofbiz/party/party/PartySimpleMethods.xml (copia locale)
> @@ -85,7 +85,7 @@
> <field-map field-name="userLoginId" value="system"/>
> </entity-one>
>
> - <call-service service-name="createUserLogin" in-map-name="createUlInMap"/>
> + <call-service service-name="createUserLogin" in-map-name="createUlInMap" include-user-login="false"/>
> <entity-one entity-name="UserLogin" value-name="newUserLogin"/>
>
> <field-to-result field-name="newUserLogin"/>
>
>
> ------------------------------------------------------------------------
>
> Index: framework/minilang/src/org/ofbiz/minilang/method/callops/CallServiceAsynch.java
> ===================================================================
> --- framework/minilang/src/org/ofbiz/minilang/method/callops/CallServiceAsynch.java (revisione 503593)
> +++ framework/minilang/src/org/ofbiz/minilang/method/callops/CallServiceAsynch.java (copia locale)
> @@ -68,8 +68,9 @@
> if (includeUserLogin) {
> GenericValue userLogin = methodContext.getUserLogin();
>
> - if (userLogin != null)
> + if (userLogin != null && inMap.get("userLogin") == null) {
> inMap.put("userLogin", userLogin);
> + }
> }
>
> // always add Locale to context unless null
> Index: framework/minilang/src/org/ofbiz/minilang/method/callops/CallService.java
> ===================================================================
> --- framework/minilang/src/org/ofbiz/minilang/method/callops/CallService.java (revisione 503593)
> +++ framework/minilang/src/org/ofbiz/minilang/method/callops/CallService.java (copia locale)
> @@ -213,7 +213,7 @@
> if (includeUserLogin) {
> GenericValue userLogin = methodContext.getUserLogin();
>
> - if (userLogin != null) {
> + if (userLogin != null && inMap.get("userLogin") == null) {
> inMap.put("userLogin", userLogin);
> }
> }
Re: Possible auth issue in the createPartyAndUserLogin service
Posted by Jacopo Cappellato <ti...@sastau.it>.
Should we take some action about this subject?
I know that probably my explanations were not clear enough, so I'm
proposing the two ways of fixing what I think it's an outstanding issue;
I hope that, reviewing the (extremely simple) fixes I'm proposing will
be easier to understand the issue I'm trying to report.
Solution #1: just fix the service call to "createUserLogin" inside of
createPersonAndUserLogin by adding the include-user-login="false"; in
this way the manually "system" user, manually put in the input context
will be not overriden by the user available in the context of the
calling service ("createPersonAndUserLogin"); see fix1.patch
Solution #2: fix for the minilang service call methods; if
include-user-login is not set or it is true, the user login is put in
the context but *only* if in the context there is not already a user
login; see fix2.patch
I really think that one of the two solutions should be committed.
What do you think?
Jacopo
Jacopo Cappellato wrote:
> David,
>
> I really did not describe it very well, sorry.
> I don't think there is a problem with the createUserLogin service
> definition or implementation.
> The issue (maybe) could be the way the framework handles the minilang
> attribute include-user-login="true": it just gets the "userLogin" object
> from the context of the calling service and put it in the context of the
> called service, even if in the context of the called service there is
> already a "userLogin" object.
> So for example the code in the createPersonAndUserLogin service (for
> which I have provided the patch) is:
>
> <!-- call the service with the system account to get around security
> constraints for this special create -->
> <entity-one entity-name="UserLogin" value-name="createUlInMap.userLogin">
> <field-map field-name="userLoginId" value="system"/>
> </entity-one>
>
> <call-service service-name="createUserLogin" in-map-name="createUlInMap"
> include-user-login="false"/>
>
> Unfotunately the "system" userLogin object that is manually put in the
> input context for the createUserLogin service is overriden (by the
> framework), if one is available in the context of the
> createPersonAndUserLogin service because the include-user-login
> attribute defaults to true.
>
> Just adding the include-user-login="false" attribute to the service call
> fixed the issue.
>
> Does it make sense?
>
> Jacopo
>
>
>
> David E. Jones wrote:
>>
>> On Jan 25, 2007, at 7:41 AM, Jacopo Cappellato wrote:
>>
>>> Hi all,
>>>
>>> could you have a look at the attached simple patch? It fixes an
>>> authorization problem under some special situations (switching from
>>> anonymous user to authenticated one...); however the issue is that,
>>> when the "createUserLogin" is called, if the attribute
>>> include-user-login is not set to false the manually passed in
>>> "system" user is overwritten by the user in the context.
>>
>> I just traced through the code to try to figure out why this might be
>> happening, and either I don't understand what you're describing, or I
>> just can't find why this would happen.
>>
>> In the implementation of the service
>> (LoginServices.java:createUserLogin) I don't see anything that would
>> do this. In the service definition I don't see it either because the
>> service definition does not have a "userLogin" parameter going out.
>>
>> Have you traced down the code that would be putting the user in the
>> context? Also, do you mean by this the context of the current service
>> call (ie for other services called by ECA rules, etc), or in the
>> result (returned) Map of the service, or somewhere else?
>>
>> I apologize for pushing on the detail, I may just be too brain-dead to
>> see it right now.
>>
>> -David
>>
>>
>>> Should I commit this patch?
>>> Or, in general, would be better, even if include-user-login is true,
>>> to set the user login only if one is not already there in the service
>>> in map?
>>>
>>> Jacopo
>>> Index:
>>> applications/party/script/org/ofbiz/party/party/PartySimpleMethods.xml
>>> ===================================================================
>>> ---
>>> applications/party/script/org/ofbiz/party/party/PartySimpleMethods.xml
>>> (revisione 499802)
>>> +++
>>> applications/party/script/org/ofbiz/party/party/PartySimpleMethods.xml
>>> (copia locale)
>>> @@ -85,7 +85,7 @@
>>> <field-map field-name="userLoginId" value="system"/>
>>> </entity-one>
>>>
>>> - <call-service service-name="createUserLogin"
>>> in-map-name="createUlInMap"/>
>>> + <call-service service-name="createUserLogin"
>>> in-map-name="createUlInMap" include-user-login="false"/>
>>> <entity-one entity-name="UserLogin" value-name="newUserLogin"/>
>>>
>>> <field-to-result field-name="newUserLogin"/>
>>
>
Re: Possible auth issue in the createPartyAndUserLogin service
Posted by Jacopo Cappellato <ti...@sastau.it>.
David,
I really did not describe it very well, sorry.
I don't think there is a problem with the createUserLogin service
definition or implementation.
The issue (maybe) could be the way the framework handles the minilang
attribute include-user-login="true": it just gets the "userLogin" object
from the context of the calling service and put it in the context of the
called service, even if in the context of the called service there is
already a "userLogin" object.
So for example the code in the createPersonAndUserLogin service (for
which I have provided the patch) is:
<!-- call the service with the system account to get around security
constraints for this special create -->
<entity-one entity-name="UserLogin" value-name="createUlInMap.userLogin">
<field-map field-name="userLoginId" value="system"/>
</entity-one>
<call-service service-name="createUserLogin" in-map-name="createUlInMap"
include-user-login="false"/>
Unfotunately the "system" userLogin object that is manually put in the
input context for the createUserLogin service is overriden (by the
framework), if one is available in the context of the
createPersonAndUserLogin service because the include-user-login
attribute defaults to true.
Just adding the include-user-login="false" attribute to the service call
fixed the issue.
Does it make sense?
Jacopo
David E. Jones wrote:
>
> On Jan 25, 2007, at 7:41 AM, Jacopo Cappellato wrote:
>
>> Hi all,
>>
>> could you have a look at the attached simple patch? It fixes an
>> authorization problem under some special situations (switching from
>> anonymous user to authenticated one...); however the issue is that,
>> when the "createUserLogin" is called, if the attribute
>> include-user-login is not set to false the manually passed in "system"
>> user is overwritten by the user in the context.
>
> I just traced through the code to try to figure out why this might be
> happening, and either I don't understand what you're describing, or I
> just can't find why this would happen.
>
> In the implementation of the service
> (LoginServices.java:createUserLogin) I don't see anything that would do
> this. In the service definition I don't see it either because the
> service definition does not have a "userLogin" parameter going out.
>
> Have you traced down the code that would be putting the user in the
> context? Also, do you mean by this the context of the current service
> call (ie for other services called by ECA rules, etc), or in the result
> (returned) Map of the service, or somewhere else?
>
> I apologize for pushing on the detail, I may just be too brain-dead to
> see it right now.
>
> -David
>
>
>> Should I commit this patch?
>> Or, in general, would be better, even if include-user-login is true,
>> to set the user login only if one is not already there in the service
>> in map?
>>
>> Jacopo
>> Index:
>> applications/party/script/org/ofbiz/party/party/PartySimpleMethods.xml
>> ===================================================================
>> ---
>> applications/party/script/org/ofbiz/party/party/PartySimpleMethods.xml
>> (revisione 499802)
>> +++
>> applications/party/script/org/ofbiz/party/party/PartySimpleMethods.xml
>> (copia locale)
>> @@ -85,7 +85,7 @@
>> <field-map field-name="userLoginId" value="system"/>
>> </entity-one>
>>
>> - <call-service service-name="createUserLogin"
>> in-map-name="createUlInMap"/>
>> + <call-service service-name="createUserLogin"
>> in-map-name="createUlInMap" include-user-login="false"/>
>> <entity-one entity-name="UserLogin" value-name="newUserLogin"/>
>>
>> <field-to-result field-name="newUserLogin"/>
>
Re: Possible auth issue in the createPartyAndUserLogin service
Posted by "David E. Jones" <jo...@hotwaxmedia.com>.
On Jan 25, 2007, at 7:41 AM, Jacopo Cappellato wrote:
> Hi all,
>
> could you have a look at the attached simple patch? It fixes an
> authorization problem under some special situations (switching from
> anonymous user to authenticated one...); however the issue is that,
> when the "createUserLogin" is called, if the attribute include-user-
> login is not set to false the manually passed in "system" user is
> overwritten by the user in the context.
I just traced through the code to try to figure out why this might be
happening, and either I don't understand what you're describing, or I
just can't find why this would happen.
In the implementation of the service
(LoginServices.java:createUserLogin) I don't see anything that would
do this. In the service definition I don't see it either because the
service definition does not have a "userLogin" parameter going out.
Have you traced down the code that would be putting the user in the
context? Also, do you mean by this the context of the current service
call (ie for other services called by ECA rules, etc), or in the
result (returned) Map of the service, or somewhere else?
I apologize for pushing on the detail, I may just be too brain-dead
to see it right now.
-David
> Should I commit this patch?
> Or, in general, would be better, even if include-user-login is
> true, to set the user login only if one is not already there in the
> service in map?
>
> Jacopo
> Index: applications/party/script/org/ofbiz/party/party/
> PartySimpleMethods.xml
> ===================================================================
> --- applications/party/script/org/ofbiz/party/party/
> PartySimpleMethods.xml (revisione 499802)
> +++ applications/party/script/org/ofbiz/party/party/
> PartySimpleMethods.xml (copia locale)
> @@ -85,7 +85,7 @@
> <field-map field-name="userLoginId" value="system"/>
> </entity-one>
>
> - <call-service service-name="createUserLogin" in-map-
> name="createUlInMap"/>
> + <call-service service-name="createUserLogin" in-map-
> name="createUlInMap" include-user-login="false"/>
> <entity-one entity-name="UserLogin" value-
> name="newUserLogin"/>
>
> <field-to-result field-name="newUserLogin"/>