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.

Sign up for GitHub

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

Fix identity platform default_supported_idp_config idp_id #2969

Merged
merged 2 commits into from
Jan 14, 2020
Merged

Fix identity platform default_supported_idp_config idp_id #2969

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
6e13e50
Fix identity platform default_supported_idp_config. Needs separate id…
slevenick Jan 13, 2020
9734346
Update tracked submodules -> HEAD on Tue Jan 14 18:09:17 UTC 2020
modular-magician Jan 14, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion build/inspec
Open in desktop
Submodule inspec updated 0 files
2 changes: 1 addition & 1 deletion build/terraform
Open in desktop
Submodule terraform updated from 8ee9fe to 09520e
2 changes: 1 addition & 1 deletion build/terraform-beta
Open in desktop
Submodule terraform-beta updated from 54a629 to 7db486
2 changes: 1 addition & 1 deletion build/terraform-mapper
Open in desktop
Submodule terraform-mapper updated 2 files
+1 −1 google/identityplatform_default_supported_idp_config.go
+1 −1 google/identityplatform_tenant_default_supported_idp_config.go
66 changes: 60 additions & 6 deletions products/identityplatform/api.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,8 +30,8 @@ objects:
- !ruby/object:Api::Resource
name: 'DefaultSupportedIdpConfig'
base_url: 'projects/{{project}}/defaultSupportedIdpConfigs'
self_link: 'projects/{{project}}/defaultSupportedIdpConfigs/{{client_id}}'
create_url: 'projects/{{project}}/defaultSupportedIdpConfigs?idpId={{client_id}}'
self_link: 'projects/{{project}}/defaultSupportedIdpConfigs/{{idp_id}}'
create_url: 'projects/{{project}}/defaultSupportedIdpConfigs?idpId={{idp_id}}'
update_verb: :PATCH
update_mask: true
description: |
Expand All @@ -46,11 +46,38 @@ objects:
output: true
description: |
The name of the DefaultSupportedIdpConfig resource
- !ruby/object:Api::Type::String
name: 'idpId'
description: |
ID of the IDP. Possible values include:

* `apple.com`

* `facebook.com`

* `gc.apple.com`

* `github.com`

* `google.com`

* `linkedin.com`

* `microsoft.com`

* `playgames.google.com`

* `twitter.com`

* `yahoo.com`

input: true
url_param_only: true
required: true
- !ruby/object:Api::Type::String
name: 'clientId'
description: |
OAuth client ID
input: true
required: true
- !ruby/object:Api::Type::String
name: 'clientSecret'
Expand All @@ -64,8 +91,8 @@ objects:
- !ruby/object:Api::Resource
name: 'TenantDefaultSupportedIdpConfig'
base_url: 'projects/{{project}}/tenants/{{tenant}}/defaultSupportedIdpConfigs'
self_link: 'projects/{{project}}/tenants/{{tenant}}/defaultSupportedIdpConfigs/{{client_id}}'
create_url: 'projects/{{project}}/tenants/{{tenant}}/defaultSupportedIdpConfigs?idpId={{client_id}}'
self_link: 'projects/{{project}}/tenants/{{tenant}}/defaultSupportedIdpConfigs/{{idp_id}}'
create_url: 'projects/{{project}}/tenants/{{tenant}}/defaultSupportedIdpConfigs?idpId={{idp_id}}'
update_verb: :PATCH
update_mask: true
description: |
Expand All @@ -80,6 +107,34 @@ objects:
output: true
description: |
The name of the default supported IDP config resource
- !ruby/object:Api::Type::String
name: 'idpId'
description: |
ID of the IDP. Possible values include:

* `apple.com`

* `facebook.com`

* `gc.apple.com`

* `github.com`

* `google.com`

* `linkedin.com`

* `microsoft.com`

* `playgames.google.com`

* `twitter.com`

* `yahoo.com`

