jacopoc commented on PR #918:
URL: https://github.com/apache/ofbiz-framework/pull/918#issuecomment-3517352860
Just for the record, the three unit tests that are failing after my changes
are doing so because they were previously passing only due to a side effect.
One of the methods in JWTManager is incorrectly implemented:
```
/**
* Get the JWT secret key from database or security.properties.
* @param delegator the delegator
* @return the JWT secret key
*/
private static String getJWTKey(Delegator delegator, String salt) {
String key = UtilProperties.getPropertyValue("security",
"security.token.key");
if (key.length() < 64) { // The key must be 512 bits (ie 64 chars)
as we use HMAC512 to create the token, cf. OFBIZ-12724
throw new SecurityException("The JWT secret key is too short. It
must be at least 512 bites.");
}
if (salt != null) {
return StringUtil.toHexString(salt.getBytes()) + key;
}
return key;
}
```
As you can see, the method accepts a Delegator object and claims to read the
secret key from either the database or security.properties, but in reality, it
only reads from security.properties — the Delegator is not used at all.
Therefore, the three tests that simply pass a mock Delegator were not
failing before because getJWTKey wasn’t actually using it. Now, however, the
tests are failing because my updated code correctly fetches the settings from
the database or security.properties, and therefore does require a valid
Delegator object.
I’ll look into how we can improve the tests and then fix getJWTKey too.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
