-
Notifications
You must be signed in to change notification settings - Fork 9.2k
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
aws_ec2_client_vpn_endpoint: add federated auth #14171
aws_ec2_client_vpn_endpoint: add federated auth #14171
Conversation
While AWS *can* act as an SSO IdP, I don't think there's a public API for enabling it. Automated testing might be troublesome.
…n_endpoint-federated-saml-idp-authentication # Conflicts: # aws/resource_aws_ec2_client_vpn_endpoint_test.go
Any updates? |
@divyaraghavann I don't think the PR needs any more updates, unless I'm mistaken. Last week, @bflad mentioned he would get some eyes on this PR in the next week or so: #13401 (comment), so there should be some movement soon! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
minor changes
…n_endpoint-federated-saml-idp-authentication
…n_endpoint-federated-saml-idp-authentication # Conflicts: # aws/resource_aws_ec2_client_vpn_endpoint_test.go
Thanks @DrFaust92! Those issues should be resolved! I also merged master to the branch as it was a bit behind. |
Awesome, thanks y'all! 👍 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The functionality looks implemented well but unfortunately the acceptance testing is currently failing. We'll need to get that fixed up before we can merge this. Please reach out if you have any questions or do not have time/ability to fix it up. 👍
return testAccEc2ClientVpnEndpointConfigAcmCertificateBase() + fmt.Sprintf(` | ||
resource "aws_iam_saml_provider" "default" { | ||
name = "myprovider-%s" | ||
saml_metadata_document = file("./test-fixtures/saml-metadata.xml") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
When running this acceptance test, it looks like EC2 Client VPN is validating the actual certificate included in this SAML Metadata:
TestAccAwsEc2ClientVpn_serial/Endpoint_federated: resource_aws_ec2_client_vpn_endpoint_test.go:243: Step 1/2 error: terraform failed: exit status 1
stderr:
Error: Error creating Client VPN endpoint: InvalidParameterValue: Cert with DN C=USA, ST=CA, L=San Francisco, O=Salesforce.com, OU=00D24000000pAoA, CN=SelfSignedCert_02Sep2015_182653 expired at Sat Sep 02 12:00:00 UTC 2017
status code: 400, request id: e8518c9b-d44c-4bb8-a7a3-ff6552af5931
<?xml version="1.0" encoding="UTF-8"?><md:EntityDescriptor xmlns:md="urn:oasis:names:tc:SAML:2.0:metadata" xmlns:ds="http://www.w3.org/2000/09/xmldsig#" entityID="https://terraform-dev-ed.my.salesforce.com" validUntil="2025-09-02T18:27:19.710Z">
<md:IDPSSODescriptor protocolSupportEnumeration="urn:oasis:names:tc:SAML:2.0:protocol">
<md:KeyDescriptor use="signing">
<ds:KeyInfo>
<ds:X509Data>
<ds:X509Certificate>MIIErDCCA5SgAwIBAgIOAU+PT8RBAAAAAHxJXEcwDQYJKoZIhvcNAQELBQAwgZAxKDAmBgNVBAMMH1NlbGZTaWduZWRDZXJ0XzAyU2VwMjAxNV8xODI2NTMxGDAWBgNVBAsMDzAwRDI0MDAwMDAwcEFvQTEXMBUGA1UECgwOU2FsZXNmb3JjZS5jb20xFjAUBgNVBAcMDVNhbiBGcmFuY2lzY28xCzAJBgNVBAgMAkNBMQwwCgYDVQQGEwNVU0EwHhcNMTUwOTAyMTgyNjUzWhcNMTcwOTAyMTIwMDAwWjCBkDEoMCYGA1UEAwwfU2VsZlNpZ25lZENlcnRfMDJTZXAyMDE1XzE4MjY1MzEYMBYGA1UECwwPMDBEMjQwMDAwMDBwQW9BMRcwFQYDVQQKDA5TYWxlc2ZvcmNlLmNvbTEWMBQGA1UEBwwNU2FuIEZyYW5jaXNjbzELMAkGA1UECAwCQ0ExDDAKBgNVBAYTA1VTQTCCASIwDQYJKoZIhvcNAQEBBQADggEPADCCAQoCggEBAJp/wTRr9n1IWJpkRTjNpep47OKJrD2E6rGbJ18TG2RxtIz+zCn2JwH2aP3TULh0r0hhcg/pecv51RRcG7O19DBBaTQ5+KuoICQyKZy07/yDXSiZontTwkEYs06ssTwTHUcRXbcwTKv16L7omt0MjIhTTGfvtLOYiPwyvKvzAHg4eNuAcli0duVM78UIBORtdmy9C9ZcMh8yRJo5aPBq85wsE3JXU58ytyZzCHTBLH+2xFQrjYnUSEW+FOEEpI7o33MVdFBvWWg1R17HkWzcve4C30lqOHqvxBzyESZ/N1mMlmSt8gPFyB+mUXY99StJDJpnytbY8DwSzMQUo/sOVB0CAwEAAaOCAQAwgf0wHQYDVR0OBBYEFByu1EQqRQS0bYQBKS9K5qwKi+6IMA8GA1UdEwEB/wQFMAMBAf8wgcoGA1UdIwSBwjCBv4AUHK7URCpFBLRthAEpL0rmrAqL7oihgZakgZMwgZAxKDAmBgNVBAMMH1NlbGZTaWduZWRDZXJ0XzAyU2VwMjAxNV8xODI2NTMxGDAWBgNVBAsMDzAwRDI0MDAwMDAwcEFvQTEXMBUGA1UECgwOU2FsZXNmb3JjZS5jb20xFjAUBgNVBAcMDVNhbiBGcmFuY2lzY28xCzAJBgNVBAgMAkNBMQwwCgYDVQQGEwNVU0GCDgFPj0/EQQAAAAB8SVxHMA0GCSqGSIb3DQEBCwUAA4IBAQA9O5o1tC71qJnkq+ABPo4A1aFKZVT/07GcBX4/wetcbYySL4Q2nR9pMgfPYYS1j+P2E3viPsQwPIWDUBwFkNsjjX5DSGEkLAioVGKRwJshRSCSynMcsVZbQkfBUiZXqhM0wzvoa/ALvGD+aSSb1m+x7lEpDYNwQKWaUW2VYcHWv9wjujMyy7dlj8E/jqM71mw7ThNl6k4+3RQ802dMa14txm8pkF0vZgfpV3tkqhBqtjBAicVCaveqr3r3iGqjvyilBgdY+0NR8szqzm7CD/Bkb22+/IgM/mXQuL9KHD/WADlSGmYKmG3SSahmcZxznYCnzcRNN9LVuXlz5cbljmBj</ds:X509Certificate>
</ds:X509Data>
</ds:KeyInfo>
</md:KeyDescriptor>
<md:NameIDFormat>urn:oasis:names:tc:SAML:1.1:nameid-format:unspecified</md:NameIDFormat>
<md:SingleSignOnService Binding="urn:oasis:names:tc:SAML:2.0:bindings:HTTP-POST" Location="https://terraform-dev-ed.my.salesforce.com/idp/endpoint/HttpPost"/>
<md:SingleSignOnService Binding="urn:oasis:names:tc:SAML:2.0:bindings:HTTP-Redirect" Location="https://terraform-dev-ed.my.salesforce.com/idp/endpoint/HttpRedirect"/>
</md:IDPSSODescriptor>
</md:EntityDescriptor>
Are you able to run this test, @jgeurts, and potentially fix the issue? We'll need to double check other tests that use that file as well after updates or potentially create it dynamically with templatefile()
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't have an account that I can run the acceptance tests on at the moment. I might be able to set one up, but that would take a few days to get working. In the mean time, I updated the saml metadata X509 cert to expire in 50 years. I hope that solves things for the time being!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you so much for that update -- seems to work well for all the existing tests and the new one 👍
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks great, thank you so much @jgeurts 🚀
Output from acceptance testing:
--- PASS: TestAccAWSCognitoIdentityPool_samlProviderArns (41.61s)
--- PASS: TestAccAwsEc2ClientVpn_serial (5.60s)
--- PASS: TestAccAwsEc2ClientVpn_serial/Endpoint_basic (18.01s)
--- PASS: TestAccAwsEc2ClientVpn_serial/AuthorizationRule_groups (81.88s)
--- PASS: TestAccAwsEc2ClientVpn_serial/Route_disappears (546.04s)
--- PASS: TestAccAwsEc2ClientVpn_serial/NetworkAssociation_securityGroups (555.75s)
--- PASS: TestAccAwsEc2ClientVpn_serial/Route_description (544.37s)
--- PASS: TestAccAwsEc2ClientVpn_serial/AuthorizationRule_disappears (43.63s)
--- PASS: TestAccAwsEc2ClientVpn_serial/Route_basic (573.48s)
--- PASS: TestAccAwsEc2ClientVpn_serial/AuthorizationRule_basic (37.60s)
--- PASS: TestAccAwsEc2ClientVpn_serial/Endpoint_splitTunnel (28.37s)
--- PASS: TestAccAwsEc2ClientVpn_serial/Endpoint_withLogGroup (27.13s)
--- PASS: TestAccAwsEc2ClientVpn_serial/Endpoint_federated (16.50s)
--- PASS: TestAccAwsEc2ClientVpn_serial/AuthorizationRule_Subnets (102.24s)
--- PASS: TestAccAwsEc2ClientVpn_serial/Endpoint_tags (39.76s)
--- PASS: TestAccAwsEc2ClientVpn_serial/NetworkAssociation_disappears (526.89s)
--- PASS: TestAccAwsEc2ClientVpn_serial/NetworkAssociation_basic (548.73s)
--- PASS: TestAccAwsEc2ClientVpn_serial/Endpoint_withDNSServers (26.48s)
--- PASS: TestAccAwsEc2ClientVpn_serial/Endpoint_disappears (14.29s)
--- PASS: TestAccAwsEc2ClientVpn_serial/Endpoint_mutualAuthAndMsAD (1767.10s)
--- PASS: TestAccAwsEc2ClientVpn_serial/Endpoint_msAD (1716.55s)
--- PASS: TestAccAWSIAMSamlProvider_basic (41.36s)
--- PASS: TestAccAWSIAMSamlProvider_disappears (15.38s)
--- PASS: TestAccAWSWorkLinkFleet_IdentityProvider (83.09s)
This has been released in version 3.5.0 of the Terraform AWS provider. Please see the Terraform documentation on provider versioning or reach out if you need any assistance upgrading. For further feature requests or bug reports with this functionality, please create a new GitHub issue following the template for triage. Thanks! |
I'm going to lock this issue because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active issues. If you feel this issue should be reopened, we encourage creating a new issue linking back to this one for added context. Thanks! |
This is an addendum to #13769, to add a test and documentation. Apologies if there is a better way to accomplish this - I wasn't able to commit to the fork source for #13769
Community Note
Relates OR Closes #13401
Release note for CHANGELOG:
Output from acceptance testing: