Re: svn commit: r1813679 - in /ofbiz/ofbiz-framework/trunk: ./ framework/common/groovyScripts/ framework/webapp/src/main/java/org/apache/ofbiz/webapp/control/

2018-01-19 Thread Jacques Le Roux

Hi Jinghai,

Actually both are accepted. I did not find a place where this is wrote, but you 
can try both work.

Anyway I changed at revision: 1821600

Jacques


Le 18/01/2018 à 06:49, Shi Jinghai a écrit :

Should it be "Authorization"? z, not s. If s is right for some specific environments, we can add an 
additional step to check "Authorisation" if getting "Authorization" header failed.

-邮件原件-
发件人: Jacques Le Roux [mailto:jacques.le.r...@les7arts.com]
发送时间: 2018年1月16日 18:52
收件人: dev@ofbiz.apache.org
主题: Re: svn commit: r1813679 - in /ofbiz/ofbiz-framework/trunk: ./ 
framework/common/groovyScripts/ 
framework/webapp/src/main/java/org/apache/ofbiz/webapp/control/

Thanks Deepak,

Ha indeed, I see your point now.

Sorry for the confusion!

Jacques


Le 16/01/2018 à 11:30, Deepak Dixit a écrit :

Thanks Jacques for detail,
but I think name is not always Authorisation in code we are having
lots of request.getHeader usage and its breaks their usage.
I'll confirm and reply here (just for reference.)

Need to backport this to 17.12 as well.

Thanks & Regards
--
Deepak Dixit
www.hotwaxsystems.com
www.hotwax.co

On Tue, Jan 16, 2018 at 3:26 PM, Jacques Le Roux <
jacques.le.r...@les7arts.com> wrote:


Le 16/01/2018 à 09:53, Deepak Dixit a écrit :


+return super.getHeader("Authorisation");

I think this should be

return super.getHeader(name);


Thanks Deepak,

Actually let me explain the context here (maybe not for you but at
large) In the case of ExternalLoginKeysManager, it's always
"Authorisation". It's explained there
https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/Authorizati
on https://en.wikipedia.org/wiki/Basic_access_authentication
And it's the only usage of the wrapper so far.

Note that in the case of JWT token used in OAuth 2 you normally need
to use a bearer token
https://www.google.fr/search?q=http+authorization+header+bearer&ie=UT
F-8 But in the case I committed it was not necessary (it's not OAuth
2, just a JWT token) and I must say I got issue trying to encode
things with it

Anyway you are right, why not using name there, it will not change
things, and the wrapper idea could be then reused/refactored when
adding other related features, will do :)

Jacques






Re: svn commit: r1813679 - in /ofbiz/ofbiz-framework/trunk: ./ framework/common/groovyScripts/ framework/webapp/src/main/java/org/apache/ofbiz/webapp/control/

2018-01-18 Thread Shi Jinghai
Love you. :)

-邮件原件-
发件人: Jacques Le Roux [mailto:jacques.le.r...@les7arts.com] 
发送时间: 2018年1月18日 21:02
收件人: dev@ofbiz.apache.org
主题: Re: svn commit: r1813679 - in /ofbiz/ofbiz-framework/trunk: ./ 
framework/common/groovyScripts/ 
framework/webapp/src/main/java/org/apache/ofbiz/webapp/control/

You are right Jinghai,

This got unnoticed because it's only used internally. But the right word is 
indeed "Authorization"

I'll change that. It will have no side effects because this is only used 
temporarily during the translation to the other server.

It would be misleading to keep it as is. Because in the general case it's only 
"Authorization" and not "Authorisation"

I used "Authorisation" because of my intuitive preference for British 
English[1] I must say due of my French background[2],  a bit pedantic... phew ;)

Thanks!

Jacques

[1] https://www.etymonline.com/word/authorisation

[2] https://fr.wiktionary.org/wiki/autoriser (so the US are right finally: 
https://en.wiktionary.org/wiki/auctorizare)

