Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[WFCORE-5709][WFCORE-5705] Make ElytronExpressionResolver useful in S… #4868

Merged
merged 1 commit into from
Nov 26, 2021

Conversation

bstansberry
Copy link
Contributor

…tage.MODEL

Create a separate ExpressionResolverExtension interface that core expression resolver plugins like ElytronExpressionResolver implement instead of ExpressionResolver

Add a capability that can be used by ExpressionResolverExtension impls to register themselves with the core expression resolver, allowing their use in Stage.MODEL

Separate ExpressionResolverExtension initialization from resolution so initialization failures can be handled separately and not be treated as indicating the extension regarded a given expression as relevant to it, but failed to resolve it.

Clarify ExpressionResolverExtension resolution exception handling to distinguish user problems from server problems and not throw OperationFailedException.

Update ReadAttributeHandler.ResolveAttributeHandler to understand the ExpressionResolverExtension exceptions and to treat their presence as indication that a secure expression was present.

Add manualmode test case analogous to the old CustomVaultInModuleTestCase, but using credential expressions.

https://issues.redhat.com/browse/WFCORE-5709
https://issues.redhat.com/browse/WFCORE-5705

@github-actions github-actions bot added the deps-ok Dependencies have been checked, and there are no significant changes label Nov 16, 2021
@bstansberry
Copy link
Contributor Author

@fjuma @darranl this is ready for review

It's a foundation piece for the https://issues.redhat.com/browse/WFCORE-5696 PR, as it brings in the basic secure expression handling testing that used to be in CustomVaultInModuleTestCase, but only for vault. #4865 then adds the deployment handling part. That one is still in Draft until I wire up the full WF side and restore the old deployment secure expression testing, but using credential store expressions instead of vault.

@wildfly-ci
Copy link

Core - Full Integration Build 11057 outcome was FAILURE using a merge of 22df967
Summary: Tests passed: 3875, ignored: 39; exit code 1 (Step: Build & test full (Maven)) (new) Build time: 01:38:36

protected void recordCapabilitiesAndRequirements(OperationContext context, ModelNode operation, Resource resource)
throws OperationFailedException {
super.recordCapabilitiesAndRequirements(context, operation, resource);
}
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Note that this looks like an add in this class but that's an artifact of the diffing algorithm; the block was already there but the diff puts the existing code block in the new ExpressionResolverRemoveHandler class.

That said I don't see the point of either of them. :)

* @param expression a string that matches begins with {@code ${}} and ends with {@code } and that does not have
* any substrings that match that pattern.
* @param context the current {@code OperationContext} to provide additional contextual information.
* @return a string representing the resolve expression, or @{code null} if {@code expression} is not of a
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Very minor details in this JavaDoc:

s/@{code null}/{@code null}

s/with {@code ${}} and ends with {@code }/with <code>${</code> and ends with <code>}</code>

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @yersan -- fixed.

…tage.MODEL

Create a seperate ExpressionResolverExtension interface that core expression resolver plugins like ElytronExpressionResolver implement instead of ExpressionResolver

Add a capability that can be used by ExpressionResolverExtension impls to register themselves with the core expression resolver, allowing their use in Stage.MODEL

Separate ExpressionResolverExtension initialization from resolution so initialization failures can be handled separately and not be treated as indicating the extension
regarded a given expression as relevant to it, but failed to resolve it.

Clarify ExpressionResolverExtension resolution exception handling to distinguish user problems from server problems and not throw OperationFailedException.

Update ReadAttributeHandler.ResolveAttributeHandler to understand the ExpressionResolverExtension exceptions and to treat their presence as indication that
a secure expression was present.

Add manualmode test case analogous to the old CustomVaultInModuleTestCase, but using credential expressions.
@bstansberry bstansberry removed the hold Do not merge this PR label Nov 22, 2021
@wildfly wildfly deleted a comment from wildfly-ci Nov 25, 2021
@darranl darranl mentioned this pull request Nov 25, 2021
@darranl darranl merged commit 295a77d into wildfly:main Nov 26, 2021
@bstansberry bstansberry deleted the WFCORE-5709 branch November 28, 2021 20:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
deps-ok Dependencies have been checked, and there are no significant changes
Projects
None yet
5 participants