correctly transfer username validation to auth0 #258

etwillbefine · 2020-07-26T09:11:51Z

Proposed Changes

Correctly sending username validation to Auth0

Fixes #238

Acceptance Test Output

make testacc TESTS=TestAccConnection
==> Checking that code complies with gofmt requirements...
?   	github.com/terraform-providers/terraform-provider-auth0	[no test files]
=== RUN   TestAccConnection
--- PASS: TestAccConnection (2.01s)
=== RUN   TestAccConnectionAD
--- PASS: TestAccConnectionAD (0.85s)
=== RUN   TestAccConnectionAzureAD
--- PASS: TestAccConnectionAzureAD (7.16s)
=== RUN   TestAccConnectionOIDC
--- PASS: TestAccConnectionOIDC (1.80s)
=== RUN   TestAccConnectionWithEnbledClients
--- PASS: TestAccConnectionWithEnbledClients (19.63s)
=== RUN   TestAccConnectionSMS
--- PASS: TestAccConnectionSMS (0.90s)
=== RUN   TestAccConnectionEmail
--- PASS: TestAccConnectionEmail (6.55s)
=== RUN   TestAccConnectionSalesforce
--- PASS: TestAccConnectionSalesforce (0.90s)
=== RUN   TestAccConnectionGoogleOAuth2
--- PASS: TestAccConnectionGoogleOAuth2 (0.83s)
=== RUN   TestAccConnectionFacebook
--- PASS: TestAccConnectionFacebook (7.67s)
=== RUN   TestAccConnectionApple
--- PASS: TestAccConnectionApple (1.73s)
=== RUN   TestAccConnectionLinkedin
--- PASS: TestAccConnectionLinkedin (7.84s)
=== RUN   TestAccConnectionGitHub
--- PASS: TestAccConnectionGitHub (0.97s)
=== RUN   TestAccConnectionConfiguration
--- PASS: TestAccConnectionConfiguration (7.83s)
=== RUN   TestAccConnectionSAML
--- PASS: TestAccConnectionSAML (7.75s)
PASS
coverage: 27.3% of statements
ok  	github.com/terraform-providers/terraform-provider-auth0/auth0	74.726s	coverage: 27.3% of statements
?   	github.com/terraform-providers/terraform-provider-auth0/auth0/internal/debug	[no test files]
testing: warning: no tests to run
PASS
coverage: 0.0% of statements
ok  	github.com/terraform-providers/terraform-provider-auth0/auth0/internal/random	0.262s	coverage: 0.0% of statements [no tests to run]
testing: warning: no tests to run
PASS
coverage: 0.0% of statements
ok  	github.com/terraform-providers/terraform-provider-auth0/auth0/internal/validation	0.288s	coverage: 0.0% of statements [no tests to run]
?   	github.com/terraform-providers/terraform-provider-auth0/version	[no test files]
...

Community Note

Please vote on this pull request by adding a 👍 reaction to the original pull request comment to help the community and maintainers prioritize this request
Please do not leave "+1" comments, they generate extra noise for pull request followers and do not help prioritize the request

Note: Contains breaking changes:

Moved options.requires_username to options.validation
Changed Data Structure of options.validation

closes alexkappa#238

etwillbefine · 2020-07-26T09:19:17Z

As suggested in the README make test runs successful. But for the CI run (make testacc ..) it fails on Travis with "Missing required argument: The argument "client_secret" is required, but was not set". Help appreciated.

alexkappa · 2020-07-27T21:04:40Z

Hi, thanks for giving this a go!

Regarding the tests failing on CI, its kind of expected as PRs wont have access to secrets. You should run them locally though and paste the output in the right section in the description.

As this is a breaking change, we should add a migration so as to not break existing users.

I wasn't able to see any changes to flattening/expanding. Do the right values get passed along to the sdk? I’m on mobile so I might have missed something.

Cheers,
Alex

etwillbefine · 2020-07-28T22:41:25Z

The Auth0 Client supports validation already with a generic interface type.

During Testing I realised that the requires_username actually works. Not sure how one of my connection had it set to false and true in Terraform but when testing with a fresh Connection it works. When sending requires_username within the validation object Auth0 returns a Bad Request that this property is not allowed. That means Auth0's Documentation is not correct about requires_username property. https://auth0.com/docs/connections/references/options-mgmt-api

I created the corresponding mapping for validation, removed changes related to requires_username and added Acceptance Test Output to the original Description.

auth0/resource_auth0_connection.go

alexkappa

Indeed the documentation seems to be wrong on this, I'll let the folks at Auth0 know.

Apart from reverting some changes, the only thing that needs to be done here is state migration. Otherwise we're good 🙂

auth0/resource_auth0_connection_test.go

alexkappa · 2020-08-23T07:55:24Z

auth0/resource_auth0_connection.go

@@ -81,8 +81,31 @@ var connectionSchema = map[string]*schema.Schema{
 		Elem: &schema.Resource{
 			Schema: map[string]*schema.Schema{
 				"validation": {
-					Type:     schema.TypeMap,
-					Elem:     &schema.Schema{Type: schema.TypeString},
+					Type:     schema.TypeList,


This change will likely break state for some users. We should write a migration to safely migrate state from a map to a list.

Thats going to take same time as Im new to Go and Terraform Provider Migrations. Will try to provide it within the next few days.

I understand :) I’m here if you need any help.

For inspiration, you can see the migration of connections schema a while back.

etwillbefine · 2020-09-05T08:20:47Z

I added the migration to move validation.min|max to validation.username.min|max. I'm not sure if I need to make sure to convert the previous string value to int. structure_auth0_validation.go contains already converting to Int value.

etwillbefine · 2020-09-05T09:31:44Z

Here is an updated Test Output:

$ make testacc TESTS=TestAccConnection
==> Checking that code complies with gofmt requirements...
?   	github.com/terraform-providers/terraform-provider-auth0	[no test files]
=== RUN   TestAccConnection
--- PASS: TestAccConnection (2.20s)
=== RUN   TestAccConnectionAD
--- PASS: TestAccConnectionAD (0.89s)
=== RUN   TestAccConnectionAzureAD
--- PASS: TestAccConnectionAzureAD (7.25s)
=== RUN   TestAccConnectionOIDC
--- PASS: TestAccConnectionOIDC (1.68s)
=== RUN   TestAccConnectionWithEnbledClients
--- PASS: TestAccConnectionWithEnbledClients (13.82s)
=== RUN   TestAccConnectionSMS
--- PASS: TestAccConnectionSMS (7.09s)
=== RUN   TestAccConnectionEmail
--- PASS: TestAccConnectionEmail (1.63s)
=== RUN   TestAccConnectionSalesforce
--- PASS: TestAccConnectionSalesforce (7.10s)
=== RUN   TestAccConnectionGoogleOAuth2
--- PASS: TestAccConnectionGoogleOAuth2 (0.95s)
=== RUN   TestAccConnectionFacebook
--- PASS: TestAccConnectionFacebook (7.76s)
=== RUN   TestAccConnectionApple
--- PASS: TestAccConnectionApple (1.64s)
=== RUN   TestAccConnectionLinkedin
--- PASS: TestAccConnectionLinkedin (7.75s)
=== RUN   TestAccConnectionGitHub
--- PASS: TestAccConnectionGitHub (0.91s)
=== RUN   TestAccConnectionConfiguration
--- PASS: TestAccConnectionConfiguration (7.66s)
=== RUN   TestAccConnectionSAML
--- PASS: TestAccConnectionSAML (1.68s)
PASS
coverage: 26.9% of statements
ok  	github.com/terraform-providers/terraform-provider-auth0/auth0	70.348s	coverage: 26.9% of statements
?   	github.com/terraform-providers/terraform-provider-auth0/auth0/internal/debug	[no test files]
testing: warning: no tests to run
PASS
coverage: 0.0% of statements
ok  	github.com/terraform-providers/terraform-provider-auth0/auth0/internal/random	0.254s	coverage: 0.0% of statements [no tests to run]
testing: warning: no tests to run
PASS
coverage: 0.0% of statements
ok  	github.com/terraform-providers/terraform-provider-auth0/auth0/internal/validation	0.081s	coverage: 0.0% of statements [no tests to run]
?   	github.com/terraform-providers/terraform-provider-auth0/version	[no test files]

github-actions · 2020-11-05T00:33:45Z

This PR has become stale as it has been open 30 days with no activity.
Stale PRs will be closed after 5 days if no action is taken. If you
think this PR should not be closed, remove the stale label.

fbarbare · 2020-11-17T08:58:59Z

Any updates on that PR? We need to configure those options too
@alexkappa

alexkappa · 2020-11-25T17:58:29Z

Thank you @etwillbefine, I've merged your changes in. Will cut a new release soon with the validation fix in place, cheers!

correctly transfer username validation to auth0

41e8bfb

closes alexkappa#238

etwillbefine mentioned this pull request Jul 26, 2020

Unable to specify username requirements #238

Closed

etwillbefine added 2 commits July 29, 2020 00:42

add mapping for validation options

1cf2d1f

correct tests and remove changes for requires_username

3a5d2f1

etwillbefine force-pushed the username-validation branch from 0a4c4c2 to 3a5d2f1 Compare July 28, 2020 22:43

etwillbefine mentioned this pull request Jul 28, 2020

Wrong Documentation for requires_username auth0/docs#9278

Open

alexkappa reviewed Aug 23, 2020

View reviewed changes

auth0/resource_auth0_connection.go Outdated Show resolved Hide resolved

alexkappa suggested changes Aug 23, 2020

View reviewed changes

etwillbefine added 2 commits August 26, 2020 18:38

reset unrelated diff

fbbe2b1

migrate validation map to validation.username

84f9725

username is a list of maps

9ea0ee6

github-actions bot added the stale label Nov 5, 2020

github-actions bot closed this Nov 13, 2020

alexkappa added preserve and removed stale labels Nov 23, 2020

alexkappa reopened this Nov 23, 2020

alexkappa self-requested a review November 23, 2020 18:17

alexkappa merged commit 09592f2 into alexkappa:master Nov 25, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

correctly transfer username validation to auth0 #258

correctly transfer username validation to auth0 #258

etwillbefine commented Jul 26, 2020 •

edited

Loading

etwillbefine commented Jul 26, 2020 •

edited

Loading

alexkappa commented Jul 27, 2020

etwillbefine commented Jul 28, 2020

alexkappa left a comment

alexkappa Aug 23, 2020

etwillbefine Aug 26, 2020

alexkappa Aug 26, 2020

etwillbefine commented Sep 5, 2020 •

edited

Loading

etwillbefine commented Sep 5, 2020 •

edited

Loading

github-actions bot commented Nov 5, 2020

fbarbare commented Nov 17, 2020

alexkappa commented Nov 25, 2020

correctly transfer username validation to auth0 #258

correctly transfer username validation to auth0 #258

Conversation

etwillbefine commented Jul 26, 2020 • edited Loading

Proposed Changes

Acceptance Test Output

Community Note

Note: Contains breaking changes:

etwillbefine commented Jul 26, 2020 • edited Loading

alexkappa commented Jul 27, 2020

etwillbefine commented Jul 28, 2020

alexkappa left a comment

Choose a reason for hiding this comment

alexkappa Aug 23, 2020

Choose a reason for hiding this comment

etwillbefine Aug 26, 2020

Choose a reason for hiding this comment

alexkappa Aug 26, 2020

Choose a reason for hiding this comment

etwillbefine commented Sep 5, 2020 • edited Loading

etwillbefine commented Sep 5, 2020 • edited Loading

github-actions bot commented Nov 5, 2020

fbarbare commented Nov 17, 2020

alexkappa commented Nov 25, 2020

etwillbefine commented Jul 26, 2020 •

edited

Loading

etwillbefine commented Jul 26, 2020 •

edited

Loading

etwillbefine commented Sep 5, 2020 •

edited

Loading

etwillbefine commented Sep 5, 2020 •

edited

Loading