Le 18/01/2018 à 06:49, Shi Jinghai a écrit :
> Should it be "Authorization"? z, not s. If s is right for some specific 
> environments, we can add an additional step to check "Authorisation" if 
> getting "Authorization" header failed.
>
> -邮件原件-
> 发件人: Jacques Le Roux [mailto:jacques.le.r...@les7arts.com]
> 发送时间: 2018年1月16日 18:52
> 收件人: dev@ofbiz.apache.org
> 主题: Re: svn commit: r1813679 - in /ofbiz/ofbiz-framework/trunk: ./ 
> framework/common/groovyScripts/ 
> framework/webapp/src/main/java/org/apache/ofbiz/webapp/control/
>
> Thanks Deepak,
>
> Ha indeed, I see your point now.
>
> Sorry for the confusion!
>
> Jacques
>
>
> Le 16/01/2018 à 11:30, Deepak Dixit a écrit :
>> Thanks Jacques for detail,
>> but I think name is not always Authorisation in code we are having 
>> lots of request.getHeader usage and its breaks their usage.
>> I'll confirm and reply here (just for reference.)
>>
>> Need to backport this to 17.12 as well.
>>
>> Thanks & Regards
>> --
>> Deepak Dixit
>> www.hotwaxsystems.com
>> www.hotwax.co
>>
>> On Tue, Jan 16, 2018 at 3:26 PM, Jacques Le Roux < 
>> jacques.le.r...@les7arts.com> wrote:
>>
>>> Le 16/01/2018 à 09:53, Deepak Dixit a écrit :
>>>
>>>> +return super.getHeader("Authorisation");
>>>>> I think this should be
>>>> return super.getHeader(name);
>>>>
>>> Thanks Deepak,
>>>
>>> Actually let me explain the context here (maybe not for you but at
>>> large) In the case of ExternalLoginKeysManager, it's always 
>>> "Authorisation". It's explained there 
>>> https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/Authorizat
>>> i on https://en.wikipedia.org/wiki/Basic_access_authentication
>>> And it's the only usage of the wrapper so far.
>>>
>>> Note that in the case of JWT token used in OAuth 2 you normally need 
>>> to use a bearer token 
>>> https://www.google.fr/search?q=http+authorization+header+bearer&ie=U
>>> T
>>> F-8 But in the case I committed it was not necessary (it's not OAuth 
>>> 2, just a JWT token) and I must say I got issue trying to encode 
>>> things with it
>>>
>>> Anyway you are right, why not using name there, it will not change 
>>> things, and the wrapper idea could be then reused/refactored when 
>>> adding other related features, will do :)
>>>
>>> Jacques
>>>
>>>



Re: svn commit: r1813679 - in /ofbiz/ofbiz-framework/trunk: ./ framework/common/groovyScripts/ framework/webapp/src/main/java/org/apache/ofbiz/webapp/control/

2018-01-18 Thread Jacques Le Roux

You are right Jinghai,

This got unnoticed because it's only used internally. But the right word is indeed 
"Authorization"

I'll change that. It will have no side effects because this is only used 
temporarily during the translation to the other server.

It would be misleading to keep it as is. Because in the general case it's only 
"Authorization" and not "Authorisation"

I used "Authorisation" because of my intuitive preference for British 
English[1] I must say due of my French background[2],  a bit pedantic... phew ;)

Thanks!

Jacques

[1] https://www.etymonline.com/word/authorisation

[2] https://fr.wiktionary.org/wiki/autoriser (so the US are right finally: 
https://en.wiktionary.org/wiki/auctorizare)

Le 18/01/2018 à 06:49, Shi Jinghai a écrit :

Should it be "Authorization"? z, not s. If s is right for some specific environments, we can add an 
additional step to check "Authorisation" if getting "Authorization" header failed.

-邮件原件-
发件人: Jacques Le Roux [mailto:jacques.le.r...@les7arts.com]
发送时间: 2018年1月16日 18:52
收件人: dev@ofbiz.apache.org
主题: Re: svn commit: r1813679 - in /ofbiz/ofbiz-framework/trunk: ./ 
framework/common/groovyScripts/ 
framework/webapp/src/main/java/org/apache/ofbiz/webapp/control/

Thanks Deepak,

Ha indeed, I see your point now.

Sorry for the confusion!

Jacques


Le 16/01/2018 à 11:30, Deepak Dixit a écrit :

Thanks Jacques for detail,
but I think name is not always Authorisation in code we are having
lots of request.getHeader usage and its breaks their usage.
I'll confirm and reply here (just for reference.)

Need to backport this to 17.12 as well.

Thanks & Regards
--
Deepak Dixit
www.hotwaxsystems.com
www.hotwax.co

On Tue, Jan 16, 2018 at 3:26 PM, Jacques Le Roux <
jacques.le.r...@les7arts.com> wrote:


Le 16/01/2018 à 09:53, Deepak Dixit a écrit :


