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

Use AzCLI instead of client secret creds #2906

Open
wants to merge 11 commits into
base: main
Choose a base branch
from
+11 −16
Open

Use AzCLI instead of client secret creds #2906

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
11 commits
Select commit Hold shift + click to select a range
3d83cf8
migrate tests from client secrete to azcli
wonwuakpa-msft Jan 6, 2025
731e78c
removed env var
wonwuakpa-msft Jan 7, 2025
a095b2b
Merge branch 'main' into wendi/azcli-auth
wonwuakpa-msft Jan 7, 2025
4f7b149
removed req
wonwuakpa-msft Jan 7, 2025
6d8589d
Merge branch 'main' of https://github.com/Azure/azure-storage-azcopy …
wonwuakpa-msft Jan 7, 2025
9d2d1cc
Merge branch 'wendi/azcli-auth' of https://github.com/Azure/azure-sto…
wonwuakpa-msft Jan 7, 2025
6dfa2ad
update
wonwuakpa-msft Jan 8, 2025
879ba60
static/ dynamic cases
wonwuakpa-msft Jan 8, 2025
b471059
removed commit from other branch
wonwuakpa-msft Jan 14, 2025
17048a6
Merge branch 'main' into wendi/azcli-auth
wonwuakpa-msft Jan 14, 2025
632ec0d
Merge branch 'main' into wendi/azcli-auth
wonwuakpa-msft Jan 16, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 2 additions & 4 deletions e2etest/newe2e_config.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -50,10 +50,8 @@ type NewE2EConfig struct {
} `env:",required"`

StaticStgAcctInfo struct {
StaticOAuth struct {
Copy link
Member

Choose a reason for hiding this comment

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

nit: this could be done as an either/or setup

weigh this with a grain of salt, most of the team will probably do inherited auth

Copy link
Member

Choose a reason for hiding this comment

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

on a similar note, could it be set up to allow AzCLI or AzPwsh inheritance? Some people on the team use one or the other, not usually both

TenantID string `env:"NEW_E2E_STATIC_TENANT_ID"`
ApplicationID string `env:"NEW_E2E_STATIC_APPLICATION_ID,required"`
ClientSecret string `env:"NEW_E2E_STATIC_CLIENT_SECRET,required"`
StaticOAuth struct { //defaults to AzCLI auth
TenantID string `env:"NEW_E2E_STATIC_TENANT_ID"`
}

// todo: should we automate this somehow? Currently each of these accounts needs some marginal boilerplate.
Expand Down
15 changes: 8 additions & 7 deletions e2etest/newe2e_oauth_cache.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@ import (
"github.com/Azure/azure-sdk-for-go/sdk/azcore"
"github.com/Azure/azure-sdk-for-go/sdk/azcore/policy"
"github.com/Azure/azure-sdk-for-go/sdk/azidentity"
"github.com/Azure/azure-storage-azcopy/v10/common"
"sync"
"time"
)
Expand Down Expand Up @@ -36,14 +35,16 @@ func SetupOAuthCache(a Asserter) {
cred, err = azidentity.NewDefaultAzureCredential(&azidentity.DefaultAzureCredentialOptions{
TenantID: tenantId,
})
} else if useStatic { // default to AzCLI for static accts
cred, err = azidentity.NewAzureCLICredential(&azidentity.AzureCLICredentialOptions{
wonwuakpa-msft marked this conversation as resolved.
Show resolved Hide resolved
TenantID: staticLoginInfo.TenantID,
})
} else {
tenantId = common.Iff(useStatic, staticLoginInfo.TenantID, dynamicLoginInfo.DynamicOAuth.SPNSecret.TenantID)
cred, err = azidentity.NewClientSecretCredential(
wonwuakpa-msft marked this conversation as resolved.
Show resolved Hide resolved
tenantId,
common.Iff(useStatic, staticLoginInfo.ApplicationID, dynamicLoginInfo.DynamicOAuth.SPNSecret.ApplicationID),
common.Iff(useStatic, staticLoginInfo.ClientSecret, dynamicLoginInfo.DynamicOAuth.SPNSecret.ClientSecret),
nil, // Hopefully the defaults should be OK?
)
dynamicLoginInfo.DynamicOAuth.SPNSecret.TenantID,
dynamicLoginInfo.DynamicOAuth.SPNSecret.ApplicationID,
dynamicLoginInfo.DynamicOAuth.SPNSecret.ClientSecret,
nil)
}
a.NoError("create credentials", err)

Expand Down
6 changes: 1 addition & 5 deletions e2etest/newe2e_task_runazcopy.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -193,12 +193,8 @@ func (c *AzCopyCommand) applyTargetAuth(a Asserter, target ResourceManager) stri

if c.Environment.AutoLoginMode == nil && c.Environment.ServicePrincipalAppID == nil && c.Environment.ServicePrincipalClientSecret == nil && c.Environment.AutoLoginTenantID == nil {
if GlobalConfig.StaticResources() {
c.Environment.AutoLoginMode = pointerTo("SPN")
c.Environment.AutoLoginMode = pointerTo(common.EAutoLoginType.AzCLI().String())
oAuthInfo := GlobalConfig.E2EAuthConfig.StaticStgAcctInfo.StaticOAuth
a.AssertNow("At least NEW_E2E_STATIC_APPLICATION_ID and NEW_E2E_STATIC_CLIENT_SECRET must be specified to use OAuth.", Empty{true}, oAuthInfo.ApplicationID, oAuthInfo.ClientSecret)

c.Environment.ServicePrincipalAppID = &oAuthInfo.ApplicationID
c.Environment.ServicePrincipalClientSecret = &oAuthInfo.ClientSecret
c.Environment.AutoLoginTenantID = common.Iff(oAuthInfo.TenantID != "", &oAuthInfo.TenantID, nil)
} else {
// oauth should reliably work
Expand Down
Loading