Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Version v5.0.0 wish list #1183

Open
monde opened this issue Jun 29, 2022 · 27 comments
Open

Version v5.0.0 wish list #1183

monde opened this issue Jun 29, 2022 · 27 comments
Labels
enhancement Asking for new behavior or feature

Comments

@monde
Copy link
Collaborator

monde commented Jun 29, 2022

Using this issue to gather suggestions for v5.0.0 of the provider which will occur sometime this year. Anything that is incompatible with v3.X.X / v4.X.X or incongruent with TF design principles https://developer.hashicorp.com/terraform/plugin/best-practices/hashicorp-provider-design-principles is on the table.

  • Fully utilize v3 of okta-sdk-golang
  • Replace provider config arguments org_name + base_url with org_url
    • For ENV VARs OKTA_ORG_NAME + OKTA_BASE_URL replaced by OKTA_ORG_URL
    • e.g. OKTA_ORG_URL="https://example.okta.com"
  • (keeping this for historical purposes, input validation was removed in v4 of the provider) Remove artificial input validation in the provider and allow the API responses dictate correct configuration
    • e.g. okta_idp_saml has property groups_action that (according to the hard coded validation in provider code) can only have values "NONE", "SYNC", "APPEND", or "ASSIGN"
    • If the Okta API changes then any artificial input validation in the provider's code can drift
  • Remove okta_email_sender and okta_email_sender_verification
  • Remove fido_webauthn and other outdated MFA authenticator
  • Remove brand_id in brand
  • Include default in base resource and remove default resource (authentication policy, need to audit for more)
  • Remove default in idp_type in resource_okta_policy_rule_idp_discovery and add support for dynamic idp add support for dynamic idp #1822
  • Deprecated okta_app_oauth_redirect_uri and okta_app_oauth_post_logout_redirect_uri
  • Remove default in inactivity_period in resource app_signon_policy_rule Remove inactivity_period default value from app_signon_policy_rule #2070
  • Change from required to optional field priority in resource okta_auth_server_policy
  • Rewrite okta_authenticator
@monde monde added the enhancement Asking for new behavior or feature label Jun 29, 2022
@monde monde pinned this issue Jun 29, 2022
@BalaGanaparthi
Copy link

BalaGanaparthi commented Jun 30, 2022

Resource definitions must be conducive to dynamic blocks.

Example : The current resource resource okta_policy_mfa has individual authenticators defined like

Note : Picking on okta_policy_mfa just to highlight the concept, but the same thought process is applicable for new resources or adapting the principles to the existing resource (or datasource) definitions .. where ever possible

resource "okta_policy_mfa" "MFAPolicy_master" {
  name        = "Service / Master Accounts"
  status      = "ACTIVE"
  description = "MFA Policy for Master Accounts"
  priority    = 1

  okta_verify = {
    enroll = "NOT_ALLOWED"
  }
  phone_number = {
    enroll = "NOT_ALLOWED"
  }
  webauthn = {
    enroll = "NOT_ALLOWED"
  }
  fido_u2f = {
    enroll = "NOT_ALLOWED"
  }
  okta_question = {
    enroll = "REQUIRED"
  }
  yubikey_token = {
    enroll = "NOT_ALLOWED"
  }
}

This resource definition can be formatted as

resource "okta_policy_mfa" "MFAPolicy_master" {
  name        = "Service / Master Accounts"
  status      = "ACTIVE"
  description = "MFA Policy for Master Accounts"
  priority    = 1

   authenticator  {
      type = okta_verify
      enroll = "NOT_ALLOWED"
   }
   authenticator  {
      type = phone_number
      enroll = "NOT_ALLOWED"
   }
   authenticator  {
      type = webauthn
      enroll = "NOT_ALLOWED"
   }
   authenticator  {
      type = fido_u2f
      enroll = "NOT_ALLOWED"
   }
   authenticator  {
      type = okta_question
      enroll = "REQUIRED"
   }
   authenticator {
      type = yubikey_token
      enroll = "NOT_ALLOWED"
   }
}

With the new resource definition format the resource block can be made very concise when multiple resource with different internal configurations are required to be managed.. where the script and data separation can be achieved... like the following

Data in JSON format

