[WFLY-14984] Add support for encrypted expressions in the wildfly-config.xml #42
Conversation
src/main/java/org/wildfly/client/config/ConfigurationXMLStreamReader.java
Outdated
Show resolved
Hide resolved
src/main/java/org/wildfly/client/config/ConfigurationXMLStreamReader.java
Outdated
Show resolved
Hide resolved
| } else try { | ||
| return Expression.compile(attributeValue, flags); | ||
| } catch (IllegalArgumentException ex) { | ||
| throw msg.expressionParseException(ex, getAttributeName(index), getLocation()); | ||
| } | ||
| } | ||
|
|
||
| default Expression resolveEncryptedExpression(String attributeValue, Expression.Flag... flags) throws ConfigXMLParseException { | ||
| Iterator resolverProviderIterator = ServiceLoader.load(ResolverProvider.class).iterator(); |
There was a problem hiding this comment.
What class loader is expected to be used for loading the provider?
There was a problem hiding this comment.
I am not exactly sure.
Here is the implementation of the ResolverClass
And this is the class that we are trying to access using the service loader.
There was a problem hiding this comment.
We could consider supporting multiple if we can detect if a resolver supported a specific expression format or not so if a resolver did not resolve the expression the next one can be tried.
There was a problem hiding this comment.
I think then that the appropriate thing would be to load from its own class loader; e.g. ServiceLoader.load(ResovlerProvider.class, ConfigurationXMLStreamReader.class.getClassLoader()).
Be sure to wrap the whole interaction with the iterator - including both the hasNext and next calls - in a try/catch to catch ServiceConfigurationError. You can either skip it (if it is OK to try another provider), or wrap & throw it (if there is a sole provider which is expected to work).
There was a problem hiding this comment.
I have made the updates. Thanks for your review! Please feel free to let me know if there are any other changes I should made.
Also, should I add a new WFCC issue? Or is the WFLY issue okay? I have a separate ELY issue related to this RFE, so I can do the same for WFCC.
PrarthonaPaul
force-pushed
the
WFLY-14984
branch
from
58a4ace to
730595b
Compare
PrarthonaPaul
force-pushed
the
WFLY-14984
branch
from
730595b to
a38c502
Compare
PrarthonaPaul
force-pushed
the
WFLY-14984
branch
from
a38c502 to
37c2fb0
Compare
|
Superseded by: #45 which is targeting proper branch. |
| if (envVar.contains("ENC:")) { | ||
| return resolveEncryptedExpression(envVar, flags); | ||
| } else { | ||
| return Expression.compile(envVar, flags); | ||
| } |
There was a problem hiding this comment.
This same if-block 733-737 is used in 4 places in this method.
I suggest moving it into a private method.
There was a problem hiding this comment.
No need. It would be a nice to have when someone else adds to the file in the future.
Issue: https://issues.redhat.com/browse/WFLY-14984
Related PR: wildfly-security/wildfly-elytron#2084
Proposal: wildfly/wildfly-proposals#545
Note: Both wildfly-elytron and wildfly-core projects have dependencies for wildfly-client-config. So, to do integration testing, the version would need to be updated on both projects.