Thanks for the review Taher,

Sorry I completely forgot this thread. When I wrote the 
autoLogoutFromAllBackendSessions() method I let a TODO there

// remove all the autoLoginCookies but if in ecommerce/ecomseo and webpos (it's 
done manually there, not sure for webpos TODO: check)

and I remember I thought about harcoding some applications was not good. And especially, how to let know users, who need this feature, to keep the autologin cookie in their custom plugins applications.

I know you don't like having too much properties. But to avoid hard wiring these applications in the framework, Isuggest to have a new keep-autologin-cookie security properties, with default OOTB plugins applications

# -- Names of the component where the autologin cookie should be kept one year
keep-autologin-cookie=ecommerce,ecomseo,webpos

This way, users who need to keep the autologin cookie just have to add the 
concerned application/s to this list.

And autoLogoutFromAllBackendSessions() can then use this list to avoid removing 
autologin cookies from these applications.

Would this work for you?

As I wrote in the TODO, I'm not sure why webpos have a the autoLogout request-map in its controller. I'm not even sure the webpos really implements it, like ecommerce does.
So we could remove webpos from the  keep-autologin-cookie list.
I'll check that.

If it's really needed, as the autologin-cookie feature just remembers you but does not sign you automatically (you need to know the password) I don't think it's a security flaw.
Anyway, if it was the only problem with the webpos :/

Jacques


Le 12/02/2018 à 19:48, Taher Alkhateeb a écrit :
I just checked this code and it looks really worrying to me. You have
hard wired the ecommerce component with logic into the heart of the
framework, I think we need to review the entire body of work and maybe
revert it.

On Sat, Feb 10, 2018 at 2:38 PM, Jacques Le Roux
<[email protected]> wrote:
Le 10/02/2018 à 12:33, Jacques Le Roux a écrit :
Hi,

Almost 6 years ago OFBIZ-4959 "Logout do not remove autoLogin" was created
and I closed as incomplete.

Recently while working on OFBIZ-10206 "Security issue in Token Based
Authentication" which followed my work in OFBIZ-9833 "Token Based
Authentication" I needed a way to get the userLoginId (or userLogin) from
the session.
But, as explained in OFBIZ-10206, at this stage it was unavailable. So I
decided to go with autoLoginCookies. I then " remembered" OFBIZ-4959.

So I'd like to commit the patch I provided at OFBIZ-4959. But before that
I want to discuss about autoLoginCookies and the feature to be sure we are
all on the same field.

The auto login feature is used in ecommerce applications (ie OOTB
ecommerce and ecomseo) to welcome an user when s/he gets back. It does not
really log the user in but eases the login process. From the code, the same
feature exists in the webpos, I did not check.

AutoLoginCookies are also generated for all applications, but are not used
for the auto login feature like in ecommerce applications. It can be
nevertheless useful as proves OFBIZ-10206 "Security issue in Token Based
Authentication". But for OFBIZ-10206 and security in general it's better to
remove the autoLoginCookies of the other applications (ie no ecommerce and
webpos) when the user logout. Of course if the user quits the session w/o
login out the autoLoginCookies remains so it's best to start with a clean
state and remove the autoLoginCookies at start.

Without negative opinions I'll commit the OFBIZ-4959.patch in 1 week.

Jacques


Forgot to say that the autoLoginCookies have a time to live of 1 year.

Jacques


Reply via email to