+return super.getHeader("Authorisation");

I think this should be

return super.getHeader(name);


Thanks Deepak,

Actually let me explain the context here (maybe not for you but at
large) In the case of ExternalLoginKeysManager, it's always
"Authorisation". It's explained there
https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/Authorizati
on https://en.wikipedia.org/wiki/Basic_access_authentication
And it's the only usage of the wrapper so far.

Note that in the case of JWT token used in OAuth 2 you normally need
to use a bearer token
https://www.google.fr/search?q=http+authorization+header+bearer&ie=UT
F-8 But in the case I committed it was not necessary (it's not OAuth
2, just a JWT token) and I must say I got issue trying to encode
things with it

Anyway you are right, why not using name there, it will not change
things, and the wrapper idea could be then reused/refactored when
adding other related features, will do :)

Jacques






Re: svn commit: r1813679 - in /ofbiz/ofbiz-framework/trunk: ./ framework/common/groovyScripts/ framework/webapp/src/main/java/org/apache/ofbiz/webapp/control/

2018-01-17 Thread Shi Jinghai
Should it be "Authorization"? z, not s. If s is right for some specific 
environments, we can add an additional step to check "Authorisation" if getting 
"Authorization" header failed.

-邮件原件-
发件人: Jacques Le Roux [mailto:jacques.le.r...@les7arts.com] 
发送时间: 2018年1月16日 18:52
收件人: dev@ofbiz.apache.org
主题: Re: svn commit: r1813679 - in /ofbiz/ofbiz-framework/trunk: ./ 
framework/common/groovyScripts/ 
framework/webapp/src/main/java/org/apache/ofbiz/webapp/control/

Thanks Deepak,

Ha indeed, I see your point now.

Sorry for the confusion!

Jacques


Le 16/01/2018 à 11:30, Deepak Dixit a écrit :
> Thanks Jacques for detail,
> but I think name is not always Authorisation in code we are having 
> lots of request.getHeader usage and its breaks their usage.
> I'll confirm and reply here (just for reference.)
>
> Need to backport this to 17.12 as well.
>
> Thanks & Regards
> --
> Deepak Dixit
> www.hotwaxsystems.com
> www.hotwax.co
>
> On Tue, Jan 16, 2018 at 3:26 PM, Jacques Le Roux < 
> jacques.le.r...@les7arts.com> wrote:
>
>> Le 16/01/2018 à 09:53, Deepak Dixit a écrit :
>>
>>> +return super.getHeader("Authorisation");
>>>> I think this should be
>>> return super.getHeader(name);
>>>
>> Thanks Deepak,
>>
>> Actually let me explain the context here (maybe not for you but at 
>> large) In the case of ExternalLoginKeysManager, it's always 
>> "Authorisation". It's explained there 
>> https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/Authorizati
>> on https://en.wikipedia.org/wiki/Basic_access_authentication
>> And it's the only usage of the wrapper so far.
>>
>> Note that in the case of JWT token used in OAuth 2 you normally need 
>> to use a bearer token
>> https://www.google.fr/search?q=http+authorization+header+bearer&ie=UT
>> F-8 But in the case I committed it was not necessary (it's not OAuth 
>> 2, just a JWT token) and I must say I got issue trying to encode 
>> things with it
>>
>> Anyway you are right, why not using name there, it will not change 
>> things, and the wrapper idea could be then reused/refactored when 
>> adding other related features, will do :)
>>
>> Jacques
>>
>>



Re: svn commit: r1813679 - in /ofbiz/ofbiz-framework/trunk: ./ framework/common/groovyScripts/ framework/webapp/src/main/java/org/apache/ofbiz/webapp/control/

2018-01-16 Thread Jacques Le Roux

Thanks Deepak,

Ha indeed, I see your point now.

Sorry for the confusion!

Jacques


Le 16/01/2018 à 11:30, Deepak Dixit a écrit :

Thanks Jacques for detail,
but I think name is not always Authorisation in code we are having lots of
request.getHeader usage and its breaks their usage.
I'll confirm and reply here (just for reference.)

Need to backport this to 17.12 as well.

Thanks & Regards
--
Deepak Dixit
www.hotwaxsystems.com
www.hotwax.co

On Tue, Jan 16, 2018 at 3:26 PM, Jacques Le Roux <
jacques.le.r...@les7arts.com> wrote:


Le 16/01/2018 à 09:53, Deepak Dixit a écrit :


+return super.getHeader("Authorisation");

