Skip to content

Commit

Permalink
[entraid] ask for mfa reuse during entraid plugin setup (#48506)
Browse files Browse the repository at this point in the history
This PR fixes a bug where MFA for admin actions prevents creating
multiple objects during the entra id setup guide.

Signed-off-by: Tiago Silva <[email protected]>
  • Loading branch information
tigrato authored Nov 6, 2024
1 parent 42d312d commit 8d94f16
Show file tree
Hide file tree
Showing 4 changed files with 19 additions and 6 deletions.
2 changes: 1 addition & 1 deletion lib/auth/auth_with_roles.go
Original file line number Diff line number Diff line change
Expand Up @@ -3651,7 +3651,7 @@ func (a *ServerWithRoles) CreateSAMLConnector(ctx context.Context, connector typ
return nil, trace.Wrap(err)
}

if err := a.context.AuthorizeAdminAction(); err != nil {
if err := a.context.AuthorizeAdminActionAllowReusedMFA(); err != nil {
return nil, trace.Wrap(err)
}

Expand Down
15 changes: 11 additions & 4 deletions tool/tctl/common/plugin/entraid.go
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@ import (
"github.com/gravitational/trace"

pluginspb "github.com/gravitational/teleport/api/gen/proto/go/teleport/plugins/v1"
"github.com/gravitational/teleport/api/mfa"
"github.com/gravitational/teleport/api/types"
entraapiutils "github.com/gravitational/teleport/api/utils/entraid"
"github.com/gravitational/teleport/lib/integrations/azureoidc"
Expand Down Expand Up @@ -65,7 +66,7 @@ To rerun the script, type 'exit' to close and then restart the process.
`

step2Template = `
` + bold("Step 2: Input Tenant ID and Client ID") + `
With the output of Step 1, please copy and paste the following information:
Expand Down Expand Up @@ -120,9 +121,7 @@ type entraSettings struct {
tenantID string
}

var (
errCancel = trace.BadParameter("operation canceled")
)
var errCancel = trace.BadParameter("operation canceled")

func (p *PluginsCommand) entraSetupGuide(proxyPublicAddr string) (entraSettings, error) {
pwd, err := os.Getwd()
Expand Down Expand Up @@ -234,6 +233,14 @@ func (p *PluginsCommand) InstallEntra(ctx context.Context, args installPluginArg
}
}

// Prompt for admin action MFA if required, allowing reuse for UpsertToken and CreateBot.
mfaResponse, err := mfa.PerformAdminActionMFACeremony(ctx, args.authClient.PerformMFACeremony, true /*allowReuse*/)
if err == nil {
ctx = mfa.ContextWithMFAResponse(ctx, mfaResponse)
} else if !errors.Is(err, &mfa.ErrMFANotRequired) && !errors.Is(err, &mfa.ErrMFANotSupported) {
return trace.Wrap(err)
}

saml, err := types.NewSAMLConnector(inputs.entraID.authConnectorName, types.SAMLConnectorSpecV2{
AssertionConsumerService: strings.TrimRight(proxyPublicAddr, "/") + "/v1/webapi/saml/acs/" + inputs.entraID.authConnectorName,
AllowIDPInitiated: true,
Expand Down
3 changes: 2 additions & 1 deletion tool/tctl/common/plugin/plugins_command.go
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@ import (
"github.com/gravitational/teleport"
"github.com/gravitational/teleport/api/client/proto"
pluginsv1 "github.com/gravitational/teleport/api/gen/proto/go/teleport/plugins/v1"
"github.com/gravitational/teleport/api/mfa"
"github.com/gravitational/teleport/api/types"
"github.com/gravitational/teleport/lib/auth/authclient"
"github.com/gravitational/teleport/lib/service/servicecfg"
Expand Down Expand Up @@ -128,7 +129,6 @@ func (p *PluginsCommand) initInstallSCIM(parent *kingpin.CmdClause) {
Short('f').
Default("false").
BoolVar(&p.install.scim.force)

}

func (p *PluginsCommand) initDelete(parent *kingpin.CmdClause) {
Expand Down Expand Up @@ -208,6 +208,7 @@ type authClient interface {
GetIntegration(ctx context.Context, name string) (types.Integration, error)
UpdateIntegration(ctx context.Context, ig types.Integration) (types.Integration, error)
Ping(ctx context.Context) (proto.PingResponse, error)
PerformMFACeremony(ctx context.Context, challengeRequest *proto.CreateAuthenticateChallengeRequest, promptOpts ...mfa.PromptOpt) (*proto.MFAAuthenticateResponse, error)
}

type pluginsClient interface {
Expand Down
5 changes: 5 additions & 0 deletions tool/tctl/common/plugin/plugins_command_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@ import (

"github.com/gravitational/teleport/api/client/proto"
pluginsv1 "github.com/gravitational/teleport/api/gen/proto/go/teleport/plugins/v1"
"github.com/gravitational/teleport/api/mfa"
"github.com/gravitational/teleport/api/types"
"github.com/gravitational/teleport/lib/service/servicecfg"
)
Expand Down Expand Up @@ -494,5 +495,9 @@ func (m *mockAuthClient) Ping(ctx context.Context) (proto.PingResponse, error) {
return result.Get(0).(proto.PingResponse), result.Error(1)
}

func (m *mockAuthClient) PerformMFACeremony(ctx context.Context, challengeRequest *proto.CreateAuthenticateChallengeRequest, promptOpts ...mfa.PromptOpt) (*proto.MFAAuthenticateResponse, error) {
return &proto.MFAAuthenticateResponse{}, nil
}

// anyContext is an argument matcher for testify mocks that matches any context.
var anyContext any = mock.MatchedBy(func(context.Context) bool { return true })

0 comments on commit 8d94f16

Please sign in to comment.