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

Sets use_microsoft_graph_api to true by default #90

Merged
merged 1 commit into from
Feb 16, 2022

Conversation

austingebauer
Copy link
Contributor

@austingebauer austingebauer commented Feb 16, 2022

Overview

This PR sets the default for the use_microsoft_graph_api configuration parameter to true. This means that new configurations after this PR will use the Microsoft Graph API by default. Existing configurations will need to manually upgrade to set the parameter to true before the June 30, 2022 deadline (see Microsoft FAQ).

Additionally, this PR fixes a couple of bugs:

  • defaultRootPasswordTTL was not a duration, which resulted in adding 4380 nanoseconds to the rotate root expiration for configs that hadn't set a root_password_ttl instead of the default of 6 months.
  • The update logic for root_password_ttl would not retain values previously set by always setting the default.

Testing

I've restructured and added additional coverage for defaults to one of the existing tests.

Manual test output:

$ vault write azure/config \
      subscription_id="redacted" \
      tenant_id="redacted" \
      client_id="redacted" \
      client_secret="redacted"
Success! Data written to: azure/config

$ vault read azure/config       
Key                        Value
---                        -----
client_id                  redacted
environment                n/a
root_password_ttl          4380h
subscription_id            redacted
tenant_id                  redacted
use_microsoft_graph_api    true

Acceptance test output:

# Config with creds with permissions for using MS graph
$ go test -v -run TestCredentialInteg_msgraph
=== RUN   TestCredentialInteg_msgraph
=== RUN   TestCredentialInteg_msgraph/service_principals
=== PAUSE TestCredentialInteg_msgraph/service_principals
=== CONT  TestCredentialInteg_msgraph/service_principals
--- PASS: TestCredentialInteg_msgraph (0.00s)
    --- PASS: TestCredentialInteg_msgraph/service_principals (16.91s)
PASS
ok      github.com/hashicorp/vault-plugin-secrets-azure 17.283s

# Config with creds with permissions for using AAD graph
$ go test -v -run TestCredentialInteg_aad
=== RUN   TestCredentialInteg_aad
=== RUN   TestCredentialInteg_aad/service_principals
=== PAUSE TestCredentialInteg_aad/service_principals
=== RUN   TestCredentialInteg_aad/static_service_principals
=== PAUSE TestCredentialInteg_aad/static_service_principals
=== CONT  TestCredentialInteg_aad/service_principals
=== CONT  TestCredentialInteg_aad/static_service_principals
--- PASS: TestCredentialInteg_aad (0.00s)
    --- PASS: TestCredentialInteg_aad/static_service_principals (85.50s)
    --- PASS: TestCredentialInteg_aad/service_principals (101.18s)
PASS
ok      github.com/hashicorp/vault-plugin-secrets-azure 101.485s

@austingebauer austingebauer added this to the 1.10 milestone Feb 16, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants