Skip to content
This repository has been archived by the owner on Feb 12, 2024. It is now read-only.

feature/enh290 Set OIDC clientSecret to be set via a secret #303

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

feature/enh290 Set OIDC clientSecret to be set via a secret #303

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -153,7 +153,8 @@ The following table lists the configurable parameters of the nifi chart and the
| `auth.oidc.enabled` | Enable User auth via oidc | `false` |
| `auth.oidc.discoveryUrl` | oidc discover url | `https://<provider>/.well-known/openid-configuration` |
| `auth.oidc.clientId` | oidc clientId | `nil` |
| `auth.oidc.clientSecret` | oidc clientSecret | `nil` |
| `auth.oidc.clientSecret` | oidc clientSecret (plaintext secret that could get stored in git, logs, etc.) | `nil` |
| `auth.oidc.existingSecret` | Name of an existing secret with the oidc clientSecret | `nil` |
| `auth.oidc.claimIdentifyingUser` | oidc claimIdentifyingUser | `email` |
| `auth.oidc.admin` | Default OIDC admin identity | `[email protected]` |
| Note that OIDC authentication to a multi-NiFi-node cluster requires Ingress sticky sessions | See [background](https://community.cloudera.com/t5/Support-Questions/OIDC-With-Azure-AD/m-p/232324#M194163) | Also [how](https://kubernetes.github.io/ingress-nginx/examples/affinity/cookie/) |
Expand Down
8 changes: 4 additions & 4 deletions configs/login-identity-providers-ldap.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,7 @@
<provider>
<identifier>ldap-provider</identifier>
<class>org.apache.nifi.ldap.LdapProvider</class>
<property name="Authentication Strategy">SIMPLE</property>
<property name="Authentication Strategy">{{.Values.auth.ldap.authStrategy}}</property>
Copy link
Contributor

Choose a reason for hiding this comment

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

I see the master branch already has the code you want (not hardcoded with SIMPLE. Not sure why the PR still shows SIMPE on the left pane??

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hmm, I don't recall changing this file.
For people who do GitOps we need a existingSecret to be referred to. Not using values. So the secrets aren't stored in git.

<property name="Manager DN">{{.Values.auth.ldap.admin}}</property>
<property name="Manager Password">{{.Values.auth.ldap.pass}}</property>
<property name="TLS - Keystore">/opt/nifi/nifi-current/conf/{{.Release.Name}}-nifi-0.{{.Release.Name}}-nifi-headless.{{.Release.Namespace}}.svc.cluster.local/keystore.jks</property>
Expand All @@ -83,8 +83,8 @@
<property name="Read Timeout">10 secs</property>
<property name="Url">{{.Values.auth.ldap.host}}</property>
<property name="User Search Base">{{.Values.auth.ldap.searchBase}}</property>
<property name="User Search Filter">(cn={0})</property>
<property name="User Search Filter">({{.Values.auth.ldap.userIdentityAttribute}}={0})</property>
<property name="Identity Strategy">{{.Values.auth.ldap.IdentityStrategy}}</property>
<property name="Authentication Expiration">12 hours</property>
<property name="Authentication Expiration">{{.Values.auth.ldap.authExpiration}}</property>
</provider>
</loginIdentityProviders>
</loginIdentityProviders>
14 changes: 14 additions & 0 deletions templates/secret.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
{{- if and .Values.auth.oidc.enabled (not (.Values.auth.oidc.existingSecret)) }}
apiVersion: v1
kind: Secret
metadata:
name: {{ template "apache-nifi.fullname" . }}-oidc
labels:
app: {{ include "apache-nifi.name" . | quote }}
chart: "{{ .Chart.Name }}-{{ .Chart.Version | replace "+" "_" }}"
release: {{ .Release.Name | quote }}
heritage: {{ .Release.Service | quote }}
data:
clientSecret: {{ .Values.auth.oidc.clientSecret | b64enc }}

{{- end }}
16 changes: 15 additions & 1 deletion templates/statefulset.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -146,7 +146,7 @@ spec:
prop_replace nifi.security.user.authorizer managed-authorizer
prop_replace nifi.security.user.oidc.discovery.url {{ .Values.auth.oidc.discoveryUrl }}
prop_replace nifi.security.user.oidc.client.id {{ .Values.auth.oidc.clientId }}
prop_replace nifi.security.user.oidc.client.secret {{ .Values.auth.oidc.clientSecret }}
prop_replace nifi.security.user.oidc.client.secret $(cat /mnt/secrets/oidc/clientSecret)
prop_replace nifi.security.user.oidc.claim.identifying.user {{ .Values.auth.oidc.claimIdentifyingUser }}
xmlstarlet ed --inplace --delete "//authorizers/authorizer[identifier='single-user-authorizer']" "${NIFI_HOME}/conf/authorizers.xml"
xmlstarlet ed --inplace --update "//authorizers/userGroupProvider/property[@name='Users File']" -v './auth-conf/users.xml' "${NIFI_HOME}/conf/authorizers.xml"
Expand Down Expand Up @@ -523,6 +523,11 @@ spec:
{{- end }}
{{- end }}
{{- end }}
{{- if .Values.auth.oidc.enabled }}
- name: oidc-secret
mountPath: /mnt/secrets/oidc
readOnly: true
{{- end }}
{{- if .Values.certManager.enabled }}
- name: "tls"
mountPath: /opt/nifi/nifi-current/tls
Expand Down Expand Up @@ -783,6 +788,15 @@ spec:
- name: logs
emptyDir: {}
{{- end }}
{{- if .Values.auth.oidc.enabled }}
- name: oidc-secret
secret:
{{- if not .Values.auth.oidc.existingSecret }}
secretName: {{ template "apache-nifi.fullname" . }}-oidc
{{- else }}
secretName: {{ .Values.auth.oidc.existingSecret }}
{{- end }}
{{- end }}
{{- if .Values.extraVolumes }}
{{ toYaml .Values.extraVolumes | indent 6 }}
{{- end }}
Expand Down
3 changes: 3 additions & 0 deletions values.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -147,6 +147,9 @@ auth:
discoveryUrl: #http://<oidc_provider_address>:<oidc_provider_port>/auth/realms/<client_realm>/.well-known/openid-configuration
clientId: #<client_name_in_oidc_provider>
clientSecret: #<client_secret_in_oidc_provider>
# try to use an existing secret that has been sourced from a key vault so the clientSecret isn't stored in plaintext
# if this is set then the clientSecret above is ignored.
existingSecret:
Copy link
Contributor

Choose a reason for hiding this comment

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

Wh not use the same property "clientSecret" with value comes from the vault or any other tools for secrets? Personally I think it adds confusion with two properties for the same purpose.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The way I did it is what most public Helm charts I've seen do.
It lets the dev/test people just use a plaintext password. clientSecret is the actual password, not a reference to a k8s secret. Also in future the existingSecret could contain more than just one key, like it may need for ldap or the simple method.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It is also backwards compatible for existing user's config

Copy link

Choose a reason for hiding this comment

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

I've seen this pattern before too, it's pretty common

claimIdentifyingUser: email
admin: [email protected]
## Request additional scopes, for example profile
Expand Down