onelogin / terraform-provider-onelogin

GNU General Public License v3.0
27 stars 19 forks source link

onelogin_oidc_apps displays sensitive data #36

Closed paulstakem closed 3 years ago

paulstakem commented 3 years ago

Hello,

When updating an onelogin_oidc_apps app resource that is already in state, the OIDC client_id and client_secret

Example

------------------------------------------------------------------------

An execution plan has been generated and is shown below.
Resource actions are indicated with the following symbols:
  ~ update in-place

Terraform will perform the following actions:

  # onelogin_oidc_apps.example will be updated in-place
  ~ resource "onelogin_oidc_apps" "example" {
        allow_assumed_signin = false
        auth_method          = 8
      ~ configuration        = {
          ~ "login_url"                  = "https://localhost" -> "https://localhost:3000"
            "oidc_application_type"      = "0"
          ~ "redirect_uri"               = "https://localhost/oauth2/callback" -> "https://localhost:3000/oauth2/callback"
            "token_endpoint_auth_method" = "1"
        }
        connector_id         = 38568
        created_at           = "2020-12-17 09:46:50.742 +0000 UTC"
        description          = "Example"
        icon_url             = "/images/missing_connector_icon/square/original.png"
        id                   = "392906"
        name                 = "Example OIDC App"
        notes                = "Example"
        policy_id            = 0
        provisioning         = {
            "enabled" = false
        }
        sso                  = {
            "client_id"     = "bbbbbbbb-2222-1111-3333-12312312312312312"
            "client_secret" = "d5f29b6cd5f29b6cd5f29b6cd5f29b6cd5f29b6cd5f29b6cd5f29b6cd5f29b6c"
        }
        tab_id               = 0
        updated_at           = "2020-12-17 09:46:50.742 +0000 UTC"
        visible              = false

        parameters {
            attributes_transformations = "group_list"
            include_in_saml_assertion  = false
            label                      = "Groups"
            param_id                   = 55555
            param_key_name             = "groups"
            provisioned_entitlements   = false
            safe_entitlements_enabled  = false
            skip_if_blank              = false
            user_attribute_mappings    = "member_of"
        }
    }

Plan: 0 to add, 1 to change, 0 to destroy.

------------------------------------------------------------------------

Code affected https://github.com/onelogin/terraform-provider-onelogin/blob/7f9a226960fbfd741de3e9c2c99246c95910be66/onelogin/resource_onelogin_oidc_apps.go#L26-L30

Suggested fix

    appSchema["sso"] = &schema.Schema{
        Type:     schema.TypeMap,
        Computed: true,
        Elem:     &schema.Schema{Type: schema.TypeString},
        Sensitive: true,
    }

Setting the Sensitive Bool as described here https://www.terraform.io/docs/extend/schemas/schema-methods.html

Which results in the client_id and client_secret are marked as sensitive.

Terraform will perform the following actions:
  # onelogin_oidc_apps.example will be updated in-place
  ~ resource "onelogin_oidc_apps" "example" {
        allow_assumed_signin = false
        auth_method          = 8
        brand_id             = 0
      ~ configuration        = {
          ~ "login_url"                  = "https://localhost" -> "https://localhost:3000"
            "oidc_application_type"      = "0"
          ~ "redirect_uri"               = "https://localhost/oauth2/callback" -> "https://localhost:3000/oauth2/callback"
            "token_endpoint_auth_method" = "1"
        }
        connector_id         = 38568
        created_at           = "2020-12-17 09:46:50.742 +0000 UTC"
        description          = "Example"
        icon_url             = "/images/missing_connector_icon/square/original.png"
        id                   = "392906"
        name                 = "Example OIDC App"
        notes                = "Example"
        policy_id            = 0
        provisioning         = {
            "enabled" = false
        }
        sso                  = (sensitive value)
        tab_id               = 0
        updated_at           = "2020-12-17 09:46:50.742 +0000 UTC"
        visible              = false

        parameters {
            attributes_transformations = "group_list"
            include_in_saml_assertion  = false
            label                      = "Groups"
            param_id                   = 55555
            param_key_name             = "groups"
            provisioned_entitlements   = false
            safe_entitlements_enabled  = false
            skip_if_blank              = false
            user_attribute_mappings    = "member_of"
        }
    }
dcaponi commented 3 years ago

Hey @paulstakem Thanks for the awesome, well constructed issue 😄

I'll add the sensitive flag as soon as I get a gap in my schedule to work on this... should be pretty soon

Could you please tell me which TF version you're using? I have 0.12.24 and I wasnt able to get it to work right off the bat. I also saw this open issue regarding the sensitive flag and I wonder if I just need an upgrade of TF to see/verify this works on my end.

paulstakem commented 3 years ago

Hey @paulstakem Thanks for the awesome, well constructed issue 😄

I'll add the sensitive flag as soon as I get a gap in my schedule to work on this... should be pretty soon

Could you please tell me which TF version you're using? I have 0.12.24 and I wasnt able to get it to work right off the bat. I also saw this open issue regarding the sensitive flag and I wonder if I just need an upgrade of TF to see/verify this works on my end.

Thanks @dcaponi really appreciate the quick response.

We're keeping up with the terraform releases given the v1.0 announcements earlier this year.

❯ terraform version
Terraform v0.13.5
+ provider onelogin.com/onelogin/onelogin v0.1.0

terraform {
  required_providers {
    onelogin = {
      source  = "onelogin.com/onelogin/onelogin"
      version = "0.1.0"
    }
  }
  required_version = ">= 0.13"
}

I used the sideload makefile task to test Sensitive flag, hence the provider is v0.1.0

dcaponi commented 3 years ago

Ah ha, thats what it was... needed to upgrade on my end. Your change works, and I deployed the fix so please upgrade your onelogin provider to version = "0.1.6" and your change should be there.

P.S. There is a Terraform v0.14 that is out. Since you said you were generally keeping up with the latest, I thought I should call your attention to that. I tested your change with v0.14 as well and found that it generally hides unchanged fields (including SSO) but Im not sure if/where the sensitive field mask is logged permanently.

Here's a sample of what happens when I upgrade to v0.14

onelogin_oidc_apps.test: Refreshing state... [id=123]

An execution plan has been generated and is shown below.
Resource actions are indicated with the following symbols:
  ~ update in-place

Terraform will perform the following actions:

  # onelogin_oidc_apps.saml will be updated in-place
  ~ resource "onelogin_oidc_apps" "test" {
      ~ description          = "hidden secret test" -> "hidden secret"
        id                           = "123"
        name                     = "TF OIDC App"
        # (13 unchanged attributes hidden)

        # (1 unchanged block hidden)
    }

Plan: 0 to add, 1 to change, 0 to destroy.

Do you want to perform these actions?
  Terraform will perform the actions described above.
  Only 'yes' will be accepted to approve.

  Enter a value: yes

@paulstakem if this is working on your end, could you please close out the issue to let me know that this has in fact been resolved.

paulstakem commented 3 years ago

@dcaponi Works as expected, thanks again.