[
    {
       "name"          : "MFA Policy-1,
       "status"         : "ACTIVE",
       "description" : "This is MFA Policy One",
       "is_oie"          : True,
       "authenticators" : [
              {
                  "type" : "okta_verify"
                  "enroll" : "NOT_ALLOWED"
              },
              {
                  "type" : "phone_number"
                  "enroll" : "NOT_ALLOWED"
              },
              {
                  "type" : "webauthn"
                  "enroll" : "NOT_ALLOWED"
              },
              {
                  "type" : "okta_question"
                  "enroll" : "REQUIRED"
              },
       ]
    },
    {
       "name"          : "MFA Policy-2,
       "status"         : "ACTIVE",
       "description" : "This is MFA Policy Two",
       "is_oie"          : True,
       "authenticators" : [
              {
                  "type" : "okta_verify"
                  "enroll" : "REQUIRED"
              },
              {
                  "type" : "phone_number"
                  "enroll" : "NOT_ALLOWED"
              },
              {
                  "type" : "webauthn"
                  "enroll" : "NOT_ALLOWED"
              },
              {
                  "type" : "okta_question"
                  "enroll" : "DISABLED"
              },
       ]
    },
]

The script can be made concise and modularized for the data to be passed from external sources hence achieving good data and code separation

resource "okta_policy_mfa" "mfa_policies" {
   for_each = var.mfa_enroll_policy_list

   name          = each.value.name
   status         = each.value.status
   description = each.value.description
   is_oie          = each.value.is_oie
   
   dynamic "authenticator" {
      for_each = each.value.authenticators
         content {
            type = each.value.type
            enroll = each.value.enroll
         }
   }
}

The result : Can achieve separation of concerns by isolating the scripts from data.. where each can be managed relevant personnel. Data can evolve (with format conformance with the script) and no need to repeat the resource blocks within the script when multiple resources of the same types must be managed.

@noinarisak
Copy link
Contributor

noinarisak commented Jun 30, 2022

Documentation

Example: We need a Guides section that would allow new users to understand the following topics: How get started configuring the Provider, Migration instructions on the deprecated resource or data sources, etc.

Example: More consistency within each resources and data sources around examples or best practices.

@BalaGanaparthi
Copy link

Documentation

Every resource (and datasource) with a section of Required Okta OAuth Scopes that provides all required scopes to successfully execute the resource (or datasource) when OKTA_API_PRIVATE_KEY is used for credentials.

@monde
Copy link
Collaborator Author

monde commented Jun 30, 2022

A mechanism to partition feature behavior based on what kind of org the provider is pointed at and what can of feature flags are enabled on that org. I believe there are plans to expose a public API endpoint that will surface the feature flags/capabilities of an org that will allow this.

Also, something similar in the ACC tests. The ACC suite takes about 20 minutes to complete and I have to run it against four or more test orgs that have different capabilities to find truely failing ACC tests.

@delize
Copy link

delize commented Jul 6, 2022

Documentation Improvement


Personally, I would like to see the documentation include what the expected data types are for each argument resource.

An example of this is for the Sign On Policy.

The following arguments are supported:

[name](https://registry.terraform.io/providers/okta/okta/latest/docs/resources/policy_signon#name) - (Required) Policy Name.

[description](https://registry.terraform.io/providers/okta/okta/latest/docs/resources/policy_signon#description) - (Optional) Policy Description.

[priority](https://registry.terraform.io/providers/okta/okta/latest/docs/resources/policy_signon#priority) - (Optional) Priority of the policy.

[status](https://registry.terraform.io/providers/okta/okta/latest/docs/resources/policy_signon#status) - (Optional) Policy Status: "ACTIVE" or "INACTIVE".

[groups_included](https://registry.terraform.io/providers/okta/okta/latest/docs/resources/policy_signon#groups_included) - List of Group IDs to Include.

For each of these, what is the expected value data type outlined per line.

Some might be obvious (name, and description are usually strings), however others like priority are less obvious - Is this a number or a string? It isn't clear on first glance.

This is helpful when setting up "default values" in variables for the expected resource from scratch, rather than importing existing values first.

I also might be overlooking this and it may already be explained somewhere that I am just not looking at.

@monde
Copy link
Collaborator Author

monde commented Jul 10, 2022

#999

@tgoodsell-tempus
Copy link
Contributor

Lots of deprecated resources already trying to deal with this, however more strict focus on following the Terraform best practices of having the resources not mix and match various API objects: https://www.terraform.io/plugin/hashicorp-provider-design-principles#resources-should-represent-a-single-api-object

For example, the apps, groups, and users APIs should avoid calling both the "root" CRUD APIs as well as the "sub-apis" such as users and groups.

@ymylei
Copy link
Contributor

ymylei commented Jul 12, 2022

Sign On Policy

Adding onto this, may be easier to support this request overall if time is spent in re-doing the docs and generating using tfplugindocs. See https://www.terraform.io/registry/providers/docs#generating-documentation

@ktham
Copy link

ktham commented Jul 14, 2022

Remove artificial input validation in the provider and allow the API responses dictate correct configuration [...] If the Okta API changes then any artificial input validation in the provider's code can drift

As long as the input validation still happens at plan-time (and not apply-time), I am ok with that. I want to minimize situations where I merge in a PR that generated a valid TF plan, only to have it fail during apply-time.

@monde
Copy link
Collaborator Author

monde commented Jul 18, 2022

Remove deprecated resources.

@BalaGanaparthi
Copy link

BalaGanaparthi commented Jul 28, 2022

Datasources

Use-case this feature will solve

When a customer with existing Okta implementation without terraform would like manage all the existing configurations via terraform going forward

Feature requested

List Data Source :
Each Resource have a corresponding list Data Source, where the IDs of all the entities/objects configured in the tenant of that Resource Type are fetched

Full-Entity-Data Data Source :
Each Resource have a corresponding list Data Source, where full data of a specific entity/object configured in the tenant of that Resource Type is fetched based on the entity-ID provided as input to the Data Source. The Entity-ID can be fetched from the List Data Source. The output of the Data Source must be directly mapped to the attributes of the corresponding Resource

Slim-Entity-Data Data Source :
Each Resource have a corresponding list Data Source, where only attribute data of a specific entity/object configured in the tenant of that Resource Type is fetched based on the entity-ID provided as input to the Data Source. The Entity-ID can be fetched from the List Data Source.

Variation to Slim-Entity-Data Data Source : Accept the list of attribute of the entity/object to be returned.

How the feature will be used

  • Create a new tenant (with the FFs matching the existing tenant)
  • Create TF scripts with two Okta providers (one for existing/source tenant and another for new/target tenant)
  • For each resource, create two modules. One of existing/source tenant with the Data Sources and pass the corresponding provider and the other with the Resource (and pass the corresponding provider)
  • (Need to check if the providers for source and target can be of the same tenant : That way can generate the state of the tenant with this method)

Multiple Provider Configurations

provider "okta" {
  alias = source
  # Configuration options
}

provider "okta" {
  alias = target
  # Configuration options
}

module "groups-source" {
  source          = "./modules/groups-src"
  # Module to get existing group details from source tenant (by using datasources)
  providers = {
    okta = okta.source
  }
} 

module "groups-target" {
  source          = "./modules/groups-target"
  # Module to create groups (using Resource) with the output of groups-source module piped as input to this module
  providers = {
    okta = okta.target
  }
} 

@delize
Copy link

delize commented Aug 5, 2022

Official Okta Modules

Would be nice to have Okta officially come up with dynamic/flexible module uses where it makes sense to package or bundle the resources that work together or are highly functional together (EG: Sign On Policies & Sign On Rule Policies).

While it is great that these are separate resources, they could function together as a module. Having Okta officially outline the "best practice" (cringey word, but at least Okta's internal best practice) would be nice.

@monde
Copy link
Collaborator Author

monde commented Aug 5, 2022

Thanks @delize . I'm not sure on the follow through, but I do know our product people want to get some sort of reference examples/documention/configurations published for the TF provider. As there are many different ways to use the provider I would think there might be multiple examples.

@ruimarinho
Copy link

@monde I think I can provide a script in terms of what I think are onboarding requirements and how those could be translated into this TF provider. E.g. dashboard configuration, authenticator requirements, profile mappings from HR system (which I think are not supported yet?), dynamic group assignments with rules, first SAML app, etc.

@monde
Copy link
Collaborator Author

monde commented Aug 8, 2022

@ruimarinho awesome, share your config in a gist here if it can be shared in the public else send me your artifacts [email protected] and I'll be able to share with product/documentation.

@ruimarinho @delize , coincidentally, my recollection was correct, just saw a planning deck today, we are in Q3 at Okta, and providing reference architectures for all of our SDKs and the TF provider is a pilot project this quarter.

@talmarco
Copy link

The "Refresh application data" button, which is currently only available from the user interface, would be great to see implemented.
Okta is heavily used and integrates with Terraform for automating the creation of applications, groups, and assignments, but there is still a need to manually refresh application data and then re-run Terraform.

@monde monde changed the title Version v4.0.0 wish list Version v5.0.0 wish list Oct 25, 2022
@monde
Copy link
Collaborator Author

monde commented Oct 25, 2022

renamed v5.0.0 . The v4.0.0 release plan is explained here: v4.0.0 release plan #1338

@tgoodsell-tempus
Copy link
Contributor

Something to consider for the v5 milestone, HashiCorp is now recommending we move to terraform-provider-framework now that they've 1.0'd it: https://developer.hashicorp.com/terraform/plugin/which-sdk#are-you-writing-a-new-provider-or-maintaining-an-existing-provider

@github-actions
Copy link

This issue is stale because it has been open 60 days with no activity. Remove stale label or comment or this will be closed in 5 days

@github-actions github-actions bot added the stale label Feb 19, 2023
@github-actions github-actions bot closed this as not planned Won't fix, can't repro, duplicate, stale Feb 25, 2023
@monde monde removed the stale label May 31, 2023
@monde monde reopened this May 31, 2023
@duytiennguyen-okta duytiennguyen-okta unpinned this issue Jun 27, 2023
@duytiennguyen-okta duytiennguyen-okta pinned this issue Jun 27, 2023
@tgoodsell-tempus
Copy link
Contributor

Something else for this if considered:

We should cutover the Plugin Protocol used for this provider to V6, see: https://developer.hashicorp.com/terraform/plugin/terraform-plugin-protocol#protocol-version-6

Should open up features and things as we implement (and re-implement) more of the provider in the terraform-plugin-framework.

@exitcode0
Copy link
Contributor

It might make sense for this one to go in as a part of a V5 release
it might require a state file migration? or perhaps a breaking change?
#1474

@tgoodsell-tempus
Copy link
Contributor

Based on #1769

I think we should also remove all the client_secret management from okta_app_oauth, this should all be transferred to a separate resource which handles all the nuance and dual value support better.

@duytiennguyen-okta
Copy link
Contributor

Something else for this if considered:

We should cutover the Plugin Protocol used for this provider to V6, see: https://developer.hashicorp.com/terraform/plugin/terraform-plugin-protocol#protocol-version-6

Should open up features and things as we implement (and re-implement) more of the provider in the terraform-plugin-framework.

It is not possible if we still have resource in plugin sdk as v5 protocol is how hashi mux the 2 server together. Have to stay with v5 for now

@tgoodsell-tempus
Copy link
Contributor

Something else for this if considered:
We should cutover the Plugin Protocol used for this provider to V6, see: https://developer.hashicorp.com/terraform/plugin/terraform-plugin-protocol#protocol-version-6
Should open up features and things as we implement (and re-implement) more of the provider in the terraform-plugin-framework.

It is not possible if we still have resource in plugin sdk as v5 protocol is how hashi mux the 2 server together. Have to stay with v5 for now

This statement is false, there is a 5to6 mux the same way a 6to5 mux exists, see: https://developer.hashicorp.com/terraform/plugin/mux/translating-protocol-version-5-to-6

Protocol using this is backwards compatible per: https://developer.hashicorp.com/terraform/plugin/terraform-plugin-protocol

@duytiennguyen-okta
Copy link
Contributor

Interesting, I will look into it

@monde
Copy link
Collaborator Author

monde commented Dec 14, 2023

I'm seeing merit in bringing back input validation. The SDK client okta-sdk-golang v3 that we've slowly been transitioning to already does it by default on enum parameters. We'd only have to bring back validation on data surfaces like min/max for integers and min/max string length, etc. . See #1839

It's a pain to maintain artificial input validation and do timely updates with this provider. It can often block customers using some new ability in the Okta API. However, it seems like everyone would prefer stability that input validation gives on the plan.

@lukezilch
Copy link

Would be great if we could get push groups added into the okta provider, one of my single PIA is push groups, some apps have so many that it would be great to add the groups to the “push groups” section via my terraform instead of having to manually assign them

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement Asking for new behavior or feature
Projects
None yet
Development

No branches or pull requests