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

Jump to bottom

feature: Use default auth method if no auth method ID is provided for provider #385

Merged
merged 3 commits into from
Apr 28, 2023
Merged

feature: Use default auth method if no auth method ID is provided for provider #385

merged 3 commits into from
Apr 28, 2023

Conversation

elimt
Copy link
Member

@elimt elimt commented Apr 25, 2023

Enable easier authentication by automatically setting the auth method id for the TF provider. Instead of passing the auth_methoid_id to the provider, the provider will set the auth_method_id.

This is similar to Boundary CLI using the primary authentication for a given scope: hashicorp/boundary#2725

Using the Primary Auth Method in Global Scope

provider "boundary" {
  addr                            = "http://127.0.0.1:9200"
  password_auth_method_login_name = "myuser"
  password_auth_method_password   = "passpass"
}

Using the Primary Auth Method in a passed-in scope:

provider "boundary" {
  addr                            = "http://127.0.0.1:9200"
  password_auth_method_login_name = "myuser"
  password_auth_method_password   = "passpass"
  scope_id                        = "s_1234567890"
}

Using auth_method_id (traditional)

provider "boundary" {
  addr                            = "http://127.0.0.1:9200"
  auth_method_id                  = "ampw_1234567890"
  password_auth_method_login_name = "myuser"
  password_auth_method_password   = "passpass"
}

Setting Auth Method ID

It sets the auth_method_id based on these rules:

  • If an auth_method_id is passed in, that auth_method_id is used
  • If there is only one auth method, it'll return it even if it's not the primary auth method
  • If there are multiple auth methods, it'll only return the auth method for the primary auth method. OIDC auth method isn't supported
  • If scope ID is empty or no primary auth method is found, it returns an error

@elimt elimt force-pushed the elimt-default-provider-auth-method branch from 4c40ab4 to 9011676 Compare April 25, 2023 18:59
@elimt elimt force-pushed the elimt-default-provider-auth-method branch from 9011676 to f98cf49 Compare April 25, 2023 19:04
@elimt elimt requested review from louisruch, ddebko and jimlambrt April 25, 2023 19:09
jimlambrt
jimlambrt requested changes Apr 26, 2023
View reviewed changes
Copy link
Contributor

@jimlambrt jimlambrt left a comment

Choose a reason for hiding this comment

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

couple questions, but overall looks great.

internal/provider/provider.go Show resolved Hide resolved
internal/provider/provider.go Outdated Show resolved Hide resolved
internal/provider/provider.go
@@ -176,14 +204,14 @@ func providerAuthenticate(ctx context.Context, d *schema.ResourceData, md *metaD
"login_name": authMethodLoginName,
"password": authMethodPassword,
}

case strings.HasPrefix(authMethodId.(string), "amoidc"):
Copy link
Contributor

Choose a reason for hiding this comment

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

do we want to allow ldap auth methods in this switch?

Copy link
Member Author

Choose a reason for hiding this comment

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

Yeah, I think we can add support for that as well. A separate PR where we add LDAP support and also use the default primary method for that.

internal/provider/provider.go
// getDefaultAuthMethodId iterates over boundary client.List() to find the default auth method ID for the given scopeId.
// If there is only one auth method, it'll return it even if it's not the primary auth method
// If scope ID is empty or no primary auth method is found, it returns an error.
func getDefaultAuthMethodId(ctx context.Context, client *authmethods.Client, scopeId, amType string) (string, error) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Do we want to support ldap auth methods in this func?

Copy link
Member Author

Choose a reason for hiding this comment

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

Yeah, I think we can add support for that as well. A separate PR where we add LDAP support and also use the default primary method for that.

internal/provider/provider.go
recoveryKmsHcl, recoveryKmsHclOk := d.GetOk("recovery_kms_hcl")
if token, ok := d.GetOk("token"); ok {
md.client.SetToken(token.(string))
}

// If auth_method_id is not set, get the default auth method ID for the given scope ID
if !authMethodIdOk {
defaultAuthMethodId, err := getDefaultAuthMethodId(ctx, amClient, providerScope, PASSWORD_AUTH_METHOD_PREFIX)
Copy link
Contributor

Choose a reason for hiding this comment

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

do we want to support ldap as well?

Copy link
Member Author

Choose a reason for hiding this comment

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

Yeah, I think we can add support for that as well. A separate PR where we add LDAP support and also use the default primary method for that.

@elimt
Address PR Feedback
fc1a02d 
- Make provider easier to read
- Fix error typo
@elimt elimt merged commit 0191874 into main Apr 28, 2023
@elimt elimt deleted the elimt-default-provider-auth-method branch April 28, 2023 17:13
@elimt elimt modified the milestone: 0.13.x May 3, 2023
elimt added a commit that referenced this pull request May 3, 2023
@elimt
feature: Use default auth method if no auth method ID is provided for…
e4a48fd 
… provider (#385)

* Use default auth method if no auth method ID is provided for provider
elimt added a commit that referenced this pull request May 5, 2023
@elimt @Dan-Heath
cherry-picks for release 1.1.x (#394)
ede5377 
* feature: Use default auth method if no auth method ID is provided for provider (#385)

* Use default auth method if no auth method ID is provided for provider

* docs: Fix typo in Managed Group resource page (#370)

* fix(credlib): Force new resource on credential_type change (#389)

* update changelog

---------

Co-authored-by: Dan Heath <[email protected]>
Co-authored-by: Hugo <[email protected]>
grantorchard pushed a commit to grantorchard/terraform-provider-boundary that referenced this pull request Aug 28, 2023
@elimt @grantorchard
feature: Use default auth method if no auth method ID is provided for…
c84e9b7 
… provider (hashicorp#385)

* Use default auth method if no auth method ID is provided for provider
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jimlambrt jimlambrt jimlambrt approved these changes

@louisruch louisruch Awaiting requested review from louisruch

@ddebko ddebko Awaiting requested review from ddebko

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@elimt @jimlambrt