input: true
url_param_only: true
required: true
- !ruby/object:Api::Type::String
name: 'tenant'
required: true
Expand All @@ -89,7 +144,6 @@ objects:
The name of the tenant where this DefaultSupportedIdpConfig resource exists
- !ruby/object:Api::Type::String
name: 'clientId'
input: true
required: true
description: |
OAuth client ID
Expand Down
4 changes: 2 additions & 2 deletions products/identityplatform/terraform.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@
--- !ruby/object:Provider::Terraform::Config
overrides: !ruby/object:Overrides::ResourceOverrides
DefaultSupportedIdpConfig: !ruby/object:Overrides::Terraform::ResourceOverride
import_format: ["projects/{{project}}/defaultSupportedIdpConfigs/{{client_id}}"]
import_format: ["projects/{{project}}/defaultSupportedIdpConfigs/{{idp_id}}"]
examples:
- !ruby/object:Provider::Terraform::Examples
name: "identity_platform_default_supported_idp_config_basic"
Expand All @@ -23,7 +23,7 @@ overrides: !ruby/object:Overrides::ResourceOverrides
# If we could spin up a project and enable identity platform we could test this separately
skip_test: true
TenantDefaultSupportedIdpConfig: !ruby/object:Overrides::Terraform::ResourceOverride
import_format: ["projects/{{project}}/tenants/{{tenant}}/defaultSupportedIdpConfigs/{{client_id}}"]
import_format: ["projects/{{project}}/tenants/{{tenant}}/defaultSupportedIdpConfigs/{{idp_id}}"]
examples:
- !ruby/object:Provider::Terraform::Examples
name: "identity_platform_tenant_default_supported_idp_config_basic"
Expand Down
3 changes: 2 additions & 1 deletion templates/terraform/examples/identity_platform_default_supported_idp_config_basic.tf.erb
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
resource "google_identity_platform_default_supported_idp_config" "<%= ctx[:primary_resource_id] %>" {
enabled = true
client_id = "playgames.google.com"
idp_id = "playgames.google.com"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does this mean that anyone who previously configured GCIP will now have to change their config?

Copy link
Contributor Author

@slevenick slevenick Jan 14, 2020 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes I don't believe anyone could have successfully configured their IDP config with the client_id and the idp_id as the same value. Generally the client_id would be a generated value, so the API accepts any value, but the correct value would need to be issued by the IDP that is being used for authentication (playgames.google.com in this case).

At least that's my (likely incomplete) understanding of OAuth at this point

client_id = "client-id"
client_secret = "secret"
}
3 changes: 2 additions & 1 deletion ...tes/terraform/examples/identity_platform_tenant_default_supported_idp_config_basic.tf.erb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ resource "google_identity_platform_tenant" "tenant" {
resource "google_identity_platform_tenant_default_supported_idp_config" "<%= ctx[:primary_resource_id] %>" {
enabled = true
tenant = google_identity_platform_tenant.tenant.name
client_id = "playgames.google.com"
idp_id = "playgames.google.com"
client_id = "my-client-id"
client_secret = "secret"
}
6 changes: 4 additions & 2 deletions third_party/terraform/tests/resource_identity_platform_default_supported_idp_config_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,8 @@ func testAccIdentityPlatformDefaultSupportedIdpConfig_defaultSupportedIdpConfigB
return Nprintf(`
resource "google_identity_platform_default_supported_idp_config" "idp_config" {
enabled = true
client_id = "playgames.google.com"
idp_id = "playgames.google.com"
client_id = "client-id"
client_secret = "secret"
}
`, context)
Expand All @@ -81,7 +82,8 @@ func testAccIdentityPlatformDefaultSupportedIdpConfig_defaultSupportedIdpConfigU
return Nprintf(`
resource "google_identity_platform_default_supported_idp_config" "idp_config" {
enabled = false
client_id = "playgames.google.com"
idp_id = "playgames.google.com"
client_id = "client-id"
client_secret = "anothersecret"
}
`, context)
Expand Down
6 changes: 4 additions & 2 deletions ...ty/terraform/tests/resource_identity_platform_tenant_default_supported_idp_config_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,8 @@ resource "google_identity_platform_tenant" "tenant" {
resource "google_identity_platform_tenant_default_supported_idp_config" "idp_config" {
enabled = true
tenant = google_identity_platform_tenant.tenant.name
client_id = "playgames.google.com"
idp_id = "playgames.google.com"
client_id = "client-id"
client_secret = "secret"
}
`, context)
Expand All @@ -65,7 +66,8 @@ resource "google_identity_platform_tenant" "tenant" {
resource "google_identity_platform_tenant_default_supported_idp_config" "idp_config" {
enabled = false
tenant = google_identity_platform_tenant.tenant.name
client_id = "playgames.google.com"
idp_id = "playgames.google.com"
client_id = "client-id2"
client_secret = "differentsecret"
}
`, context)
Expand Down