usebruno / bruno

Opensource IDE For Exploring and Testing Api's (lightweight alternative to postman/insomnia)
https://www.usebruno.com/
MIT License
25.72k stars 1.17k forks source link

Environment Secrets #199

Closed helloanoop closed 1 year ago

helloanoop commented 1 year ago

Objective Enable developers to use secrets in environment variables without storing them in {env}.bru files This is so that the secrets don't get committed to git.

Implementation is based on some ideas put forth by @brainomite and @fcr--

For earlier discussions - please check #122 and #109 Creating a new issue to track a specific solution approach

Part 1 The DSL (Bru Lang)

Choosing the below approach over saving the var as token: $secret$ I feel it better for the DSL to explicitly specify secrets as a separate block for the below reasons

vars:secret { token: null }


**Part 2**
UI Updates

We add another column called secret which has a checkbox to indicate whether a var is a secret or not.

**Part 3**
Interceptor to store the secret in a file (at the electron layer). This component will replace the secret vars as null before writing them to `{env}.bru` files and hydrate them back using the cached secret values while loading them in UI.

Store the secrets inside a file `preferences.json` (which we already use) via `electron-store` with secrets encrypted via [safe-storage](https://www.electronjs.org/docs/latest/api/safe-storage)
```json
{
  "secrets": { 
    "collections": [{
      "path": "/Users/anoop/Code/acme-acpi-collection",
      "environments" : [{
        "name": "Local",
        "secrets": [{
          "name": "token",
          "value": "abracadabra"  
        }]
      }]
    }]
  }
}

Tasks

All changes are tracked in feature/env-secrets branch. I have already completed implementation for the Bru Lang updates

@brainomite I saw that you already working on Tasks 2 and 3 Let me know if you'd like to make any changes on the implementation spec. I wrote this since I saw you already working on this feature so that we have a consensus on the implementation approach.

szarrougtw commented 1 year ago

Thanks for making us a branch and for doing the changes for the bru lang updates!

My original thought was that we don't put secrets in the env files at all. So with your example above, the env file would be

vars {
  url: http://localhost:3000
  ~status: active
}

and the electon-store would have

{
  "secrets": { 
    "collections": [{
      "path": "/Users/anoop/Code/acme-acpi-collection",
      "environments" : [{
        "path": "Local.bru",
        "secrets": [{
          "name": "token",
          "value": "abracadabra"  
        }]
      }]
    }]
  }
}

If you want the secrets explicitly stated in the env files, maybe instead of doing

vars {
  url: http://localhost:3000
  ~status: active
}

vars:secret {
  token: null
}

we can do something like

vars {
  url: http://localhost:3000
  ~status: active
}

vars:secret [
  token1
]

That way we don't have to nullify at all. We would never reference the value of the secret variable at all making it less likely that we introduce a bug that prints the real value.

helloanoop commented 1 year ago

Hey @szarrougtw !

The advantage of stating the secret names explicitly is that - whenever a developer clones (ex: via git) and opens an api collection, they are nudged in the environments settings ui to fill the secret values

vars:secret [
  token1
]

I concur with your suggestion to store the list of var names using an array syntax. I will spend some time tomorrow on the bru lang updates for the same.

szarrougtw commented 1 year ago

Thanks!

helloanoop commented 1 year ago

@szarrougtw @brainomite I have made the Bru Lang updates to support the array syntax for storing secret names

mirkogolze commented 1 year ago

Hi, and how will it work when running the collections with the CLI. Lets say in Gitlab CI. For this I would prefer a solution to get the secret values from ENV-Variables.

variable comes from Electron Store

vars:secret [
  $STORE$token1
]

variable comes from process environment variable.

vars:secret [
  $ENV$token1
]
helloanoop commented 1 year ago

@mirkogolze While running in CLI, We can pass the env arg via command like.

bru run --env-var access_key=topsecret
helloanoop commented 1 year ago

There were some prettier updates that had to be done on the main branch. I tried to rebase the current changes with main branch, but that ended being messy.

The new branch feature/env-secrets now has the bru lang updates and also has the latest changes from the main branch

helloanoop commented 1 year ago

@brainomite @szarrougtw

I have completed all the functionalities and this is now available in v0.15.0 of Bruno. Please install the latest version from the website.

My apologies. I know you guys would have loved to deliver this, but there were too many requests and this has been a long standing feature ask from the community. Hence I had to expedite and complete dev on this.

@szarrougtw I must thank you again for you suggestion to use the array syntax instead of saving it using existing object syntax (storing the values as null) was. It's very eloquent.

vars:secret [
  token
]

@brainomite I went through your fork, and your writeup on use electron safeStorage also helped me formulate a vision to store the secrets in an encrypted manner in the app. FYI, I also added a fallback to use AES256 in-case safeStorage is not available.

I am also happy to share that we are introducing 2 ways to manage secrets. You can read more about it in the documentation

If you are interested to know the code changes that were done, here is the pr

I must also thank @mirkogolze for doing the Bru CLI updates for supporting --env-var name=val The Bru CLI v0.8.0 has been published.

Thanks everyone !!

brainomite commented 1 year ago

@helloanoop I will say the following, @szarrougtw did all of the work on my fork. She deserves all the credit not I.

szarroug3 commented 1 year ago

@brainomite @szarrougtw

I have completed all the functionalities and this is now available in v0.15.0 of Bruno. Please install the latest version from the website.

My apologies. I know you guys would have loved to deliver this, but there were too many requests and this has been a long standing feature ask from the community. Hence I had to expedite and complete dev on this.

@szarrougtw I must thank you again for you suggestion to use the array syntax instead of saving it using existing object syntax (storing the values as null) was. It's very eloquent.

vars:secret [
  token
]

@brainomite I went through your fork, and your writeup on use electron safeStorage also helped me formulate a vision to store the secrets in an encrypted manner in the app. FYI, I also added a fallback to use AES256 in-case safeStorage is not available.

I am also happy to share that we are introducing 2 ways to manage secrets. You can read more about it in the documentation

If you are interested to know the code changes that were done, here is the pr

I must also thank @mirkogolze for doing the Bru CLI updates for supporting --env-var name=val The Bru CLI v0.8.0 has been published.

Thanks everyone !!

No worries! I got a bit busy this past week so I'm just glad it got done honestly!

mirkogolze commented 1 year ago

Hi @helloanoop,

thanks for the fast implementation of this feature. The additional variant with .env file does not work in the same way the secret vars are working. They are not activated in scripts.

For bru.getEnvVar only the envVariables are injected in the constructor. I think here should be implemented the interpolation with processEnvVars .

For Test VarTesting.zip

helloanoop commented 1 year ago

@mirkogolze Yup. This is a bug. Created #216 to track this.