Improve Secrets Handling

[cgardens]

Tell us about the problem you're trying to solve

• When requesting previously configured connectors, secrets are returned from from the API and viewable in plain text.
• Out of scope: Obscuring secrets as I type them the first time.

Describe the solution you’d like

• credit for this impl goes to @michel-tricot , i'm just writing it down.
• Add support for an attribute / convention in our jsonschema for sensitive fields. e.g.

  "password": {
    "type": "string",
    "sensitive": true
  }
  

• API Get / List: For fields with this flag, the API should remove the value when returning it on a read request and instead insert the value **********
• API Put: For for fields with this flag, if the value returned from the API is **********, then the API keeps the previous value of the field. Performs a merge of the object provided in the PUT and the existing object.
  • Note: This breaks our original contract in our API docs around puts (that we always replace the whole object. we should document this)
• UI Read: When it receives from the API a field with this flag it can act as normal, it just displays **********
• UI Update: This has some edge cases.
  • If user updates config but does not touch sensitive field, UI sends ********** to the API (which means it is a no op)
  • If user deletes the field (removes all of the *), then UI sends empty value to API, which gets interpretted as deleting the value.
  • If user does something in the middle? Deletes some of the dots. Goes to enter a new password and then realized they don't want to edit that field?
    • talked to artem, ui will have a special secrets component to handle this case.
    • @michel-tricot - I don't think we discussed this case.
    • options:
      1. any number of asterisk. (handles first case well but doesn't handle second case)
      2. add a reset button. (more works but handles both cases)

Additional context

• Preferring this over adding patch updates to the API.
• Whoever takes this on, please create child tickets, one for API and one for UI.

[sherifnada]

Isn't this going to lead to a bad UX from the API side because it would require running a get-spec for every integration that we run? it may be time to consider doing spec caching to avoid this issue. The tradeoff there would be:

1. dynamic specs wouldn't work with caching (though we could get around this by adding a airbyteCacheable: false field)
2. hotfixing a docker image with a new spec wouldn't work unless clients force pull the docker image. Alternatively we could check for a new image everytime we run a docker process and this wouldn't be an issue 99% of the time (the 1% being running standalone containers).

[cgardens]

do we need to do it now? we already have this problem all over in the ui and we tolerate it. agreed it's not good, but not sure that now is the time to fix it.