1Password / connect

Access your 1Password secrets using a 1Password Connect Server
https://developer.1password.com/docs/connect
149 stars 28 forks source link

The "Getting Started" section in the kubernetes example appears to be incorrect #62

Open monitz87 opened 1 year ago

monitz87 commented 1 year ago

This might be because my use case is weird, but still.

I'm using Kustomize with the helmCharts option to inflate the 1password connect charts, but I'm deploying the secret that contains 1password-credentials.json using the command in the kubernetes example.

This is the command I'm referring to

kubectl create secret generic op-credentials --from-file=1password-credentials.json=./1password-credentials.json

That produces the following secret:

apiVersion: v1
data:
  1password-credentials.json:  <base64_encoded_file_contents>
kind: Secret
metadata:
  creationTimestamp: "2023-03-04T05:14:48Z"
  name: op-credentials
  namespace: default
  resourceVersion: "9983"
  uid: 4454843d-8d82-4a87-8f7a-366eaf34e753
type: Opaque

The problem is that what gets assigned to the OP_SESSION env variable in the connect-api container is already decoded, so it's the plain JSON contents of the file (I quadruple checked by opening an ssh tunnel into the container), but apparently the container expects the value to still be base64 encoded, and tries to decode it again.

This causes 500 errors when making requests to the API, which look like this in the service logs:

Server: (unable to get credentials and initialize API, retrying in 30s), Wrapped: (failed to FindCredentialsUniqueKey), Wrapped: (failed to loadCredentialsFile), Wrapped: (LoadLocalAuthV2 failed to credentialsDataFromBase64), illegal base64 data at input byte 0"

I managed to make things work by encoding the JSON to base64, encoding that base64 value to base64 AGAIN, and then manually creating the secret manifest file and assigning that doubly-encoded value to the data.1password-credentials.json key.

There's a chance that this is caused by a combination of the kubernetes (v1.25.4) and chart (non-specified, so I assume latest) versions I'm using. The images for both connect-api and connect-sync are at version 1.5.7.

It feels more like either a bug in the container or an error in the example docs, although my money is on the bug, I see no sensible reason why the file should be encoded/decoded twice.

I'm equal parts frustrated and proud for having finally figured this out.

Cheers

pjmagee commented 1 year ago

I just had this problem today following the docs

monitz87 commented 1 year ago

I'm going to shamelessly bump this.

I know fixing the bug is not trivial (lots of moving parts, updating the image, helm chart, tests, making sure everything works, etc etc).

On the other hand, adding a simple note in the docs explaining the bug and how to work around it should take less than 5 minutes, and it would save countless hours of dev time for users.

I'll happily submit a PR for the docs, but I'm not sure if you'd rather present it as a bug or intended behavior.

damoun commented 1 year ago

I've got the same issue and so I also had to double encode 1password-credentals.json.

cat 1password-credentials.json|base64| tr -d \\n|base64

monitz87 commented 1 year ago

If anyone from the team can tell me whether they want to present this in the docs as intended behavior or a bug with its corresponding workaround, I will gladly submit a PR that addresses the issue.

heliochronix commented 1 year ago

I too have ran into this issue when attempting to create a sealed secret for the credentials. It's a bit confusing how to go about this with the double encoding. Should I create the secret with --from-literal=1password-credentials.json=<base64 encoded 1password-credentials.json>?

dannysauer commented 3 months ago

@heliochronix , I assume you've just given up by now... But for future seekers of information: The key I finally stumbled across after way too long is to remove the line wraps base64 adds by default. If your chart is deployed to the namespace onepassword-whatever, which would be weird, and you're using the other helm chart defaults:

mv 1password-credentials.json 1password-credentials.json.pre-encode
base64 -w 0 < 1password-credentials.json.pre-encode > 1password-credentials.json
kubectl --namespace onepassword-whatever create secret generic op-credentials --from-file 1password-credentials.json

Another option for that middle step is cat 1password-credentials.json.pre-encode | base64 | tr -d \\n > 1password-credentials.json, though there are probably lots of other options too.

Either way, when you use the base64 CLI, you need to stop it from inserting newlines every 80 chars, because the second encode includes those newlines and results in a broken file. It shouldn't break, since most base64 decode implementations ignore newlines. I'd argue that's a bug. But this whole behavior is a little sketchy to begin with, so what's one more quirk? :D

pabloaugusto commented 2 weeks ago

Same issue here deploying 1password-connect do kubernetes managed by flux OS. After 2 days trying make a deploy works. Finally done just encoding 1password-credentials.json to base64 after store it as a secret on ks8.

Please, update documentation or fix the bug!