I think this should be

return super.getHeader(name);


Thanks Deepak,

Actually let me explain the context here (maybe not for you but at large)
In the case of ExternalLoginKeysManager, it's always "Authorisation". It's
explained there
https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/Authorization
https://en.wikipedia.org/wiki/Basic_access_authentication
And it's the only usage of the wrapper so far.

Note that in the case of JWT token used in OAuth 2 you normally need to
use a bearer token
https://www.google.fr/search?q=http+authorization+header+bearer&ie=UTF-8
But in the case I committed it was not necessary (it's not OAuth 2, just a
JWT token) and I must say I got issue trying to encode things with it

Anyway you are right, why not using name there, it will not change things,
and the wrapper idea could be then reused/refactored when adding other
related features, will do :)

Jacques






Re: svn commit: r1813679 - in /ofbiz/ofbiz-framework/trunk: ./ framework/common/groovyScripts/ framework/webapp/src/main/java/org/apache/ofbiz/webapp/control/

2018-01-16 Thread Deepak Dixit
Thanks Jacques for detail,
but I think name is not always Authorisation in code we are having lots of
request.getHeader usage and its breaks their usage.
I'll confirm and reply here (just for reference.)

Need to backport this to 17.12 as well.

Thanks & Regards
--
Deepak Dixit
www.hotwaxsystems.com
www.hotwax.co

On Tue, Jan 16, 2018 at 3:26 PM, Jacques Le Roux <
jacques.le.r...@les7arts.com> wrote:

> Le 16/01/2018 à 09:53, Deepak Dixit a écrit :
>
>> +return super.getHeader("Authorisation");
>>>
>>> I think this should be
>>
>> return super.getHeader(name);
>>
> Thanks Deepak,
>
> Actually let me explain the context here (maybe not for you but at large)
> In the case of ExternalLoginKeysManager, it's always "Authorisation". It's
> explained there
> https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/Authorization
> https://en.wikipedia.org/wiki/Basic_access_authentication
> And it's the only usage of the wrapper so far.
>
> Note that in the case of JWT token used in OAuth 2 you normally need to
> use a bearer token
> https://www.google.fr/search?q=http+authorization+header+bearer&ie=UTF-8
> But in the case I committed it was not necessary (it's not OAuth 2, just a
> JWT token) and I must say I got issue trying to encode things with it
>
> Anyway you are right, why not using name there, it will not change things,
> and the wrapper idea could be then reused/refactored when adding other
> related features, will do :)
>
> Jacques
>
>


Re: svn commit: r1813679 - in /ofbiz/ofbiz-framework/trunk: ./ framework/common/groovyScripts/ framework/webapp/src/main/java/org/apache/ofbiz/webapp/control/

2018-01-16 Thread Jacques Le Roux

Le 16/01/2018 à 09:53, Deepak Dixit a écrit :

+return super.getHeader("Authorisation");


I think this should be

return super.getHeader(name);

Thanks Deepak,

Actually let me explain the context here (maybe not for you but at large)
In the case of ExternalLoginKeysManager, it's always "Authorisation". It's 
explained there
https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/Authorization
https://en.wikipedia.org/wiki/Basic_access_authentication
And it's the only usage of the wrapper so far.

Note that in the case of JWT token used in OAuth 2 you normally need to use a 
bearer token
https://www.google.fr/search?q=http+authorization+header+bearer&ie=UTF-8
But in the case I committed it was not necessary (it's not OAuth 2, just a JWT 
token) and I must say I got issue trying to encode things with it

Anyway you are right, why not using name there, it will not change things, and the wrapper idea could be then reused/refactored when adding other 
related features, will do :)


Jacques



Re: svn commit: r1813679 - in /ofbiz/ofbiz-framework/trunk: ./ framework/common/groovyScripts/ framework/webapp/src/main/java/org/apache/ofbiz/webapp/control/

2018-01-16 Thread Deepak Dixit
Hi Jacques,



