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

Chart: add support for basic authentication on initial apprepositories #3372

Merged
merged 3 commits into from Sep 9, 2021
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
8 changes: 4 additions & 4 deletions chart/kubeapps/Chart.lock
Expand Up @@ -4,9 +4,9 @@ dependencies:
version: 1.8.0
- name: postgresql
repository: https://charts.bitnami.com/bitnami
version: 10.9.4
version: 10.9.5
- name: redis
repository: https://charts.bitnami.com/bitnami
version: 15.3.0
digest: sha256:aeaec0be82187933fa9771b63d3c8bb6b82f33e0cbac6d548cfe182efda852f5
generated: "2021-09-02T00:14:17.499728161Z"
version: 15.3.1
digest: sha256:2d4b55776549e3f0ea0a32531734d311a825fed3a433ff1e3312fee1c2b04ae1
generated: "2021-09-08T05:11:24.71302194+02:00"
14 changes: 10 additions & 4 deletions chart/kubeapps/templates/apprepository/apprepositories-secret.yaml
@@ -1,5 +1,5 @@
{{- range .Values.apprepository.initialRepos }}
{{- if or .caCert .authorizationHeader }}
{{- if or .caCert .authorizationHeader .basicAuth }}
apiVersion: v1
kind: Secret
metadata:
Expand All @@ -21,9 +21,15 @@ data:
ca.crt: |-
{{ .caCert | b64enc }}
{{- end }}
{{- $authorizationHeader := "" }}
{{- if .authorizationHeader }}
{{- $authorizationHeader = .authorizationHeader | b64enc }}
{{- else if .basicAuth }}
{{- $authorizationHeader = printf "Basic %s" ((printf "%s:%s" .basicAuth.user .basicAuth.password) | b64enc) }}
{{- end }}
{{- if $authorizationHeader }}
authorizationHeader: |-
{{ .authorizationHeader | b64enc }}
{{ $authorizationHeader | b64enc }}
Copy link
Contributor

Choose a reason for hiding this comment

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

Isn't $authorizationHeader now double-base64 encoded? I'm guessing you want to remove the b64 encoding on lines 26 and 28 (or here and below on line 56).

Copy link
Contributor Author

@dud225 dud225 Sep 9, 2021

Choose a reason for hiding this comment

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

Hello
good catch there were one to many b64enc call, however for the basic authentication there are 2 b64enc involved:

If the "Basic" authentication scheme is used, the credentials are constructed by first combining the username and the password with a colon (aladdin:opensesame), then by encoding the resulting string in base64 (YWxhZGRpbjpvcGVuc2VzYW1l).
source: https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/Authorization

then when building the secret, the values for all keys in the data field have to be base64-encoded strings.

Here is an example:

$ helm template . --set apprepository.initialRepos[0].name=local --set apprepository.initialRepos[0].url=http://localhost --set apprepository.initialRepos[0].basicAuth.user=myuser --set apprepository.initialRepos[0].basicAuth.password=mypassword

yields:

# Source: kubeapps/templates/apprepository/apprepositories-secret.yaml
apiVersion: v1
kind: Secret
metadata:
  name: apprepo-local-secrets
  namespace: "default"
  labels:
    app.kubernetes.io/name: kubeapps
    helm.sh/chart: kubeapps-7.4.1-dev0
    app.kubernetes.io/instance: RELEASE-NAME
    app.kubernetes.io/managed-by: Helm
data:
  authorizationHeader: |-
    QmFzaWMgYlhsMWMyVnlPbTE1Y0dGemMzZHZjbVE9

which is decoded as follows:

$ echo -n QmFzaWMgYlhsMWMyVnlPbTE1Y0dGemMzZHZjbVE9 | base64 -d
Basic bXl1c2VyOm15cGFzc3dvcmQ=
$ echo -n QmFzaWMgYlhsMWMyVnlPbTE1Y0dGemMzZHZjbVE9 | base64 -d | sed 's/Basic //' | base64 -d
myuser:mypassword

whereas

$ helm template . --set apprepository.initialRepos[0].name=local --set apprepository.initialRepos[0].url=http://localhost --set apprepository.initialRepos[0].authorizationHeader="Bearer mytoken"

yields

# Source: kubeapps/templates/apprepository/apprepositories-secret.yaml
apiVersion: v1
kind: Secret
metadata:
  name: apprepo-local-secrets
  namespace: "default"
  labels:
    app.kubernetes.io/name: kubeapps
    helm.sh/chart: kubeapps-7.4.1-dev0
    app.kubernetes.io/instance: RELEASE-NAME
    app.kubernetes.io/managed-by: Helm
data:
  authorizationHeader: |-
    QmVhcmVyIG15dG9rZW4=

which is decoded as follows:

$ echo -n QmVhcmVyIG15dG9rZW4= | base64 -d
Bearer mytoken

Copy link
Contributor

@antgamdia antgamdia Sep 9, 2021

Choose a reason for hiding this comment

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

I initially thought the same, but I think it's right as is?

The header should be b64-ed itself, and the Basic auth is b64(user:pass), so, the whole thing is: b64("Basic b64(<user>:<pass>)") , no?

Haha, commenting at the same time 😆

Copy link
Contributor

Choose a reason for hiding this comment

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

For the basic auth it was, but an extra one had been added to the non-basic auth path. Thanks for the fix in 97830ba Herve.

{{- end }}
---
{{/* credentials are required in the release namespace for syncer jobs */}}
Expand All @@ -45,9 +51,9 @@ data:
ca.crt: |-
{{ .caCert | b64enc }}
{{- end }}
{{- if .authorizationHeader }}
{{- if $authorizationHeader }}
authorizationHeader: |-
{{ .authorizationHeader | b64enc }}
{{ $authorizationHeader | b64enc }}
{{- end }}
---
{{- end }}
Expand Down
4 changes: 2 additions & 2 deletions chart/kubeapps/templates/apprepository/apprepositories.yaml
Expand Up @@ -45,15 +45,15 @@ spec:
nodeSelector: {{- toYaml .nodeSelector | nindent 8 }}
{{- end }}
{{- end }}
{{- if or .caCert .authorizationHeader }}
{{- if or .caCert .authorizationHeader .basicAuth }}
auth:
{{- if .caCert }}
customCA:
secretKeyRef:
key: ca.crt
name: {{ printf "apprepo-%s-secrets" .name }}
{{- end }}
{{- if .authorizationHeader }}
{{- if or .authorizationHeader .basicAuth }}
header:
secretKeyRef:
key: authorizationHeader
Expand Down
6 changes: 5 additions & 1 deletion chart/kubeapps/values.yaml
Expand Up @@ -706,8 +706,12 @@ apprepository:
## url: https://chartmuseum.default:8080
## nodeSelector:
## somelabel: somevalue
## # Specify an Authorization Header if you are using an authentication method.
## # Specify an Authorization Header if you are using an authentication method:
## authorizationHeader: "Bearer xrxNC..."
## # Specify the credentials if you are using a basic authentication method:
## basicAuth:
## user:
## password:
## # If you're providing your own certificates, please use this to add the certificates as secrets.
## # It should start with -----BEGIN CERTIFICATE----- or
## # -----BEGIN RSA PRIVATE KEY-----
Expand Down