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

feat(auth-jwt), fix|docs(charts): Security Assessment #27

Conversation

drcgjung
Copy link
Contributor

@drcgjung drcgjung commented Aug 28, 2023

WHAT

Provides a useful default authentication configuration (based on api-key hashes) which can be easily adopted, e.g. to use jwt-based authentication.

WHY

Security Assessment Threat Mitigation

FURTHER NOTES

Currently, we need to copy the EDC charts, because the helm value/template structure is significantly changed by introducing multiple dataplanes (of which the agent plane is one option). A reference to the original chart has been added right into the documentation.

The chartlint workflow already looks like the final upgrade workflow, but since the EDC charts are always depending on some external service (hashicorp, azure-vault), we cannot easily deploy (and hence upgrade-test) the charts.

Closes #24
Explains but not closes #26

… useful default authentication configuration of the data/agent plane's default endpoint that is targetted towards applications.
… explanation of the redirection filter expressions in the agent plane configuration.
@drcgjung drcgjung marked this pull request as ready for review August 28, 2023 11:44
Copy link
Member

@scherersebastian scherersebastian left a comment

Choose a reason for hiding this comment

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

Sorry, but I am a bit overwhelmed - in a positive way.
This is a lot of code. My Spring Boot is a bit rusty :)
values.yaml(s) look good.
I also checked the JwtAuthenticationService, because it says "auth-jwt" for default in the issue (maybe I am wrong). Looks also good.
By default, the auth type is set to "api-key", is this intentional? While this approach may be simpler, it could also be riskier.

@drcgjung
Copy link
Contributor Author

Thx much for caring and the good hint:

By default, the auth type is set to "api-key", is this intentional? While this approach may be simpler, it could also be riskier.

We started with the idea of having JWT as the "natural" way to secure an application-facing endpoint of the EDC for enterprises. And found out that maybe a stack of composable authentication services would be a great addition to EDC. However, this would always have an external dependency on an IDP (or at least its public key).

Due to the need to ship a chart which can be installed on a fingertip, we choose to extend/switch to the api-key approach in the default config (and document and even foster the switch to jwt in the documentation).

So the modules focus is more on auth-compose, amybe we should rename it in the next release.

@scherersebastian
Copy link
Member

Got it. Thanks for clarifying

@SebastianBezold SebastianBezold merged commit b255fd1 into eclipse-tractusx:main Aug 31, 2023
@SebastianBezold SebastianBezold mentioned this pull request Aug 31, 2023
77 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add helm upgrade workflow Clarification: Tractus-X EDC Chart usage
3 participants