PAYARA-2718 Added Microprofile Config support to OAuth definition ann… #2704
Conversation
|
Jenkins test please |
|
Quick build and test passed! |
| */ | ||
| public OAuth2AuthenticationMechanism setDefinition(OAuth2AuthenticationDefinition definition) {; | ||
| public OAuth2AuthenticationMechanism setDefinition(OAuth2AuthenticationDefinition definition) { |
There was a problem hiding this comment.
I think you could reduce the complexity of this method with a utility method that does the check of which config to use if present. Especially as they are all Strings.
something like
authConfig = getConfig(definition.authEndpoint())
| <version>${microprofile-config.version}</version> | ||
| <type>jar</type> | ||
| </dependency> | ||
| <dependency> |
There was a problem hiding this comment.
Add <scope>provided</scope> here to prevent problems
… into Cousjava-PAYARA-2718-config
|
Jenkins test please |
|
Quick build and test passed! |
| */ | ||
| public OAuth2AuthenticationMechanism setDefinition(OAuth2AuthenticationDefinition definition) {; | ||
| public OAuth2AuthenticationMechanism setDefinition(OAuth2AuthenticationDefinition definition) { | ||
| Config provider = Globals.getDefaultBaseServiceLocator().getService(ConfigProviderResolverImpl.class).getConfig(); |
There was a problem hiding this comment.
You should be able to just use Microprofile config apis to get the Config service. No need to use the internal service.
In fact you could just Optional inject the values.
| <optional>true</optional> | ||
| </dependency> | ||
| <!-- phomehome package --> | ||
| <dependency> |
There was a problem hiding this comment.
What does any of this have to do with the name on the PR
Cousjava
force-pushed
the
PAYARA-2718-config
branch
from
edab05c to
1e7a5dd
Compare
|
Jenkins test please |
|
Quick build and test passed! |
|
The general approach needs some changes. The code should not directly treat the annotation attribute as a potential MP Config key. The convention/
Where mp.jwt.verify.publickey is a global well known key. MP Fault tolerance uses something similar, but based on composite keys: Finally, the issue originally asked for expression language (EL) support in the annotation attributes, which is consistent with EE Security. See https://javaee.github.io/javaee-spec/javadocs/javax/security/enterprise/package-summary.html Since it's also a Payara annotation, which already uses the "${}" syntax, it might be best to only support the "#{}" syntax and treat that as resolve once / immediate-like. |
Cousjava
force-pushed
the
PAYARA-2718-config
branch
from
1e7a5dd to
c9803d1
Compare
|
Jenkins test please |
|
Quick build and test passed! |
|
Jenkins test please |
|
Quick build and test passed! |
|
Jenkins test |
|
Quick build and test passed! |
| String result = value; | ||
| Optional<String> configResult = provider.getOptionalValue(mpConfigKey, String.class); | ||
| if (configResult.isPresent()) { | ||
| result = configResult.get(); |
There was a problem hiding this comment.
If config result is present, just return it. Don't let that value go through the processors.
(a concern was raised that this might introduce security loop holes, as the MP Config values can be set externally)
Cousjava
force-pushed
the
PAYARA-2718-config
branch
from
7e13b83 to
0859422
Compare
|
Jenkins test please |
|
Quick build and test passed! |
payara#2704) * PAYARA-2718 Added Microprofile Config support to OAuth definition annotation * PAYARA-2718 added method to reduce duplication * PAYARA-2718 Added EL# support * Moved annotation to different package * PAYARA-2718 Made keys well-known * PAYARA-2718 return MP value if present directly
payara#2704) * PAYARA-2718 Added Microprofile Config support to OAuth definition annotation * PAYARA-2718 added method to reduce duplication * PAYARA-2718 Added EL# support * Moved annotation to different package * PAYARA-2718 Made keys well-known * PAYARA-2718 return MP value if present directly
…otation