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

PAYARA-4077: Implement Weld SecurityContext #4210

Merged
merged 5 commits into from Sep 11, 2019

Conversation

@pdudits
Copy link
Contributor

commented Sep 10, 2019

This enable security context propagation to asynchronous event observers.

This is build on top of PAYARA-4078 to make CDI TCK pass.

pdudits added 2 commits Sep 9, 2019
Existing InjectionServices work well for classes within single module,
looking directly into deployment metadata, but interceptors need
standard JNDI based injection.

Naming exceptions during lookup are ignored, resulting in null
injection, which is compatible with previous behavior.
@pdudits pdudits requested a review from cubastanley Sep 10, 2019
@pdudits

This comment has been minimized.

Copy link
Contributor Author

commented Sep 10, 2019

Jenkins test please

@pdudits

This comment has been minimized.

Copy link
Contributor Author

commented Sep 10, 2019

jenkins test please

@pdudits pdudits force-pushed the pdudits:payara-4077 branch from a5b99a0 to 1100762 Sep 10, 2019
@pdudits

This comment has been minimized.

Copy link
Contributor Author

commented Sep 10, 2019

Jenkins test please

@pdudits pdudits force-pushed the pdudits:payara-4077 branch from 1100762 to 9f34437 Sep 10, 2019
@pdudits

This comment has been minimized.

Copy link
Contributor Author

commented Sep 10, 2019

Jenkins test please

@pdudits pdudits force-pushed the pdudits:payara-4077 branch from 9f34437 to f587008 Sep 10, 2019
@pdudits

This comment has been minimized.

Copy link
Contributor Author

commented Sep 10, 2019

jenkins test please

@pdudits

This comment has been minimized.

Copy link
Contributor Author

commented Sep 11, 2019

jenkins test please

@pdudits

This comment has been minimized.

Copy link
Contributor Author

commented Sep 11, 2019

Jenkins test please

@dmatej
dmatej approved these changes Sep 11, 2019
import org.glassfish.weld.services.ProxyServicesImpl;
import org.glassfish.weld.services.SecurityServicesImpl;
import org.glassfish.weld.services.TransactionServicesImpl;
import org.glassfish.weld.services.*;

This comment has been minimized.

Copy link
@dmatej

dmatej Sep 11, 2019

Contributor

Whe should agree on some rule how many imports should be replaced with asterisk ... I have 20 as default ... and I would not like it under 10. Only minor comment here, does not block PR.

This comment has been minimized.

Copy link
@Pandrex247

Pandrex247 Sep 11, 2019

Member

Yeah IntelliJ defaults to something like 5 which I think is way too low.
We don't currently have one set, but I agree we should agree one.

Personally I hate wildcard imports in all situations.
You pretty much never need to import everything, and IDEs can auto-hide imports anyway so the "readability" argument is moot.

This comment has been minimized.

Copy link
@dmatej

dmatej Sep 11, 2019

Contributor

Yep, SonarQube also has "too many imports" rule ... so usually asterisk should be unreachable :)
But we can aim this later ... also ordering of imports (alphabetical with blank line between imports with different first block, fish, com, org, etc)

This comment has been minimized.

Copy link
@pdudits

pdudits Sep 11, 2019

Author Contributor

It was set to 9 :). I'll turn it up a bit.

try {
this.namingContext = new InitialContext();
} catch (NamingException e) {
throw new RuntimeException("Initial context not available at deployment time", e);

This comment has been minimized.

Copy link
@dmatej

dmatej Sep 11, 2019

Contributor

Use IllegalStateException, don't use RTE directly ...

@pdudits pdudits merged commit 6c1cc81 into payara:master Sep 11, 2019
24 of 28 checks passed
24 of 28 checks passed
security/snyk - appserver/appclient/pom.xml (payara-ci) Test in progress
security/snyk - appserver/webservices/pom.xml (payara-ci) Test in progress
security/snyk - nucleus/hk2/pom.xml (payara-ci) Test in progress
security/snyk - nucleus/tests/pom.xml (payara-ci) Test in progress
Payara Quick Build and Test Quick build and test passed!
Details
security/snyk - api/payara-api/pom.xml (payara-ci) No new issues
Details
security/snyk - api/pom.xml (payara-ci) No new issues
Details
security/snyk - appserver/batch/pom.xml (payara-ci) No new issues
Details
security/snyk - appserver/common/pom.xml (payara-ci) No new issues
Details
security/snyk - appserver/connectors/pom.xml (payara-ci) No new issues
Details
security/snyk - appserver/distributions/pom.xml (payara-ci) No new issues
Details
security/snyk - appserver/ejb/pom.xml (payara-ci) No new issues
Details
security/snyk - appserver/featuresets/pom.xml (payara-ci) No new issues
Details
security/snyk - appserver/grizzly/pom.xml (payara-ci) No new issues
Details
security/snyk - appserver/load-balancer/pom.xml (payara-ci) No new issues
Details
security/snyk - appserver/osgi-platforms/pom.xml (payara-ci) No new issues
Details
security/snyk - appserver/packager/pom.xml (payara-ci) No new issues
Details
security/snyk - appserver/security/pom.xml (payara-ci) No new issues
Details
security/snyk - appserver/transaction/pom.xml (payara-ci) No new issues
Details
security/snyk - nucleus/cluster/pom.xml (payara-ci) No new issues
Details
security/snyk - nucleus/core/pom.xml (payara-ci) No new issues
Details
security/snyk - nucleus/deployment/pom.xml (payara-ci) No new issues
Details
security/snyk - nucleus/distributions/pom.xml (payara-ci) No new issues
Details
security/snyk - nucleus/flashlight/pom.xml (payara-ci) No new issues
Details
security/snyk - nucleus/grizzly/pom.xml (payara-ci) No new issues
Details
security/snyk - nucleus/packager/pom.xml (payara-ci) No new issues
Details
security/snyk - nucleus/resources-l10n/pom.xml (payara-ci) No new issues
Details
security/snyk - nucleus/resources/pom.xml (payara-ci) No new issues
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants
You can’t perform that action at this time.