OpenId Deployment and Recipes have some issues

[deanmarcussen]

There are a number of issues with the OpenId Recipe and Deployment Steps

• The Deployment Step for the Server Settings produces completely incorrect JSON to match what the Recipe Step is expecting (the settings directly, when the step expects a Model)

• Some of the Steps use Models, some use CreateViewModels

• Most of the Steps are storing their data directly in the step property, where they should be keyed. (Minor issue)

• The Application Step can only be run once, as it does not check to see if the application already exists, so throws an exception the second time.

[Tsjerno]

Added some code to fix the issues
See #7646

[kaipm]

I'm working on the view model refactor, but I got stuck a little. It's more a decision, rather than a technical problem.

Due to the recipe using a view model, currently only the following format works in tests:

  "steps": [
    {
      "name": "OpenIdApplication",
      ...
      "roleEntries": [
        {
          "name": "role1",
          "selected": true
        }
      ]
    }
  ]

selected missing or equal to false results in the role beeing ignored.

All the other list-properties use a string, delimited by space like this:

...
"redirectUris": "https://uri1 https://uri2",
...

I'm not sure how I should fix this now. I could:

• replicate the view model format with custom models, so existing recipes don't break
• introduce breaking changes:
  • make the property a string, delimited by space, to match other properties
  • eventually also rename it to roles or clientCredentialRoles
• try to support both formats, with the caveat of additional complexity

Any advice would be appreciated!

[sebastienros]

I understand the issue, the step is using a "View Model" that is designed for checkboxes, and in aspnet it needs a bool to map the state of the checkbox. In the step it just need a presence. So maybe change the step logic by removing the Select(r => r.Selected == true) and take all the steps that are listed. Or use a different class that also has RoleEntries but no selected field. And stop using the View model class.

[kaipm]

Right, I'll do that then, thank you!

[kaipm]

There are a number of issues with the OpenId Recipe and Deployment Steps

• The Deployment Step for the Server Settings produces completely incorrect JSON to match what the Recipe Step is expecting (the settings directly, when the step expects a Model)
• Some of the Steps use Models, some use CreateViewModels
• Most of the Steps are storing their data directly in the step property, where they should be keyed. (Minor issue)
• The Application Step can only be run once, as it does not check to see if the application already exists, so throws an exception the second time.

@deanmarcussen, the above points, except the 3rd point, are fixed and merged now. No. 3 requires breaking changes but was deemed a minor issue, so I was wondering if this issue can be closed?

[deanmarcussen]

Fixed by @kaipm various prs. Thanks

[ShaneCourtrille]

@deanmarcussen Is there any cleanup that can be done to resolve "An application with the same client identifier already exists." when recreating a tenant? Or does it depend on these PRs?