> Modified: ofbiz/ofbiz-framework/trunk/framework/webapp/src/main/
> java/org/apache/ofbiz/webapp/control/ContextFilter.java
> URL: http://svn.apache.org/viewvc/ofbiz/ofbiz-framework/trunk/
> framework/webapp/src/main/java/org/apache/ofbiz/webapp/
> control/ContextFilter.java?rev=1813679&r1=1813678&r2=1813679&view=diff
> 
> ==
> --- ofbiz/ofbiz-framework/trunk/framework/webapp/src/main/
> java/org/apache/ofbiz/webapp/control/ContextFilter.java (original)
> +++ ofbiz/ofbiz-framework/trunk/framework/webapp/src/main/
> java/org/apache/ofbiz/webapp/control/ContextFilter.java Sun Oct 29
> 11:02:00 2017
> @@ -18,11 +18,8 @@
>   
> ***/
>  package org.apache.ofbiz.webapp.control;
>
> -import static org.apache.ofbiz.base.util.UtilGenerics.checkMap;
> -
>  import java.io.IOException;
>  import java.util.Enumeration;
> -import java.util.Map;
>
>  import javax.servlet.Filter;
>  import javax.servlet.FilterChain;
> @@ -31,13 +28,12 @@ import javax.servlet.ServletException;
>  import javax.servlet.ServletRequest;
>  import javax.servlet.ServletResponse;
>  import javax.servlet.http.HttpServletRequest;
> +import javax.servlet.http.HttpServletRequestWrapper;
>  import javax.servlet.http.HttpServletResponse;
>
>  import org.apache.ofbiz.base.util.Debug;
> -import org.apache.ofbiz.base.util.StringUtil;
>  import org.apache.ofbiz.base.util.UtilGenerics;
>  import org.apache.ofbiz.base.util.UtilHttp;
> -import org.apache.ofbiz.base.util.UtilObject;
>  import org.apache.ofbiz.base.util.UtilValidate;
>  import org.apache.ofbiz.entity.Delegator;
>  import org.apache.ofbiz.entity.DelegatorFactory;
> @@ -192,8 +188,29 @@ public class ContextFilter implements Fi
>  }
>  }
>
> +HttpServletRequestWrapper wrapper = new 
> HttpServletRequestWrapper(httpRequest)
> {
> +@Override
> +public String getHeader(String name) {
> +String externalServerUserLoginId = request.getParameter(
> ExternalLoginKeysManager.EXTERNAL_SERVER_LOGIN_KEY);
> +String value = null;
> +if (externalServerUserLoginId != null) {
> +// ExternalLoginKeysManager .createJwt() arguments in
> order:
> +// id an Id, I suggest userLoginId
> +// issuer is who/what issued the token. I suggest the
> server DNS
> +// subject is the subject of the token. I suggest the
> destination webapp
> +// timeToLive is the token maximum duration
> +String webAppName = UtilHttp.getApplicationName(
> httpRequest);
> +String dnsName = ExternalLoginKeysManager.
> getExternalServerName(httpRequest);
> +long timeToLive = ExternalLoginKeysManager.
> getJwtTokenTimeToLive(httpRequest);
> +// We would need a Bearer token (in Authorisation
> request header) if we were using Oauth2, here we don't, so no Bearer
> +value = 
> ExternalLoginKeysManager.createJwt(externalServerUserLoginId,
> dnsName, webAppName , timeToLive);
> +}
> +if (value != null) return value;
> +return super.getHeader("Authorisation");
>


I think this should be

return super.getHeader(name);
>





> +}
> +};
>  // we're done checking; continue on
> -chain.doFilter(request, httpResponse);
> +chain.doFilter(wrapper, httpResponse);
>  }
>
>  /**
>
>
>


Thanks & Regards
--
Deepak Dixit
www.hotwaxsystems.com
www.hotwax.co


Re: svn commit: r1813679 - in /ofbiz/ofbiz-framework/trunk: ./ framework/common/groovyScripts/ framework/webapp/src/main/java/org/apache/ofbiz/webapp/control/

2017-10-30 Thread Jacques Le Roux

Oops, thanks Deepak :)

Fixed

Jacques


Le 30/10/2017 à 06:43, Deepak Dixit a écrit :

Hi Jacques,


+if (delegator != null && 
"Y".equals(EntityUtilProperties.getPropertyValue("embisphere",
"use-external-server", "Y", delegator))) {


I think ti should be security instead of embisphere :)





Re: svn commit: r1813679 - in /ofbiz/ofbiz-framework/trunk: ./ framework/common/groovyScripts/ framework/webapp/src/main/java/org/apache/ofbiz/webapp/control/

2017-10-29 Thread Deepak Dixit
Hi Jacques,




> +if (delegator != null && 
> "Y".equals(EntityUtilProperties.getPropertyValue("embisphere",
> "use-external-server", "Y", delegator))) {
>

I think ti should be security instead of embisphere :)