-
Notifications
You must be signed in to change notification settings - Fork 3
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
[WIP] authenticated microservices #1238
base: master
Are you sure you want to change the base?
Conversation
* Precompiled script plugin for kotlin-jvm plugin application and configuration * Move some dependencies to libs.versions.toml
…nt` to `fabric8-client` in save-backend
…ed-microservices # Conflicts: # buildSrc/src/main/kotlin/com/saveourtool/save/buildutils/kotlin-jvm-configuration.gradle.kts # gradle/libs.versions.toml # save-backend/build.gradle.kts # save-cloud-charts/save-cloud/templates/backend-deployment.yaml # save-orchestrator/build.gradle.kts
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Found 15 potential problems in the proposed changes. Check the Files changed tab for more details.
save-backend/src/main/kotlin/com/saveourtool/save/backend/configs/WebSecurityConfig.kt
Fixed
Show fixed
Hide fixed
save-backend/src/main/kotlin/com/saveourtool/save/backend/configs/WebSecurityConfig.kt
Fixed
Show fixed
Hide fixed
...-backend/src/main/kotlin/com/saveourtool/save/backend/utils/KubernetesAuthenticationUtils.kt
Fixed
Show fixed
Hide fixed
* Added WebFilter for applying token in headers
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ktlint found more than 10 potential problems in the proposed changes. Check the Files changed tab for more details.
save-backend/src/main/kotlin/com/saveourtool/save/backend/configs/WebSecurityConfig.kt
Fixed
Show fixed
Hide fixed
save-backend/src/main/kotlin/com/saveourtool/save/backend/configs/WebSecurityConfig.kt
Fixed
Show fixed
Hide fixed
...-backend/src/main/kotlin/com/saveourtool/save/backend/utils/KubernetesAuthenticationUtils.kt
Fixed
Show fixed
Hide fixed
…g it in in the preprocessor
…ed-microservices # Conflicts: # save-backend/src/main/kotlin/com/saveourtool/save/backend/utils/ConvertingAuthenticationManager.kt # save-backend/src/main/kotlin/com/saveourtool/save/backend/utils/CustomAuthenticationBasicConverter.kt
save-backend/src/main/kotlin/com/saveourtool/save/backend/configs/WebSecurityConfig.kt
Fixed
Show fixed
Hide fixed
save-backend/src/main/kotlin/com/saveourtool/save/backend/configs/WebSecurityConfig.kt
Fixed
Show fixed
Hide fixed
2097ab0
to
e390516
Compare
…ed-microservices # Conflicts: # authentication-service/build.gradle.kts # save-cloud-charts/save-cloud/templates/backend-configmap.yaml # save-orchestrator/src/main/kotlin/com/saveourtool/save/orchestrator/service/BackendOrchestratorAgentService.kt # save-preprocessor/src/main/kotlin/com/saveourtool/save/preprocessor/SavePreprocessor.kt # save-sandbox/src/main/kotlin/com/saveourtool/save/sandbox/SaveSandbox.kt
…ces' into feature/authenticated-microservices
*/ | ||
@Configuration | ||
class SecurityWebClientCustomizers { | ||
@Bean |
Check failure
Code scanning / ktlint
[KDOC_WITHOUT_RETURN_TAG] all methods which return values should have @return tag in KDoc: serviceAccountTokenHeaderWebClientCustomizer
# Conflicts: # .github/workflows/helm_push.yml # gradle/libs.versions.toml # save-backend/build.gradle.kts # save-backend/src/main/kotlin/com/saveourtool/save/backend/SaveApplication.kt # save-cloud-charts/save-cloud/templates/backend-deployment.yaml # save-sandbox/src/main/kotlin/com/saveourtool/save/sandbox/SaveSandbox.kt
@@ -18,6 +18,7 @@ spec: | |||
annotations: | |||
{{- include "pod.common.annotations" (dict "service" .Values.backend ) | nindent 8 }} | |||
spec: | |||
serviceAccountName: microservice-sa |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As far as I can see, all the microservices that are able to send requests should have microservice-sa
(e.g. demo-cpg
does not send anything so service account for it is not required(?))
Moreover, it seems that orchestrator and demo role bindings should also reference microservice
cluster role
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The idea was, that communication between some microservices can still be secure without security, because we were planning to account only for not trusted executed tools and their network activity could be restricted with a NetworkPolicy
on agent pods. This way using SA token for authentication is only required for microservices which receive requests from agents. But then, microservices that send requests to these services also need to authenticate, so it may be easier to unify everything.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As far as I can see, many things have been done: KubernetesAuthenticaitonUtils
provide almost everything for X-Service-Account-Token
check. So the question is: what hasn't been done yet? (regarding this PR) Seems that ktor needs to be able to add custom headers. And it seems that some Bean
s are required (e.g. KubernetesClient
bean in KubernetesAuthenticationUtils
). Am I right?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I suppose it was already more or less working. This PR affects only services, not agents, and I'm not sure if we have ktor on server-side. But yes, the part that validates token against k8s api server requires KubernetesClient
. The part that adds it as a custom header requires the token to be mounted (added to YAML spec as projected volume
)
@SpringBootApplication(exclude = [ | ||
ReactiveSecurityAutoConfiguration::class, | ||
ReactiveUserDetailsServiceAutoConfiguration::class, | ||
ReactiveManagementWebSecurityAutoConfiguration::class, | ||
]) | ||
@EnableWebFlux | ||
@EnableConfigurationProperties(ConfigProperties::class) | ||
@Import(SecurityWebClientCustomizers::class) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It seems that all the microservices should also be configured with SecurityWebClientCustomizers
. What if ktor
client is used? Should we implement some kind of plugin that adds required header?
save-backend/src/main/kotlin/com/saveourtool/save/backend/SaveApplication.kt
Outdated
Show resolved
Hide resolved
@@ -18,6 +18,7 @@ spec: | |||
annotations: | |||
{{- include "pod.common.annotations" (dict "service" .Values.backend ) | nindent 8 }} | |||
spec: | |||
serviceAccountName: microservice-sa |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The idea was, that communication between some microservices can still be secure without security, because we were planning to account only for not trusted executed tools and their network activity could be restricted with a NetworkPolicy
on agent pods. This way using SA token for authentication is only required for microservices which receive requests from agents. But then, microservices that send requests to these services also need to authenticate, so it may be easier to unify everything.
877c276
to
620a3ea
Compare
HttpSecurityChain
have lowest precedence, because it captures all endpoints (/**
) and more specific matchers should be able to override it by using higher precedence. MakeConvertingAuthenticationManager
primary bean to avoid conflict when autowiring by typeServiceAccount
tokens (similarly to this authentication mode of Vault)ServiceAccount
microservice-sa
for all microservices; mount its token as a projected volumeWebClientCustomizer
to add this token as a custom header; token is updated every 5 minutesspring-security
components to authorize request based on the service account token/internal
endpoints of backend and for orchestartor and sandboxspring-cloud-kubernetes
implementation fromkubernetes-client
tofabric8-client
in save-backendConfigMap
s usingspring-cloud-kubernetes
integration (for some reason resolving${spring.datasource.backend-url}
won't work with existing loading of optional properties file)Part of #1049