Le 27/03/2018 à 11:38, Taher Alkhateeb a écrit :
On Tue, Mar 27, 2018 at 11:13 AM, Jacques Le Roux
<[email protected]> wrote:
Hi Taher,
That's good questions and I'll try to build a technical perspective here
answering them one by one and as possible in chronology.
1. Changes to cookies was introduced with OFBIZ-4959. Actually it was not
really a bug rather a clean-up. The autoLogin cookies were only used by the
ecommerce component and maybe webpos. But all applications were creating
such cookies with a one year duration. They were useless until I needed
them for the "Navigate from a domain to another with automated signed in
authentication" OFBIZ-10307 feature. But even if they were safe
(httponly) then I needed them to be clean, not a one year duration (to be
as safe as possible, temporary cookies are better). So with your
suggestion[1] (thanks) I introduced the keep-autologin-cookie <webapp>
attribute in ofbiz-component.xml. It's used to remove not kept cookies when
login in or out. So those cookies are only kept during a session. Also a
cookie is created when an user jumps from one application to another.
These cookies are used when navigating from a domain to another to
guarantee the safety of the user who jumps from a domain to another (unlike
my
mistake of using a request parameter initially). Note that protected
cookies (httponly) are one of the safer ways to store information, js script
can't use them[2].
No one helped you in the design, there were issues in it which I
objected to, you did not collaborate with us on the fix and just
re-committed something else and when I objected you said you're
welcome to fix it. I don't like your first solution at all (reverse
dependency which I explained at length) and I am not comfortable with
your second solution either.
What does make you uncomfortable?
2. Http redirects, I'm not sure what you mean by that. I guess it's the part
which is in OFBIZ-10307 (following CORS standard) to allow an user to
jump from a domain to another. For that I use Ajax to send a JWT token in
a HTTP header (as recommended by CORS).
I mean disabling http URLs and their switch to https (port 8080 to
8443). That old behavior was removed by you in a commit again without
community consensus or discussion, you just did it and you committed
your work. Also, look at the JIRA you provided, I only see Jacques
doing stuff in it.
After my explanations I thought it was OK, so I applied a lazy consensus
3. Authentication, for that I use the checkExternalServerLogin pre-processor
in the same vein than checkExternalLoginKey, nothing extraordinary. Just
that it check a JWT token in a HTTP header (as recommended by CORS)
rather than a request parameter. I must say that the devil is in the
technical
details (of CORS) and I'll not explain that here.
Again, you did not cooperate or consult with the community, and you
touched a core component of the system on something which is complex
and intrusive. There were questionable issues in the design here.
I have a patch waiting for review at
https://issues.apache.org/jira/browse/OFBIZ-10307.
I'm working to have another domain to test my work before reverting the whole
and submit a new complete patch there
I don't want to break it all before having tested it on another domain. For now it works on trunk demo and I can test it. It's temporary but I'd like
to contribute this feature and let users able to test it on the trunk demo later.
I hope this helps, of course reviews are welcome.
It seems unfortunately that our reviews are not welcome. You are not
reverting and not cooperating.
What do you exactly want to revert and why? What are your propositions?
Jacques
Jacques
[1] https://s.apache.org/qLGC
[2]
https://stormpath.com/blog/where-to-store-your-jwts-cookies-vs-html5-web-storage
Le 23/03/2018 à 10:22, Taher Alkhateeb a écrit :
I have issues with multiple decisions all around that same topic that
never got community consensus. Changes to cookies, http redirects,
authentication, and other commits that did not get a proper review
from the community. Such major design decisions need proper review IMO
On Fri, Mar 23, 2018 at 11:38 AM, Jacques Le Roux
<[email protected]> wrote:
Le 23/03/2018 à 09:33, Jacques Le Roux a écrit :
Le 23/03/2018 à 09:21, Jacopo Cappellato a écrit :
On Fri, Mar 23, 2018 at 8:36 AM, Jacques Le Roux <
[email protected]> wrote:
Did you try what I said?
You can easily check by svn updating to r1819133 and removing the
wrapper
in ContextFilter.java.
Maybe we need to revert Tomcat SSO then?
A thorough review of that feature is actually on my todo list since
some
time, after I have noticed some potential design issues.
Jacopo
Thanks Jacopo,
I'll also review ASAP since I seconded this feature
Jacques
BTW, forgot to say but the proposed feature at OFBIZ-10307 could be also
used locally (dropping the CORS part).
I tested it initially before crossing CORS (pun intended) and it works
perfectly. It's safe because, like JSESSION, it's build upon safe
AutoLogin
cookies
So we could use it instead of ExternalLoginKey or TomcatSSO. I did not
test
in a cluster environment though...
Anyway just saying for now.
Jacques
