From da5197fa1774f2d5b41753c5dd5d5a64f4c40cee Mon Sep 17 00:00:00 2001 From: Gavin Frazar Date: Mon, 23 Sep 2024 23:07:12 -0700 Subject: [PATCH] make configure awsoidc-idp actions transparent Applies to the integration command that the web UI discover flows tell users to run in AWS CloudShell to setup the AWS OIDC identity provider: teleport integration configure awsoidc-idp The command describes itself, its actions, and the desired state after it runs. It then prompts the user (by default) to confirm the action plan before proceeding. The confirmation prompt can be overridden with cli flag --confirm if desired. The IAM role it configures is no longer required to have the "ownership" tags that teleport applies if it's created by teleport, since the user is now prompted for confirmation before making changes. This allows a user to configure an existing IAM role without tagging the role for configuration by teleport. The command will still attempt to ensure the IAM role it configures has teleport tags, but failing to do so is only a warning. --- lib/cloud/provisioning/awsactions/common.go | 32 ++ .../awsactions/create_oidc_idp.go | 94 +++++ .../provisioning/awsactions/create_role.go | 237 +++++++++++ lib/cloud/provisioning/operations.go | 226 ++++++++++ lib/cloud/provisioning/operations_test.go | 391 ++++++++++++++++++ lib/integrations/awsoidc/idp_iam_config.go | 154 +++---- .../awsoidc/idp_iam_config_test.go | 65 ++- .../testdata/TestConfigureIdPIAMOutput.golden | 42 ++ tool/teleport/common/integration_configure.go | 1 + tool/teleport/common/teleport.go | 1 + 10 files changed, 1135 insertions(+), 108 deletions(-) create mode 100644 lib/cloud/provisioning/awsactions/common.go create mode 100644 lib/cloud/provisioning/awsactions/create_oidc_idp.go create mode 100644 lib/cloud/provisioning/awsactions/create_role.go create mode 100644 lib/cloud/provisioning/operations.go create mode 100644 lib/cloud/provisioning/operations_test.go create mode 100644 lib/integrations/awsoidc/testdata/TestConfigureIdPIAMOutput.golden diff --git a/lib/cloud/provisioning/awsactions/common.go b/lib/cloud/provisioning/awsactions/common.go new file mode 100644 index 0000000000000..bd4a23f97c64b --- /dev/null +++ b/lib/cloud/provisioning/awsactions/common.go @@ -0,0 +1,32 @@ +/* + * Teleport + * Copyright (C) 2024 Gravitational, Inc. + * + * This program is free software: you can redistribute it and/or modify + * it under the terms of the GNU Affero General Public License as published by + * the Free Software Foundation, either version 3 of the License, or + * (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU Affero General Public License for more details. + * + * You should have received a copy of the GNU Affero General Public License + * along with this program. If not, see . + */ + +package awsactions + +import ( + "encoding/json" + + "github.com/gravitational/trace" +) + +func formatDetails(in any) (string, error) { + const prefix = "" + const indent = " " + out, err := json.MarshalIndent(in, prefix, indent) + return string(out), trace.Wrap(err) +} diff --git a/lib/cloud/provisioning/awsactions/create_oidc_idp.go b/lib/cloud/provisioning/awsactions/create_oidc_idp.go new file mode 100644 index 0000000000000..81bfe6d34894a --- /dev/null +++ b/lib/cloud/provisioning/awsactions/create_oidc_idp.go @@ -0,0 +1,94 @@ +/* + * Teleport + * Copyright (C) 2024 Gravitational, Inc. + * + * This program is free software: you can redistribute it and/or modify + * it under the terms of the GNU Affero General Public License as published by + * the Free Software Foundation, either version 3 of the License, or + * (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU Affero General Public License for more details. + * + * You should have received a copy of the GNU Affero General Public License + * along with this program. If not, see . + */ + +package awsactions + +import ( + "context" + "log/slog" + + "github.com/aws/aws-sdk-go-v2/service/iam" + "github.com/gravitational/trace" + + awslib "github.com/gravitational/teleport/lib/cloud/aws" + "github.com/gravitational/teleport/lib/cloud/provisioning" + "github.com/gravitational/teleport/lib/integrations/awsoidc/tags" +) + +// OpenIDConnectProviderCreator can create an OpenID Connect Identity Provider +// (OIDC IdP) in AWS IAM. +type OpenIDConnectProviderCreator interface { + // CreateOpenIDConnectProvider creates an AWS IAM OIDC IdP. + CreateOpenIDConnectProvider(ctx context.Context, params *iam.CreateOpenIDConnectProviderInput, optFns ...func(*iam.Options)) (*iam.CreateOpenIDConnectProviderOutput, error) +} + +// CreateOIDCProvider wraps a [OpenIDConnectPRoviderCreator] in a +// [provisioning.Action] that creates an OIDC IdP in AWS IAM when invoked. +func CreateOIDCProvider( + clt OpenIDConnectProviderCreator, + thumbprints []string, + issuerURL string, + clientIDs []string, + tags tags.AWSTags, +) (*provisioning.Action, error) { + details, err := formatDetails(identityProviderDetails{ + IssuerURL: issuerURL, + ClientIDs: clientIDs, + Thumbprints: thumbprints, + }) + if err != nil { + return nil, trace.Wrap(err) + } + + config := provisioning.ActionConfig{ + Name: "CreateOpenIDConnectIdentityProvider", + Summary: "Create an OpenID Connect identity provider in AWS IAM for your Teleport cluster", + Details: details, + RunnerFn: func(ctx context.Context) error { + slog.InfoContext(ctx, "Creating OpenID Connect identity provider", + "issuer_url", issuerURL, + ) + _, err = clt.CreateOpenIDConnectProvider(ctx, &iam.CreateOpenIDConnectProviderInput{ + ThumbprintList: thumbprints, + Url: &issuerURL, + ClientIDList: clientIDs, + Tags: tags.ToIAMTags(), + }) + if err != nil { + awsErr := awslib.ConvertIAMv2Error(err) + if trace.IsAlreadyExists(awsErr) { + slog.InfoContext(ctx, "OpenID Connect identity provider already exists", + "issuer_url", issuerURL, + ) + return nil + } + + return trace.Wrap(err) + } + return nil + }, + } + action, err := provisioning.NewAction(config) + return action, trace.Wrap(err) +} + +type identityProviderDetails struct { + IssuerURL string `json:"issuer_url"` + ClientIDs []string `json:"client_id_list"` + Thumbprints []string `json:"thumbprint_list"` +} diff --git a/lib/cloud/provisioning/awsactions/create_role.go b/lib/cloud/provisioning/awsactions/create_role.go new file mode 100644 index 0000000000000..2d1b962138128 --- /dev/null +++ b/lib/cloud/provisioning/awsactions/create_role.go @@ -0,0 +1,237 @@ +/* + * Teleport + * Copyright (C) 2024 Gravitational, Inc. + * + * This program is free software: you can redistribute it and/or modify + * it under the terms of the GNU Affero General Public License as published by + * the Free Software Foundation, either version 3 of the License, or + * (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU Affero General Public License for more details. + * + * You should have received a copy of the GNU Affero General Public License + * along with this program. If not, see . + */ + +package awsactions + +import ( + "context" + "fmt" + "log/slog" + + "github.com/aws/aws-sdk-go-v2/aws" + "github.com/aws/aws-sdk-go-v2/service/iam" + iamtypes "github.com/aws/aws-sdk-go-v2/service/iam/types" + "github.com/gravitational/trace" + + awslib "github.com/gravitational/teleport/lib/cloud/aws" + "github.com/gravitational/teleport/lib/cloud/provisioning" + "github.com/gravitational/teleport/lib/integrations/awsoidc/tags" +) + +// RoleCreator can create an IAM role. +type RoleCreator interface { + // CreateRole creates a new IAM Role. + CreateRole(ctx context.Context, params *iam.CreateRoleInput, optFns ...func(*iam.Options)) (*iam.CreateRoleOutput, error) +} + +// RoleGetter can get an IAM role. +type RoleGetter interface { + // GetRole retrieves information about the specified role, including the + // role's path, GUID, ARN, and the role's trust policy that grants + // permission to assume the role. + GetRole(ctx context.Context, params *iam.GetRoleInput, optFns ...func(*iam.Options)) (*iam.GetRoleOutput, error) +} + +// AssumeRolePolicyUpdater can update an IAM role's trust policy. +type AssumeRolePolicyUpdater interface { + // UpdateAssumeRolePolicy updates the policy that grants an IAM entity + // permission to assume a role. + // This is typically referred to as the "role trust policy". + UpdateAssumeRolePolicy(ctx context.Context, params *iam.UpdateAssumeRolePolicyInput, optFns ...func(*iam.Options)) (*iam.UpdateAssumeRolePolicyOutput, error) +} + +// RoleTagger can tag an AWS IAM role. +type RoleTagger interface { + // TagRole adds one or more tags to an IAM role. The role can be a regular + // role or a service-linked role. If a tag with the same key name already + // exists, then that tag is overwritten with the new value. + TagRole(ctx context.Context, params *iam.TagRoleInput, optFns ...func(*iam.Options)) (*iam.TagRoleOutput, error) +} + +// CreateRole returns a [provisioning.Action] that creates or updates an IAM +// role when invoked. +func CreateRole( + clt interface { + AssumeRolePolicyUpdater + RoleCreator + RoleGetter + RoleTagger + }, + roleName string, + description string, + trustPolicy *awslib.PolicyDocument, + tags tags.AWSTags, +) (*provisioning.Action, error) { + details, err := formatDetails(createRoleDetails{ + Name: roleName, + Description: description, + TrustPolicy: *trustPolicy, + Tags: tags, + }) + if err != nil { + return nil, trace.Wrap(err) + } + + config := provisioning.ActionConfig{ + Name: "CreateRole", + Summary: fmt.Sprintf("Create IAM role %q with a custom trust policy", roleName), + Details: details, + RunnerFn: func(ctx context.Context) error { + slog.InfoContext(ctx, "Checking for existing IAM role", + "role", roleName, + ) + getRoleOut, err := clt.GetRole(ctx, &iam.GetRoleInput{ + RoleName: &roleName, + }) + if err != nil { + convertedErr := awslib.ConvertIAMv2Error(err) + if !trace.IsNotFound(convertedErr) { + return trace.Wrap(convertedErr) + } + slog.InfoContext(ctx, "Creating IAM role", "role", roleName) + return trace.Wrap(createRole(ctx, clt, roleName, description, trustPolicy, tags)) + } + + slog.InfoContext(ctx, "IAM role already exists", + "role", roleName, + ) + existingTrustPolicy, err := awslib.ParsePolicyDocument(aws.ToString(getRoleOut.Role.AssumeRolePolicyDocument)) + if err != nil { + return trace.Wrap(err) + } + err = ensureTrustPolicy(ctx, clt, roleName, trustPolicy, existingTrustPolicy) + if err != nil { + return trace.Wrap(err) + } + + err = ensureTags(ctx, clt, roleName, tags, getRoleOut.Role.Tags) + if err != nil { + // Tagging an existing role after we update it is a + // nice-to-have, but not a need-to-have. + slog.WarnContext(ctx, "Failed to update IAM role tags", + "role", roleName, + "error", err, + "tags", tags, + ) + } + return nil + }, + } + action, err := provisioning.NewAction(config) + return action, trace.Wrap(err) +} + +func createRole( + ctx context.Context, + clt RoleCreator, + roleName string, description string, + trustPolicy *awslib.PolicyDocument, + tags tags.AWSTags, +) error { + trustPolicyJSON, err := trustPolicy.Marshal() + if err != nil { + return trace.Wrap(err) + } + _, err = clt.CreateRole(ctx, &iam.CreateRoleInput{ + RoleName: &roleName, + Description: &description, + AssumeRolePolicyDocument: &trustPolicyJSON, + Tags: tags.ToIAMTags(), + }) + if err != nil { + return trace.Wrap(awslib.ConvertIAMv2Error(err)) + } + slog.InfoContext(ctx, "IAM role created", "role", roleName) + return nil +} + +func ensureTrustPolicy( + ctx context.Context, + clt AssumeRolePolicyUpdater, + roleName string, + trustPolicy *awslib.PolicyDocument, + existingTrustPolicy *awslib.PolicyDocument, +) error { + slog.InfoContext(ctx, "Checking IAM role trust policy", + "role", roleName, + ) + + var needsUpdate bool +Outer: + for _, statement := range trustPolicy.Statements { + for _, existingStatement := range existingTrustPolicy.Statements { + if existingStatement.EqualStatement(statement) { + continue Outer + } + } + needsUpdate = true + existingTrustPolicy.Statements = append(existingTrustPolicy.Statements, statement) + } + if !needsUpdate { + slog.InfoContext(ctx, "IAM role trust policy does not require update", + "role", roleName, + ) + return nil + } + + slog.InfoContext(ctx, "Updating IAM role trust policy", + "role", roleName, + ) + trustPolicyJSON, err := existingTrustPolicy.Marshal() + if err != nil { + return trace.Wrap(err) + } + + _, err = clt.UpdateAssumeRolePolicy(ctx, &iam.UpdateAssumeRolePolicyInput{ + RoleName: &roleName, + PolicyDocument: &trustPolicyJSON, + }) + return trace.Wrap(err) +} + +func ensureTags( + ctx context.Context, + clt RoleTagger, + roleName string, + tags tags.AWSTags, + existingTags []iamtypes.Tag, +) error { + slog.InfoContext(ctx, "Checking for tags on IAM role", + "role", roleName, + "tags", tags, + ) + if tags.MatchesIAMTags(existingTags) { + slog.InfoContext(ctx, "IAM role tags already present", + "role", roleName, + ) + return nil + } + + _, err := clt.TagRole(ctx, &iam.TagRoleInput{ + RoleName: &roleName, + Tags: tags.ToIAMTags(), + }) + return trace.Wrap(err) +} + +type createRoleDetails struct { + Name string `json:"name"` + Description string `json:"description"` + TrustPolicy awslib.PolicyDocument `json:"trust_policy"` + Tags map[string]string `json:"tags"` +} diff --git a/lib/cloud/provisioning/operations.go b/lib/cloud/provisioning/operations.go new file mode 100644 index 0000000000000..dcc1e056b7839 --- /dev/null +++ b/lib/cloud/provisioning/operations.go @@ -0,0 +1,226 @@ +/* + * Teleport + * Copyright (C) 2024 Gravitational, Inc. + * + * This program is free software: you can redistribute it and/or modify + * it under the terms of the GNU Affero General Public License as published by + * the Free Software Foundation, either version 3 of the License, or + * (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU Affero General Public License for more details. + * + * You should have received a copy of the GNU Affero General Public License + * along with this program. If not, see . + */ + +package provisioning + +import ( + "context" + "fmt" + "io" + "log/slog" + "os" + "regexp" + "strings" + "text/template" + + "github.com/gravitational/trace" + + "github.com/gravitational/teleport/api/utils/prompt" +) + +// validName is used to ensure that [OperationConfig] and [ActionConfig] names +// start with a letter and only consist of letters, numbers, and hyphen +// characters thereafter. +var validName = regexp.MustCompile(`^[a-zA-Z][a-zA-Z0-9-]+$`) + +// OperationConfig is the configuration for an operation. +type OperationConfig struct { + // Name is the operation name. Must consist of only letters and hyphens. + Name string + // Summary is the summary the operation in prose. + Summary string + // Actions is the list of actions that make up the operation. + Actions []Action + // AutoConfirm is whether to skip the operation plan confirmation prompt. + AutoConfirm bool + // Output is an [io.Writer] where the operation plan and confirmation prompt + // are written to. + // Defaults to [os.Stdout]. + Output io.Writer +} + +// checkAndSetDefaults validates the operation config and sets defaults. +func (c *OperationConfig) checkAndSetDefaults() error { + c.Name = strings.TrimSpace(c.Name) + c.Summary = strings.TrimSpace(c.Summary) + if c.Name == "" { + return trace.BadParameter("missing operation name") + } + if !validName.MatchString(c.Name) { + return trace.BadParameter( + "operation name %q does not match regex used for validation %q", + c.Name, validName.String(), + ) + } + if c.Summary == "" { + return trace.BadParameter("missing operation summary") + } + if len(c.Actions) == 0 { + return trace.BadParameter("missing operation actions") + } + if c.Output == nil { + c.Output = os.Stdout + } + return nil +} + +// Action wraps a runnable function to provide a name, summary, and detailed +// explanation of the behavior. +type Action struct { + // config is an unexported value-type to prevent mutation after it's been + // validated by the checkAndSetDefaults func. + config ActionConfig +} + +// GetName returns the action's configured name. +func (a *Action) GetName() string { + return a.config.Name +} + +// GetSummary returns the action's configured summary. +func (a *Action) GetSummary() string { + return a.config.Summary +} + +// GetDetails returns the action's configured details. +func (a *Action) GetDetails() string { + return a.config.Details +} + +// Run runs the action. +func (a *Action) Run(ctx context.Context) error { + return a.config.RunnerFn(ctx) +} + +// NewAction creates a new [Action]. +func NewAction(config ActionConfig) (*Action, error) { + if err := config.checkAndSetDefaults(); err != nil { + return nil, trace.Wrap(err) + } + return &Action{ + config: config, + }, nil +} + +// ActionConfig is the configuration for an [Action]. +type ActionConfig struct { + // Name is the action name. + Name string + // Summary is the action summary in prose. + Summary string + // Details is the detailed explanation of the action, explaining exactly + // what it will do. + Details string + // RunnerFn is a function that actually runs the action. + RunnerFn func(context.Context) error +} + +// checkAndSetDefaults validates the action config and sets defaults. +func (c *ActionConfig) checkAndSetDefaults() error { + c.Name = strings.TrimSpace(c.Name) + c.Summary = strings.TrimSpace(c.Summary) + c.Details = strings.TrimSpace(c.Details) + if c.Name == "" { + return trace.BadParameter("missing action name") + } + if !validName.MatchString(c.Name) { + return trace.BadParameter( + "action name %q does not match regex used for validation %q", + c.Name, validName.String(), + ) + } + if c.Summary == "" { + return trace.BadParameter("missing action summary") + } + if c.Details == "" { + return trace.BadParameter("missing action details") + } + if c.RunnerFn == nil { + return trace.BadParameter("missing action runner") + } + + return nil +} + +// Run writes the operation plan, optionally prompts for user confirmation, +// then executes the operation plan. +func Run(ctx context.Context, config OperationConfig) error { + if err := config.checkAndSetDefaults(); err != nil { + return trace.Wrap(err) + } + + if err := writeOperationPlan(config); err != nil { + return trace.Wrap(err) + } + + if !config.AutoConfirm { + question := fmt.Sprintf("Do you want %q to perform these actions?", config.Name) + ok, err := prompt.Confirmation(ctx, config.Output, prompt.Stdin(), question) + if err != nil { + return trace.Wrap(err) + } + if !ok { + return trace.BadParameter("operation %q canceled", config.Name) + } + } + + enumerateSteps := len(config.Actions) > 1 + for i, action := range config.Actions { + if enumerateSteps { + slog.InfoContext(ctx, "Running", "step", i+1, "action", action.config.Name) + } else { + slog.InfoContext(ctx, "Running", "action", action.config.Name) + } + + if err := action.Run(ctx); err != nil { + if enumerateSteps { + return trace.Wrap(err, "step %d %q failed", i+1, action.config.Name) + } + return trace.Wrap(err, "%q failed", action.config.Name) + } + } + slog.InfoContext(ctx, "Success!", "operation", config.Name) + return nil +} + +// writeOperationPlan writes the operational plan to the given [io.Writer] as +// a structured summary of the operation and the actions that compose it. +func writeOperationPlan(config OperationConfig) error { + data := map[string]any{ + "config": config, + "showStepNumbers": len(config.Actions) > 1, + } + return trace.Wrap(operationPlanTemplate.Execute(config.Output, data)) +} + +var operationPlanTemplate = template.Must(template.New("plan"). + Funcs(template.FuncMap{ + // used to enumerate the action steps starting from 1 instead of 0. + "addOne": func(x int) int { return x + 1 }, + }). + Parse(` +{{- $global := . }} +{{- range $index, $action := .config.Actions }} +{{- if $global.showStepNumbers }}{{ $index | addOne }}. {{ end -}}{{$action.GetSummary}}. +{{$action.GetName}}: {{$action.GetDetails}} + +{{end -}} +Summary: {{.config.Summary}}. + +"{{ .config.Name }}" will perform the actions above automatically. +`)) diff --git a/lib/cloud/provisioning/operations_test.go b/lib/cloud/provisioning/operations_test.go new file mode 100644 index 0000000000000..902a80d3f3419 --- /dev/null +++ b/lib/cloud/provisioning/operations_test.go @@ -0,0 +1,391 @@ +/* + * Teleport + * Copyright (C) 2024 Gravitational, Inc. + * + * This program is free software: you can redistribute it and/or modify + * it under the terms of the GNU Affero General Public License as published by + * the Free Software Foundation, either version 3 of the License, or + * (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU Affero General Public License for more details. + * + * You should have received a copy of the GNU Affero General Public License + * along with this program. If not, see . + */ + +package provisioning + +import ( + "bytes" + "context" + "os" + "strings" + "testing" + "time" + + "github.com/google/go-cmp/cmp" + "github.com/google/go-cmp/cmp/cmpopts" + "github.com/gravitational/trace" + "github.com/stretchr/testify/require" +) + +func withErrCheck(fn func(*testing.T, error)) require.ErrorAssertionFunc { + return func(t require.TestingT, err error, msgAndArgs ...interface{}) { + tt := t.(*testing.T) + tt.Helper() + fn(tt, err) + } +} + +var badParameterCheck = withErrCheck(func(t *testing.T, err error) { + t.Helper() + require.True(t, trace.IsBadParameter(err), `expected "bad parameter", but got %v`, err) +}) + +func TestOperationCheckAndSetDefaults(t *testing.T) { + stubAction := Action{} + baseCfg := OperationConfig{ + Name: "name", + Summary: "summary", + Actions: []Action{stubAction}, + AutoConfirm: true, + Output: os.Stdout, + } + tests := []struct { + desc string + getConfig func() OperationConfig + want OperationConfig + wantErrContains string + }{ + { + desc: "valid config", + getConfig: func() OperationConfig { return baseCfg }, + want: baseCfg, + }, + { + desc: "defaults output to stdout", + getConfig: func() OperationConfig { + cfg := baseCfg + cfg.Output = nil + return cfg + }, + want: baseCfg, + }, + { + desc: "trims trailing and leading spaces", + getConfig: func() OperationConfig { + cfg := baseCfg + cfg.Name = "\t\n " + cfg.Name + "\t\n " + cfg.Summary = "\t\n " + cfg.Summary + "\t\n " + return cfg + }, + want: baseCfg, + }, + { + desc: "missing name is an error", + getConfig: func() OperationConfig { + cfg := baseCfg + cfg.Name = "" + return cfg + }, + wantErrContains: "missing operation name", + }, + { + desc: "spaces in name is an error", + getConfig: func() OperationConfig { + cfg := baseCfg + cfg.Name = "some thing" + return cfg + }, + wantErrContains: "does not match regex", + }, + { + desc: "missing summary is an error", + getConfig: func() OperationConfig { + cfg := baseCfg + cfg.Summary = "" + return cfg + }, + wantErrContains: "missing operation summary", + }, + { + desc: "missing actions is an error", + getConfig: func() OperationConfig { + cfg := baseCfg + cfg.Actions = nil + return cfg + }, + wantErrContains: "missing operation actions", + }, + } + for _, test := range tests { + t.Run(test.desc, func(t *testing.T) { + got := test.getConfig() + err := got.checkAndSetDefaults() + if test.wantErrContains != "" { + require.Error(t, err) + require.ErrorContains(t, err, test.wantErrContains) + return + } + require.Empty(t, cmp.Diff(test.want, got, + cmp.AllowUnexported(Action{}), + cmpopts.IgnoreFields(OperationConfig{}, "Output"), + )) + require.NotNil(t, got.Output) + }) + } +} + +func TestNewAction(t *testing.T) { + noOpRunnerFn := func(context.Context) error { return nil } + baseCfg := ActionConfig{ + Name: "name", + Summary: "summary", + Details: "details", + RunnerFn: noOpRunnerFn, + } + tests := []struct { + desc string + getConfig func() ActionConfig + errCheck require.ErrorAssertionFunc + want Action + }{ + { + desc: "valid config", + getConfig: func() ActionConfig { return baseCfg }, + errCheck: require.NoError, + want: Action{config: baseCfg}, + }, + { + desc: "trims trailing and leading spaces", + getConfig: func() ActionConfig { + cfg := baseCfg + cfg.Name = "\t\n " + cfg.Name + "\t\n " + cfg.Summary = "\t\n " + cfg.Summary + "\t\n " + return cfg + }, + errCheck: require.NoError, + want: Action{config: baseCfg}, + }, + { + desc: "missing name is an error", + getConfig: func() ActionConfig { + cfg := baseCfg + cfg.Name = "" + return cfg + }, + errCheck: badParameterCheck, + }, + { + desc: "spaces in name is an error", + getConfig: func() ActionConfig { + cfg := baseCfg + cfg.Name = "some thing" + return cfg + }, + errCheck: badParameterCheck, + }, + { + desc: "missing summary is an error", + getConfig: func() ActionConfig { + cfg := baseCfg + cfg.Summary = "" + return cfg + }, + errCheck: badParameterCheck, + }, + { + desc: "missing details is an error", + getConfig: func() ActionConfig { + cfg := baseCfg + cfg.Details = "" + return cfg + }, + errCheck: badParameterCheck, + }, + { + desc: "missing runner is an error", + getConfig: func() ActionConfig { + cfg := baseCfg + cfg.RunnerFn = nil + return cfg + }, + errCheck: badParameterCheck, + }, + } + for _, test := range tests { + t.Run(test.desc, func(t *testing.T) { + _, err := NewAction(test.getConfig()) + test.errCheck(t, err) + }) + } +} + +func TestRun(t *testing.T) { + actionA := Action{ + config: ActionConfig{ + Name: "actionA", + Summary: "actionA summary", + Details: "actionA details", + RunnerFn: func(context.Context) error { return nil }, + }, + } + actionB := Action{ + config: ActionConfig{ + Name: "actionB", + Summary: "actionB summary", + Details: "actionB details", + RunnerFn: func(context.Context) error { return nil }, + }, + } + failingAction := Action{ + config: ActionConfig{ + Name: "actionC", + Summary: "actionC summary", + Details: "actionC details", + RunnerFn: func(context.Context) error { return trace.AccessDenied("access denied") }, + }, + } + + tests := []struct { + desc string + config OperationConfig + errCheck require.ErrorAssertionFunc + }{ + { + desc: "success", + config: OperationConfig{ + Name: "op-name", + Summary: "op summary", + Actions: []Action{ + actionA, + actionB, + }, + AutoConfirm: true, + }, + errCheck: require.NoError, + }, + { + desc: "failed action does not enumerate the failed step", + config: OperationConfig{ + Name: "op-name", + Summary: "op summary", + Actions: []Action{ + failingAction, + }, + AutoConfirm: true, + }, + errCheck: withErrCheck(func(t *testing.T, err error) { + t.Helper() + require.Error(t, err) + require.True(t, trace.IsAccessDenied(err)) + // the error should indicate the failed action + require.Contains(t, err.Error(), `"actionC" failed`) + // but it should not number the step, since there's only + // one action. + require.NotContains(t, err.Error(), `1`) + }), + }, + { + desc: "failed action fails the entire operation", + config: OperationConfig{ + Name: "op-name", + Summary: "op summary", + Actions: []Action{ + actionA, + failingAction, + actionB, + }, + AutoConfirm: true, + }, + errCheck: withErrCheck(func(t *testing.T, err error) { + t.Helper() + require.Error(t, err) + require.True(t, trace.IsAccessDenied(err)) + // error should indicate step number and action name that failed + require.Contains(t, err.Error(), `step 2 "actionC" failed`) + }), + }, + } + for _, test := range tests { + t.Run(test.desc, func(t *testing.T) { + ctx, cancel := context.WithTimeout(context.Background(), time.Second*3) + defer cancel() + err := Run(ctx, test.config) + test.errCheck(t, err) + }) + } +} + +func TestWriteOperationPlan(t *testing.T) { + actionA := Action{ + config: ActionConfig{ + Name: "actionA", + Summary: "", + Details: "", + RunnerFn: func(context.Context) error { return trace.NotImplemented("not implemented") }, + }, + } + actionB := Action{ + config: ActionConfig{ + Name: "actionB", + Summary: "", + Details: "", + RunnerFn: func(context.Context) error { return trace.NotImplemented("not implemented") }, + }, + } + tests := []struct { + desc string + config OperationConfig + want string + }{ + { + desc: "operation with only one action does not enumerate the step", + config: OperationConfig{ + Name: "op-name", + Summary: "", + Actions: []Action{actionA}, + AutoConfirm: true, + }, + want: strings.TrimLeft(` +. +actionA: + +Summary: . + +"op-name" will perform the actions above automatically. +`, "\n"), + }, + { + desc: "operation with multiple actions enumerates the steps", + config: OperationConfig{ + Name: "op-name", + Summary: "", + Actions: []Action{actionA, actionB}, + AutoConfirm: true, + }, + want: strings.TrimLeft(` +1. . +actionA: + +2. . +actionB: + +Summary: . + +"op-name" will perform the actions above automatically. +`, "\n"), + }, + } + for _, test := range tests { + t.Run(test.desc, func(t *testing.T) { + var got bytes.Buffer + test.config.Output = &got + require.NoError(t, writeOperationPlan(test.config)) + require.Empty(t, cmp.Diff(test.want, got.String())) + }) + } +} diff --git a/lib/integrations/awsoidc/idp_iam_config.go b/lib/integrations/awsoidc/idp_iam_config.go index 4a6b345092bbd..39af42c0156e2 100644 --- a/lib/integrations/awsoidc/idp_iam_config.go +++ b/lib/integrations/awsoidc/idp_iam_config.go @@ -20,7 +20,8 @@ package awsoidc import ( "context" - "log/slog" + "fmt" + "io" "net/http" "net/url" @@ -32,6 +33,8 @@ import ( "github.com/gravitational/teleport/api/types" awslib "github.com/gravitational/teleport/lib/cloud/aws" + "github.com/gravitational/teleport/lib/cloud/provisioning" + "github.com/gravitational/teleport/lib/cloud/provisioning/awsactions" "github.com/gravitational/teleport/lib/defaults" "github.com/gravitational/teleport/lib/integrations/awsoidc/tags" ) @@ -59,6 +62,9 @@ type IdPIAMConfigureRequest struct { // Eg, https://.teleport.sh, https://proxy.example.org:443, https://teleport.ec2.aws:3080 ProxyPublicAddress string + // AutoConfirm skips user confirmation of the operation plan if true. + AutoConfirm bool + // issuer is the above value but only contains the host. // Eg, .teleport.sh, proxy.example.org issuer string @@ -70,6 +76,12 @@ type IdPIAMConfigureRequest struct { IntegrationRole string ownershipTags tags.AWSTags + + // stdout is used to override stdout output in tests. + stdout io.Writer + // fakeThumbprint is used to override thumbprint in output tests, to produce + // consistent output. + fakeThumbprint string } // CheckAndSetDefaults ensures the required fields are present. @@ -109,21 +121,11 @@ func (r *IdPIAMConfigureRequest) CheckAndSetDefaults() error { // There is no guarantee that the client is thread safe. type IdPIAMConfigureClient interface { callerIdentityGetter - - // CreateOpenIDConnectProvider creates an IAM OIDC IdP. - CreateOpenIDConnectProvider(ctx context.Context, params *iam.CreateOpenIDConnectProviderInput, optFns ...func(*iam.Options)) (*iam.CreateOpenIDConnectProviderOutput, error) - - // CreateRole creates a new IAM Role. - CreateRole(ctx context.Context, params *iam.CreateRoleInput, optFns ...func(*iam.Options)) (*iam.CreateRoleOutput, error) - - // GetRole retrieves information about the specified role, including the role's path, - // GUID, ARN, and the role's trust policy that grants permission to assume the - // role. - GetRole(ctx context.Context, params *iam.GetRoleInput, optFns ...func(*iam.Options)) (*iam.GetRoleOutput, error) - - // UpdateAssumeRolePolicy updates the policy that grants an IAM entity permission to assume a role. - // This is typically referred to as the "role trust policy". - UpdateAssumeRolePolicy(ctx context.Context, params *iam.UpdateAssumeRolePolicyInput, optFns ...func(*iam.Options)) (*iam.UpdateAssumeRolePolicyOutput, error) + awsactions.AssumeRolePolicyUpdater + awsactions.OpenIDConnectProviderCreator + awsactions.RoleCreator + awsactions.RoleGetter + awsactions.RoleTagger } type defaultIdPIAMConfigureClient struct { @@ -183,99 +185,57 @@ func ConfigureIdPIAM(ctx context.Context, clt IdPIAMConfigureClient, req IdPIAMC req.AccountID = aws.ToString(callerIdentity.Account) } - slog.InfoContext(ctx, "Creating IAM OpenID Connect Provider", "url", req.issuerURL) - if err := ensureOIDCIdPIAM(ctx, clt, req); err != nil { - return trace.Wrap(err) - } - - slog.InfoContext(ctx, "Creating IAM Role", "role", req.IntegrationRole) - if err := upsertIdPIAMRole(ctx, clt, req); err != nil { - return trace.Wrap(err) - } - - return nil -} - -func ensureOIDCIdPIAM(ctx context.Context, clt IdPIAMConfigureClient, req IdPIAMConfigureRequest) error { - thumbprint, err := ThumbprintIdP(ctx, req.ProxyPublicAddress) - if err != nil { - return trace.Wrap(err) - } - - _, err = clt.CreateOpenIDConnectProvider(ctx, &iam.CreateOpenIDConnectProviderInput{ - ThumbprintList: []string{thumbprint}, - Url: &req.issuerURL, - ClientIDList: []string{types.IntegrationAWSOIDCAudience}, - Tags: req.ownershipTags.ToIAMTags(), - }) + createOIDCIdP, err := createOIDCIdPAction(ctx, clt, req) if err != nil { - awsErr := awslib.ConvertIAMv2Error(err) - if trace.IsAlreadyExists(awsErr) { - return nil - } - return trace.Wrap(err) } - return nil -} - -func createIdPIAMRole(ctx context.Context, clt IdPIAMConfigureClient, req IdPIAMConfigureRequest) error { - integrationRoleAssumeRoleDocument, err := awslib.NewPolicyDocument( - awslib.StatementForAWSOIDCRoleTrustRelationship(req.AccountID, req.issuer, []string{types.IntegrationAWSOIDCAudience}), - ).Marshal() + createIdPIAMRole, err := createIdPIAMRoleAction(clt, req) if err != nil { return trace.Wrap(err) } - _, err = clt.CreateRole(ctx, &iam.CreateRoleInput{ - RoleName: &req.IntegrationRole, - Description: aws.String(descriptionOIDCIdPRole), - AssumeRolePolicyDocument: &integrationRoleAssumeRoleDocument, - Tags: req.ownershipTags.ToIAMTags(), - }) - return trace.Wrap(err) + return trace.Wrap(provisioning.Run(ctx, provisioning.OperationConfig{ + Name: "awsoidc-idp", + Summary: fmt.Sprintf("Configure an AWS IAM OIDC Identity Provider (IdP) "+ + "and configure IAM role %q to trust the IdP", + req.IntegrationRole, + ), + Actions: []provisioning.Action{ + *createOIDCIdP, + *createIdPIAMRole, + }, + AutoConfirm: req.AutoConfirm, + Output: req.stdout, + })) } -func upsertIdPIAMRole(ctx context.Context, clt IdPIAMConfigureClient, req IdPIAMConfigureRequest) error { - getRoleOut, err := clt.GetRole(ctx, &iam.GetRoleInput{ - RoleName: &req.IntegrationRole, - }) - if err != nil { - convertedErr := awslib.ConvertIAMv2Error(err) - if !trace.IsNotFound(convertedErr) { - return trace.Wrap(convertedErr) - } - - return trace.Wrap(createIdPIAMRole(ctx, clt, req)) - } - - if !req.ownershipTags.MatchesIAMTags(getRoleOut.Role.Tags) { - return trace.BadParameter("IAM Role %q already exists but is not managed by Teleport. "+ - "Add the following tags to allow Teleport to manage this Role: %s", req.IntegrationRole, req.ownershipTags) - } - - trustRelationshipDoc, err := awslib.ParsePolicyDocument(aws.ToString(getRoleOut.Role.AssumeRolePolicyDocument)) - if err != nil { - return trace.Wrap(err) - } - - trustRelationshipForIdP := awslib.StatementForAWSOIDCRoleTrustRelationship(req.AccountID, req.issuer, []string{types.IntegrationAWSOIDCAudience}) - for _, existingStatement := range trustRelationshipDoc.Statements { - if existingStatement.EqualStatement(trustRelationshipForIdP) { - return nil +func createOIDCIdPAction(ctx context.Context, clt IdPIAMConfigureClient, req IdPIAMConfigureRequest) (*provisioning.Action, error) { + var thumbprint string + if req.fakeThumbprint != "" { + // only happens in tests. + thumbprint = req.fakeThumbprint + } else { + var err error + thumbprint, err = ThumbprintIdP(ctx, req.ProxyPublicAddress) + if err != nil { + return nil, trace.Wrap(err) } } - trustRelationshipDoc.Statements = append(trustRelationshipDoc.Statements, trustRelationshipForIdP) - trustRelationshipDocString, err := trustRelationshipDoc.Marshal() - if err != nil { - return trace.Wrap(err) - } + clientIDs := []string{types.IntegrationAWSOIDCAudience} + thumbprints := []string{thumbprint} + return awsactions.CreateOIDCProvider(clt, thumbprints, req.issuerURL, clientIDs, req.ownershipTags) +} - _, err = clt.UpdateAssumeRolePolicy(ctx, &iam.UpdateAssumeRolePolicyInput{ - RoleName: &req.IntegrationRole, - PolicyDocument: &trustRelationshipDocString, - }) - return trace.Wrap(err) +func createIdPIAMRoleAction(clt IdPIAMConfigureClient, req IdPIAMConfigureRequest) (*provisioning.Action, error) { + integrationRoleAssumeRoleDocument := awslib.NewPolicyDocument( + awslib.StatementForAWSOIDCRoleTrustRelationship(req.AccountID, req.issuer, []string{types.IntegrationAWSOIDCAudience}), + ) + return awsactions.CreateRole(clt, + req.IntegrationRole, + descriptionOIDCIdPRole, + integrationRoleAssumeRoleDocument, + req.ownershipTags, + ) } diff --git a/lib/integrations/awsoidc/idp_iam_config_test.go b/lib/integrations/awsoidc/idp_iam_config_test.go index 70012936ac7b2..894a059f55a81 100644 --- a/lib/integrations/awsoidc/idp_iam_config_test.go +++ b/lib/integrations/awsoidc/idp_iam_config_test.go @@ -19,6 +19,7 @@ package awsoidc import ( + "bytes" "context" "fmt" "net/http/httptest" @@ -35,6 +36,7 @@ import ( "github.com/gravitational/teleport/lib" "github.com/gravitational/teleport/lib/integrations/awsoidc/tags" + "github.com/gravitational/teleport/lib/utils/golden" ) func TestIdPIAMConfigReqDefaults(t *testing.T) { @@ -44,6 +46,7 @@ func TestIdPIAMConfigReqDefaults(t *testing.T) { IntegrationName: "myintegration", IntegrationRole: "integrationrole", ProxyPublicAddress: "https://proxy.example.com", + AutoConfirm: true, } } @@ -69,6 +72,7 @@ func TestIdPIAMConfigReqDefaults(t *testing.T) { "teleport.dev/integration": "myintegration", "teleport.dev/origin": "integration_awsoidc", }, + AutoConfirm: true, }, }, { @@ -166,6 +170,7 @@ func TestConfigureIdPIAM(t *testing.T) { IntegrationName: "myintegration", IntegrationRole: "integrationrole", ProxyPublicAddress: tlsServer.URL, + AutoConfirm: true, } } @@ -195,15 +200,7 @@ func TestConfigureIdPIAM(t *testing.T) { errCheck: require.NoError, }, { - name: "role exists, no ownership tags", - mockAccountID: "123456789012", - mockExistingIdPUrl: []string{}, - mockExistingRoles: map[string]mockRole{"integrationrole": {}}, - req: baseIdPIAMConfigReqWithTLServer, - errCheck: badParameterCheck, - }, - { - name: "role exists, ownership tags, no assume role", + name: "role exists with empty trust policy", mockAccountID: "123456789012", mockExistingIdPUrl: []string{}, mockExistingRoles: map[string]mockRole{"integrationrole": { @@ -225,7 +222,7 @@ func TestConfigureIdPIAM(t *testing.T) { }, }, { - name: "role exists, ownership tags, with existing assume role", + name: "role exists with existing trust policy", mockAccountID: "123456789012", mockExistingIdPUrl: []string{}, mockExistingRoles: map[string]mockRole{"integrationrole": { @@ -250,7 +247,7 @@ func TestConfigureIdPIAM(t *testing.T) { }, }, { - name: "role exists, ownership tags, assume role already exists", + name: "role exists with matching trust policy", mockAccountID: "123456789012", mockExistingIdPUrl: []string{}, mockExistingRoles: map[string]mockRole{"integrationrole": { @@ -291,6 +288,32 @@ func TestConfigureIdPIAM(t *testing.T) { } } +func TestConfigureIdPIAMOutput(t *testing.T) { + ctx := context.Background() + var buf bytes.Buffer + req := IdPIAMConfigureRequest{ + Cluster: "mycluster", + IntegrationName: "myintegration", + IntegrationRole: "integrationrole", + ProxyPublicAddress: "https://example.com", + AutoConfirm: true, + stdout: &buf, + fakeThumbprint: "15dbd260c7465ecca6de2c0b2181187f66ee0d1a", + } + + clt := mockIdPIAMConfigClient{ + callerIdentityGetter: mockSTSClient{accountID: "123456789012"}, + existingRoles: map[string]mockRole{}, + existingIDPUrl: []string{}, + } + + require.NoError(t, ConfigureIdPIAM(ctx, &clt, req)) + if golden.ShouldSet() { + golden.Set(t, buf.Bytes()) + } + require.Equal(t, string(golden.Get(t)), buf.String()) +} + type mockRole struct { assumeRolePolicyDoc *string tags []iamtypes.Tag @@ -366,6 +389,26 @@ func (m *mockIdPIAMConfigClient) UpdateAssumeRolePolicy(ctx context.Context, par return &iam.UpdateAssumeRolePolicyOutput{}, nil } +func (m *mockIdPIAMConfigClient) TagRole(ctx context.Context, params *iam.TagRoleInput, _ ...func(*iam.Options)) (*iam.TagRoleOutput, error) { + roleName := aws.ToString(params.RoleName) + role, found := m.existingRoles[roleName] + if !found { + return nil, trace.NotFound("role not found") + } + +Outer: + for _, addTag := range params.Tags { + for _, roleTag := range role.tags { + if aws.ToString(roleTag.Key) == aws.ToString(addTag.Key) { + roleTag.Value = addTag.Value + continue Outer + } + } + role.tags = append(role.tags, addTag) + } + return &iam.TagRoleOutput{}, nil +} + func TestNewIdPIAMConfigureClient(t *testing.T) { t.Run("no aws_region env var, returns an error", func(t *testing.T) { _, err := NewIdPIAMConfigureClient(context.Background()) diff --git a/lib/integrations/awsoidc/testdata/TestConfigureIdPIAMOutput.golden b/lib/integrations/awsoidc/testdata/TestConfigureIdPIAMOutput.golden new file mode 100644 index 0000000000000..b438f6918d787 --- /dev/null +++ b/lib/integrations/awsoidc/testdata/TestConfigureIdPIAMOutput.golden @@ -0,0 +1,42 @@ +1. Create an OpenID Connect identity provider in AWS IAM for your Teleport cluster. +CreateOpenIDConnectIdentityProvider: { + "issuer_url": "https://example.com", + "client_id_list": [ + "discover.teleport" + ], + "thumbprint_list": [ + "15dbd260c7465ecca6de2c0b2181187f66ee0d1a" + ] +} + +2. Create IAM role "integrationrole" with a custom trust policy. +CreateRole: { + "name": "integrationrole", + "description": "Used by Teleport to provide access to AWS resources.", + "trust_policy": { + "Version": "2012-10-17", + "Statement": [ + { + "Effect": "Allow", + "Action": "sts:AssumeRoleWithWebIdentity", + "Principal": { + "Federated": "arn:aws:iam::123456789012:oidc-provider/example.com" + }, + "Condition": { + "StringEquals": { + "example.com:aud": "discover.teleport" + } + } + } + ] + }, + "tags": { + "teleport.dev/cluster": "mycluster", + "teleport.dev/integration": "myintegration", + "teleport.dev/origin": "integration_awsoidc" + } +} + +Summary: Configure an AWS IAM OIDC Identity Provider (IdP) and configure IAM role "integrationrole" to trust the IdP. + +"awsoidc-idp" will perform the actions above automatically. diff --git a/tool/teleport/common/integration_configure.go b/tool/teleport/common/integration_configure.go index 20b839155a0e2..66e8f5bccfca8 100644 --- a/tool/teleport/common/integration_configure.go +++ b/tool/teleport/common/integration_configure.go @@ -151,6 +151,7 @@ func onIntegrationConfAWSOIDCIdP(ctx context.Context, clf config.CommandLineFlag IntegrationName: clf.IntegrationConfAWSOIDCIdPArguments.Name, IntegrationRole: clf.IntegrationConfAWSOIDCIdPArguments.Role, ProxyPublicAddress: clf.IntegrationConfAWSOIDCIdPArguments.ProxyPublicURL, + AutoConfirm: clf.IntegrationConfAWSOIDCIdPArguments.AutoConfirm, } return trace.Wrap(awsoidc.ConfigureIdPIAM(ctx, iamClient, confReq)) } diff --git a/tool/teleport/common/teleport.go b/tool/teleport/common/teleport.go index 1c891d3c5aab7..d5b72668b904a 100644 --- a/tool/teleport/common/teleport.go +++ b/tool/teleport/common/teleport.go @@ -515,6 +515,7 @@ func Run(options Options) (app *kingpin.Application, executedCommand string, con IntegrationConfAWSOIDCIdPArguments.Role) integrationConfAWSOIDCIdPCmd.Flag("proxy-public-url", "Proxy Public URL (eg https://mytenant.teleport.sh).").Required().StringVar(&ccf. IntegrationConfAWSOIDCIdPArguments.ProxyPublicURL) + integrationConfAWSOIDCIdPCmd.Flag("confirm", "Do not prompt user and auto-confirm all actions.").BoolVar(&ccf.IntegrationConfAWSOIDCIdPArguments.AutoConfirm) integrationConfAWSOIDCIdPCmd.Flag("insecure", "Insecure mode disables certificate validation.").BoolVar(&ccf.InsecureMode) integrationConfListDatabasesCmd := integrationConfigureCmd.Command("listdatabases-iam", "Adds required IAM permissions to List RDS Databases (Instances and Clusters).")