From 5a16843f3b19942d16bb8863a704f467f131d7d3 Mon Sep 17 00:00:00 2001 From: Wallace Breza Date: Mon, 10 Jun 2024 17:19:28 -0700 Subject: [PATCH 1/5] Migrates loginGuard to middleware set with action descriptor options --- cli/azd/cmd/actions/action_descriptor.go | 2 + cli/azd/cmd/container.go | 4 +- cli/azd/cmd/middleware/login_guard.go | 46 +++++++++++++ cli/azd/cmd/middleware/login_guard_test.go | 78 ++++++++++++++++++++++ cli/azd/cmd/pipeline.go | 3 +- cli/azd/cmd/root.go | 4 ++ cli/azd/cmd/up.go | 2 - cli/azd/pkg/auth/guard.go | 25 ------- cli/azd/pkg/auth/manager.go | 5 ++ 9 files changed, 139 insertions(+), 30 deletions(-) create mode 100644 cli/azd/cmd/middleware/login_guard.go create mode 100644 cli/azd/cmd/middleware/login_guard_test.go delete mode 100644 cli/azd/pkg/auth/guard.go diff --git a/cli/azd/cmd/actions/action_descriptor.go b/cli/azd/cmd/actions/action_descriptor.go index c4b089a4c7f..d1aad783fbe 100644 --- a/cli/azd/cmd/actions/action_descriptor.go +++ b/cli/azd/cmd/actions/action_descriptor.go @@ -191,6 +191,8 @@ type ActionDescriptorOptions struct { HelpOptions ActionHelpOptions // Defines grouping options for the command GroupingOptions CommandGroupOptions + // Whether or not the command requires a principal login + RequireLogin bool } // Completion function used for cobra command flag completion diff --git a/cli/azd/cmd/container.go b/cli/azd/cmd/container.go index eb6895efc26..2eae89f28b9 100644 --- a/cli/azd/cmd/container.go +++ b/cli/azd/cmd/container.go @@ -152,7 +152,6 @@ func registerCommonDependencies(container *ioc.NestedContainer) { ioc.RegisterInstance[auth.HttpClient](container, client) // Auth - container.MustRegisterSingleton(auth.NewLoggedInGuard) container.MustRegisterSingleton(auth.NewMultiTenantCredentialProvider) container.MustRegisterSingleton(func(mgr *auth.Manager) CredentialProviderFn { return mgr.CredentialForCurrentUser @@ -579,6 +578,9 @@ func registerCommonDependencies(container *ioc.NestedContainer) { }) container.MustRegisterScoped(auth.NewManager) container.MustRegisterSingleton(azapi.NewUserProfileService) + container.MustRegisterScoped(func(authManager *auth.Manager) middleware.CurrentUserAuthManager { + return authManager + }) container.MustRegisterSingleton(account.NewSubscriptionsService) container.MustRegisterSingleton(account.NewManager) container.MustRegisterSingleton(account.NewSubscriptionsManager) diff --git a/cli/azd/cmd/middleware/login_guard.go b/cli/azd/cmd/middleware/login_guard.go new file mode 100644 index 00000000000..6e5dbeb410b --- /dev/null +++ b/cli/azd/cmd/middleware/login_guard.go @@ -0,0 +1,46 @@ +package middleware + +import ( + "context" + + "github.com/Azure/azure-sdk-for-go/sdk/azcore" + "github.com/azure/azure-dev/cli/azd/cmd/actions" + "github.com/azure/azure-dev/cli/azd/pkg/auth" + "github.com/azure/azure-dev/cli/azd/pkg/cloud" +) + +type CurrentUserAuthManager interface { + Cloud() *cloud.Cloud + CredentialForCurrentUser( + ctx context.Context, + options *auth.CredentialForCurrentUserOptions, + ) (azcore.TokenCredential, error) +} + +// LoginGuardMiddleware ensures that the user is logged in otherwise it returns an error +type LoginGuardMiddleware struct { + authManager CurrentUserAuthManager +} + +// NewLoginGuardMiddleware creates a new instance of the LoginGuardMiddleware +func NewLoginGuardMiddleware(authManager CurrentUserAuthManager) Middleware { + return &LoginGuardMiddleware{ + authManager: authManager, + } +} + +// Run ensures that the user is logged in otherwise it returns an error +func (l *LoginGuardMiddleware) Run(ctx context.Context, next NextFn) (*actions.ActionResult, error) { + cred, err := l.authManager.CredentialForCurrentUser(ctx, nil) + if err != nil { + return nil, err + } + + _, err = auth.EnsureLoggedInCredential(ctx, cred, l.authManager.Cloud()) + if err != nil { + return nil, err + } + + // At this point we have ensured a logged in user, continue execution of the action + return next(ctx) +} diff --git a/cli/azd/cmd/middleware/login_guard_test.go b/cli/azd/cmd/middleware/login_guard_test.go new file mode 100644 index 00000000000..132d3830b9c --- /dev/null +++ b/cli/azd/cmd/middleware/login_guard_test.go @@ -0,0 +1,78 @@ +package middleware + +import ( + "context" + "testing" + + "github.com/Azure/azure-sdk-for-go/sdk/azcore" + "github.com/azure/azure-dev/cli/azd/cmd/actions" + "github.com/azure/azure-dev/cli/azd/pkg/auth" + "github.com/azure/azure-dev/cli/azd/pkg/cloud" + "github.com/azure/azure-dev/cli/azd/test/mocks" + "github.com/stretchr/testify/mock" + "github.com/stretchr/testify/require" +) + +func Test_LoginGuard_Run(t *testing.T) { + t.Run("LoggedIn", func(t *testing.T) { + mockContext := mocks.NewMockContext(context.Background()) + + mockAuthManager := &mockCurrentUserAuthManager{} + mockAuthManager.On("Cloud").Return(cloud.AzurePublic()) + mockAuthManager. + On("CredentialForCurrentUser", *mockContext.Context, mock.Anything). + Return(mockContext.Credentials, nil) + + middleware := LoginGuardMiddleware{ + authManager: mockAuthManager, + } + + result, err := middleware.Run(*mockContext.Context, next) + require.NoError(t, err) + require.NotNil(t, result) + }) + t.Run("NotLoggedIn", func(t *testing.T) { + mockContext := mocks.NewMockContext(context.Background()) + + mockAuthManager := &mockCurrentUserAuthManager{} + mockAuthManager.On("Cloud").Return(cloud.AzurePublic()) + mockAuthManager. + On("CredentialForCurrentUser", *mockContext.Context, mock.Anything). + Return(nil, auth.ErrNoCurrentUser) + + middleware := LoginGuardMiddleware{ + authManager: mockAuthManager, + } + + result, err := middleware.Run(*mockContext.Context, next) + require.Error(t, err) + require.Nil(t, result) + }) +} + +func next(ctx context.Context) (*actions.ActionResult, error) { + return &actions.ActionResult{}, nil +} + +type mockCurrentUserAuthManager struct { + mock.Mock +} + +func (m *mockCurrentUserAuthManager) Cloud() *cloud.Cloud { + args := m.Called() + return args.Get(0).(*cloud.Cloud) +} + +func (m *mockCurrentUserAuthManager) CredentialForCurrentUser( + ctx context.Context, + options *auth.CredentialForCurrentUserOptions, +) (azcore.TokenCredential, error) { + args := m.Called(ctx, options) + + tokenVal := args.Get(0) + if tokenVal == nil { + return nil, args.Error(1) + } + + return tokenVal.(azcore.TokenCredential), args.Error(1) +} diff --git a/cli/azd/cmd/pipeline.go b/cli/azd/cmd/pipeline.go index 7806c9af13c..0a064782bda 100644 --- a/cli/azd/cmd/pipeline.go +++ b/cli/azd/cmd/pipeline.go @@ -10,7 +10,6 @@ import ( "github.com/MakeNowJust/heredoc/v2" "github.com/azure/azure-dev/cli/azd/cmd/actions" "github.com/azure/azure-dev/cli/azd/internal" - "github.com/azure/azure-dev/cli/azd/pkg/auth" "github.com/azure/azure-dev/cli/azd/pkg/environment" "github.com/azure/azure-dev/cli/azd/pkg/infra/provisioning" "github.com/azure/azure-dev/cli/azd/pkg/input" @@ -89,6 +88,7 @@ func pipelineActions(root *actions.ActionDescriptor) *actions.ActionDescriptor { GroupingOptions: actions.CommandGroupOptions{ RootLevelHelp: actions.CmdGroupBeta, }, + RequireLogin: true, }) group.Add("config", &actions.ActionDescriptorOptions{ @@ -134,7 +134,6 @@ type pipelineConfigAction struct { func newPipelineConfigAction( env *environment.Environment, - _ auth.LoggedInGuard, console input.Console, flags *pipelineConfigFlags, prompters prompt.Prompter, diff --git a/cli/azd/cmd/root.go b/cli/azd/cmd/root.go index 854e7a45f55..ac4ca81364b 100644 --- a/cli/azd/cmd/root.go +++ b/cli/azd/cmd/root.go @@ -303,6 +303,7 @@ func NewRootCmd( GroupingOptions: actions.CommandGroupOptions{ RootLevelHelp: actions.CmdGroupStart, }, + RequireLogin: true, }). UseMiddleware("hooks", middleware.NewHooksMiddleware). UseMiddleware("extensions", middleware.NewExtensionsMiddleware) @@ -356,6 +357,9 @@ func NewRootCmd( UseMiddleware("ux", middleware.NewUxMiddleware). UseMiddlewareWhen("telemetry", middleware.NewTelemetryMiddleware, func(descriptor *actions.ActionDescriptor) bool { return !descriptor.Options.DisableTelemetry + }). + UseMiddlewareWhen("loginGuard", middleware.NewLoginGuardMiddleware, func(descriptor *actions.ActionDescriptor) bool { + return descriptor.Options.RequireLogin }) // Register common dependencies for the IoC rootContainer diff --git a/cli/azd/cmd/up.go b/cli/azd/cmd/up.go index 41bfdbd18b3..df9f8bea434 100644 --- a/cli/azd/cmd/up.go +++ b/cli/azd/cmd/up.go @@ -13,7 +13,6 @@ import ( "github.com/azure/azure-dev/cli/azd/cmd/actions" "github.com/azure/azure-dev/cli/azd/internal" "github.com/azure/azure-dev/cli/azd/internal/cmd" - "github.com/azure/azure-dev/cli/azd/pkg/auth" "github.com/azure/azure-dev/cli/azd/pkg/environment" "github.com/azure/azure-dev/cli/azd/pkg/infra/provisioning" "github.com/azure/azure-dev/cli/azd/pkg/infra/provisioning/bicep" @@ -83,7 +82,6 @@ func newUpAction( flags *upFlags, console input.Console, env *environment.Environment, - _ auth.LoggedInGuard, projectConfig *project.ProjectConfig, provisioningManager *provisioning.Manager, envManager environment.Manager, diff --git a/cli/azd/pkg/auth/guard.go b/cli/azd/pkg/auth/guard.go deleted file mode 100644 index a0bdb659182..00000000000 --- a/cli/azd/pkg/auth/guard.go +++ /dev/null @@ -1,25 +0,0 @@ -// Copyright (c) Microsoft Corporation. All rights reserved. -// Licensed under the MIT License. - -package auth - -import "context" - -// LoggedInGuard doesn't hold anything. -// It simply represents a type that can be used to expressed the logged in constraint. -type LoggedInGuard struct{} - -// NewLoggedInGuard checks if the user is logged in. An error is returned if the user is not logged in. -func NewLoggedInGuard(manager *Manager, ctx context.Context) (LoggedInGuard, error) { - cred, err := manager.CredentialForCurrentUser(ctx, nil) - if err != nil { - return LoggedInGuard{}, err - } - - _, err = EnsureLoggedInCredential(ctx, cred, manager.cloud) - if err != nil { - return LoggedInGuard{}, err - } - - return LoggedInGuard{}, nil -} diff --git a/cli/azd/pkg/auth/manager.go b/cli/azd/pkg/auth/manager.go index a6eca564cde..2e55f8790e2 100644 --- a/cli/azd/pkg/auth/manager.go +++ b/cli/azd/pkg/auth/manager.go @@ -173,6 +173,11 @@ func (m *Manager) LoginScopes() []string { return LoginScopes(m.cloud) } +// Cloud returns the cloud that the manager is configured to use. +func (m *Manager) Cloud() *cloud.Cloud { + return m.cloud +} + func loginScopesMap(cloud *cloud.Cloud) map[string]struct{} { resourceManagerUrl := cloud.Configuration.Services[azcloud.ResourceManager].Endpoint From 667c8dfa9c8583d533f473d8bb1f54485e9e7ba7 Mon Sep 17 00:00:00 2001 From: Wallace Breza Date: Thu, 15 May 2025 11:58:03 -0700 Subject: [PATCH 2/5] Adds 'RequireLogin' setting to additional actions --- cli/azd/cmd/root.go | 3 +++ 1 file changed, 3 insertions(+) diff --git a/cli/azd/cmd/root.go b/cli/azd/cmd/root.go index ac4ca81364b..77e61950588 100644 --- a/cli/azd/cmd/root.go +++ b/cli/azd/cmd/root.go @@ -238,6 +238,7 @@ func NewRootCmd( GroupingOptions: actions.CommandGroupOptions{ RootLevelHelp: actions.CmdGroupAzure, }, + RequireLogin: true, }). UseMiddlewareWhen("hooks", middleware.NewHooksMiddleware, func(descriptor *actions.ActionDescriptor) bool { if onPreview, _ := descriptor.Options.Command.Flags().GetBool("preview"); onPreview { @@ -286,6 +287,7 @@ func NewRootCmd( GroupingOptions: actions.CommandGroupOptions{ RootLevelHelp: actions.CmdGroupAzure, }, + RequireLogin: true, }). UseMiddleware("hooks", middleware.NewHooksMiddleware). UseMiddleware("extensions", middleware.NewExtensionsMiddleware) @@ -335,6 +337,7 @@ func NewRootCmd( GroupingOptions: actions.CommandGroupOptions{ RootLevelHelp: actions.CmdGroupAzure, }, + RequireLogin: true, }). UseMiddleware("hooks", middleware.NewHooksMiddleware). UseMiddleware("extensions", middleware.NewExtensionsMiddleware) From 1946fe77386af817aafccec4bef434baac99d0a2 Mon Sep 17 00:00:00 2001 From: Wallace Breza Date: Thu, 15 May 2025 15:51:03 -0700 Subject: [PATCH 3/5] Fixes linter issues --- cli/azd/.vscode/cspell-azd-dictionary.txt | 1 + cli/azd/cmd/middleware/login_guard.go | 3 +++ cli/azd/cmd/middleware/login_guard_test.go | 3 +++ 3 files changed, 7 insertions(+) diff --git a/cli/azd/.vscode/cspell-azd-dictionary.txt b/cli/azd/.vscode/cspell-azd-dictionary.txt index f757367c022..2f2adfa4b80 100644 --- a/cli/azd/.vscode/cspell-azd-dictionary.txt +++ b/cli/azd/.vscode/cspell-azd-dictionary.txt @@ -196,6 +196,7 @@ requirepass resourcegraph restoreapp retriable +runtimes rzip secureobject securestring diff --git a/cli/azd/cmd/middleware/login_guard.go b/cli/azd/cmd/middleware/login_guard.go index 6e5dbeb410b..992d64deb7c 100644 --- a/cli/azd/cmd/middleware/login_guard.go +++ b/cli/azd/cmd/middleware/login_guard.go @@ -1,3 +1,6 @@ +// Copyright (c) Microsoft Corporation. All rights reserved. +// Licensed under the MIT License. + package middleware import ( diff --git a/cli/azd/cmd/middleware/login_guard_test.go b/cli/azd/cmd/middleware/login_guard_test.go index 132d3830b9c..e7daf95a023 100644 --- a/cli/azd/cmd/middleware/login_guard_test.go +++ b/cli/azd/cmd/middleware/login_guard_test.go @@ -1,3 +1,6 @@ +// Copyright (c) Microsoft Corporation. All rights reserved. +// Licensed under the MIT License. + package middleware import ( From b3f9bf9e779edbf31084c2de40f924b9477562c9 Mon Sep 17 00:00:00 2001 From: Wallace Breza Date: Thu, 15 May 2025 16:02:40 -0700 Subject: [PATCH 4/5] Check if any parents require login --- cli/azd/cmd/root.go | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-) diff --git a/cli/azd/cmd/root.go b/cli/azd/cmd/root.go index 77e61950588..79ac275b2be 100644 --- a/cli/azd/cmd/root.go +++ b/cli/azd/cmd/root.go @@ -362,7 +362,17 @@ func NewRootCmd( return !descriptor.Options.DisableTelemetry }). UseMiddlewareWhen("loginGuard", middleware.NewLoginGuardMiddleware, func(descriptor *actions.ActionDescriptor) bool { - return descriptor.Options.RequireLogin + // Check if the command or any of its parents require login + current := descriptor + for current != nil { + if current.Options != nil && current.Options.RequireLogin { + return true + } + + current = current.Parent() + } + + return false }) // Register common dependencies for the IoC rootContainer From 1c83cfbe15b9bba2622f760bd413bafc098b8ec9 Mon Sep 17 00:00:00 2001 From: Wallace Breza Date: Fri, 16 May 2025 16:21:53 -0700 Subject: [PATCH 5/5] Prompt to continue with login --- cli/azd/cmd/middleware/login_guard.go | 67 ++++++++++++++++++++-- cli/azd/cmd/middleware/login_guard_test.go | 16 +++++- 2 files changed, 77 insertions(+), 6 deletions(-) diff --git a/cli/azd/cmd/middleware/login_guard.go b/cli/azd/cmd/middleware/login_guard.go index 992d64deb7c..04b0f21a273 100644 --- a/cli/azd/cmd/middleware/login_guard.go +++ b/cli/azd/cmd/middleware/login_guard.go @@ -10,6 +10,9 @@ import ( "github.com/azure/azure-dev/cli/azd/cmd/actions" "github.com/azure/azure-dev/cli/azd/pkg/auth" "github.com/azure/azure-dev/cli/azd/pkg/cloud" + "github.com/azure/azure-dev/cli/azd/pkg/input" + "github.com/azure/azure-dev/cli/azd/pkg/output" + "github.com/azure/azure-dev/cli/azd/pkg/workflow" ) type CurrentUserAuthManager interface { @@ -22,19 +25,26 @@ type CurrentUserAuthManager interface { // LoginGuardMiddleware ensures that the user is logged in otherwise it returns an error type LoginGuardMiddleware struct { - authManager CurrentUserAuthManager + authManager CurrentUserAuthManager + console input.Console + workflowRunner *workflow.Runner } // NewLoginGuardMiddleware creates a new instance of the LoginGuardMiddleware -func NewLoginGuardMiddleware(authManager CurrentUserAuthManager) Middleware { +func NewLoginGuardMiddleware( + console input.Console, + authManager CurrentUserAuthManager, + workflowRunner *workflow.Runner) Middleware { return &LoginGuardMiddleware{ - authManager: authManager, + authManager: authManager, + console: console, + workflowRunner: workflowRunner, } } // Run ensures that the user is logged in otherwise it returns an error func (l *LoginGuardMiddleware) Run(ctx context.Context, next NextFn) (*actions.ActionResult, error) { - cred, err := l.authManager.CredentialForCurrentUser(ctx, nil) + cred, err := l.ensureLogin(ctx) if err != nil { return nil, err } @@ -47,3 +57,52 @@ func (l *LoginGuardMiddleware) Run(ctx context.Context, next NextFn) (*actions.A // At this point we have ensured a logged in user, continue execution of the action return next(ctx) } + +// ensureLogin checks if the user is logged in and if not, it prompts the user to continue with log in +func (l *LoginGuardMiddleware) ensureLogin(ctx context.Context) (azcore.TokenCredential, error) { + cred, credentialErr := l.authManager.CredentialForCurrentUser(ctx, nil) + if credentialErr != nil { + loginWarning := output.WithWarningFormat("WARNING: You must be logged into Azure perform this action") + l.console.Message(ctx, loginWarning) + + // Prompt the user to log in + continueWithLogin, err := l.console.Confirm(ctx, input.ConsoleOptions{ + Message: "Would you like to log in now?", + DefaultValue: true, + }) + l.console.Message(ctx, "") + + if err != nil { + return nil, err + } + + if !continueWithLogin { + return nil, credentialErr + } + + err = l.workflowRunner.Run(ctx, &workflow.Workflow{ + Name: "Login", + Steps: []*workflow.Step{ + { + AzdCommand: workflow.Command{ + Args: []string{"auth", "login"}, + }, + }, + }, + }) + + if err != nil { + return nil, err + } + + // Retry to get the credential after login + cred, err = l.authManager.CredentialForCurrentUser(ctx, nil) + if err != nil { + return nil, err + } + + l.console.Message(ctx, "") + } + + return cred, nil +} diff --git a/cli/azd/cmd/middleware/login_guard_test.go b/cli/azd/cmd/middleware/login_guard_test.go index e7daf95a023..d89f9b2e8cd 100644 --- a/cli/azd/cmd/middleware/login_guard_test.go +++ b/cli/azd/cmd/middleware/login_guard_test.go @@ -5,12 +5,15 @@ package middleware import ( "context" + "strings" "testing" "github.com/Azure/azure-sdk-for-go/sdk/azcore" "github.com/azure/azure-dev/cli/azd/cmd/actions" "github.com/azure/azure-dev/cli/azd/pkg/auth" "github.com/azure/azure-dev/cli/azd/pkg/cloud" + "github.com/azure/azure-dev/cli/azd/pkg/input" + "github.com/azure/azure-dev/cli/azd/pkg/workflow" "github.com/azure/azure-dev/cli/azd/test/mocks" "github.com/stretchr/testify/mock" "github.com/stretchr/testify/require" @@ -27,7 +30,9 @@ func Test_LoginGuard_Run(t *testing.T) { Return(mockContext.Credentials, nil) middleware := LoginGuardMiddleware{ - authManager: mockAuthManager, + console: mockContext.Console, + authManager: mockAuthManager, + workflowRunner: &workflow.Runner{}, } result, err := middleware.Run(*mockContext.Context, next) @@ -36,6 +41,11 @@ func Test_LoginGuard_Run(t *testing.T) { }) t.Run("NotLoggedIn", func(t *testing.T) { mockContext := mocks.NewMockContext(context.Background()) + mockContext.Console. + WhenConfirm(func(options input.ConsoleOptions) bool { + return strings.Contains(options.Message, "Would you like to log in now?") + }). + Respond(false) mockAuthManager := &mockCurrentUserAuthManager{} mockAuthManager.On("Cloud").Return(cloud.AzurePublic()) @@ -44,7 +54,9 @@ func Test_LoginGuard_Run(t *testing.T) { Return(nil, auth.ErrNoCurrentUser) middleware := LoginGuardMiddleware{ - authManager: mockAuthManager, + console: mockContext.Console, + authManager: mockAuthManager, + workflowRunner: &workflow.Runner{}, } result, err := middleware.Run(*mockContext.Context, next)