[
https://issues.apache.org/jira/browse/OFBIZ-4361?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16916529#comment-16916529
]
Jacques Le Roux commented on OFBIZ-4361:
----------------------------------------
I wrote above
{quote}There is one thing wich is worrying me, why have the JWT in a hidden
form parameter? Could you not put it in a cookie?
{quote}
It's not needed. The hidden form parameter is
{quote}<input type="hidden" name="TOKEN" value="${parameters.TOKEN!}"/>
{quote}
in ChangePassword.ftl. It's only handled on the server side, so no worries.
Apart that I reviewed and tested, it's OK with me
Stuff I found while reviewing:
Unused vars in LoginEvents.java:
{quote}private static final String keyValue =
UtilProperties.getPropertyValue(LoginWorker.securityProperties,
"login.secret_key_string");
{quote}
in forgotPassword()
{quote}GenericDelegator delegator = (GenericDelegator)
request.getAttribute("delegator");
{quote}
{quote}String errMsg = null;
{quote}
in emailPasswordRequest()
{quote}Locale locale = UtilHttp.getLocale(request);
{quote}
All that mostly thanks to Eclipse ;)
In emailPasswordRequest()
{code:java}
if (UtilValidate.isEmpty(userLoginId)) {
String errMsg = UtilProperties.getMessage(resource,
"loginevents.username_was_empty_reenter",
UtilHttp.getLocale(request));
request.setAttribute("_ERROR_MESSAGE_", errMsg);
return "error";
}
{code}
is useless, it's already checked in forgotPassword() which is the only method
calling emailPasswordRequest()
In comment
// Generate a JWT with *defaut* retention time
should be *default* ;)
I don't think changes in general.properties are wanted. Notably mail.debug.on=Y
is dangerous. It can be exploited to look at the message sent, like (even if w/
the secret key it remains hard to decipher)
{noformat}
<!-- Begin Template component://securityext/template/email/PasswordEmail.ftl -->
<html>
<head>
</head>
<body>
<div>This email is in response to your request to have password sent to
you.</div>
<br />
<div>
<form method="post"
action="https://localhost:8443/partymgr/control/passwordChange?USERNAME=DemoCustomer&TOKEN=eyJ0eXAiOiJKV1QiLCJhbGciOiJIUzUxMiJ9.eyJ1c2VyTG9naW5JZCI6IkRlbW9DdXN0b21lciIsImlzcyI6IkFwYWNoZU9G
6IiwiZXhwIjoxNTY2ODMyNjgwLCJpYXQiOjE1NjY4MzA4ODB9.e70au01BK4ubiv-i2S6KutG1mRQtLhl4APJa_Hi5wMWdLLkNtdIIlgLXu5-gqfMQ2RlOFFlO92KIr38NfEwzjA&forgotPwdFlag=true&tenantId="
name="loginform" id="loginform" target="_blank">
<input type="submit" name="submit" value="Click Here To Reset Password"
/>
</form>
</div>
</body>
</html>
<!-- End Template component://securityext/template/email/PasswordEmail.ftl -->
{noformat}
I agree about security.jwt.token.expireTime=1800
ForgotPassword.ftl has a duplicated ASL2 header
Also it should be noted (was already like that) that for an user to be able to
change the password this user must have the permission to access the partymgr
webapp. So every ecommmerce clients must have this permission....!
> Any ecommerce user has the ability to reset anothers password (including
> admin) via "Forget Your Password"
> ----------------------------------------------------------------------------------------------------------
>
> Key: OFBIZ-4361
> URL: https://issues.apache.org/jira/browse/OFBIZ-4361
> Project: OFBiz
> Issue Type: Sub-task
> Components: framework
> Affects Versions: Release Branch 11.04, Release Branch 13.07, Release
> Branch 14.12, Trunk, Release Branch 15.12, Release Branch 16.11, Release
> Branch 17.12
> Environment: Ubuntu and others
> Reporter: mz4wheeler
> Assignee: Jacques Le Roux
> Priority: Major
> Labels: security
> Attachments: OFBIZ-4361.patch, OFBIZ-4361_OneScreen.patch,
> OFBIZ-4361_ReworkPasswordLogic.patch, OFBIZ-4361_ReworkPasswordLogic.patch,
> OFBIZ-4361_Token-Password-Registration.patch
>
>
> Currently, any user (via ecommerce "Forget Your Password") has the ability to
> reset another users password, including "admin" without permission. By
> simply entering "admin" and clicking "Email Password", the following is
> displayed.
> The following occurred:
> A new password has been created and sent to you. Please check your Email.
> This now forces the user of the ERP to change their password. It is also
> possible to generate a dictionary attack against ofbiz because there is no
> capta code required. This is serious security risk.
> This feature could be reduced to a certain sub-set of users, whose login name
> is optionally in the format of an email address, and maybe require a capta
> code to prevent dictionary attacks.
> For example, limit the feature to role "Customer" of type "Person" which was
> generated via an ecommerce transaction.
--
This message was sent by Atlassian Jira
(v8.3.2#803003)
