OFBIZ-10307: Navigate from a domain to another with automated signed in authentication
Locked
123
123
Re: OFBIZ-10307: Navigate from a domain to another with automated signed in authentication
 
Administrator
|
Le 21/08/2018 à 10:41, Michael Brohl a écrit :
> Jacques, > > Am 21.08.18 um 10:27 schrieb Jacques Le Roux: >> You are totaly right on this Michael >> >> I do treat the trunk as much care as possible as the evolution of this work shows. This work is now ready and good, so I'll soon commit it. > > So you want to ignore raised concerns and commit despite of them? > > Jacques |
Re: OFBIZ-10307: Navigate from a domain to another with automated signed in authentication
|
I mean the concerns which already were raised, e.g. by Taher and Scott,
who made you rethink your work you were going to commit. You cannot simply ignore them and say you will commit your work. The history of this issue clearly shows that it is necessary to have a close look at this work. So please wait until committers have had enough time to review the updated/corrected patches. Thanks, Michael Am 21.08.18 um 11:40 schrieb Jacques Le Roux: > Le 21/08/2018 à 10:41, Michael Brohl a écrit : >> Jacques, >> >> Am 21.08.18 um 10:27 schrieb Jacques Le Roux: >>> You are totaly right on this Michael >>> >>> I do treat the trunk as much care as possible as the evolution of >>> this work shows. This work is now ready and good, so I'll soon >>> commit it. >> >> So you want to ignore raised concerns and commit despite of them? >> >> > What are your raised concerns? Are they technically sustained? > > Jacques > |
smime.p7s (5K)
Re: OFBIZ-10307: Navigate from a domain to another with automated signed in authentication
|
Administrator
|
Le 21/08/2018 à 12:20, Michael Brohl a écrit :
> The history of this issue clearly shows that it is necessary to have a close look at this work. So please wait until committers have had enough time > to review the updated/corrected patches. OK, how much time do you estimate for that? Note that the last change, after Scott's review is very simple, so I hope I'll not have to wait for months for other reviews. Jacques |
Re: OFBIZ-10307: Navigate from a domain to another with automated signed in authentication
|
|
Hello Jacques, I will take the time to review it, please give me one week :)
Nicolas On 21/08/2018 13:48, Jacques Le Roux wrote: > Le 21/08/2018 à 12:20, Michael Brohl a écrit : >> The history of this issue clearly shows that it is necessary to have >> a close look at this work. So please wait until committers have had >> enough time to review the updated/corrected patches. > OK, how much time do you estimate for that? > > Note that the last change, after Scott's review is very simple, so I > hope I'll not have to wait for months for other reviews. > > Jacques > > |
Re: OFBIZ-10307: Navigate from a domain to another with automated signed in authentication
|
Administrator
|
Hi Nicolas,
One week is OK with me. You will see, it's plain simple now. Thanks Jacques Le 21/08/2018 à 14:11, Nicolas Malin a écrit : > Hello Jacques, I will take the time to review it, please give me one week :) > > Nicolas > > > On 21/08/2018 13:48, Jacques Le Roux wrote: >> Le 21/08/2018 à 12:20, Michael Brohl a écrit : >>> The history of this issue clearly shows that it is necessary to have a close look at this work. So please wait until committers have had enough >>> time to review the updated/corrected patches. >> OK, how much time do you estimate for that? >> >> Note that the last change, after Scott's review is very simple, so I hope I'll not have to wait for months for other reviews. >> >> Jacques >> >> > > |
Re: OFBIZ-10307: Navigate from a domain to another with automated signed in authentication
|
Administrator
|
In reply to this post by Shi Jinghai-3
Hi Jinghai,
You maybe read it already, what did redislabs very recently might be affecting you https://redislabs.com/community/commons-clause/ Jacques Le 15/08/2018 à 14:37, Shi Jinghai a écrit : > Dear Jacques, > > On how to store the Tokens, as a token is a key, value is the UserLogin entity and/or other info, a key-value db, Redis[1] is a good choice. Redis is no.7 in db ranking in Aug 2018[2], becomes more and more popular. Goldman Sachs invested Redis team in last year[3]. It's common view now in China that Redis is better than any others including Gemfire of Pivotal, the railway ticket system of China replaced its 3 Gemfire clusters with 3 Redis clusters last year and then there are much less complains on how difficulties to buy spring festival tickets. > > Mr. Dai Haipeng contributed a Redis component in Jira[4]. > > [1] https://redis.io/ > [2] https://db-engines.com/en/ranking > [3] https://redislabs.com/press/redis-labs-secures-44-million-funding-led-goldman-sachs-private-capital-investing-strengthen-database-leadership/ > [4] https://issues.apache.org/jira/browse/OFBIZ-9829 > > BTW, I'll try to review the patches. > > Kind Regards, > > Shi Jinghai > > -----邮件原件----- > 发件人: Jacques Le Roux [mailto:[hidden email]] > 发送时间: 2018年8月15日 15:09 > 收件人: [hidden email] > 主题: OFBIZ-10307: Navigate from a domain to another with automated signed in authentication > > Hi, > > Some time ago I created https://issues.apache.org/jira/browse/OFBIZ-10307. > > I asked for reviews but only Taher answered and he asked to know the goal of this new feature. > > It was actually developed for a client who needed to get from one OFBiz instance on a server (on a domain) to another OFBiz instance on another > server (on another domain) without having to sign up between the 2 while keeping things secure. > > There could be many reasons why you want to split OFBiz application on servers. In their case it was for performance issues. > > The technology used is as secure as possible. Like OAuth 2.0 it uses a token but it does not need a middle authorization server (think to two-factor > authentication) because it's only for OFBiz instances of the same version. > > To commit this work we need 1st to agree an commit the work done by Deepak at OFBIZ-9833 "Token Based Authentication" that I use in my last patch. > > For me there is only one question outstanding: how to store the Token secret. But this should not prevent us to commit Deepak's work. > > It's now a long time (9 months) since I started this work. And my last patch is ready for a month. > > I crossed several issues which are now all resolved. So please review and answer to this thread. > > Without negative comments well argumented I'll commit both OFBIZ-9833 and OFBIZ-10307 in a week. You can always test and review later, we use RTC. > > Also a veto on a commit is always possible... Of course, as ever, a good consensus is preferred. > > Let me know if you need more information about the goal. For the technical details I think I already provided them the in OFBIZ-10307. > > Jacques > |
Re: OFBIZ-10307: Navigate from a domain to another with automated signed in authentication
|
|
In reply to this post by Jacques Le Roux
Hi Michael,
Release means from private to public, i.e. upload to github. -----邮件原件----- 发件人: Michael Brohl [mailto:[hidden email]] 发送时间: 2018年8月21日 15:24 收件人: [hidden email] 主题: Re: OFBIZ-10307: Navigate from a domain to another with automated signed in authentication Hi Shi, what do you mean when you say you are going to release the plugin? Where will this take place? Regards, Michael Am 19.08.18 um 22:00 schrieb Shi Jinghai: > Thanks Jacques! > > If so, I'll release a CAS plugin to make OFBiz offer OAuth2 alliance next week. I have cas 4.2.x version running in production environment, I'll upgrade it to cas 5.2.x and then release it. > > > > -----邮件原件----- > 发件人: Jacques Le Roux [mailto:[hidden email]] > 发送时间: 2018年8月19日 18:34 > 收件人: [hidden email] > 主题: Re: OFBIZ-10307: Navigate from a domain to another with automated signed in authentication > > Hi Jinghai, > > Actually I did not pick auth0 (not to be confused with https://en.wikipedia.org/wiki/OAuth) nor https://oauth.net/2/ because those need a central > Identify server (as is the SAML protocol). > > I simply send a JWT token: https://en.wikipedia.org/wiki/JSON_Web_Token and https://jwt.io/ to > > Please refer to OFBIZ-10307 "Navigate from a domain to another with automated signed in authentication" > > Thanks for your interest. > > Jacques > > > Le 17/08/2018 à 09:02, Shi Jinghai a écrit : >> Hi Jacques, >> >> OK, I think the redis topic is jumped to next step. >> >> I have read the patches carelly, as a fan of Apereo CAS[1], I wonder why choose auth0[2] rather than CAS. And is the implement OAuth2 alliance? >> >> [1] https://github.com/apereo/cas >> [2] https://auth0.com/ >> >> Kind Regards, >> >> Shi Jinghai >> >> >> -----邮件原件----- >> 发件人: Jacques Le Roux [mailto:[hidden email]] >> 发送时间: 2018年8月16日 2:08 >> 收件人: [hidden email] >> 主题: Re: OFBIZ-10307: Navigate from a domain to another with automated signed in authentication >> >> Hi Jinghai, >> >> The problem with the token master secret key is to guarantee its secrecy at max. >> >> We already discussed different solutions at https://s.apache.org/7yyR and https://s.apache.org/IBDM >> >> How is Redis more secure than Postgres for storing values? >> >> Thanks >> >> Jacques >> >> >> Le 15/08/2018 à 14:37, Shi Jinghai a écrit : >>> Dear Jacques, >>> >>> On how to store the Tokens, as a token is a key, value is the UserLogin entity and/or other info, a key-value db, Redis[1] is a good choice. Redis is no.7 in db ranking in Aug 2018[2], becomes more and more popular. Goldman Sachs invested Redis team in last year[3]. It's common view now in China that Redis is better than any others including Gemfire of Pivotal, the railway ticket system of China replaced its 3 Gemfire clusters with 3 Redis clusters last year and then there are much less complains on how difficulties to buy spring festival tickets. >>> >>> Mr. Dai Haipeng contributed a Redis component in Jira[4]. >>> >>> [1] https://redis.io/ >>> [2] https://db-engines.com/en/ranking >>> [3] https://redislabs.com/press/redis-labs-secures-44-million-funding-led-goldman-sachs-private-capital-investing-strengthen-database-leadership/ >>> [4] https://issues.apache.org/jira/browse/OFBIZ-9829 >>> >>> BTW, I'll try to review the patches. >>> >>> Kind Regards, >>> >>> Shi Jinghai >>> >>> -----邮件原件----- >>> 发件人: Jacques Le Roux [mailto:[hidden email]] >>> 发送时间: 2018年8月15日 15:09 >>> 收件人: [hidden email] >>> 主题: OFBIZ-10307: Navigate from a domain to another with automated signed in authentication >>> >>> Hi, >>> >>> Some time ago I created https://issues.apache.org/jira/browse/OFBIZ-10307. >>> >>> I asked for reviews but only Taher answered and he asked to know the goal of this new feature. >>> >>> It was actually developed for a client who needed to get from one OFBiz instance on a server (on a domain) to another OFBiz instance on another >>> server (on another domain) without having to sign up between the 2 while keeping things secure. >>> >>> There could be many reasons why you want to split OFBiz application on servers. In their case it was for performance issues. >>> >>> The technology used is as secure as possible. Like OAuth 2.0 it uses a token but it does not need a middle authorization server (think to two-factor >>> authentication) because it's only for OFBiz instances of the same version. >>> >>> To commit this work we need 1st to agree an commit the work done by Deepak at OFBIZ-9833 "Token Based Authentication" that I use in my last patch. >>> >>> For me there is only one question outstanding: how to store the Token secret. But this should not prevent us to commit Deepak's work. >>> >>> It's now a long time (9 months) since I started this work. And my last patch is ready for a month. >>> >>> I crossed several issues which are now all resolved. So please review and answer to this thread. >>> >>> Without negative comments well argumented I'll commit both OFBIZ-9833 and OFBIZ-10307 in a week. You can always test and review later, we use RTC. >>> >>> Also a veto on a commit is always possible... Of course, as ever, a good consensus is preferred. >>> >>> Let me know if you need more information about the goal. For the technical details I think I already provided them the in OFBIZ-10307. >>> >>> Jacques >>> |
Re: OFBIZ-10307: Navigate from a domain to another with automated signed in authentication
|
|
In reply to this post by Jacques Le Roux
Thank you Jacques!
Perhaps we can build a cloud-ready (SAAS version) OFBiz and add such a common clause :) -----邮件原件----- 发件人: Jacques Le Roux [mailto:[hidden email]] 发送时间: 2018年8月22日 12:11 收件人: [hidden email] 主题: Re: OFBIZ-10307: Navigate from a domain to another with automated signed in authentication Hi Jinghai, You maybe read it already, what did redislabs very recently might be affecting you https://redislabs.com/community/commons-clause/ Jacques Le 15/08/2018 à 14:37, Shi Jinghai a écrit : > Dear Jacques, > > On how to store the Tokens, as a token is a key, value is the UserLogin entity and/or other info, a key-value db, Redis[1] is a good choice. Redis is no.7 in db ranking in Aug 2018[2], becomes more and more popular. Goldman Sachs invested Redis team in last year[3]. It's common view now in China that Redis is better than any others including Gemfire of Pivotal, the railway ticket system of China replaced its 3 Gemfire clusters with 3 Redis clusters last year and then there are much less complains on how difficulties to buy spring festival tickets. > > Mr. Dai Haipeng contributed a Redis component in Jira[4]. > > [1] https://redis.io/ > [2] https://db-engines.com/en/ranking > [3] https://redislabs.com/press/redis-labs-secures-44-million-funding-led-goldman-sachs-private-capital-investing-strengthen-database-leadership/ > [4] https://issues.apache.org/jira/browse/OFBIZ-9829 > > BTW, I'll try to review the patches. > > Kind Regards, > > Shi Jinghai > > -----邮件原件----- > 发件人: Jacques Le Roux [mailto:[hidden email]] > 发送时间: 2018年8月15日 15:09 > 收件人: [hidden email] > 主题: OFBIZ-10307: Navigate from a domain to another with automated signed in authentication > > Hi, > > Some time ago I created https://issues.apache.org/jira/browse/OFBIZ-10307. > > I asked for reviews but only Taher answered and he asked to know the goal of this new feature. > > It was actually developed for a client who needed to get from one OFBiz instance on a server (on a domain) to another OFBiz instance on another > server (on another domain) without having to sign up between the 2 while keeping things secure. > > There could be many reasons why you want to split OFBiz application on servers. In their case it was for performance issues. > > The technology used is as secure as possible. Like OAuth 2.0 it uses a token but it does not need a middle authorization server (think to two-factor > authentication) because it's only for OFBiz instances of the same version. > > To commit this work we need 1st to agree an commit the work done by Deepak at OFBIZ-9833 "Token Based Authentication" that I use in my last patch. > > For me there is only one question outstanding: how to store the Token secret. But this should not prevent us to commit Deepak's work. > > It's now a long time (9 months) since I started this work. And my last patch is ready for a month. > > I crossed several issues which are now all resolved. So please review and answer to this thread. > > Without negative comments well argumented I'll commit both OFBIZ-9833 and OFBIZ-10307 in a week. You can always test and review later, we use RTC. > > Also a veto on a commit is always possible... Of course, as ever, a good consensus is preferred. > > Let me know if you need more information about the goal. For the technical details I think I already provided them the in OFBIZ-10307. > > Jacques > |
Re: OFBIZ-10307: Navigate from a domain to another with automated signed in authentication
|
Administrator
|
Hi Jinghai,
You can do if you want, but that's not possible in the context of the ASF. Also I'm for the ASL2 license as is, not to add a common clause. Of course if you pay me for that I would :D But I guess it's a joke, isn'it? Jacques Le 22/08/2018 à 14:52, Shi Jinghai a écrit : > Thank you Jacques! > > Perhaps we can build a cloud-ready (SAAS version) OFBiz and add such a common clause :) > > > -----邮件原件----- > 发件人: Jacques Le Roux [mailto:[hidden email]] > 发送时间: 2018年8月22日 12:11 > 收件人: [hidden email] > 主题: Re: OFBIZ-10307: Navigate from a domain to another with automated signed in authentication > > Hi Jinghai, > > You maybe read it already, what did redislabs very recently might be affecting you > > https://redislabs.com/community/commons-clause/ > > Jacques > > > Le 15/08/2018 à 14:37, Shi Jinghai a écrit : >> Dear Jacques, >> >> On how to store the Tokens, as a token is a key, value is the UserLogin entity and/or other info, a key-value db, Redis[1] is a good choice. Redis is no.7 in db ranking in Aug 2018[2], becomes more and more popular. Goldman Sachs invested Redis team in last year[3]. It's common view now in China that Redis is better than any others including Gemfire of Pivotal, the railway ticket system of China replaced its 3 Gemfire clusters with 3 Redis clusters last year and then there are much less complains on how difficulties to buy spring festival tickets. >> >> Mr. Dai Haipeng contributed a Redis component in Jira[4]. >> >> [1] https://redis.io/ >> [2] https://db-engines.com/en/ranking >> [3] https://redislabs.com/press/redis-labs-secures-44-million-funding-led-goldman-sachs-private-capital-investing-strengthen-database-leadership/ >> [4] https://issues.apache.org/jira/browse/OFBIZ-9829 >> >> BTW, I'll try to review the patches. >> >> Kind Regards, >> >> Shi Jinghai >> >> -----邮件原件----- >> 发件人: Jacques Le Roux [mailto:[hidden email]] >> 发送时间: 2018年8月15日 15:09 >> 收件人: [hidden email] >> 主题: OFBIZ-10307: Navigate from a domain to another with automated signed in authentication >> >> Hi, >> >> Some time ago I created https://issues.apache.org/jira/browse/OFBIZ-10307. >> >> I asked for reviews but only Taher answered and he asked to know the goal of this new feature. >> >> It was actually developed for a client who needed to get from one OFBiz instance on a server (on a domain) to another OFBiz instance on another >> server (on another domain) without having to sign up between the 2 while keeping things secure. >> >> There could be many reasons why you want to split OFBiz application on servers. In their case it was for performance issues. >> >> The technology used is as secure as possible. Like OAuth 2.0 it uses a token but it does not need a middle authorization server (think to two-factor >> authentication) because it's only for OFBiz instances of the same version. >> >> To commit this work we need 1st to agree an commit the work done by Deepak at OFBIZ-9833 "Token Based Authentication" that I use in my last patch. >> >> For me there is only one question outstanding: how to store the Token secret. But this should not prevent us to commit Deepak's work. >> >> It's now a long time (9 months) since I started this work. And my last patch is ready for a month. >> >> I crossed several issues which are now all resolved. So please review and answer to this thread. >> >> Without negative comments well argumented I'll commit both OFBIZ-9833 and OFBIZ-10307 in a week. You can always test and review later, we use RTC. >> >> Also a veto on a commit is always possible... Of course, as ever, a good consensus is preferred. >> >> Let me know if you need more information about the goal. For the technical details I think I already provided them the in OFBIZ-10307. >> >> Jacques >> |
|
I reviewed the code and explanation in the beginning of this thread
and I have the following comments: - First, in case of a result.containsKey(ModelService.ERROR_MESSAGE) you are still returning "success". Does not make sense at all. - Also the comments are unnecessary and do not make sense in the same code block - The repetition of the return "success" with a validation if separating them is a sign of a code smell. - The comment you wrote: "Check it's the right tenant in case username and password are the same in different tenants. Not sure this is really useful in the case of external server, does not hurt anyway" is self-defeating. Never put in code you won't use. We have enough mess in the code base. - Calling LoginWorker.doBasicLogin(userLogin, request) after the if/else block does not make sense. The else block guarantees falling into the exception. This whole code block needs refactoring - A testing method needs to be provided in detail. For example I'm not sure how the Javascript portions will execute and when. So some provision of a clear testing mechanism is necessary. - Finally, I'm not sure this feature is needed at all. Why not just assign an LDAP server to take care of OFBiz and non-OFBiz in a single sign-on environment. I see too much code being written for something that might not be of great value and there are other better solutions out there. Using JWT tokens might come with problems [1] that need to be justified before we adopt it. [1] https://en.wikipedia.org/wiki/JSON_Web_Token#Vulnerabilities_and_criticism On Wed, Aug 22, 2018 at 5:22 PM Jacques Le Roux <[hidden email]> wrote: > > Hi Jinghai, > > You can do if you want, but that's not possible in the context of the ASF. > > Also I'm for the ASL2 license as is, not to add a common clause. > > Of course if you pay me for that I would :D But I guess it's a joke, isn'it? > > Jacques > > > Le 22/08/2018 à 14:52, Shi Jinghai a écrit : > > Thank you Jacques! > > > > Perhaps we can build a cloud-ready (SAAS version) OFBiz and add such a common clause :) > > > > > > -----邮件原件----- > > 发件人: Jacques Le Roux [mailto:[hidden email]] > > 发送时间: 2018年8月22日 12:11 > > 收件人: [hidden email] > > 主题: Re: OFBIZ-10307: Navigate from a domain to another with automated signed in authentication > > > > Hi Jinghai, > > > > You maybe read it already, what did redislabs very recently might be affecting you > > > > https://redislabs.com/community/commons-clause/ > > > > Jacques > > > > > > Le 15/08/2018 à 14:37, Shi Jinghai a écrit : > >> Dear Jacques, > >> > >> On how to store the Tokens, as a token is a key, value is the UserLogin entity and/or other info, a key-value db, Redis[1] is a good choice. Redis is no.7 in db ranking in Aug 2018[2], becomes more and more popular. Goldman Sachs invested Redis team in last year[3]. It's common view now in China that Redis is better than any others including Gemfire of Pivotal, the railway ticket system of China replaced its 3 Gemfire clusters with 3 Redis clusters last year and then there are much less complains on how difficulties to buy spring festival tickets. > >> > >> Mr. Dai Haipeng contributed a Redis component in Jira[4]. > >> > >> [1] https://redis.io/ > >> [2] https://db-engines.com/en/ranking > >> [3] https://redislabs.com/press/redis-labs-secures-44-million-funding-led-goldman-sachs-private-capital-investing-strengthen-database-leadership/ > >> [4] https://issues.apache.org/jira/browse/OFBIZ-9829 > >> > >> BTW, I'll try to review the patches. > >> > >> Kind Regards, > >> > >> Shi Jinghai > >> > >> -----邮件原件----- > >> 发件人: Jacques Le Roux [mailto:[hidden email]] > >> 发送时间: 2018年8月15日 15:09 > >> 收件人: [hidden email] > >> 主题: OFBIZ-10307: Navigate from a domain to another with automated signed in authentication > >> > >> Hi, > >> > >> Some time ago I created https://issues.apache.org/jira/browse/OFBIZ-10307. > >> > >> I asked for reviews but only Taher answered and he asked to know the goal of this new feature. > >> > >> It was actually developed for a client who needed to get from one OFBiz instance on a server (on a domain) to another OFBiz instance on another > >> server (on another domain) without having to sign up between the 2 while keeping things secure. > >> > >> There could be many reasons why you want to split OFBiz application on servers. In their case it was for performance issues. > >> > >> The technology used is as secure as possible. Like OAuth 2.0 it uses a token but it does not need a middle authorization server (think to two-factor > >> authentication) because it's only for OFBiz instances of the same version. > >> > >> To commit this work we need 1st to agree an commit the work done by Deepak at OFBIZ-9833 "Token Based Authentication" that I use in my last patch. > >> > >> For me there is only one question outstanding: how to store the Token secret. But this should not prevent us to commit Deepak's work. > >> > >> It's now a long time (9 months) since I started this work. And my last patch is ready for a month. > >> > >> I crossed several issues which are now all resolved. So please review and answer to this thread. > >> > >> Without negative comments well argumented I'll commit both OFBIZ-9833 and OFBIZ-10307 in a week. You can always test and review later, we use RTC. > >> > >> Also a veto on a commit is always possible... Of course, as ever, a good consensus is preferred. > >> > >> Let me know if you need more information about the goal. For the technical details I think I already provided them the in OFBIZ-10307. > >> > >> Jacques > >> > |
Re: OFBIZ-10307: Navigate from a domain to another with automated signed in authentication
|
Administrator
|
Hi Taher,
Thanks for your review, much appreciated. Le 22/08/2018 à 18:42, Taher Alkhateeb a écrit : > I reviewed the code and explanation in the beginning of this thread > and I have the following comments: > > - First, in case of a result.containsKey(ModelService.ERROR_MESSAGE) > you are still returning "success". Does not make sense at all. You are right I should have used "error". I did not notice (actually forgot a last own review after much variations in code) because it's an event preprocessor and no response is expected/handled. Not to bad, in case of error just an EventHandlerException was not thrown. > - Also the comments are unnecessary and do not make sense in the same code block You mean "// Something unexpected happened here", or ? > - The comment you wrote: "Check it's the right tenant in case username > and password are the same in different tenants. Not sure this is > really useful in the case of external server, does not hurt anyway" is > self-defeating. Never put in code you won't use. We have enough mess > in the code base. I must say I initially copied that from ExternalLoginKeysManager::checkExternalLoginKey and adapted it to the context. It's roughly the same code. I let it there because I was then unsure. But when you think about it, there is no reason to not make this check. So I'll simply remove the comment. I should have put a TODO. BTW this is out of subject and I'll start a new thread about it after reading https://www.linkedin.com/pulse/architecture-constraints-end-multi-tenancy-gregor-hohpe/ The subject will be: "Should we keep the multi-tenants feature in OFBiz." I though think the tenants in OFBiz are still useful when you only needs dozens or maybe even few hundreds tenants (begin to be a lot of DBs!). But I faced that with a startup which wanted to handle thousands, if not millions (actually they failed), of tenants, obviously OFBiz can't do that. > - Calling LoginWorker.doBasicLogin(userLogin, request) after the > if/else block does not make sense. The else block guarantees falling > into the exception. This whole code block needs refactoring You are totally right, I have uploaded a new patch where I put the call of LoginWorker.doBasicLogin(userLogin, request); in the positive part of the if and used the same block pattern than above where there is a warning log then returning an error. I was so focused on the other parts of the mechanism, especially to secure it, than I forgot to review the Java part. My bad, thank you again for your review. > - A testing method needs to be provided in detail. For example I'm not > sure how the Javascript portions will execute and when. So some > provision of a clear testing mechanism is necessary. Yep, right, I'll document all the mechanism and how to use it. Pierre even proposed to make a graph in AsciiDoc, not sure I'll be able to, by I'll try. Actually it's only a matter of following what should be in the example component, see last OFBIZ-10307-test from example.patch. I thought it would be enough for devs. I understand it's not, a bit of clear documentation is always welcomed, and I often advocate for it. > - Finally, I'm not sure this feature is needed at all. Why not just > assign an LDAP server to take care of OFBiz and non-OFBiz in a single > sign-on environment. Because you don't need to have (install, maintain, etc.) a LDAP server, or whatnot (CAS, Oauth2, SAML, you name it) if you use this code, it's ready OOTB. > I see too much code being written for something > that might not be of great value and there are other better solutions > out there. Which ones do you think about? And why they are better at doing this specific feature? > Using JWT tokens might come with problems [1] that need to > be justified before we adopt it. That's very interesting, let's check, after reading https://connect2id.com/products/nimbus-jose-jwt/vulnerabilities 1. Never let the JWT header alone drive verification * Not our case, we have the loginUserId ciphered in and checked. The crucial point was how to send the loginUserId securely. 2. Know the algorithms * This part is handled by JWTManager class. We have begun to discuss where to store the secret key at https://s.apache.org/IiE0 3. Use an appropriate key size * This part is handled by JWTManager class, which uses HMAC with SHA-512. And that's OK reading the article Wikipedia refers to.| | |Jacques| > > [1] https://en.wikipedia.org/wiki/JSON_Web_Token#Vulnerabilities_and_criticism > On Wed, Aug 22, 2018 at 5:22 PM Jacques Le Roux > <[hidden email]> wrote: >> Hi Jinghai, >> >> You can do if you want, but that's not possible in the context of the ASF. >> >> Also I'm for the ASL2 license as is, not to add a common clause. >> >> Of course if you pay me for that I would :D But I guess it's a joke, isn'it? >> >> Jacques >> >> >> Le 22/08/2018 à 14:52, Shi Jinghai a écrit : >>> Thank you Jacques! >>> >>> Perhaps we can build a cloud-ready (SAAS version) OFBiz and add such a common clause :) >>> >>> >>> -----邮件原件----- >>> 发件人: Jacques Le Roux [mailto:[hidden email]] >>> 发送时间: 2018年8月22日 12:11 >>> 收件人: [hidden email] >>> 主题: Re: OFBIZ-10307: Navigate from a domain to another with automated signed in authentication >>> >>> Hi Jinghai, >>> >>> You maybe read it already, what did redislabs very recently might be affecting you >>> >>> https://redislabs.com/community/commons-clause/ >>> >>> Jacques >>> >>> >>> Le 15/08/2018 à 14:37, Shi Jinghai a écrit : >>>> Dear Jacques, >>>> >>>> On how to store the Tokens, as a token is a key, value is the UserLogin entity and/or other info, a key-value db, Redis[1] is a good choice. Redis is no.7 in db ranking in Aug 2018[2], becomes more and more popular. Goldman Sachs invested Redis team in last year[3]. It's common view now in China that Redis is better than any others including Gemfire of Pivotal, the railway ticket system of China replaced its 3 Gemfire clusters with 3 Redis clusters last year and then there are much less complains on how difficulties to buy spring festival tickets. >>>> >>>> Mr. Dai Haipeng contributed a Redis component in Jira[4]. >>>> >>>> [1] https://redis.io/ >>>> [2] https://db-engines.com/en/ranking >>>> [3] https://redislabs.com/press/redis-labs-secures-44-million-funding-led-goldman-sachs-private-capital-investing-strengthen-database-leadership/ >>>> [4] https://issues.apache.org/jira/browse/OFBIZ-9829 >>>> >>>> BTW, I'll try to review the patches. >>>> >>>> Kind Regards, >>>> >>>> Shi Jinghai >>>> >>>> -----邮件原件----- >>>> 发件人: Jacques Le Roux [mailto:[hidden email]] >>>> 发送时间: 2018年8月15日 15:09 >>>> 收件人: [hidden email] >>>> 主题: OFBIZ-10307: Navigate from a domain to another with automated signed in authentication >>>> >>>> Hi, >>>> >>>> Some time ago I created https://issues.apache.org/jira/browse/OFBIZ-10307. >>>> >>>> I asked for reviews but only Taher answered and he asked to know the goal of this new feature. >>>> >>>> It was actually developed for a client who needed to get from one OFBiz instance on a server (on a domain) to another OFBiz instance on another >>>> server (on another domain) without having to sign up between the 2 while keeping things secure. >>>> >>>> There could be many reasons why you want to split OFBiz application on servers. In their case it was for performance issues. >>>> >>>> The technology used is as secure as possible. Like OAuth 2.0 it uses a token but it does not need a middle authorization server (think to two-factor >>>> authentication) because it's only for OFBiz instances of the same version. >>>> >>>> To commit this work we need 1st to agree an commit the work done by Deepak at OFBIZ-9833 "Token Based Authentication" that I use in my last patch. >>>> >>>> For me there is only one question outstanding: how to store the Token secret. But this should not prevent us to commit Deepak's work. >>>> >>>> It's now a long time (9 months) since I started this work. And my last patch is ready for a month. >>>> >>>> I crossed several issues which are now all resolved. So please review and answer to this thread. >>>> >>>> Without negative comments well argumented I'll commit both OFBIZ-9833 and OFBIZ-10307 in a week. You can always test and review later, we use RTC. >>>> >>>> Also a veto on a commit is always possible... Of course, as ever, a good consensus is preferred. >>>> >>>> Let me know if you need more information about the goal. For the technical details I think I already provided them the in OFBIZ-10307. >>>> >>>> Jacques >>>> |
Re: OFBIZ-10307: Navigate from a domain to another with automated signed in authentication
|
Administrator
|
In reply to this post by Jacques Le Roux
Hi Jinghai,
Actually you had a point speaking about auth0, in relation to the JWT secret key. I forgot that in OFBIZ-9833 at "20/Feb/18 18:02" I considered https://github.com/auth0/java-jwt#using-a-keyprovider because using a KeyProvider it's an alternative solution to using a static secret key. But actually that's not part of my work but Deepak's at OFBIZ-9833. I now rely on OFBIZ-9833 for OFBIZ-10307. Jacques Le 19/08/2018 à 12:33, Jacques Le Roux a écrit : > Hi Jinghai, > > Actually I did not pick auth0 (not to be confused with https://en.wikipedia.org/wiki/OAuth) nor https://oauth.net/2/ because those need a central > Identify server (as is the SAML protocol). > > I simply send a JWT token: https://en.wikipedia.org/wiki/JSON_Web_Token and https://jwt.io/ to > > Please refer to OFBIZ-10307 "Navigate from a domain to another with automated signed in authentication" > > Thanks for your interest. > > Jacques > > > Le 17/08/2018 à 09:02, Shi Jinghai a écrit : >> Hi Jacques, >> >> OK, I think the redis topic is jumped to next step. >> >> I have read the patches carelly, as a fan of Apereo CAS[1], I wonder why choose auth0[2] rather than CAS. And is the implement OAuth2 alliance? >> >> [1] https://github.com/apereo/cas >> [2] https://auth0.com/ >> >> Kind Regards, >> >> Shi Jinghai >> >> >> -----邮件原件----- >> 发件人: Jacques Le Roux [mailto:[hidden email]] >> 发送时间: 2018年8月16日 2:08 >> 收件人: [hidden email] >> 主题: Re: OFBIZ-10307: Navigate from a domain to another with automated signed in authentication >> >> Hi Jinghai, >> >> The problem with the token master secret key is to guarantee its secrecy at max. >> >> We already discussed different solutions at https://s.apache.org/7yyR and https://s.apache.org/IBDM >> >> How is Redis more secure than Postgres for storing values? >> >> Thanks >> >> Jacques >> >> >> Le 15/08/2018 à 14:37, Shi Jinghai a écrit : >>> Dear Jacques, >>> >>> On how to store the Tokens, as a token is a key, value is the UserLogin entity and/or other info, a key-value db, Redis[1] is a good choice. Redis >>> is no.7 in db ranking in Aug 2018[2], becomes more and more popular. Goldman Sachs invested Redis team in last year[3]. It's common view now in >>> China that Redis is better than any others including Gemfire of Pivotal, the railway ticket system of China replaced its 3 Gemfire clusters with 3 >>> Redis clusters last year and then there are much less complains on how difficulties to buy spring festival tickets. >>> >>> Mr. Dai Haipeng contributed a Redis component in Jira[4]. >>> >>> [1] https://redis.io/ >>> [2] https://db-engines.com/en/ranking >>> [3] https://redislabs.com/press/redis-labs-secures-44-million-funding-led-goldman-sachs-private-capital-investing-strengthen-database-leadership/ >>> [4] https://issues.apache.org/jira/browse/OFBIZ-9829 >>> >>> BTW, I'll try to review the patches. >>> >>> Kind Regards, >>> >>> Shi Jinghai >>> >>> -----邮件原件----- >>> 发件人: Jacques Le Roux [mailto:[hidden email]] >>> 发送时间: 2018年8月15日 15:09 >>> 收件人: [hidden email] >>> 主题: OFBIZ-10307: Navigate from a domain to another with automated signed in authentication >>> >>> Hi, >>> >>> Some time ago I created https://issues.apache.org/jira/browse/OFBIZ-10307. >>> >>> I asked for reviews but only Taher answered and he asked to know the goal of this new feature. >>> >>> It was actually developed for a client who needed to get from one OFBiz instance on a server (on a domain) to another OFBiz instance on another >>> server (on another domain) without having to sign up between the 2 while keeping things secure. >>> >>> There could be many reasons why you want to split OFBiz application on servers. In their case it was for performance issues. >>> >>> The technology used is as secure as possible. Like OAuth 2.0 it uses a token but it does not need a middle authorization server (think to two-factor >>> authentication) because it's only for OFBiz instances of the same version. >>> >>> To commit this work we need 1st to agree an commit the work done by Deepak at OFBIZ-9833 "Token Based Authentication" that I use in my last patch. >>> >>> For me there is only one question outstanding: how to store the Token secret. But this should not prevent us to commit Deepak's work. >>> >>> It's now a long time (9 months) since I started this work. And my last patch is ready for a month. >>> >>> I crossed several issues which are now all resolved. So please review and answer to this thread. >>> >>> Without negative comments well argumented I'll commit both OFBIZ-9833 and OFBIZ-10307 in a week. You can always test and review later, we use RTC. >>> >>> Also a veto on a commit is always possible... Of course, as ever, a good consensus is preferred. >>> >>> Let me know if you need more information about the goal. For the technical details I think I already provided them the in OFBIZ-10307. >>> >>> Jacques >>> > > |
|
|
In reply to this post by Jacques Le Roux
I already found problems in this code to give me pause and to show
that it is not properly studied and designed. If you want to continue the effort, then I recommend reviewing your code thoroughly, providing a clear testing mechanism (especially the web scripts), and ask for reviews and tests. On Sun, Aug 26, 2018 at 12:33 PM Jacques Le Roux <[hidden email]> wrote: > > Hi Taher, > > Thanks for your review, much appreciated. > > > Le 22/08/2018 à 18:42, Taher Alkhateeb a écrit : > > I reviewed the code and explanation in the beginning of this thread > > and I have the following comments: > > > > - First, in case of a result.containsKey(ModelService.ERROR_MESSAGE) > > you are still returning "success". Does not make sense at all. > You are right I should have used "error". > I did not notice (actually forgot a last own review after much variations in code) because it's an event preprocessor and no response is > expected/handled. > Not to bad, in case of error just an EventHandlerException was not thrown. > > > - Also the comments are unnecessary and do not make sense in the same code block > You mean "// Something unexpected happened here", or ? > > > - The comment you wrote: "Check it's the right tenant in case username > > and password are the same in different tenants. Not sure this is > > really useful in the case of external server, does not hurt anyway" is > > self-defeating. Never put in code you won't use. We have enough mess > > in the code base. > I must say I initially copied that from ExternalLoginKeysManager::checkExternalLoginKey and adapted it to the context. It's roughly the same code. > I let it there because I was then unsure. But when you think about it, there is no reason to not make this check. So I'll simply remove the comment. I > should have put a TODO. > > BTW this is out of subject and I'll start a new thread about it after reading > https://www.linkedin.com/pulse/architecture-constraints-end-multi-tenancy-gregor-hohpe/ > The subject will be: "Should we keep the multi-tenants feature in OFBiz." > I though think the tenants in OFBiz are still useful when you only needs dozens or maybe even few hundreds tenants (begin to be a lot of DBs!). > But I faced that with a startup which wanted to handle thousands, if not millions (actually they failed), of tenants, obviously OFBiz can't do that. > > > - Calling LoginWorker.doBasicLogin(userLogin, request) after the > > if/else block does not make sense. The else block guarantees falling > > into the exception. This whole code block needs refactoring > You are totally right, I have uploaded a new patch where I put the call of > LoginWorker.doBasicLogin(userLogin, request); > in the positive part of the if and used the same block pattern than above where there is a warning log then returning an error. > I was so focused on the other parts of the mechanism, especially to secure it, than I forgot to review the Java part. > My bad, thank you again for your review. > > > - A testing method needs to be provided in detail. For example I'm not > > sure how the Javascript portions will execute and when. So some > > provision of a clear testing mechanism is necessary. > Yep, right, I'll document all the mechanism and how to use it. Pierre even proposed to make a graph in AsciiDoc, not sure I'll be able to, by I'll try. > Actually it's only a matter of following what should be in the example component, see last OFBIZ-10307-test from example.patch. > I thought it would be enough for devs. I understand it's not, a bit of clear documentation is always welcomed, and I often advocate for it. > > > - Finally, I'm not sure this feature is needed at all. Why not just > > assign an LDAP server to take care of OFBiz and non-OFBiz in a single > > sign-on environment. > Because you don't need to have (install, maintain, etc.) a LDAP server, or whatnot (CAS, Oauth2, SAML, you name it) if you use this code, it's ready OOTB. > > > I see too much code being written for something > > that might not be of great value and there are other better solutions > > out there. > Which ones do you think about? And why they are better at doing this specific feature? > > > Using JWT tokens might come with problems [1] that need to > > be justified before we adopt it. > That's very interesting, let's check, after reading https://connect2id.com/products/nimbus-jose-jwt/vulnerabilities > > 1. Never let the JWT header alone drive verification > * Not our case, we have the loginUserId ciphered in and checked. The crucial point was how to send the loginUserId securely. > 2. Know the algorithms > * This part is handled by JWTManager class. We have begun to discuss where to store the secret key at https://s.apache.org/IiE0 > 3. Use an appropriate key size > * This part is handled by JWTManager class, which uses HMAC with SHA-512. And that's OK reading the article Wikipedia refers to.| > | > > |Jacques| > > > > > [1] https://en.wikipedia.org/wiki/JSON_Web_Token#Vulnerabilities_and_criticism > > On Wed, Aug 22, 2018 at 5:22 PM Jacques Le Roux > > <[hidden email]> wrote: > >> Hi Jinghai, > >> > >> You can do if you want, but that's not possible in the context of the ASF. > >> > >> Also I'm for the ASL2 license as is, not to add a common clause. > >> > >> Of course if you pay me for that I would :D But I guess it's a joke, isn'it? > >> > >> Jacques > >> > >> > >> Le 22/08/2018 à 14:52, Shi Jinghai a écrit : > >>> Thank you Jacques! > >>> > >>> Perhaps we can build a cloud-ready (SAAS version) OFBiz and add such a common clause :) > >>> > >>> > >>> -----邮件原件----- > >>> 发件人: Jacques Le Roux [mailto:[hidden email]] > >>> 发送时间: 2018年8月22日 12:11 > >>> 收件人: [hidden email] > >>> 主题: Re: OFBIZ-10307: Navigate from a domain to another with automated signed in authentication > >>> > >>> Hi Jinghai, > >>> > >>> You maybe read it already, what did redislabs very recently might be affecting you > >>> > >>> https://redislabs.com/community/commons-clause/ > >>> > >>> Jacques > >>> > >>> > >>> Le 15/08/2018 à 14:37, Shi Jinghai a écrit : > >>>> Dear Jacques, > >>>> > >>>> On how to store the Tokens, as a token is a key, value is the UserLogin entity and/or other info, a key-value db, Redis[1] is a good choice. Redis is no.7 in db ranking in Aug 2018[2], becomes more and more popular. Goldman Sachs invested Redis team in last year[3]. It's common view now in China that Redis is better than any others including Gemfire of Pivotal, the railway ticket system of China replaced its 3 Gemfire clusters with 3 Redis clusters last year and then there are much less complains on how difficulties to buy spring festival tickets. > >>>> > >>>> Mr. Dai Haipeng contributed a Redis component in Jira[4]. > >>>> > >>>> [1] https://redis.io/ > >>>> [2] https://db-engines.com/en/ranking > >>>> [3] https://redislabs.com/press/redis-labs-secures-44-million-funding-led-goldman-sachs-private-capital-investing-strengthen-database-leadership/ > >>>> [4] https://issues.apache.org/jira/browse/OFBIZ-9829 > >>>> > >>>> BTW, I'll try to review the patches. > >>>> > >>>> Kind Regards, > >>>> > >>>> Shi Jinghai > >>>> > >>>> -----邮件原件----- > >>>> 发件人: Jacques Le Roux [mailto:[hidden email]] > >>>> 发送时间: 2018年8月15日 15:09 > >>>> 收件人: [hidden email] > >>>> 主题: OFBIZ-10307: Navigate from a domain to another with automated signed in authentication > >>>> > >>>> Hi, > >>>> > >>>> Some time ago I created https://issues.apache.org/jira/browse/OFBIZ-10307. > >>>> > >>>> I asked for reviews but only Taher answered and he asked to know the goal of this new feature. > >>>> > >>>> It was actually developed for a client who needed to get from one OFBiz instance on a server (on a domain) to another OFBiz instance on another > >>>> server (on another domain) without having to sign up between the 2 while keeping things secure. > >>>> > >>>> There could be many reasons why you want to split OFBiz application on servers. In their case it was for performance issues. > >>>> > >>>> The technology used is as secure as possible. Like OAuth 2.0 it uses a token but it does not need a middle authorization server (think to two-factor > >>>> authentication) because it's only for OFBiz instances of the same version. > >>>> > >>>> To commit this work we need 1st to agree an commit the work done by Deepak at OFBIZ-9833 "Token Based Authentication" that I use in my last patch. > >>>> > >>>> For me there is only one question outstanding: how to store the Token secret. But this should not prevent us to commit Deepak's work. > >>>> > >>>> It's now a long time (9 months) since I started this work. And my last patch is ready for a month. > >>>> > >>>> I crossed several issues which are now all resolved. So please review and answer to this thread. > >>>> > >>>> Without negative comments well argumented I'll commit both OFBIZ-9833 and OFBIZ-10307 in a week. You can always test and review later, we use RTC. > >>>> > >>>> Also a veto on a commit is always possible... Of course, as ever, a good consensus is preferred. > >>>> > >>>> Let me know if you need more information about the goal. For the technical details I think I already provided them the in OFBIZ-10307. > >>>> > >>>> Jacques > >>>> > |
Re: OFBIZ-10307: Navigate from a domain to another with automated signed in authentication
|
Administrator
|
Hi Taher,
Before creating the last patch, I have reviewed the code a last time again, it's totally OK now. Did you test from https://localhost:8443/catalog/control/FindCatalog with the provided test patch? I'll now provide a simple and concise AsciiDoc documentation. Maybe with a graph, but that's not sure, I have to try... For the test part I'll wait for the user ML "OFBiz Sanity Test Automation" thread https://markmail.org/message/nxoyqxn7lqfupf55 to conclude before endeavouring in web tests I don't think it should stop to commit this work, which can be tested manually using the provided patches (though the one from example needs the commit to be done) Anyway I'll wait more feedbacks before committing, I'm not in a hurry and I know it's a sensitive feature. Jacques Le 27/08/2018 à 14:55, Taher Alkhateeb a écrit : > I already found problems in this code to give me pause and to show > that it is not properly studied and designed. If you want to continue > the effort, then I recommend reviewing your code thoroughly, providing > a clear testing mechanism (especially the web scripts), and ask for > reviews and tests. > On Sun, Aug 26, 2018 at 12:33 PM Jacques Le Roux > <[hidden email]> wrote: >> Hi Taher, >> >> Thanks for your review, much appreciated. >> >> >> Le 22/08/2018 à 18:42, Taher Alkhateeb a écrit : >>> I reviewed the code and explanation in the beginning of this thread >>> and I have the following comments: >>> >>> - First, in case of a result.containsKey(ModelService.ERROR_MESSAGE) >>> you are still returning "success". Does not make sense at all. >> You are right I should have used "error". >> I did not notice (actually forgot a last own review after much variations in code) because it's an event preprocessor and no response is >> expected/handled. >> Not to bad, in case of error just an EventHandlerException was not thrown. >> >>> - Also the comments are unnecessary and do not make sense in the same code block >> You mean "// Something unexpected happened here", or ? >> >>> - The comment you wrote: "Check it's the right tenant in case username >>> and password are the same in different tenants. Not sure this is >>> really useful in the case of external server, does not hurt anyway" is >>> self-defeating. Never put in code you won't use. We have enough mess >>> in the code base. >> I must say I initially copied that from ExternalLoginKeysManager::checkExternalLoginKey and adapted it to the context. It's roughly the same code. >> I let it there because I was then unsure. But when you think about it, there is no reason to not make this check. So I'll simply remove the comment. I >> should have put a TODO. >> >> BTW this is out of subject and I'll start a new thread about it after reading >> https://www.linkedin.com/pulse/architecture-constraints-end-multi-tenancy-gregor-hohpe/ >> The subject will be: "Should we keep the multi-tenants feature in OFBiz." >> I though think the tenants in OFBiz are still useful when you only needs dozens or maybe even few hundreds tenants (begin to be a lot of DBs!). >> But I faced that with a startup which wanted to handle thousands, if not millions (actually they failed), of tenants, obviously OFBiz can't do that. >> >>> - Calling LoginWorker.doBasicLogin(userLogin, request) after the >>> if/else block does not make sense. The else block guarantees falling >>> into the exception. This whole code block needs refactoring >> You are totally right, I have uploaded a new patch where I put the call of >> LoginWorker.doBasicLogin(userLogin, request); >> in the positive part of the if and used the same block pattern than above where there is a warning log then returning an error. >> I was so focused on the other parts of the mechanism, especially to secure it, than I forgot to review the Java part. >> My bad, thank you again for your review. >> >>> - A testing method needs to be provided in detail. For example I'm not >>> sure how the Javascript portions will execute and when. So some >>> provision of a clear testing mechanism is necessary. >> Yep, right, I'll document all the mechanism and how to use it. Pierre even proposed to make a graph in AsciiDoc, not sure I'll be able to, by I'll try. >> Actually it's only a matter of following what should be in the example component, see last OFBIZ-10307-test from example.patch. >> I thought it would be enough for devs. I understand it's not, a bit of clear documentation is always welcomed, and I often advocate for it. >> >>> - Finally, I'm not sure this feature is needed at all. Why not just >>> assign an LDAP server to take care of OFBiz and non-OFBiz in a single >>> sign-on environment. >> Because you don't need to have (install, maintain, etc.) a LDAP server, or whatnot (CAS, Oauth2, SAML, you name it) if you use this code, it's ready OOTB. >> >>> I see too much code being written for something >>> that might not be of great value and there are other better solutions >>> out there. >> Which ones do you think about? And why they are better at doing this specific feature? >> >>> Using JWT tokens might come with problems [1] that need to >>> be justified before we adopt it. >> That's very interesting, let's check, after reading https://connect2id.com/products/nimbus-jose-jwt/vulnerabilities >> >> 1. Never let the JWT header alone drive verification >> * Not our case, we have the loginUserId ciphered in and checked. The crucial point was how to send the loginUserId securely. >> 2. Know the algorithms >> * This part is handled by JWTManager class. We have begun to discuss where to store the secret key at https://s.apache.org/IiE0 >> 3. Use an appropriate key size >> * This part is handled by JWTManager class, which uses HMAC with SHA-512. And that's OK reading the article Wikipedia refers to.| >> | >> >> |Jacques| >> >>> [1] https://en.wikipedia.org/wiki/JSON_Web_Token#Vulnerabilities_and_criticism >>> On Wed, Aug 22, 2018 at 5:22 PM Jacques Le Roux >>> <[hidden email]> wrote: >>>> Hi Jinghai, >>>> >>>> You can do if you want, but that's not possible in the context of the ASF. >>>> >>>> Also I'm for the ASL2 license as is, not to add a common clause. >>>> >>>> Of course if you pay me for that I would :D But I guess it's a joke, isn'it? >>>> >>>> Jacques >>>> >>>> >>>> Le 22/08/2018 à 14:52, Shi Jinghai a écrit : >>>>> Thank you Jacques! >>>>> >>>>> Perhaps we can build a cloud-ready (SAAS version) OFBiz and add such a common clause :) >>>>> >>>>> >>>>> -----邮件原件----- >>>>> 发件人: Jacques Le Roux [mailto:[hidden email]] >>>>> 发送时间: 2018年8月22日 12:11 >>>>> 收件人: [hidden email] >>>>> 主题: Re: OFBIZ-10307: Navigate from a domain to another with automated signed in authentication >>>>> >>>>> Hi Jinghai, >>>>> >>>>> You maybe read it already, what did redislabs very recently might be affecting you >>>>> >>>>> https://redislabs.com/community/commons-clause/ >>>>> >>>>> Jacques >>>>> >>>>> >>>>> Le 15/08/2018 à 14:37, Shi Jinghai a écrit : >>>>>> Dear Jacques, >>>>>> >>>>>> On how to store the Tokens, as a token is a key, value is the UserLogin entity and/or other info, a key-value db, Redis[1] is a good choice. Redis is no.7 in db ranking in Aug 2018[2], becomes more and more popular. Goldman Sachs invested Redis team in last year[3]. It's common view now in China that Redis is better than any others including Gemfire of Pivotal, the railway ticket system of China replaced its 3 Gemfire clusters with 3 Redis clusters last year and then there are much less complains on how difficulties to buy spring festival tickets. >>>>>> >>>>>> Mr. Dai Haipeng contributed a Redis component in Jira[4]. >>>>>> >>>>>> [1] https://redis.io/ >>>>>> [2] https://db-engines.com/en/ranking >>>>>> [3] https://redislabs.com/press/redis-labs-secures-44-million-funding-led-goldman-sachs-private-capital-investing-strengthen-database-leadership/ >>>>>> [4] https://issues.apache.org/jira/browse/OFBIZ-9829 >>>>>> >>>>>> BTW, I'll try to review the patches. >>>>>> >>>>>> Kind Regards, >>>>>> >>>>>> Shi Jinghai >>>>>> >>>>>> -----邮件原件----- >>>>>> 发件人: Jacques Le Roux [mailto:[hidden email]] >>>>>> 发送时间: 2018年8月15日 15:09 >>>>>> 收件人: [hidden email] >>>>>> 主题: OFBIZ-10307: Navigate from a domain to another with automated signed in authentication >>>>>> >>>>>> Hi, >>>>>> >>>>>> Some time ago I created https://issues.apache.org/jira/browse/OFBIZ-10307. >>>>>> >>>>>> I asked for reviews but only Taher answered and he asked to know the goal of this new feature. >>>>>> >>>>>> It was actually developed for a client who needed to get from one OFBiz instance on a server (on a domain) to another OFBiz instance on another >>>>>> server (on another domain) without having to sign up between the 2 while keeping things secure. >>>>>> >>>>>> There could be many reasons why you want to split OFBiz application on servers. In their case it was for performance issues. >>>>>> >>>>>> The technology used is as secure as possible. Like OAuth 2.0 it uses a token but it does not need a middle authorization server (think to two-factor >>>>>> authentication) because it's only for OFBiz instances of the same version. >>>>>> >>>>>> To commit this work we need 1st to agree an commit the work done by Deepak at OFBIZ-9833 "Token Based Authentication" that I use in my last patch. >>>>>> >>>>>> For me there is only one question outstanding: how to store the Token secret. But this should not prevent us to commit Deepak's work. >>>>>> >>>>>> It's now a long time (9 months) since I started this work. And my last patch is ready for a month. >>>>>> >>>>>> I crossed several issues which are now all resolved. So please review and answer to this thread. >>>>>> >>>>>> Without negative comments well argumented I'll commit both OFBIZ-9833 and OFBIZ-10307 in a week. You can always test and review later, we use RTC. >>>>>> >>>>>> Also a veto on a commit is always possible... Of course, as ever, a good consensus is preferred. >>>>>> >>>>>> Let me know if you need more information about the goal. For the technical details I think I already provided them the in OFBIZ-10307. >>>>>> >>>>>> Jacques >>>>>> |
|
|
I couldn't find the testing mechanism and the JIRA is filled with too
much information. Sharing a simple testing mechanism would help On Wed, Aug 29, 2018 at 12:33 PM Jacques Le Roux <[hidden email]> wrote: > > Hi Taher, > > Before creating the last patch, I have reviewed the code a last time again, it's totally OK now. Did you test from > https://localhost:8443/catalog/control/FindCatalog with the provided test patch? > > I'll now provide a simple and concise AsciiDoc documentation. Maybe with a graph, but that's not sure, I have to try... > > For the test part I'll wait for the user ML "OFBiz Sanity Test Automation" thread https://markmail.org/message/nxoyqxn7lqfupf55 to conclude before > endeavouring in web tests > > I don't think it should stop to commit this work, which can be tested manually using the provided patches (though the one from example needs the > commit to be done) > > Anyway I'll wait more feedbacks before committing, I'm not in a hurry and I know it's a sensitive feature. > > Jacques > > > Le 27/08/2018 à 14:55, Taher Alkhateeb a écrit : > > I already found problems in this code to give me pause and to show > > that it is not properly studied and designed. If you want to continue > > the effort, then I recommend reviewing your code thoroughly, providing > > a clear testing mechanism (especially the web scripts), and ask for > > reviews and tests. > > On Sun, Aug 26, 2018 at 12:33 PM Jacques Le Roux > > <[hidden email]> wrote: > >> Hi Taher, > >> > >> Thanks for your review, much appreciated. > >> > >> > >> Le 22/08/2018 à 18:42, Taher Alkhateeb a écrit : > >>> I reviewed the code and explanation in the beginning of this thread > >>> and I have the following comments: > >>> > >>> - First, in case of a result.containsKey(ModelService.ERROR_MESSAGE) > >>> you are still returning "success". Does not make sense at all. > >> You are right I should have used "error". > >> I did not notice (actually forgot a last own review after much variations in code) because it's an event preprocessor and no response is > >> expected/handled. > >> Not to bad, in case of error just an EventHandlerException was not thrown. > >> > >>> - Also the comments are unnecessary and do not make sense in the same code block > >> You mean "// Something unexpected happened here", or ? > >> > >>> - The comment you wrote: "Check it's the right tenant in case username > >>> and password are the same in different tenants. Not sure this is > >>> really useful in the case of external server, does not hurt anyway" is > >>> self-defeating. Never put in code you won't use. We have enough mess > >>> in the code base. > >> I must say I initially copied that from ExternalLoginKeysManager::checkExternalLoginKey and adapted it to the context. It's roughly the same code. > >> I let it there because I was then unsure. But when you think about it, there is no reason to not make this check. So I'll simply remove the comment. I > >> should have put a TODO. > >> > >> BTW this is out of subject and I'll start a new thread about it after reading > >> https://www.linkedin.com/pulse/architecture-constraints-end-multi-tenancy-gregor-hohpe/ > >> The subject will be: "Should we keep the multi-tenants feature in OFBiz." > >> I though think the tenants in OFBiz are still useful when you only needs dozens or maybe even few hundreds tenants (begin to be a lot of DBs!). > >> But I faced that with a startup which wanted to handle thousands, if not millions (actually they failed), of tenants, obviously OFBiz can't do that. > >> > >>> - Calling LoginWorker.doBasicLogin(userLogin, request) after the > >>> if/else block does not make sense. The else block guarantees falling > >>> into the exception. This whole code block needs refactoring > >> You are totally right, I have uploaded a new patch where I put the call of > >> LoginWorker.doBasicLogin(userLogin, request); > >> in the positive part of the if and used the same block pattern than above where there is a warning log then returning an error. > >> I was so focused on the other parts of the mechanism, especially to secure it, than I forgot to review the Java part. > >> My bad, thank you again for your review. > >> > >>> - A testing method needs to be provided in detail. For example I'm not > >>> sure how the Javascript portions will execute and when. So some > >>> provision of a clear testing mechanism is necessary. > >> Yep, right, I'll document all the mechanism and how to use it. Pierre even proposed to make a graph in AsciiDoc, not sure I'll be able to, by I'll try. > >> Actually it's only a matter of following what should be in the example component, see last OFBIZ-10307-test from example.patch. > >> I thought it would be enough for devs. I understand it's not, a bit of clear documentation is always welcomed, and I often advocate for it. > >> > >>> - Finally, I'm not sure this feature is needed at all. Why not just > >>> assign an LDAP server to take care of OFBiz and non-OFBiz in a single > >>> sign-on environment. > >> Because you don't need to have (install, maintain, etc.) a LDAP server, or whatnot (CAS, Oauth2, SAML, you name it) if you use this code, it's ready OOTB. > >> > >>> I see too much code being written for something > >>> that might not be of great value and there are other better solutions > >>> out there. > >> Which ones do you think about? And why they are better at doing this specific feature? > >> > >>> Using JWT tokens might come with problems [1] that need to > >>> be justified before we adopt it. > >> That's very interesting, let's check, after reading https://connect2id.com/products/nimbus-jose-jwt/vulnerabilities > >> > >> 1. Never let the JWT header alone drive verification > >> * Not our case, we have the loginUserId ciphered in and checked. The crucial point was how to send the loginUserId securely. > >> 2. Know the algorithms > >> * This part is handled by JWTManager class. We have begun to discuss where to store the secret key at https://s.apache.org/IiE0 > >> 3. Use an appropriate key size > >> * This part is handled by JWTManager class, which uses HMAC with SHA-512. And that's OK reading the article Wikipedia refers to.| > >> | > >> > >> |Jacques| > >> > >>> [1] https://en.wikipedia.org/wiki/JSON_Web_Token#Vulnerabilities_and_criticism > >>> On Wed, Aug 22, 2018 at 5:22 PM Jacques Le Roux > >>> <[hidden email]> wrote: > >>>> Hi Jinghai, > >>>> > >>>> You can do if you want, but that's not possible in the context of the ASF. > >>>> > >>>> Also I'm for the ASL2 license as is, not to add a common clause. > >>>> > >>>> Of course if you pay me for that I would :D But I guess it's a joke, isn'it? > >>>> > >>>> Jacques > >>>> > >>>> > >>>> Le 22/08/2018 à 14:52, Shi Jinghai a écrit : > >>>>> Thank you Jacques! > >>>>> > >>>>> Perhaps we can build a cloud-ready (SAAS version) OFBiz and add such a common clause :) > >>>>> > >>>>> > >>>>> -----邮件原件----- > >>>>> 发件人: Jacques Le Roux [mailto:[hidden email]] > >>>>> 发送时间: 2018年8月22日 12:11 > >>>>> 收件人: [hidden email] > >>>>> 主题: Re: OFBIZ-10307: Navigate from a domain to another with automated signed in authentication > >>>>> > >>>>> Hi Jinghai, > >>>>> > >>>>> You maybe read it already, what did redislabs very recently might be affecting you > >>>>> > >>>>> https://redislabs.com/community/commons-clause/ > >>>>> > >>>>> Jacques > >>>>> > >>>>> > >>>>> Le 15/08/2018 à 14:37, Shi Jinghai a écrit : > >>>>>> Dear Jacques, > >>>>>> > >>>>>> On how to store the Tokens, as a token is a key, value is the UserLogin entity and/or other info, a key-value db, Redis[1] is a good choice. Redis is no.7 in db ranking in Aug 2018[2], becomes more and more popular. Goldman Sachs invested Redis team in last year[3]. It's common view now in China that Redis is better than any others including Gemfire of Pivotal, the railway ticket system of China replaced its 3 Gemfire clusters with 3 Redis clusters last year and then there are much less complains on how difficulties to buy spring festival tickets. > >>>>>> > >>>>>> Mr. Dai Haipeng contributed a Redis component in Jira[4]. > >>>>>> > >>>>>> [1] https://redis.io/ > >>>>>> [2] https://db-engines.com/en/ranking > >>>>>> [3] https://redislabs.com/press/redis-labs-secures-44-million-funding-led-goldman-sachs-private-capital-investing-strengthen-database-leadership/ > >>>>>> [4] https://issues.apache.org/jira/browse/OFBIZ-9829 > >>>>>> > >>>>>> BTW, I'll try to review the patches. > >>>>>> > >>>>>> Kind Regards, > >>>>>> > >>>>>> Shi Jinghai > >>>>>> > >>>>>> -----邮件原件----- > >>>>>> 发件人: Jacques Le Roux [mailto:[hidden email]] > >>>>>> 发送时间: 2018年8月15日 15:09 > >>>>>> 收件人: [hidden email] > >>>>>> 主题: OFBIZ-10307: Navigate from a domain to another with automated signed in authentication > >>>>>> > >>>>>> Hi, > >>>>>> > >>>>>> Some time ago I created https://issues.apache.org/jira/browse/OFBIZ-10307. > >>>>>> > >>>>>> I asked for reviews but only Taher answered and he asked to know the goal of this new feature. > >>>>>> > >>>>>> It was actually developed for a client who needed to get from one OFBiz instance on a server (on a domain) to another OFBiz instance on another > >>>>>> server (on another domain) without having to sign up between the 2 while keeping things secure. > >>>>>> > >>>>>> There could be many reasons why you want to split OFBiz application on servers. In their case it was for performance issues. > >>>>>> > >>>>>> The technology used is as secure as possible. Like OAuth 2.0 it uses a token but it does not need a middle authorization server (think to two-factor > >>>>>> authentication) because it's only for OFBiz instances of the same version. > >>>>>> > >>>>>> To commit this work we need 1st to agree an commit the work done by Deepak at OFBIZ-9833 "Token Based Authentication" that I use in my last patch. > >>>>>> > >>>>>> For me there is only one question outstanding: how to store the Token secret. But this should not prevent us to commit Deepak's work. > >>>>>> > >>>>>> It's now a long time (9 months) since I started this work. And my last patch is ready for a month. > >>>>>> > >>>>>> I crossed several issues which are now all resolved. So please review and answer to this thread. > >>>>>> > >>>>>> Without negative comments well argumented I'll commit both OFBIZ-9833 and OFBIZ-10307 in a week. You can always test and review later, we use RTC. > >>>>>> > >>>>>> Also a veto on a commit is always possible... Of course, as ever, a good consensus is preferred. > >>>>>> > >>>>>> Let me know if you need more information about the goal. For the technical details I think I already provided them the in OFBIZ-10307. > >>>>>> > >>>>>> Jacques > >>>>>> > |
Re: OFBIZ-10307: Navigate from a domain to another with automated signed in authentication
|
Administrator
|
I'll soon provide an AsciiDoc for that. I have to think about its location, any ideas?
Jacques Le 29/08/2018 à 12:06, Taher Alkhateeb a écrit : > I couldn't find the testing mechanism and the JIRA is filled with too > much information. Sharing a simple testing mechanism would help > On Wed, Aug 29, 2018 at 12:33 PM Jacques Le Roux > <[hidden email]> wrote: >> Hi Taher, >> >> Before creating the last patch, I have reviewed the code a last time again, it's totally OK now. Did you test from >> https://localhost:8443/catalog/control/FindCatalog with the provided test patch? >> >> I'll now provide a simple and concise AsciiDoc documentation. Maybe with a graph, but that's not sure, I have to try... >> >> For the test part I'll wait for the user ML "OFBiz Sanity Test Automation" thread https://markmail.org/message/nxoyqxn7lqfupf55 to conclude before >> endeavouring in web tests >> >> I don't think it should stop to commit this work, which can be tested manually using the provided patches (though the one from example needs the >> commit to be done) >> >> Anyway I'll wait more feedbacks before committing, I'm not in a hurry and I know it's a sensitive feature. >> >> Jacques >> >> >> Le 27/08/2018 à 14:55, Taher Alkhateeb a écrit : >>> I already found problems in this code to give me pause and to show >>> that it is not properly studied and designed. If you want to continue >>> the effort, then I recommend reviewing your code thoroughly, providing >>> a clear testing mechanism (especially the web scripts), and ask for >>> reviews and tests. >>> On Sun, Aug 26, 2018 at 12:33 PM Jacques Le Roux >>> <[hidden email]> wrote: >>>> Hi Taher, >>>> >>>> Thanks for your review, much appreciated. >>>> >>>> >>>> Le 22/08/2018 à 18:42, Taher Alkhateeb a écrit : >>>>> I reviewed the code and explanation in the beginning of this thread >>>>> and I have the following comments: >>>>> >>>>> - First, in case of a result.containsKey(ModelService.ERROR_MESSAGE) >>>>> you are still returning "success". Does not make sense at all. >>>> You are right I should have used "error". >>>> I did not notice (actually forgot a last own review after much variations in code) because it's an event preprocessor and no response is >>>> expected/handled. >>>> Not to bad, in case of error just an EventHandlerException was not thrown. >>>> >>>>> - Also the comments are unnecessary and do not make sense in the same code block >>>> You mean "// Something unexpected happened here", or ? >>>> >>>>> - The comment you wrote: "Check it's the right tenant in case username >>>>> and password are the same in different tenants. Not sure this is >>>>> really useful in the case of external server, does not hurt anyway" is >>>>> self-defeating. Never put in code you won't use. We have enough mess >>>>> in the code base. >>>> I must say I initially copied that from ExternalLoginKeysManager::checkExternalLoginKey and adapted it to the context. It's roughly the same code. >>>> I let it there because I was then unsure. But when you think about it, there is no reason to not make this check. So I'll simply remove the comment. I >>>> should have put a TODO. >>>> >>>> BTW this is out of subject and I'll start a new thread about it after reading >>>> https://www.linkedin.com/pulse/architecture-constraints-end-multi-tenancy-gregor-hohpe/ >>>> The subject will be: "Should we keep the multi-tenants feature in OFBiz." >>>> I though think the tenants in OFBiz are still useful when you only needs dozens or maybe even few hundreds tenants (begin to be a lot of DBs!). >>>> But I faced that with a startup which wanted to handle thousands, if not millions (actually they failed), of tenants, obviously OFBiz can't do that. >>>> >>>>> - Calling LoginWorker.doBasicLogin(userLogin, request) after the >>>>> if/else block does not make sense. The else block guarantees falling >>>>> into the exception. This whole code block needs refactoring >>>> You are totally right, I have uploaded a new patch where I put the call of >>>> LoginWorker.doBasicLogin(userLogin, request); >>>> in the positive part of the if and used the same block pattern than above where there is a warning log then returning an error. >>>> I was so focused on the other parts of the mechanism, especially to secure it, than I forgot to review the Java part. >>>> My bad, thank you again for your review. >>>> >>>>> - A testing method needs to be provided in detail. For example I'm not >>>>> sure how the Javascript portions will execute and when. So some >>>>> provision of a clear testing mechanism is necessary. >>>> Yep, right, I'll document all the mechanism and how to use it. Pierre even proposed to make a graph in AsciiDoc, not sure I'll be able to, by I'll try. >>>> Actually it's only a matter of following what should be in the example component, see last OFBIZ-10307-test from example.patch. >>>> I thought it would be enough for devs. I understand it's not, a bit of clear documentation is always welcomed, and I often advocate for it. >>>> >>>>> - Finally, I'm not sure this feature is needed at all. Why not just >>>>> assign an LDAP server to take care of OFBiz and non-OFBiz in a single >>>>> sign-on environment. >>>> Because you don't need to have (install, maintain, etc.) a LDAP server, or whatnot (CAS, Oauth2, SAML, you name it) if you use this code, it's ready OOTB. >>>> >>>>> I see too much code being written for something >>>>> that might not be of great value and there are other better solutions >>>>> out there. >>>> Which ones do you think about? And why they are better at doing this specific feature? >>>> >>>>> Using JWT tokens might come with problems [1] that need to >>>>> be justified before we adopt it. >>>> That's very interesting, let's check, after reading https://connect2id.com/products/nimbus-jose-jwt/vulnerabilities >>>> >>>> 1. Never let the JWT header alone drive verification >>>> * Not our case, we have the loginUserId ciphered in and checked. The crucial point was how to send the loginUserId securely. >>>> 2. Know the algorithms >>>> * This part is handled by JWTManager class. We have begun to discuss where to store the secret key at https://s.apache.org/IiE0 >>>> 3. Use an appropriate key size >>>> * This part is handled by JWTManager class, which uses HMAC with SHA-512. And that's OK reading the article Wikipedia refers to.| >>>> | >>>> >>>> |Jacques| >>>> >>>>> [1] https://en.wikipedia.org/wiki/JSON_Web_Token#Vulnerabilities_and_criticism >>>>> On Wed, Aug 22, 2018 at 5:22 PM Jacques Le Roux >>>>> <[hidden email]> wrote: >>>>>> Hi Jinghai, >>>>>> >>>>>> You can do if you want, but that's not possible in the context of the ASF. >>>>>> >>>>>> Also I'm for the ASL2 license as is, not to add a common clause. >>>>>> >>>>>> Of course if you pay me for that I would :D But I guess it's a joke, isn'it? >>>>>> >>>>>> Jacques >>>>>> >>>>>> >>>>>> Le 22/08/2018 à 14:52, Shi Jinghai a écrit : >>>>>>> Thank you Jacques! >>>>>>> >>>>>>> Perhaps we can build a cloud-ready (SAAS version) OFBiz and add such a common clause :) >>>>>>> >>>>>>> >>>>>>> -----邮件原件----- >>>>>>> 发件人: Jacques Le Roux [mailto:[hidden email]] >>>>>>> 发送时间: 2018年8月22日 12:11 >>>>>>> 收件人: [hidden email] >>>>>>> 主题: Re: OFBIZ-10307: Navigate from a domain to another with automated signed in authentication >>>>>>> >>>>>>> Hi Jinghai, >>>>>>> >>>>>>> You maybe read it already, what did redislabs very recently might be affecting you >>>>>>> >>>>>>> https://redislabs.com/community/commons-clause/ >>>>>>> >>>>>>> Jacques >>>>>>> >>>>>>> >>>>>>> Le 15/08/2018 à 14:37, Shi Jinghai a écrit : >>>>>>>> Dear Jacques, >>>>>>>> >>>>>>>> On how to store the Tokens, as a token is a key, value is the UserLogin entity and/or other info, a key-value db, Redis[1] is a good choice. Redis is no.7 in db ranking in Aug 2018[2], becomes more and more popular. Goldman Sachs invested Redis team in last year[3]. It's common view now in China that Redis is better than any others including Gemfire of Pivotal, the railway ticket system of China replaced its 3 Gemfire clusters with 3 Redis clusters last year and then there are much less complains on how difficulties to buy spring festival tickets. >>>>>>>> >>>>>>>> Mr. Dai Haipeng contributed a Redis component in Jira[4]. >>>>>>>> >>>>>>>> [1] https://redis.io/ >>>>>>>> [2] https://db-engines.com/en/ranking >>>>>>>> [3] https://redislabs.com/press/redis-labs-secures-44-million-funding-led-goldman-sachs-private-capital-investing-strengthen-database-leadership/ >>>>>>>> [4] https://issues.apache.org/jira/browse/OFBIZ-9829 >>>>>>>> >>>>>>>> BTW, I'll try to review the patches. >>>>>>>> >>>>>>>> Kind Regards, >>>>>>>> >>>>>>>> Shi Jinghai >>>>>>>> >>>>>>>> -----邮件原件----- >>>>>>>> 发件人: Jacques Le Roux [mailto:[hidden email]] >>>>>>>> 发送时间: 2018年8月15日 15:09 >>>>>>>> 收件人: [hidden email] >>>>>>>> 主题: OFBIZ-10307: Navigate from a domain to another with automated signed in authentication >>>>>>>> >>>>>>>> Hi, >>>>>>>> >>>>>>>> Some time ago I created https://issues.apache.org/jira/browse/OFBIZ-10307. >>>>>>>> >>>>>>>> I asked for reviews but only Taher answered and he asked to know the goal of this new feature. >>>>>>>> >>>>>>>> It was actually developed for a client who needed to get from one OFBiz instance on a server (on a domain) to another OFBiz instance on another >>>>>>>> server (on another domain) without having to sign up between the 2 while keeping things secure. >>>>>>>> >>>>>>>> There could be many reasons why you want to split OFBiz application on servers. In their case it was for performance issues. >>>>>>>> >>>>>>>> The technology used is as secure as possible. Like OAuth 2.0 it uses a token but it does not need a middle authorization server (think to two-factor >>>>>>>> authentication) because it's only for OFBiz instances of the same version. >>>>>>>> >>>>>>>> To commit this work we need 1st to agree an commit the work done by Deepak at OFBIZ-9833 "Token Based Authentication" that I use in my last patch. >>>>>>>> >>>>>>>> For me there is only one question outstanding: how to store the Token secret. But this should not prevent us to commit Deepak's work. >>>>>>>> >>>>>>>> It's now a long time (9 months) since I started this work. And my last patch is ready for a month. >>>>>>>> >>>>>>>> I crossed several issues which are now all resolved. So please review and answer to this thread. >>>>>>>> >>>>>>>> Without negative comments well argumented I'll commit both OFBIZ-9833 and OFBIZ-10307 in a week. You can always test and review later, we use RTC. >>>>>>>> >>>>>>>> Also a veto on a commit is always possible... Of course, as ever, a good consensus is preferred. >>>>>>>> >>>>>>>> Let me know if you need more information about the goal. For the technical details I think I already provided them the in OFBIZ-10307. >>>>>>>> >>>>>>>> Jacques >>>>>>>> |
Re: OFBIZ-10307: Navigate from a domain to another with automated signed in authentication
|
|
Globally (after Taher remarks :) ) I found the code clear. Instead of my
comment about the jwt generation [1] on issue OFBIZ-9833 [2] I haven't other technical remarks that can help to improve this. Now on naming vision I suggest simple naming like this : setUserLoginIdInJwtToken -> loadJWT() sendUserLoginIdInJwtToken -> sendJWT() checkExternalServerLogin -> checkJWTLogin() Because I didn't see why we call a token without userLogin and jwt always talk about a token (Jsin Web Token) not necessary to redound it Nicolas [1] https://issues.apache.org/jira/browse/OFBIZ-9833?focusedCommentId=16594702&page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#comment-16594702 [2] https://issues.apache.org/jira/browse/OFBIZ-9833 On 29/08/2018 12:37, Jacques Le Roux wrote: > I'll soon provide an AsciiDoc for that. I have to think about its > location, any ideas? > > Jacques > > > Le 29/08/2018 à 12:06, Taher Alkhateeb a écrit : >> I couldn't find the testing mechanism and the JIRA is filled with too >> much information. Sharing a simple testing mechanism would help >> On Wed, Aug 29, 2018 at 12:33 PM Jacques Le Roux >> <[hidden email]> wrote: >>> Hi Taher, >>> >>> Before creating the last patch, I have reviewed the code a last time >>> again, it's totally OK now. Did you test from >>> https://localhost:8443/catalog/control/FindCatalog with the provided >>> test patch? >>> >>> I'll now provide a simple and concise AsciiDoc documentation. Maybe >>> with a graph, but that's not sure, I have to try... >>> >>> For the test part I'll wait for the user ML "OFBiz Sanity Test >>> Automation" thread https://markmail.org/message/nxoyqxn7lqfupf55 to >>> conclude before >>> endeavouring in web tests >>> >>> I don't think it should stop to commit this work, which can be >>> tested manually using the provided patches (though the one from >>> example needs the >>> commit to be done) >>> >>> Anyway I'll wait more feedbacks before committing, I'm not in a >>> hurry and I know it's a sensitive feature. >>> >>> Jacques >>> >>> >>> Le 27/08/2018 à 14:55, Taher Alkhateeb a écrit : >>>> I already found problems in this code to give me pause and to show >>>> that it is not properly studied and designed. If you want to continue >>>> the effort, then I recommend reviewing your code thoroughly, providing >>>> a clear testing mechanism (especially the web scripts), and ask for >>>> reviews and tests. >>>> On Sun, Aug 26, 2018 at 12:33 PM Jacques Le Roux >>>> <[hidden email]> wrote: >>>>> Hi Taher, >>>>> >>>>> Thanks for your review, much appreciated. >>>>> >>>>> >>>>> Le 22/08/2018 à 18:42, Taher Alkhateeb a écrit : >>>>>> I reviewed the code and explanation in the beginning of this thread >>>>>> and I have the following comments: >>>>>> >>>>>> - First, in case of a result.containsKey(ModelService.ERROR_MESSAGE) >>>>>> you are still returning "success". Does not make sense at all. >>>>> You are right I should have used "error". >>>>> I did not notice (actually forgot a last own review after much >>>>> variations in code) because it's an event preprocessor and no >>>>> response is >>>>> expected/handled. >>>>> Not to bad, in case of error just an EventHandlerException was not >>>>> thrown. >>>>> >>>>>> - Also the comments are unnecessary and do not make sense in the >>>>>> same code block >>>>> You mean "// Something unexpected happened here", or ? >>>>> >>>>>> - The comment you wrote: "Check it's the right tenant in case >>>>>> username >>>>>> and password are the same in different tenants. Not sure this is >>>>>> really useful in the case of external server, does not hurt >>>>>> anyway" is >>>>>> self-defeating. Never put in code you won't use. We have enough mess >>>>>> in the code base. >>>>> I must say I initially copied that from >>>>> ExternalLoginKeysManager::checkExternalLoginKey and adapted it to >>>>> the context. It's roughly the same code. >>>>> I let it there because I was then unsure. But when you think about >>>>> it, there is no reason to not make this check. So I'll simply >>>>> remove the comment. I >>>>> should have put a TODO. >>>>> >>>>> BTW this is out of subject and I'll start a new thread about it >>>>> after reading >>>>> https://www.linkedin.com/pulse/architecture-constraints-end-multi-tenancy-gregor-hohpe/ >>>>> >>>>> The subject will be: "Should we keep the multi-tenants feature in >>>>> OFBiz." >>>>> I though think the tenants in OFBiz are still useful when you only >>>>> needs dozens or maybe even few hundreds tenants (begin to be a lot >>>>> of DBs!). >>>>> But I faced that with a startup which wanted to handle thousands, >>>>> if not millions (actually they failed), of tenants, obviously >>>>> OFBiz can't do that. >>>>> >>>>>> - Calling LoginWorker.doBasicLogin(userLogin, request) after the >>>>>> if/else block does not make sense. The else block guarantees falling >>>>>> into the exception. This whole code block needs refactoring >>>>> You are totally right, I have uploaded a new patch where I put the >>>>> call of >>>>> LoginWorker.doBasicLogin(userLogin, request); >>>>> in the positive part of the if and used the same block pattern >>>>> than above where there is a warning log then returning an error. >>>>> I was so focused on the other parts of the mechanism, especially >>>>> to secure it, than I forgot to review the Java part. >>>>> My bad, thank you again for your review. >>>>> >>>>>> - A testing method needs to be provided in detail. For example >>>>>> I'm not >>>>>> sure how the Javascript portions will execute and when. So some >>>>>> provision of a clear testing mechanism is necessary. >>>>> Yep, right, I'll document all the mechanism and how to use it. >>>>> Pierre even proposed to make a graph in AsciiDoc, not sure I'll be >>>>> able to, by I'll try. >>>>> Actually it's only a matter of following what should be in the >>>>> example component, see last OFBIZ-10307-test from example.patch. >>>>> I thought it would be enough for devs. I understand it's not, a >>>>> bit of clear documentation is always welcomed, and I often >>>>> advocate for it. >>>>> >>>>>> - Finally, I'm not sure this feature is needed at all. Why not just >>>>>> assign an LDAP server to take care of OFBiz and non-OFBiz in a >>>>>> single >>>>>> sign-on environment. >>>>> Because you don't need to have (install, maintain, etc.) a LDAP >>>>> server, or whatnot (CAS, Oauth2, SAML, you name it) if you use >>>>> this code, it's ready OOTB. >>>>> >>>>>> I see too much code being written for something >>>>>> that might not be of great value and there are other better >>>>>> solutions >>>>>> out there. >>>>> Which ones do you think about? And why they are better at doing >>>>> this specific feature? >>>>> >>>>>> Using JWT tokens might come with problems [1] that need to >>>>>> be justified before we adopt it. >>>>> That's very interesting, let's check, after reading >>>>> https://connect2id.com/products/nimbus-jose-jwt/vulnerabilities >>>>> >>>>> 1. Never let the JWT header alone drive verification >>>>> * Not our case, we have the loginUserId ciphered in and >>>>> checked. The crucial point was how to send the loginUserId securely. >>>>> 2. Know the algorithms >>>>> * This part is handled by JWTManager class. We have begun >>>>> to discuss where to store the secret key at https://s.apache.org/IiE0 >>>>> 3. Use an appropriate key size >>>>> * This part is handled by JWTManager class, which uses >>>>> HMAC with SHA-512. And that's OK reading the article Wikipedia >>>>> refers to.| >>>>> | >>>>> >>>>> |Jacques| >>>>> >>>>>> [1] >>>>>> https://en.wikipedia.org/wiki/JSON_Web_Token#Vulnerabilities_and_criticism >>>>>> On Wed, Aug 22, 2018 at 5:22 PM Jacques Le Roux >>>>>> <[hidden email]> wrote: >>>>>>> Hi Jinghai, >>>>>>> >>>>>>> You can do if you want, but that's not possible in the context >>>>>>> of the ASF. >>>>>>> >>>>>>> Also I'm for the ASL2 license as is, not to add a common clause. >>>>>>> >>>>>>> Of course if you pay me for that I would :D But I guess it's a >>>>>>> joke, isn'it? >>>>>>> >>>>>>> Jacques >>>>>>> >>>>>>> >>>>>>> Le 22/08/2018 à 14:52, Shi Jinghai a écrit : >>>>>>>> Thank you Jacques! >>>>>>>> >>>>>>>> Perhaps we can build a cloud-ready (SAAS version) OFBiz and add >>>>>>>> such a common clause :) >>>>>>>> >>>>>>>> >>>>>>>> -----邮件原件----- >>>>>>>> 发件人: Jacques Le Roux [mailto:[hidden email]] >>>>>>>> 发送时间: 2018年8月22日 12:11 >>>>>>>> 收件人: [hidden email] >>>>>>>> 主题: Re: OFBIZ-10307: Navigate from a domain to another with >>>>>>>> automated signed in authentication >>>>>>>> >>>>>>>> Hi Jinghai, >>>>>>>> >>>>>>>> You maybe read it already, what did redislabs very recently >>>>>>>> might be affecting you >>>>>>>> >>>>>>>> https://redislabs.com/community/commons-clause/ >>>>>>>> >>>>>>>> Jacques >>>>>>>> >>>>>>>> >>>>>>>> Le 15/08/2018 à 14:37, Shi Jinghai a écrit : >>>>>>>>> Dear Jacques, >>>>>>>>> >>>>>>>>> On how to store the Tokens, as a token is a key, value is the >>>>>>>>> UserLogin entity and/or other info, a key-value db, Redis[1] >>>>>>>>> is a good choice. Redis is no.7 in db ranking in Aug 2018[2], >>>>>>>>> becomes more and more popular. Goldman Sachs invested Redis >>>>>>>>> team in last year[3]. It's common view now in China that Redis >>>>>>>>> is better than any others including Gemfire of Pivotal, the >>>>>>>>> railway ticket system of China replaced its 3 Gemfire clusters >>>>>>>>> with 3 Redis clusters last year and then there are much less >>>>>>>>> complains on how difficulties to buy spring festival tickets. >>>>>>>>> >>>>>>>>> Mr. Dai Haipeng contributed a Redis component in Jira[4]. >>>>>>>>> >>>>>>>>> [1] https://redis.io/ >>>>>>>>> [2] https://db-engines.com/en/ranking >>>>>>>>> [3] >>>>>>>>> https://redislabs.com/press/redis-labs-secures-44-million-funding-led-goldman-sachs-private-capital-investing-strengthen-database-leadership/ >>>>>>>>> [4] https://issues.apache.org/jira/browse/OFBIZ-9829 >>>>>>>>> >>>>>>>>> BTW, I'll try to review the patches. >>>>>>>>> >>>>>>>>> Kind Regards, >>>>>>>>> >>>>>>>>> Shi Jinghai >>>>>>>>> >>>>>>>>> -----邮件原件----- >>>>>>>>> 发件人: Jacques Le Roux [mailto:[hidden email]] >>>>>>>>> 发送时间: 2018年8月15日 15:09 >>>>>>>>> 收件人: [hidden email] >>>>>>>>> 主题: OFBIZ-10307: Navigate from a domain to another with >>>>>>>>> automated signed in authentication >>>>>>>>> >>>>>>>>> Hi, >>>>>>>>> >>>>>>>>> Some time ago I created >>>>>>>>> https://issues.apache.org/jira/browse/OFBIZ-10307. >>>>>>>>> >>>>>>>>> I asked for reviews but only Taher answered and he asked to >>>>>>>>> know the goal of this new feature. >>>>>>>>> >>>>>>>>> It was actually developed for a client who needed to get from >>>>>>>>> one OFBiz instance on a server (on a domain) to another OFBiz >>>>>>>>> instance on another >>>>>>>>> server (on another domain) without having to sign up between >>>>>>>>> the 2 while keeping things secure. >>>>>>>>> >>>>>>>>> There could be many reasons why you want to split OFBiz >>>>>>>>> application on servers. In their case it was for performance >>>>>>>>> issues. >>>>>>>>> >>>>>>>>> The technology used is as secure as possible. Like OAuth 2.0 >>>>>>>>> it uses a token but it does not need a middle authorization >>>>>>>>> server (think to two-factor >>>>>>>>> authentication) because it's only for OFBiz instances of the >>>>>>>>> same version. >>>>>>>>> >>>>>>>>> To commit this work we need 1st to agree an commit the work >>>>>>>>> done by Deepak at OFBIZ-9833 "Token Based Authentication" that >>>>>>>>> I use in my last patch. >>>>>>>>> >>>>>>>>> For me there is only one question outstanding: how to store >>>>>>>>> the Token secret. But this should not prevent us to commit >>>>>>>>> Deepak's work. >>>>>>>>> >>>>>>>>> It's now a long time (9 months) since I started this work. And >>>>>>>>> my last patch is ready for a month. >>>>>>>>> >>>>>>>>> I crossed several issues which are now all resolved. So please >>>>>>>>> review and answer to this thread. >>>>>>>>> >>>>>>>>> Without negative comments well argumented I'll commit both >>>>>>>>> OFBIZ-9833 and OFBIZ-10307 in a week. You can always test and >>>>>>>>> review later, we use RTC. >>>>>>>>> >>>>>>>>> Also a veto on a commit is always possible... Of course, as >>>>>>>>> ever, a good consensus is preferred. >>>>>>>>> >>>>>>>>> Let me know if you need more information about the goal. For >>>>>>>>> the technical details I think I already provided them the in >>>>>>>>> OFBIZ-10307. >>>>>>>>> >>>>>>>>> Jacques >>>>>>>>> > > |
Re: OFBIZ-10307: Navigate from a domain to another with automated signed in authentication
|
Administrator
|
Hi Nicolas,
Thanks for your feedback, much appreciated. I know it's not something easy to review and even less to test. About the names, I see no problems to change them. I used them to try to clarify what they do. Actually for the js functions I initially used names close to what you suggest. And for checkExternalServerLogin I was inspired by checkExternalLoginKey. I'll be happy to change them if everybody agree about that. They are short and explicit enough in the context indeed. Note though that Deepak created the JWTManager class in order to use JWTs for more than an use with userLogin. Jacques Le 30/08/2018 à 13:58, Nicolas Malin a écrit : > Globally (after Taher remarks :) ) I found the code clear. Instead of my comment about the jwt generation [1] on issue OFBIZ-9833 [2] I haven't > other technical remarks that can help to improve this. > > Now on naming vision I suggest simple naming like this : > > setUserLoginIdInJwtToken -> loadJWT() > sendUserLoginIdInJwtToken -> sendJWT() > checkExternalServerLogin -> checkJWTLogin() > > Because I didn't see why we call a token without userLogin and jwt always talk about a token (Jsin Web Token) not necessary to redound it > > Nicolas > > [1] > https://issues.apache.org/jira/browse/OFBIZ-9833?focusedCommentId=16594702&page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#comment-16594702 > [2] https://issues.apache.org/jira/browse/OFBIZ-9833 > On 29/08/2018 12:37, Jacques Le Roux wrote: >> I'll soon provide an AsciiDoc for that. I have to think about its location, any ideas? >> >> Jacques >> >> >> Le 29/08/2018 à 12:06, Taher Alkhateeb a écrit : >>> I couldn't find the testing mechanism and the JIRA is filled with too >>> much information. Sharing a simple testing mechanism would help >>> On Wed, Aug 29, 2018 at 12:33 PM Jacques Le Roux >>> <[hidden email]> wrote: >>>> Hi Taher, >>>> >>>> Before creating the last patch, I have reviewed the code a last time again, it's totally OK now. Did you test from >>>> https://localhost:8443/catalog/control/FindCatalog with the provided test patch? >>>> >>>> I'll now provide a simple and concise AsciiDoc documentation. Maybe with a graph, but that's not sure, I have to try... >>>> >>>> For the test part I'll wait for the user ML "OFBiz Sanity Test Automation" thread https://markmail.org/message/nxoyqxn7lqfupf55 to conclude before >>>> endeavouring in web tests >>>> >>>> I don't think it should stop to commit this work, which can be tested manually using the provided patches (though the one from example needs the >>>> commit to be done) >>>> >>>> Anyway I'll wait more feedbacks before committing, I'm not in a hurry and I know it's a sensitive feature. >>>> >>>> Jacques >>>> >>>> >>>> Le 27/08/2018 à 14:55, Taher Alkhateeb a écrit : >>>>> I already found problems in this code to give me pause and to show >>>>> that it is not properly studied and designed. If you want to continue >>>>> the effort, then I recommend reviewing your code thoroughly, providing >>>>> a clear testing mechanism (especially the web scripts), and ask for >>>>> reviews and tests. >>>>> On Sun, Aug 26, 2018 at 12:33 PM Jacques Le Roux >>>>> <[hidden email]> wrote: >>>>>> Hi Taher, >>>>>> >>>>>> Thanks for your review, much appreciated. >>>>>> >>>>>> >>>>>> Le 22/08/2018 à 18:42, Taher Alkhateeb a écrit : >>>>>>> I reviewed the code and explanation in the beginning of this thread >>>>>>> and I have the following comments: >>>>>>> >>>>>>> - First, in case of a result.containsKey(ModelService.ERROR_MESSAGE) >>>>>>> you are still returning "success". Does not make sense at all. >>>>>> You are right I should have used "error". >>>>>> I did not notice (actually forgot a last own review after much variations in code) because it's an event preprocessor and no response is >>>>>> expected/handled. >>>>>> Not to bad, in case of error just an EventHandlerException was not thrown. >>>>>> >>>>>>> - Also the comments are unnecessary and do not make sense in the same code block >>>>>> You mean "// Something unexpected happened here", or ? >>>>>> >>>>>>> - The comment you wrote: "Check it's the right tenant in case username >>>>>>> and password are the same in different tenants. Not sure this is >>>>>>> really useful in the case of external server, does not hurt anyway" is >>>>>>> self-defeating. Never put in code you won't use. We have enough mess >>>>>>> in the code base. >>>>>> I must say I initially copied that from ExternalLoginKeysManager::checkExternalLoginKey and adapted it to the context. It's roughly the same code. >>>>>> I let it there because I was then unsure. But when you think about it, there is no reason to not make this check. So I'll simply remove the >>>>>> comment. I >>>>>> should have put a TODO. >>>>>> >>>>>> BTW this is out of subject and I'll start a new thread about it after reading >>>>>> https://www.linkedin.com/pulse/architecture-constraints-end-multi-tenancy-gregor-hohpe/ >>>>>> The subject will be: "Should we keep the multi-tenants feature in OFBiz." >>>>>> I though think the tenants in OFBiz are still useful when you only needs dozens or maybe even few hundreds tenants (begin to be a lot of DBs!). >>>>>> But I faced that with a startup which wanted to handle thousands, if not millions (actually they failed), of tenants, obviously OFBiz can't do >>>>>> that. >>>>>> >>>>>>> - Calling LoginWorker.doBasicLogin(userLogin, request) after the >>>>>>> if/else block does not make sense. The else block guarantees falling >>>>>>> into the exception. This whole code block needs refactoring >>>>>> You are totally right, I have uploaded a new patch where I put the call of >>>>>> LoginWorker.doBasicLogin(userLogin, request); >>>>>> in the positive part of the if and used the same block pattern than above where there is a warning log then returning an error. >>>>>> I was so focused on the other parts of the mechanism, especially to secure it, than I forgot to review the Java part. >>>>>> My bad, thank you again for your review. >>>>>> >>>>>>> - A testing method needs to be provided in detail. For example I'm not >>>>>>> sure how the Javascript portions will execute and when. So some >>>>>>> provision of a clear testing mechanism is necessary. >>>>>> Yep, right, I'll document all the mechanism and how to use it. Pierre even proposed to make a graph in AsciiDoc, not sure I'll be able to, by >>>>>> I'll try. >>>>>> Actually it's only a matter of following what should be in the example component, see last OFBIZ-10307-test from example.patch. >>>>>> I thought it would be enough for devs. I understand it's not, a bit of clear documentation is always welcomed, and I often advocate for it. >>>>>> >>>>>>> - Finally, I'm not sure this feature is needed at all. Why not just >>>>>>> assign an LDAP server to take care of OFBiz and non-OFBiz in a single >>>>>>> sign-on environment. >>>>>> Because you don't need to have (install, maintain, etc.) a LDAP server, or whatnot (CAS, Oauth2, SAML, you name it) if you use this code, it's >>>>>> ready OOTB. >>>>>> >>>>>>> I see too much code being written for something >>>>>>> that might not be of great value and there are other better solutions >>>>>>> out there. >>>>>> Which ones do you think about? And why they are better at doing this specific feature? >>>>>> >>>>>>> Using JWT tokens might come with problems [1] that need to >>>>>>> be justified before we adopt it. >>>>>> That's very interesting, let's check, after reading https://connect2id.com/products/nimbus-jose-jwt/vulnerabilities >>>>>> >>>>>> 1. Never let the JWT header alone drive verification >>>>>> * Not our case, we have the loginUserId ciphered in and checked. The crucial point was how to send the loginUserId securely. >>>>>> 2. Know the algorithms >>>>>> * This part is handled by JWTManager class. We have begun to discuss where to store the secret key at https://s.apache.org/IiE0 >>>>>> 3. Use an appropriate key size >>>>>> * This part is handled by JWTManager class, which uses HMAC with SHA-512. And that's OK reading the article Wikipedia refers to.| >>>>>> | >>>>>> >>>>>> |Jacques| >>>>>> >>>>>>> [1] https://en.wikipedia.org/wiki/JSON_Web_Token#Vulnerabilities_and_criticism >>>>>>> On Wed, Aug 22, 2018 at 5:22 PM Jacques Le Roux >>>>>>> <[hidden email]> wrote: >>>>>>>> Hi Jinghai, >>>>>>>> >>>>>>>> You can do if you want, but that's not possible in the context of the ASF. >>>>>>>> >>>>>>>> Also I'm for the ASL2 license as is, not to add a common clause. >>>>>>>> >>>>>>>> Of course if you pay me for that I would :D But I guess it's a joke, isn'it? >>>>>>>> >>>>>>>> Jacques >>>>>>>> >>>>>>>> >>>>>>>> Le 22/08/2018 à 14:52, Shi Jinghai a écrit : >>>>>>>>> Thank you Jacques! >>>>>>>>> >>>>>>>>> Perhaps we can build a cloud-ready (SAAS version) OFBiz and add such a common clause :) >>>>>>>>> >>>>>>>>> >>>>>>>>> -----邮件原件----- >>>>>>>>> 发件人: Jacques Le Roux [mailto:[hidden email]] >>>>>>>>> 发送时间: 2018年8月22日 12:11 >>>>>>>>> 收件人: [hidden email] >>>>>>>>> 主题: Re: OFBIZ-10307: Navigate from a domain to another with automated signed in authentication >>>>>>>>> >>>>>>>>> Hi Jinghai, >>>>>>>>> >>>>>>>>> You maybe read it already, what did redislabs very recently might be affecting you >>>>>>>>> >>>>>>>>> https://redislabs.com/community/commons-clause/ >>>>>>>>> >>>>>>>>> Jacques >>>>>>>>> >>>>>>>>> >>>>>>>>> Le 15/08/2018 à 14:37, Shi Jinghai a écrit : >>>>>>>>>> Dear Jacques, >>>>>>>>>> >>>>>>>>>> On how to store the Tokens, as a token is a key, value is the UserLogin entity and/or other info, a key-value db, Redis[1] is a good >>>>>>>>>> choice. Redis is no.7 in db ranking in Aug 2018[2], becomes more and more popular. Goldman Sachs invested Redis team in last year[3]. It's >>>>>>>>>> common view now in China that Redis is better than any others including Gemfire of Pivotal, the railway ticket system of China replaced its >>>>>>>>>> 3 Gemfire clusters with 3 Redis clusters last year and then there are much less complains on how difficulties to buy spring festival tickets. >>>>>>>>>> >>>>>>>>>> Mr. Dai Haipeng contributed a Redis component in Jira[4]. >>>>>>>>>> >>>>>>>>>> [1] https://redis.io/ >>>>>>>>>> [2] https://db-engines.com/en/ranking >>>>>>>>>> [3] >>>>>>>>>> https://redislabs.com/press/redis-labs-secures-44-million-funding-led-goldman-sachs-private-capital-investing-strengthen-database-leadership/ >>>>>>>>>> [4] https://issues.apache.org/jira/browse/OFBIZ-9829 >>>>>>>>>> >>>>>>>>>> BTW, I'll try to review the patches. >>>>>>>>>> >>>>>>>>>> Kind Regards, >>>>>>>>>> >>>>>>>>>> Shi Jinghai >>>>>>>>>> >>>>>>>>>> -----邮件原件----- >>>>>>>>>> 发件人: Jacques Le Roux [mailto:[hidden email]] >>>>>>>>>> 发送时间: 2018年8月15日 15:09 >>>>>>>>>> 收件人: [hidden email] >>>>>>>>>> 主题: OFBIZ-10307: Navigate from a domain to another with automated signed in authentication >>>>>>>>>> >>>>>>>>>> Hi, >>>>>>>>>> >>>>>>>>>> Some time ago I created https://issues.apache.org/jira/browse/OFBIZ-10307. >>>>>>>>>> >>>>>>>>>> I asked for reviews but only Taher answered and he asked to know the goal of this new feature. >>>>>>>>>> >>>>>>>>>> It was actually developed for a client who needed to get from one OFBiz instance on a server (on a domain) to another OFBiz instance on >>>>>>>>>> another >>>>>>>>>> server (on another domain) without having to sign up between the 2 while keeping things secure. >>>>>>>>>> >>>>>>>>>> There could be many reasons why you want to split OFBiz application on servers. In their case it was for performance issues. >>>>>>>>>> >>>>>>>>>> The technology used is as secure as possible. Like OAuth 2.0 it uses a token but it does not need a middle authorization server (think to >>>>>>>>>> two-factor >>>>>>>>>> authentication) because it's only for OFBiz instances of the same version. >>>>>>>>>> >>>>>>>>>> To commit this work we need 1st to agree an commit the work done by Deepak at OFBIZ-9833 "Token Based Authentication" that I use in my last >>>>>>>>>> patch. >>>>>>>>>> >>>>>>>>>> For me there is only one question outstanding: how to store the Token secret. But this should not prevent us to commit Deepak's work. >>>>>>>>>> >>>>>>>>>> It's now a long time (9 months) since I started this work. And my last patch is ready for a month. >>>>>>>>>> >>>>>>>>>> I crossed several issues which are now all resolved. So please review and answer to this thread. >>>>>>>>>> >>>>>>>>>> Without negative comments well argumented I'll commit both OFBIZ-9833 and OFBIZ-10307 in a week. You can always test and review later, we >>>>>>>>>> use RTC. >>>>>>>>>> >>>>>>>>>> Also a veto on a commit is always possible... Of course, as ever, a good consensus is preferred. >>>>>>>>>> >>>>>>>>>> Let me know if you need more information about the goal. For the technical details I think I already provided them the in OFBIZ-10307. >>>>>>>>>> >>>>>>>>>> Jacques >>>>>>>>>> >> >> > > |
Re: OFBIZ-10307: Navigate from a domain to another with automated signed in authentication
|
Administrator
|
In reply to this post by taher
Hi Taher,
Before diving in documenting this feature, here is a simple way to test this mechanism: 1. Get to https://issues.apache.org/jira/browse/OFBIZ-10307 2. Apply the main patch OFBIZ-10307.patch <https://issues.apache.org/jira/secure/attachment/12937165/OFBIZ-10307.patch> 3. Apply the test patch OFBIZ-10307-test.patch <https://issues.apache.org/jira/secure/attachment/12936183/OFBIZ-10307-test.patch> 4. Get to https://localhost:8443/catalog/control/FindCatalog 5. Click on added test button "Target URL" You should get to https://jleroux.nereide.fr/content/control/main without authenticating I have added this information at the bottom of the Jira to help anybody who would like to test too Thanks Jacques Le 29/08/2018 à 12:06, Taher Alkhateeb a écrit : > I couldn't find the testing mechanism and the JIRA is filled with too > much information. Sharing a simple testing mechanism would help > On Wed, Aug 29, 2018 at 12:33 PM Jacques Le Roux > <[hidden email]> wrote: >> Hi Taher, >> >> Before creating the last patch, I have reviewed the code a last time again, it's totally OK now. Did you test from >> https://localhost:8443/catalog/control/FindCatalog with the provided test patch? >> >> I'll now provide a simple and concise AsciiDoc documentation. Maybe with a graph, but that's not sure, I have to try... >> >> For the test part I'll wait for the user ML "OFBiz Sanity Test Automation" thread https://markmail.org/message/nxoyqxn7lqfupf55 to conclude before >> endeavouring in web tests >> >> I don't think it should stop to commit this work, which can be tested manually using the provided patches (though the one from example needs the >> commit to be done) >> >> Anyway I'll wait more feedbacks before committing, I'm not in a hurry and I know it's a sensitive feature. >> >> Jacques >> >> >> Le 27/08/2018 à 14:55, Taher Alkhateeb a écrit : >>> I already found problems in this code to give me pause and to show >>> that it is not properly studied and designed. If you want to continue >>> the effort, then I recommend reviewing your code thoroughly, providing >>> a clear testing mechanism (especially the web scripts), and ask for >>> reviews and tests. >>> On Sun, Aug 26, 2018 at 12:33 PM Jacques Le Roux >>> <[hidden email]> wrote: >>>> Hi Taher, >>>> >>>> Thanks for your review, much appreciated. >>>> >>>> >>>> Le 22/08/2018 à 18:42, Taher Alkhateeb a écrit : >>>>> I reviewed the code and explanation in the beginning of this thread >>>>> and I have the following comments: >>>>> >>>>> - First, in case of a result.containsKey(ModelService.ERROR_MESSAGE) >>>>> you are still returning "success". Does not make sense at all. >>>> You are right I should have used "error". >>>> I did not notice (actually forgot a last own review after much variations in code) because it's an event preprocessor and no response is >>>> expected/handled. >>>> Not to bad, in case of error just an EventHandlerException was not thrown. >>>> >>>>> - Also the comments are unnecessary and do not make sense in the same code block >>>> You mean "// Something unexpected happened here", or ? >>>> >>>>> - The comment you wrote: "Check it's the right tenant in case username >>>>> and password are the same in different tenants. Not sure this is >>>>> really useful in the case of external server, does not hurt anyway" is >>>>> self-defeating. Never put in code you won't use. We have enough mess >>>>> in the code base. >>>> I must say I initially copied that from ExternalLoginKeysManager::checkExternalLoginKey and adapted it to the context. It's roughly the same code. >>>> I let it there because I was then unsure. But when you think about it, there is no reason to not make this check. So I'll simply remove the comment. I >>>> should have put a TODO. >>>> >>>> BTW this is out of subject and I'll start a new thread about it after reading >>>> https://www.linkedin.com/pulse/architecture-constraints-end-multi-tenancy-gregor-hohpe/ >>>> The subject will be: "Should we keep the multi-tenants feature in OFBiz." >>>> I though think the tenants in OFBiz are still useful when you only needs dozens or maybe even few hundreds tenants (begin to be a lot of DBs!). >>>> But I faced that with a startup which wanted to handle thousands, if not millions (actually they failed), of tenants, obviously OFBiz can't do that. >>>> >>>>> - Calling LoginWorker.doBasicLogin(userLogin, request) after the >>>>> if/else block does not make sense. The else block guarantees falling >>>>> into the exception. This whole code block needs refactoring >>>> You are totally right, I have uploaded a new patch where I put the call of >>>> LoginWorker.doBasicLogin(userLogin, request); >>>> in the positive part of the if and used the same block pattern than above where there is a warning log then returning an error. >>>> I was so focused on the other parts of the mechanism, especially to secure it, than I forgot to review the Java part. >>>> My bad, thank you again for your review. >>>> >>>>> - A testing method needs to be provided in detail. For example I'm not >>>>> sure how the Javascript portions will execute and when. So some >>>>> provision of a clear testing mechanism is necessary. >>>> Yep, right, I'll document all the mechanism and how to use it. Pierre even proposed to make a graph in AsciiDoc, not sure I'll be able to, by I'll try. >>>> Actually it's only a matter of following what should be in the example component, see last OFBIZ-10307-test from example.patch. >>>> I thought it would be enough for devs. I understand it's not, a bit of clear documentation is always welcomed, and I often advocate for it. >>>> >>>>> - Finally, I'm not sure this feature is needed at all. Why not just >>>>> assign an LDAP server to take care of OFBiz and non-OFBiz in a single >>>>> sign-on environment. >>>> Because you don't need to have (install, maintain, etc.) a LDAP server, or whatnot (CAS, Oauth2, SAML, you name it) if you use this code, it's ready OOTB. >>>> >>>>> I see too much code being written for something >>>>> that might not be of great value and there are other better solutions >>>>> out there. >>>> Which ones do you think about? And why they are better at doing this specific feature? >>>> >>>>> Using JWT tokens might come with problems [1] that need to >>>>> be justified before we adopt it. >>>> That's very interesting, let's check, after reading https://connect2id.com/products/nimbus-jose-jwt/vulnerabilities >>>> >>>> 1. Never let the JWT header alone drive verification >>>> * Not our case, we have the loginUserId ciphered in and checked. The crucial point was how to send the loginUserId securely. >>>> 2. Know the algorithms >>>> * This part is handled by JWTManager class. We have begun to discuss where to store the secret key at https://s.apache.org/IiE0 >>>> 3. Use an appropriate key size >>>> * This part is handled by JWTManager class, which uses HMAC with SHA-512. And that's OK reading the article Wikipedia refers to.| >>>> | >>>> >>>> |Jacques| >>>> >>>>> [1] https://en.wikipedia.org/wiki/JSON_Web_Token#Vulnerabilities_and_criticism >>>>> On Wed, Aug 22, 2018 at 5:22 PM Jacques Le Roux >>>>> <[hidden email]> wrote: >>>>>> Hi Jinghai, >>>>>> >>>>>> You can do if you want, but that's not possible in the context of the ASF. >>>>>> >>>>>> Also I'm for the ASL2 license as is, not to add a common clause. >>>>>> >>>>>> Of course if you pay me for that I would :D But I guess it's a joke, isn'it? >>>>>> >>>>>> Jacques >>>>>> >>>>>> >>>>>> Le 22/08/2018 à 14:52, Shi Jinghai a écrit : >>>>>>> Thank you Jacques! >>>>>>> >>>>>>> Perhaps we can build a cloud-ready (SAAS version) OFBiz and add such a common clause :) >>>>>>> >>>>>>> >>>>>>> -----邮件原件----- >>>>>>> 发件人: Jacques Le Roux [mailto:[hidden email]] >>>>>>> 发送时间: 2018年8月22日 12:11 >>>>>>> 收件人: [hidden email] >>>>>>> 主题: Re: OFBIZ-10307: Navigate from a domain to another with automated signed in authentication >>>>>>> >>>>>>> Hi Jinghai, >>>>>>> >>>>>>> You maybe read it already, what did redislabs very recently might be affecting you >>>>>>> >>>>>>> https://redislabs.com/community/commons-clause/ >>>>>>> >>>>>>> Jacques >>>>>>> >>>>>>> >>>>>>> Le 15/08/2018 à 14:37, Shi Jinghai a écrit : >>>>>>>> Dear Jacques, >>>>>>>> >>>>>>>> On how to store the Tokens, as a token is a key, value is the UserLogin entity and/or other info, a key-value db, Redis[1] is a good choice. Redis is no.7 in db ranking in Aug 2018[2], becomes more and more popular. Goldman Sachs invested Redis team in last year[3]. It's common view now in China that Redis is better than any others including Gemfire of Pivotal, the railway ticket system of China replaced its 3 Gemfire clusters with 3 Redis clusters last year and then there are much less complains on how difficulties to buy spring festival tickets. >>>>>>>> >>>>>>>> Mr. Dai Haipeng contributed a Redis component in Jira[4]. >>>>>>>> >>>>>>>> [1] https://redis.io/ >>>>>>>> [2] https://db-engines.com/en/ranking >>>>>>>> [3] https://redislabs.com/press/redis-labs-secures-44-million-funding-led-goldman-sachs-private-capital-investing-strengthen-database-leadership/ >>>>>>>> [4] https://issues.apache.org/jira/browse/OFBIZ-9829 >>>>>>>> >>>>>>>> BTW, I'll try to review the patches. >>>>>>>> >>>>>>>> Kind Regards, >>>>>>>> >>>>>>>> Shi Jinghai >>>>>>>> >>>>>>>> -----邮件原件----- >>>>>>>> 发件人: Jacques Le Roux [mailto:[hidden email]] >>>>>>>> 发送时间: 2018年8月15日 15:09 >>>>>>>> 收件人: [hidden email] >>>>>>>> 主题: OFBIZ-10307: Navigate from a domain to another with automated signed in authentication >>>>>>>> >>>>>>>> Hi, >>>>>>>> >>>>>>>> Some time ago I created https://issues.apache.org/jira/browse/OFBIZ-10307. >>>>>>>> >>>>>>>> I asked for reviews but only Taher answered and he asked to know the goal of this new feature. >>>>>>>> >>>>>>>> It was actually developed for a client who needed to get from one OFBiz instance on a server (on a domain) to another OFBiz instance on another >>>>>>>> server (on another domain) without having to sign up between the 2 while keeping things secure. >>>>>>>> >>>>>>>> There could be many reasons why you want to split OFBiz application on servers. In their case it was for performance issues. >>>>>>>> >>>>>>>> The technology used is as secure as possible. Like OAuth 2.0 it uses a token but it does not need a middle authorization server (think to two-factor >>>>>>>> authentication) because it's only for OFBiz instances of the same version. >>>>>>>> >>>>>>>> To commit this work we need 1st to agree an commit the work done by Deepak at OFBIZ-9833 "Token Based Authentication" that I use in my last patch. >>>>>>>> >>>>>>>> For me there is only one question outstanding: how to store the Token secret. But this should not prevent us to commit Deepak's work. >>>>>>>> >>>>>>>> It's now a long time (9 months) since I started this work. And my last patch is ready for a month. >>>>>>>> >>>>>>>> I crossed several issues which are now all resolved. So please review and answer to this thread. >>>>>>>> >>>>>>>> Without negative comments well argumented I'll commit both OFBIZ-9833 and OFBIZ-10307 in a week. You can always test and review later, we use RTC. >>>>>>>> >>>>>>>> Also a veto on a commit is always possible... Of course, as ever, a good consensus is preferred. >>>>>>>> >>>>>>>> Let me know if you need more information about the goal. For the technical details I think I already provided them the in OFBIZ-10307. >>>>>>>> >>>>>>>> Jacques >>>>>>>> |
Re: OFBIZ-10307: Navigate from a domain to another with automated signed in authentication
|
Administrator
|
In reply to this post by Jacques Le Roux
Seems nobody is against renaming so I'll create a new last patches and attach them.
Jacques Le 30/08/2018 à 16:27, Jacques Le Roux a écrit : > Hi Nicolas, > > Thanks for your feedback, much appreciated. I know it's not something easy to review and even less to test. > > About the names, I see no problems to change them. I used them to try to clarify what they do. Actually for the js functions I initially used names > close to what you suggest. And for checkExternalServerLogin I was inspired by checkExternalLoginKey. > > I'll be happy to change them if everybody agree about that. They are short and explicit enough in the context indeed. > > Note though that Deepak created the JWTManager class in order to use JWTs for more than an use with userLogin. > > Jacques > > > Le 30/08/2018 à 13:58, Nicolas Malin a écrit : >> Globally (after Taher remarks :) ) I found the code clear. Instead of my comment about the jwt generation [1] on issue OFBIZ-9833 [2] I haven't >> other technical remarks that can help to improve this. >> >> Now on naming vision I suggest simple naming like this : >> >> setUserLoginIdInJwtToken -> loadJWT() >> sendUserLoginIdInJwtToken -> sendJWT() >> checkExternalServerLogin -> checkJWTLogin() >> >> Because I didn't see why we call a token without userLogin and jwt always talk about a token (Jsin Web Token) not necessary to redound it >> >> Nicolas >> >> [1] >> https://issues.apache.org/jira/browse/OFBIZ-9833?focusedCommentId=16594702&page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#comment-16594702 >> [2] https://issues.apache.org/jira/browse/OFBIZ-9833 >> On 29/08/2018 12:37, Jacques Le Roux wrote: >>> I'll soon provide an AsciiDoc for that. I have to think about its location, any ideas? >>> >>> Jacques >>> >>> >>> Le 29/08/2018 à 12:06, Taher Alkhateeb a écrit : >>>> I couldn't find the testing mechanism and the JIRA is filled with too >>>> much information. Sharing a simple testing mechanism would help >>>> On Wed, Aug 29, 2018 at 12:33 PM Jacques Le Roux >>>> <[hidden email]> wrote: >>>>> Hi Taher, >>>>> >>>>> Before creating the last patch, I have reviewed the code a last time again, it's totally OK now. Did you test from >>>>> https://localhost:8443/catalog/control/FindCatalog with the provided test patch? >>>>> >>>>> I'll now provide a simple and concise AsciiDoc documentation. Maybe with a graph, but that's not sure, I have to try... >>>>> >>>>> For the test part I'll wait for the user ML "OFBiz Sanity Test Automation" thread https://markmail.org/message/nxoyqxn7lqfupf55 to conclude before >>>>> endeavouring in web tests >>>>> >>>>> I don't think it should stop to commit this work, which can be tested manually using the provided patches (though the one from example needs the >>>>> commit to be done) >>>>> >>>>> Anyway I'll wait more feedbacks before committing, I'm not in a hurry and I know it's a sensitive feature. >>>>> >>>>> Jacques >>>>> >>>>> >>>>> Le 27/08/2018 à 14:55, Taher Alkhateeb a écrit : >>>>>> I already found problems in this code to give me pause and to show >>>>>> that it is not properly studied and designed. If you want to continue >>>>>> the effort, then I recommend reviewing your code thoroughly, providing >>>>>> a clear testing mechanism (especially the web scripts), and ask for >>>>>> reviews and tests. >>>>>> On Sun, Aug 26, 2018 at 12:33 PM Jacques Le Roux >>>>>> <[hidden email]> wrote: >>>>>>> Hi Taher, >>>>>>> >>>>>>> Thanks for your review, much appreciated. >>>>>>> >>>>>>> >>>>>>> Le 22/08/2018 à 18:42, Taher Alkhateeb a écrit : >>>>>>>> I reviewed the code and explanation in the beginning of this thread >>>>>>>> and I have the following comments: >>>>>>>> >>>>>>>> - First, in case of a result.containsKey(ModelService.ERROR_MESSAGE) >>>>>>>> you are still returning "success". Does not make sense at all. >>>>>>> You are right I should have used "error". >>>>>>> I did not notice (actually forgot a last own review after much variations in code) because it's an event preprocessor and no response is >>>>>>> expected/handled. >>>>>>> Not to bad, in case of error just an EventHandlerException was not thrown. >>>>>>> >>>>>>>> - Also the comments are unnecessary and do not make sense in the same code block >>>>>>> You mean "// Something unexpected happened here", or ? >>>>>>> >>>>>>>> - The comment you wrote: "Check it's the right tenant in case username >>>>>>>> and password are the same in different tenants. Not sure this is >>>>>>>> really useful in the case of external server, does not hurt anyway" is >>>>>>>> self-defeating. Never put in code you won't use. We have enough mess >>>>>>>> in the code base. >>>>>>> I must say I initially copied that from ExternalLoginKeysManager::checkExternalLoginKey and adapted it to the context. It's roughly the same >>>>>>> code. >>>>>>> I let it there because I was then unsure. But when you think about it, there is no reason to not make this check. So I'll simply remove the >>>>>>> comment. I >>>>>>> should have put a TODO. >>>>>>> >>>>>>> BTW this is out of subject and I'll start a new thread about it after reading >>>>>>> https://www.linkedin.com/pulse/architecture-constraints-end-multi-tenancy-gregor-hohpe/ >>>>>>> The subject will be: "Should we keep the multi-tenants feature in OFBiz." >>>>>>> I though think the tenants in OFBiz are still useful when you only needs dozens or maybe even few hundreds tenants (begin to be a lot of DBs!). >>>>>>> But I faced that with a startup which wanted to handle thousands, if not millions (actually they failed), of tenants, obviously OFBiz can't do >>>>>>> that. >>>>>>> >>>>>>>> - Calling LoginWorker.doBasicLogin(userLogin, request) after the >>>>>>>> if/else block does not make sense. The else block guarantees falling >>>>>>>> into the exception. This whole code block needs refactoring >>>>>>> You are totally right, I have uploaded a new patch where I put the call of >>>>>>> LoginWorker.doBasicLogin(userLogin, request); >>>>>>> in the positive part of the if and used the same block pattern than above where there is a warning log then returning an error. >>>>>>> I was so focused on the other parts of the mechanism, especially to secure it, than I forgot to review the Java part. >>>>>>> My bad, thank you again for your review. >>>>>>> >>>>>>>> - A testing method needs to be provided in detail. For example I'm not >>>>>>>> sure how the Javascript portions will execute and when. So some >>>>>>>> provision of a clear testing mechanism is necessary. >>>>>>> Yep, right, I'll document all the mechanism and how to use it. Pierre even proposed to make a graph in AsciiDoc, not sure I'll be able to, by >>>>>>> I'll try. >>>>>>> Actually it's only a matter of following what should be in the example component, see last OFBIZ-10307-test from example.patch. >>>>>>> I thought it would be enough for devs. I understand it's not, a bit of clear documentation is always welcomed, and I often advocate for it. >>>>>>> >>>>>>>> - Finally, I'm not sure this feature is needed at all. Why not just >>>>>>>> assign an LDAP server to take care of OFBiz and non-OFBiz in a single >>>>>>>> sign-on environment. >>>>>>> Because you don't need to have (install, maintain, etc.) a LDAP server, or whatnot (CAS, Oauth2, SAML, you name it) if you use this code, it's >>>>>>> ready OOTB. >>>>>>> >>>>>>>> I see too much code being written for something >>>>>>>> that might not be of great value and there are other better solutions >>>>>>>> out there. >>>>>>> Which ones do you think about? And why they are better at doing this specific feature? >>>>>>> >>>>>>>> Using JWT tokens might come with problems [1] that need to >>>>>>>> be justified before we adopt it. >>>>>>> That's very interesting, let's check, after reading https://connect2id.com/products/nimbus-jose-jwt/vulnerabilities >>>>>>> >>>>>>> 1. Never let the JWT header alone drive verification >>>>>>> * Not our case, we have the loginUserId ciphered in and checked. The crucial point was how to send the loginUserId securely. >>>>>>> 2. Know the algorithms >>>>>>> * This part is handled by JWTManager class. We have begun to discuss where to store the secret key at https://s.apache.org/IiE0 >>>>>>> 3. Use an appropriate key size >>>>>>> * This part is handled by JWTManager class, which uses HMAC with SHA-512. And that's OK reading the article Wikipedia refers to.| >>>>>>> | >>>>>>> >>>>>>> |Jacques| >>>>>>> >>>>>>>> [1] https://en.wikipedia.org/wiki/JSON_Web_Token#Vulnerabilities_and_criticism >>>>>>>> On Wed, Aug 22, 2018 at 5:22 PM Jacques Le Roux >>>>>>>> <[hidden email]> wrote: >>>>>>>>> Hi Jinghai, >>>>>>>>> >>>>>>>>> You can do if you want, but that's not possible in the context of the ASF. >>>>>>>>> >>>>>>>>> Also I'm for the ASL2 license as is, not to add a common clause. >>>>>>>>> >>>>>>>>> Of course if you pay me for that I would :D But I guess it's a joke, isn'it? >>>>>>>>> >>>>>>>>> Jacques >>>>>>>>> >>>>>>>>> >>>>>>>>> Le 22/08/2018 à 14:52, Shi Jinghai a écrit : >>>>>>>>>> Thank you Jacques! >>>>>>>>>> >>>>>>>>>> Perhaps we can build a cloud-ready (SAAS version) OFBiz and add such a common clause :) >>>>>>>>>> >>>>>>>>>> >>>>>>>>>> -----邮件原件----- >>>>>>>>>> 发件人: Jacques Le Roux [mailto:[hidden email]] >>>>>>>>>> 发送时间: 2018年8月22日 12:11 >>>>>>>>>> 收件人: [hidden email] >>>>>>>>>> 主题: Re: OFBIZ-10307: Navigate from a domain to another with automated signed in authentication >>>>>>>>>> >>>>>>>>>> Hi Jinghai, >>>>>>>>>> >>>>>>>>>> You maybe read it already, what did redislabs very recently might be affecting you >>>>>>>>>> >>>>>>>>>> https://redislabs.com/community/commons-clause/ >>>>>>>>>> >>>>>>>>>> Jacques >>>>>>>>>> >>>>>>>>>> >>>>>>>>>> Le 15/08/2018 à 14:37, Shi Jinghai a écrit : >>>>>>>>>>> Dear Jacques, >>>>>>>>>>> >>>>>>>>>>> On how to store the Tokens, as a token is a key, value is the UserLogin entity and/or other info, a key-value db, Redis[1] is a good >>>>>>>>>>> choice. Redis is no.7 in db ranking in Aug 2018[2], becomes more and more popular. Goldman Sachs invested Redis team in last year[3]. It's >>>>>>>>>>> common view now in China that Redis is better than any others including Gemfire of Pivotal, the railway ticket system of China replaced >>>>>>>>>>> its 3 Gemfire clusters with 3 Redis clusters last year and then there are much less complains on how difficulties to buy spring festival >>>>>>>>>>> tickets. >>>>>>>>>>> >>>>>>>>>>> Mr. Dai Haipeng contributed a Redis component in Jira[4]. >>>>>>>>>>> >>>>>>>>>>> [1] https://redis.io/ >>>>>>>>>>> [2] https://db-engines.com/en/ranking >>>>>>>>>>> [3] >>>>>>>>>>> https://redislabs.com/press/redis-labs-secures-44-million-funding-led-goldman-sachs-private-capital-investing-strengthen-database-leadership/ >>>>>>>>>>> [4] https://issues.apache.org/jira/browse/OFBIZ-9829 >>>>>>>>>>> >>>>>>>>>>> BTW, I'll try to review the patches. >>>>>>>>>>> >>>>>>>>>>> Kind Regards, >>>>>>>>>>> >>>>>>>>>>> Shi Jinghai >>>>>>>>>>> >>>>>>>>>>> -----邮件原件----- >>>>>>>>>>> 发件人: Jacques Le Roux [mailto:[hidden email]] >>>>>>>>>>> 发送时间: 2018年8月15日 15:09 >>>>>>>>>>> 收件人: [hidden email] >>>>>>>>>>> 主题: OFBIZ-10307: Navigate from a domain to another with automated signed in authentication >>>>>>>>>>> >>>>>>>>>>> Hi, >>>>>>>>>>> >>>>>>>>>>> Some time ago I created https://issues.apache.org/jira/browse/OFBIZ-10307. >>>>>>>>>>> >>>>>>>>>>> I asked for reviews but only Taher answered and he asked to know the goal of this new feature. >>>>>>>>>>> >>>>>>>>>>> It was actually developed for a client who needed to get from one OFBiz instance on a server (on a domain) to another OFBiz instance on >>>>>>>>>>> another >>>>>>>>>>> server (on another domain) without having to sign up between the 2 while keeping things secure. >>>>>>>>>>> >>>>>>>>>>> There could be many reasons why you want to split OFBiz application on servers. In their case it was for performance issues. >>>>>>>>>>> >>>>>>>>>>> The technology used is as secure as possible. Like OAuth 2.0 it uses a token but it does not need a middle authorization server (think to >>>>>>>>>>> two-factor >>>>>>>>>>> authentication) because it's only for OFBiz instances of the same version. >>>>>>>>>>> >>>>>>>>>>> To commit this work we need 1st to agree an commit the work done by Deepak at OFBIZ-9833 "Token Based Authentication" that I use in my >>>>>>>>>>> last patch. >>>>>>>>>>> >>>>>>>>>>> For me there is only one question outstanding: how to store the Token secret. But this should not prevent us to commit Deepak's work. >>>>>>>>>>> >>>>>>>>>>> It's now a long time (9 months) since I started this work. And my last patch is ready for a month. >>>>>>>>>>> >>>>>>>>>>> I crossed several issues which are now all resolved. So please review and answer to this thread. >>>>>>>>>>> >>>>>>>>>>> Without negative comments well argumented I'll commit both OFBIZ-9833 and OFBIZ-10307 in a week. You can always test and review later, we >>>>>>>>>>> use RTC. >>>>>>>>>>> >>>>>>>>>>> Also a veto on a commit is always possible... Of course, as ever, a good consensus is preferred. >>>>>>>>>>> >>>>>>>>>>> Let me know if you need more information about the goal. For the technical details I think I already provided them the in OFBIZ-10307. >>>>>>>>>>> >>>>>>>>>>> Jacques >>>>>>>>>>> >>> >>> >> >> > > |
«
Return to OFBiz - Dev
|
1 view|%1 views
| Free forum by Nabble | Edit this page |