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

passwordless sqlcmd sqlOperation #4140

Open
wants to merge 13 commits into
base: main
Choose a base branch
from
1 change: 1 addition & 0 deletions cli/azd/.vscode/cspell-azd-dictionary.txt
Original file line number Diff line number Diff line change
Expand Up @@ -186,6 +186,7 @@ servicebus
setenvs
snapshotter
springapp
sqlcmd
sqlserver
sstore
staticcheck
Expand Down
2 changes: 2 additions & 0 deletions cli/azd/cmd/container.go
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,7 @@ import (
"github.com/azure/azure-dev/cli/azd/pkg/tools/maven"
"github.com/azure/azure-dev/cli/azd/pkg/tools/npm"
"github.com/azure/azure-dev/cli/azd/pkg/tools/python"
"github.com/azure/azure-dev/cli/azd/pkg/tools/sqlcmd"
"github.com/azure/azure-dev/cli/azd/pkg/tools/swa"
"github.com/azure/azure-dev/cli/azd/pkg/workflow"
"github.com/mattn/go-colorable"
Expand Down Expand Up @@ -590,6 +591,7 @@ func registerCommonDependencies(container *ioc.NestedContainer) {
container.MustRegisterSingleton(dotnet.NewCli)
container.MustRegisterSingleton(git.NewCli)
container.MustRegisterSingleton(github.NewGitHubCli)
container.MustRegisterSingleton(sqlcmd.NewSqlCmdCli)
container.MustRegisterSingleton(javac.NewCli)
container.MustRegisterSingleton(kubectl.NewCli)
container.MustRegisterSingleton(maven.NewCli)
Expand Down
18 changes: 18 additions & 0 deletions cli/azd/internal/cmd/provision.go
Original file line number Diff line number Diff line change
Expand Up @@ -192,6 +192,24 @@ func (p *ProvisionAction) Run(ctx context.Context) (*actions.ActionResult, error
return nil, fmt.Errorf("initializing provisioning manager: %w", err)
}

// ** Registering post-provisioning operations **
// When azd.operations.yaml is found, the provisioning manager returns the list of operations to be executed
// as callbacks -> []func(ctx context.Context) error, error)
// See package `infra/provisioning/operations` for more details.
operations, err := p.provisionManager.Operations(ctx)
if err != nil {
return nil, fmt.Errorf("registering operations: %w", err)
}
for _, operation := range operations {
err := p.projectConfig.AddHandler(
"postprovision", func(ctx context.Context, args project.ProjectLifecycleEventArgs) error {
return operation(ctx)
})
if err != nil {
return nil, fmt.Errorf("registering operation: %w", err)
}
}

// Get Subscription to Display in Command Title Note
// Subscription and Location are ONLY displayed when they are available (found from env), otherwise, this message
// is not displayed.
Expand Down
4 changes: 2 additions & 2 deletions cli/azd/pkg/apphost/generate.go
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ import (
"github.com/azure/azure-dev/cli/azd/pkg/convert"
"github.com/azure/azure-dev/cli/azd/pkg/custommaps"
"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/operations"
"github.com/azure/azure-dev/cli/azd/pkg/osutil"
"github.com/azure/azure-dev/cli/azd/pkg/output"
"github.com/azure/azure-dev/cli/azd/resources"
Expand Down Expand Up @@ -318,7 +318,7 @@ func BicepTemplate(name string, manifest *Manifest, options AppHostOptions) (*me
}
} else {
// returning fs because this error can be handled by the caller as expected
return fs, provisioning.ErrBindMountOperationDisabled
return fs, operations.ErrBindMountOperationDisabled
}

}
Expand Down
4 changes: 2 additions & 2 deletions cli/azd/pkg/apphost/generate_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ import (
"testing"

"github.com/azure/azure-dev/cli/azd/pkg/exec"
"github.com/azure/azure-dev/cli/azd/pkg/infra/provisioning"
"github.com/azure/azure-dev/cli/azd/pkg/infra/provisioning/operations"
"github.com/azure/azure-dev/cli/azd/pkg/osutil"
"github.com/azure/azure-dev/cli/azd/pkg/tools/dotnet"
"github.com/azure/azure-dev/cli/azd/test/mocks"
Expand Down Expand Up @@ -228,7 +228,7 @@ func TestAspireContainerGeneration(t *testing.T) {
}

_, err = BicepTemplate("main", m, AppHostOptions{})
require.Error(t, err, provisioning.ErrBindMountOperationDisabled)
require.Error(t, err, operations.ErrBindMountOperationDisabled)

files, err := BicepTemplate("main", m, AppHostOptions{
AzdOperations: true,
Expand Down
29 changes: 29 additions & 0 deletions cli/azd/pkg/auth/manager.go
Original file line number Diff line number Diff line change
Expand Up @@ -446,6 +446,35 @@ func (m *Manager) GetLoggedInServicePrincipalTenantID(ctx context.Context) (*str
return currentUser.TenantID, nil
}

func (m *Manager) GetLoggedInServicePrincipalID(ctx context.Context) (*string, error) {
if m.UseExternalAuth() {
// When delegating to an external system, we have no way to determine what principal was used
return nil, nil
}

cfg, err := m.userConfigManager.Load()
if err != nil {
return nil, fmt.Errorf("fetching current user: %w", err)
}

if shouldUseLegacyAuth(cfg) {
// When delegating to az, we have no way to determine what principal was used
return nil, nil
}

authCfg, err := m.readAuthConfig()
if err != nil {
return nil, fmt.Errorf("fetching auth config: %w", err)
}

currentUser, err := readUserProperties(authCfg)
if err != nil {
return nil, ErrNoCurrentUser
}

return currentUser.ClientID, nil
}

func (m *Manager) newCredentialFromManagedIdentity(clientID string) (azcore.TokenCredential, error) {
options := &azidentity.ManagedIdentityCredentialOptions{}
if clientID != "" {
Expand Down
4 changes: 4 additions & 0 deletions cli/azd/pkg/azure/arm_template.go
Original file line number Diff line number Diff line change
Expand Up @@ -101,6 +101,10 @@ type AzdMetadataType string

const AzdMetadataTypeLocation AzdMetadataType = "location"
const AzdMetadataTypeGenerate AzdMetadataType = "generate"
const AzdMetadataTypePrincipalLogin AzdMetadataType = "principalLogin"
const AzdMetadataTypePrincipalId AzdMetadataType = "principalId"
const AzdMetadataTypePrincipalType AzdMetadataType = "principalType"
const AzdMetadataTypeIpAddress AzdMetadataType = "ipAddress"
Copy link
Member

Choose a reason for hiding this comment

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

I'm very nervous about this feature (and also the implementation) - Are we totally hosed if we don't have it? I assume this is for us to be able to scope some firewall rules to "the current user's machine" but I am very nervous about corner cases in the implementation with respect to things like NAT.

const AzdMetadataTypeGenerateOrManual AzdMetadataType = "generateOrManual"

type AzdMetadata struct {
Expand Down
47 changes: 47 additions & 0 deletions cli/azd/pkg/azureutil/principal.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ package azureutil
import (
"context"
"fmt"
"log"

"github.com/azure/azure-dev/cli/azd/pkg/auth"
"github.com/azure/azure-dev/cli/azd/pkg/tools/azcli"
Expand Down Expand Up @@ -34,3 +35,49 @@ func GetCurrentPrincipalId(ctx context.Context, userProfile *azcli.UserProfileSe

return oid, nil
}

type LoggedInPrincipalProfileData struct {
PrincipalId string
PrincipalType string
PrincipalLoginName string
}

// LoggedInPrincipalProfile returns the info about the current logged in principal
func LoggedInPrincipalProfile(
vhvb1989 marked this conversation as resolved.
Show resolved Hide resolved
ctx context.Context, userProfile *azcli.UserProfileService, tenantId string) (*LoggedInPrincipalProfileData, error) {
principalProfile, err := userProfile.SignedProfile(ctx, tenantId)
if err == nil {
return &LoggedInPrincipalProfileData{
PrincipalId: principalProfile.Id,
PrincipalType: "User",
PrincipalLoginName: principalProfile.UserPrincipalName,
}, nil
}

token, err := userProfile.GetAccessToken(ctx, tenantId)
if err != nil {
return nil, fmt.Errorf("getting access token: %w", err)
}

tokenClaims, err := auth.GetClaimsFromAccessToken(token.AccessToken)
if err != nil {
return nil, fmt.Errorf("getting oid from token: %w", err)
}

appProfile, err := userProfile.AppProfile(ctx, tenantId)
if err == nil {
return &LoggedInPrincipalProfileData{
PrincipalId: *appProfile.AppId,
PrincipalType: "Application",
PrincipalLoginName: appProfile.DisplayName,
}, nil
} else {
log.Println(fmt.Errorf("fetching current user information: %w", err))
}

return &LoggedInPrincipalProfileData{
PrincipalId: tokenClaims.LocalAccountId(),
PrincipalType: "User",
PrincipalLoginName: tokenClaims.Email,
}, nil
}
16 changes: 16 additions & 0 deletions cli/azd/pkg/httputil/util.go
Original file line number Diff line number Diff line change
Expand Up @@ -119,3 +119,19 @@ func RetryAfter(resp *http.Response) time.Duration {

return 0
}

// GetIpAddress returns the public IP address of the caller.
func GetIpAddress() (string, error) {
resp, err := http.Get("https://api.ipify.org")
Copy link
Member

Choose a reason for hiding this comment

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

If we must add this, let's figure out a way to do it using an azure backed service (ideally one we don't run!). But this feels like the sort of thing that's going to be a footgun

if err != nil {
return "", err
}
defer resp.Body.Close()

data, err := io.ReadAll(resp.Body)
if err != nil {
return "", err
}

return string(data), nil
}
10 changes: 10 additions & 0 deletions cli/azd/pkg/httputil/util_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import (
"bytes"
"encoding/json"
"io"
"net"
"net/http"
"testing"

Expand Down Expand Up @@ -38,3 +39,12 @@ func TestReadRawResponse(t *testing.T) {
require.Equal(t, *expectedResponse, *actualResponse)
})
}

func TestGetIpAddress(t *testing.T) {
ip, err := GetIpAddress()

require.NoError(t, err)
require.NotEmpty(t, ip)
validIp := net.ParseIP(ip)
require.NotNil(t, validIp)
}
71 changes: 58 additions & 13 deletions cli/azd/pkg/infra/provisioning/bicep/bicep_provider.go
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@ import (
"github.com/azure/azure-dev/cli/azd/pkg/config"
"github.com/azure/azure-dev/cli/azd/pkg/convert"
"github.com/azure/azure-dev/cli/azd/pkg/environment"
"github.com/azure/azure-dev/cli/azd/pkg/httputil"
"github.com/azure/azure-dev/cli/azd/pkg/infra"
. "github.com/azure/azure-dev/cli/azd/pkg/infra/provisioning"
"github.com/azure/azure-dev/cli/azd/pkg/input"
Expand Down Expand Up @@ -1964,6 +1965,10 @@ func (p *BicepProvider) ensureParameters(
key string
param azure.ArmTemplateParameterDefinition
}
currentPrincipalProfile, err := p.curPrincipal.CurrentPrincipalProfile(ctx)
if err != nil {
return nil, fmt.Errorf("fetching current principal profile: %w", err)
}

for _, key := range sortedKeys {
param := template.Parameters[key]
Expand Down Expand Up @@ -2006,20 +2011,60 @@ func (p *BicepProvider) ensureParameters(
// If the parameter is tagged with {type: "generate"}, skip prompting.
// We generate it once, then save to config for next attempts.`.
azdMetadata, hasMetadata := param.AzdMetadata()
if hasMetadata && parameterType == ParameterTypeString && azdMetadata.Type != nil &&
*azdMetadata.Type == azure.AzdMetadataTypeGenerate {

// - generate once
genValue, err := autoGenerate(key, azdMetadata)
if err != nil {
return nil, err
}
configuredParameters[key] = azure.ArmParameterValue{
Value: genValue,
if hasMetadata && parameterType == ParameterTypeString && azdMetadata.Type != nil {
vhvb1989 marked this conversation as resolved.
Show resolved Hide resolved
azdMetadataType := *azdMetadata.Type
switch azdMetadataType {
case azure.AzdMetadataTypeGenerate:
// - generate once
genValue, err := autoGenerate(key, azdMetadata)
if err != nil {
return nil, err
}
configuredParameters[key] = azure.ArmParameterValue{
Value: genValue,
}
mustSetParamAsConfig(key, genValue, p.env.Config, param.Secure())
configModified = true
continue
// Check metadata for auto-inject values [principalId, principalType, principalLogin]
case azure.AzdMetadataTypePrincipalLogin:
pLogin := currentPrincipalProfile.PrincipalLoginName
configuredParameters[key] = azure.ArmParameterValue{
Value: pLogin,
}
mustSetParamAsConfig(key, pLogin, p.env.Config, param.Secure())
configModified = true
continue
case azure.AzdMetadataTypePrincipalId:
pLogin := currentPrincipalProfile.PrincipalId
configuredParameters[key] = azure.ArmParameterValue{
Value: pLogin,
}
mustSetParamAsConfig(key, pLogin, p.env.Config, param.Secure())
configModified = true
continue
case azure.AzdMetadataTypePrincipalType:
pLogin := currentPrincipalProfile.PrincipalType
configuredParameters[key] = azure.ArmParameterValue{
Value: pLogin,
}
mustSetParamAsConfig(key, pLogin, p.env.Config, param.Secure())
configModified = true
continue
case azure.AzdMetadataTypeIpAddress:
ipAddress, err := httputil.GetIpAddress()
if err != nil {
return nil, fmt.Errorf("getting IP address for bicep parameter: %w", err)
}
configuredParameters[key] = azure.ArmParameterValue{
Value: ipAddress,
}
// this metadata type is not saved to config as the IP can be dynamic.
continue
default:
// Do nothing
log.Println("Skipping actions for azd unknown metadata bicep parameter with type: ", azdMetadataType)
}
mustSetParamAsConfig(key, genValue, p.env.Config, param.Secure())
configModified = true
continue
}

// No saved value for this required parameter, we'll need to prompt for it.
Expand Down
7 changes: 7 additions & 0 deletions cli/azd/pkg/infra/provisioning/bicep/prompt_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import (
"github.com/Azure/azure-sdk-for-go/sdk/azcore/to"
"github.com/azure/azure-dev/cli/azd/pkg/account"
"github.com/azure/azure-dev/cli/azd/pkg/azure"
"github.com/azure/azure-dev/cli/azd/pkg/azureutil"
"github.com/azure/azure-dev/cli/azd/pkg/cloud"
"github.com/azure/azure-dev/cli/azd/pkg/environment"
"github.com/azure/azure-dev/cli/azd/pkg/input"
Expand Down Expand Up @@ -318,6 +319,12 @@ func TestPromptForParametersLocation(t *testing.T) {

type mockCurrentPrincipal struct{}

// CurrentPrincipalProfile implements provisioning.CurrentPrincipalIdProvider.
func (m *mockCurrentPrincipal) CurrentPrincipalProfile(
ctx context.Context) (*azureutil.LoggedInPrincipalProfileData, error) {
return &azureutil.LoggedInPrincipalProfileData{}, nil
}

func (m *mockCurrentPrincipal) CurrentPrincipalId(_ context.Context) (string, error) {
return "11111111-1111-1111-1111-111111111111", nil
}
15 changes: 15 additions & 0 deletions cli/azd/pkg/infra/provisioning/current_principal_id_provider.go
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ type CurrentPrincipalIdProvider interface {
// CurrentPrincipalId returns the object id of the current logged in principal, or an error if it can not be
// determined.
CurrentPrincipalId(ctx context.Context) (string, error)
CurrentPrincipalProfile(ctx context.Context) (*azureutil.LoggedInPrincipalProfileData, error)
}

func NewPrincipalIdProvider(
Expand Down Expand Up @@ -47,3 +48,17 @@ func (p *principalIDProvider) CurrentPrincipalId(ctx context.Context) (string, e

return principalId, nil
}

func (p *principalIDProvider) CurrentPrincipalProfile(ctx context.Context) (*azureutil.LoggedInPrincipalProfileData, error) {
tenantId, err := p.subResolver.LookupTenant(ctx, p.env.GetSubscriptionId())
if err != nil {
return nil, fmt.Errorf("getting tenant id for subscription %s. Error: %w", p.env.GetSubscriptionId(), err)
}

principalProfile, err := azureutil.LoggedInPrincipalProfile(ctx, p.userProfileService, tenantId)
if err != nil {
return nil, fmt.Errorf("fetching current user information: %w", err)
}

return principalProfile, nil
}
Loading
Loading