auth0-deploy-cli: Inconsistent connection update behavior, potentially causing client_secret deletion

Description

deploying config a0deploy deploy -c tenant/config.json --input_file tenant/tenant.yaml

results in error:

error: Problem running command deploy during stage processChanges when processing type connections error: Problem updating connections {“name”:“###”,“id”:“###”} Bad Request: Before creating Windows Azure Active Directory connections you must configure the client_id and client_secret settings in the dashboard. For more information go to https://docs.auth0.com/waad-clientid

also:

error: Problem running command deploy during stage processChanges when processing type connections error: Problem updating connections {“name”:“####”,“id”:“####”} Bad Request: The signing certificate is not valid. Please provide a .pem or .cer certificate base64 encoded public key

Reproduction

Export an existing environment, including one or more AzureAD connections

We are stripping out some sensitive information on export

config.json:

{
...
  "AUTH0_ALLOW_DELETE": false,
  "AUTH0_EXPORT_IDENTIFIERS": false,
  "AUTH0_EXCLUDED_RULES": [ "rule-1-name" ],
  "AUTH0_EXCLUDED_CONNECTIONS" : [
    "####",
    "smoke-test"
  ],
  "EXCLUDED_PROPS": {
    "connections": [
        "options.client_secret",
        "options.admin_access_token",
        "options.admin_access_token_expiresin",
        "options.cert",
        "options.signingCert"
    ]
  },
    "INCLUDED_PROPS": {
      "rulesConfigs": [ "key" ]
  }
}

tenant.yaml:

connections:
  - name: ####
    strategy: waad
    enabled_clients:
      - ####
    is_domain_connection: false
    options:
      domain: ####.com
      granted: true
      tenantId: ####
      client_id: ####
      use_wsfed: false
      app_domain: ####.auth0.com
      ext_groups: true
      ext_profile: true
      thumbprints:
        - ####
        - ####
        - ####
        - ####
        - ####
        - ####
      identity_api: azure-active-directory-v1.0
      basic_profile: false
      tenant_domain: ####.com
      waad_protocol: openid-connect
      domain_aliases:
        - ####.com
      api_enable_users: false
      ext_nested_groups: true
      useCommonEndpoint: false
      should_trust_email_verified_connection: ####
    show_as_button: false

Deploy the config: a0deploy deploy -c tenant/config.json --input_file tenant/tenant.yaml

the deployment halts with the given error.

The strange thing is that this worked perfectly in 2 other environments, with almost identical configurations (azuread and samlp connections) just a few days ago. Re-testing those environments they now throw the same errors. It looks like these options are now required instead of optional.

Workaround:

for now, you can add the failing connections to the AUTH0_EXCLUDED_CONNECTIONS list to skip them entirely

Environment

Version of this library used: latest = 7.3.7

About this issue

Original URL
State: closed
Created 2 years ago
Comments: 15 (8 by maintainers)

Most upvoted comments

This was also a big thing for me, I had assumed they were bi-directional and the documentation makes no mention that it will only be adhered to on import.

My understanding is that this only affects properties nested under the options property for connections. This is because most other endpoints adhere to normal PATCH conventions where excluded properties are ignored. However, as we can see in the endpoint docs, the options property does not adhere to that convention. So for most other circumstances it turns out to work bi-directionally.

willvedd on Mar 3, 2022

@lundybernard @kanocarra thanks for being patient. We’ve addressed this in our 7.5.0 release, I encourage you folks to update and test out.

The approach taken can be looked at more closely in #430 but in brief, we’ve just added excluded properties back onto the PATCH payload, which makes the bi-directional exclusion more explicit. We did consider the __IGNORE__ syntax, but it would’ve added much more surface area to the problem when this only affects connections. So for the sake of simplicity and backwards compatibility, we decided against it for now.

I appreciate the patience and feedback on this issue; don’t ever want to have the usage of this tool result in destructive changes. Going to close this out but please raise another issue if you’re experiencing any issues with the new release.

willvedd on Mar 9, 2022

Here’s a solution to consider:

Add a special value like __IGNORE__ that can be used in the configuration files.

On export, any EXCLUDED_* values can be replaced with the special IGNORE value. That way exported connections can will show the config spec, and explicitly flag ignored values.

On import/deploy, it also gives a much more convenient way to exclude objects that we do not want to deploy. for example:

rules: __IGNORE__
connections:
  - name: my-connection
  - client_secret: __IGNORE__
  ...

would totally skip all rules and do whatever is necessary to leave the connection’s client_secret value unchanged.

Its not as elegant a solution as I had hoped for ignoring individual connections, and other objects. Because the objects are all stored in lists instead of maps with their unique identifiers as keys, its not as easy as

connections:
  my_connection: __IGNORE__

Instead we would need a slightly messier syntax like:

connections:
  - name: __IGNORE__ my_connection

connections:
  - name: my_connection
  - __IGNORE__: True

lundybernard on Mar 3, 2022

@lundybernard Understood, thanks for confirming. I think this is a reasonable use case and enough for us to explore some possible solutions in the coming weeks.

willvedd on Mar 3, 2022

@willvedd

Our use case is much the same as kanocarra.

Essentially we are using a0deploy as a configuration management system.
- We want to store the configuration in a git repo
  - so that changes are subject to review
  - history is recorded
- We use the export (pull) functionality to diff the configs and look for changes made outside of change-control
- We use the import (push) functionality:
  - to maintain a single source of truth
  - for disaster recovery
  - to coordinate changes during deployments
- Long term we would like to setup a base template, and over-ride configs for each environment, which I believe is already possible.
We do not want sensitive information stored in our github repositories.
- Long term we may use environment variables to inject those secrets in a secure way.

I also expect EXCLUDED_PROPS and EXCLUDED_CONNECTIONS, and other EXCLUDE* settings to be bi-directional, essentially acting like entries in .gitignore. Better documentation about what settings affect export/import would be valuable.

Thanks for your help!

lundybernard on Mar 3, 2022

@lundybernard Appreciate the follow-up. I am able to confirm your observations, there is inconsistent behavior within the PATCH connections endpoint depending on the connection type.

Azure AD: Validates payload for the existence of client_id and client_secret, returns error. Google Apps: Does not validate payload for client_secret . Succeeds and deletes client_secret.

It appears that there are some reasons why these differences exist, but going to work further with the API team to see if we can bring more parity there.

I’m also going to work with our team to implement warning and safeguards in this project to prevent client_secret and other important properties from being deleted to existing resources; I don’t believe there is a strong-enough use case to allow this behavior.

However, in order to provide a more long-term fix, can you provide an explanation for excluding those properties? I understand that they’re sensitive values, are you looking to prevent them from being written to plain-text?

willvedd on Mar 2, 2022