-
Notifications
You must be signed in to change notification settings - Fork 219
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
PS and CLI creds #2433
PS and CLI creds #2433
Changes from 15 commits
42f3586
01a7b8b
3886323
247965f
2936265
6918f38
1282215
33782e6
94a7d8c
bcf9666
e104dbf
6b2ba1b
e63d018
6e0de65
4f22da3
f9b5224
217756e
bad7d44
9f4f7b3
6aca9d6
5244167
1c1390f
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -134,6 +134,7 @@ jobs: | |
imageName: 'windows-2019' | ||
build_name: 'azcopy_windows_amd64.exe' | ||
display_name: "Windows" | ||
type: 'windows' | ||
go_path: 'C:\Users\VssAdministrator\go\bin\' | ||
suffix: '.exe' | ||
run_e2e: 'go test -timeout=1h -v ./e2etest > test.txt' | ||
|
@@ -152,6 +153,11 @@ jobs: | |
vmImage: $(imageName) | ||
|
||
steps: | ||
- task: PowerShell@2 | ||
condition: eq(variables.type, 'windows' ) | ||
inputs: | ||
targetType: 'inline' | ||
script: 'Install-Module -Name Az -Scope CurrentUser -Repository PSGallery -AllowClobber' | ||
- task: GoTool@0 | ||
inputs: | ||
version: $(AZCOPY_GOLANG_VERSION_COVERAGE) | ||
|
@@ -161,6 +167,11 @@ jobs: | |
go install github.com/AlekSi/[email protected] | ||
go install github.com/matm/[email protected] | ||
displayName: 'Installing dependencies' | ||
|
||
- script: | | ||
'powershell Install-Module -Name Az -Scope CurrentUser -Repository PSGallery -AllowClobber' | ||
displayName: 'Installing Azure Powershell Module' | ||
condition: eq(variables.type, 'windows') | ||
|
||
# Running E2E Tests on AMD64 | ||
- script: | | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -32,7 +32,7 @@ import ( | |
var loginCmdArg = loginCmdArgs{tenantID: common.DefaultTenantID} | ||
|
||
var loginNotice = "'azcopy %s' command will be deprecated starting release 10.22. " + | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. You could probably update this to 10.23 since we are skipping this release, right? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'll add that as a different review. Deviates from topic. |
||
"Use auto-login instead. Visit %s to know more." | ||
"Use auto-login instead. Visit %s to know more." | ||
var autoLoginURL = "https://learn.microsoft.com/en-us/azure/storage/common/storage-use-azcopy-authorize-azure-active-directory#authorize-without-a-secret-store " | ||
|
||
var lgCmd = &cobra.Command{ | ||
|
@@ -85,7 +85,7 @@ func init() { | |
lgCmd.PersistentFlags().StringVar(&loginCmdArg.certPath, "certificate-path", "", "Path to certificate for SPN authentication. Required for certificate-based service principal auth.") | ||
|
||
// Deprecate the identity-object-id flag | ||
_ = lgCmd.PersistentFlags().MarkHidden("identity-object-id") // Object ID of user-assigned identity. | ||
_ = lgCmd.PersistentFlags().MarkHidden("identity-object-id") // Object ID of user-assigned identity. | ||
lgCmd.PersistentFlags().StringVar(&loginCmdArg.identityObjectID, "identity-object-id", "", "Object ID of user-assigned identity. This parameter is deprecated. Please use client id or resource id") | ||
|
||
} | ||
|
@@ -97,6 +97,8 @@ type loginCmdArgs struct { | |
|
||
identity bool // Whether to use MSI. | ||
nakulkar-msft marked this conversation as resolved.
Show resolved
Hide resolved
|
||
servicePrincipal bool | ||
azCliCred bool | ||
psCred bool | ||
|
||
// Info of VM's user assigned identity, client or object ids of the service identity are required if | ||
// your VM has multiple user-assigned managed identities. | ||
|
@@ -191,6 +193,16 @@ func (lca loginCmdArgs) process() error { | |
} | ||
// For MSI login, info success message to user. | ||
glcm.Info("Login with identity succeeded.") | ||
case lca.azCliCred: | ||
gapra-msft marked this conversation as resolved.
Show resolved
Hide resolved
|
||
if err := uotm.AzCliLogin(lca.tenantID); err != nil { | ||
return err | ||
} | ||
glcm.Info("Login with AzCliCreds succeeded") | ||
case lca.psCred: | ||
if err := uotm.PSContextToken(lca.tenantID); err != nil { | ||
return err | ||
} | ||
glcm.Info("Login with Powershell context succeeded") | ||
default: | ||
if err := uotm.UserLogin(lca.tenantID, lca.aadEndpoint, lca.persistToken); err != nil { | ||
return err | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,183 @@ | ||
//go:build go1.18 | ||
// +build go1.18 | ||
|
||
// Copyright (c) Microsoft Corporation. All rights reserved. | ||
// Licensed under the MIT License. | ||
|
||
package common | ||
|
||
import ( | ||
"bytes" | ||
"context" | ||
"encoding/json" | ||
"errors" | ||
"fmt" | ||
"os" | ||
"os/exec" | ||
"regexp" | ||
"strconv" | ||
"strings" | ||
"sync" | ||
"time" | ||
|
||
"github.com/Azure/azure-sdk-for-go/sdk/azcore" | ||
"github.com/Azure/azure-sdk-for-go/sdk/azcore/policy" | ||
) | ||
|
||
const credNamePSContext = "PSContextCredential" | ||
|
||
type PSTokenProvider func(ctx context.Context, resource string, tenant string) ([]byte, error) | ||
func validTenantID(tenantID string) bool { | ||
match, err := regexp.MatchString("^[0-9a-zA-Z-.]+$", tenantID) | ||
if err != nil { | ||
return false | ||
} | ||
return match | ||
} | ||
|
||
func resolveTenant(defaultTenant, specified, credName string, additionalTenants []string) (string, error) { | ||
if specified == "" || specified == defaultTenant { | ||
return defaultTenant, nil | ||
} | ||
if defaultTenant == "adfs" { | ||
return "", errors.New("ADFS doesn't support tenants") | ||
} | ||
if !validTenantID(specified) { | ||
return "", errors.New("Invalid tenant") | ||
} | ||
for _, t := range additionalTenants { | ||
if t == "*" || t == specified { | ||
return specified, nil | ||
} | ||
} | ||
return "", fmt.Errorf(`%s isn't configured to acquire tokens for tenant %q. To enable acquiring tokens for this tenant add it to the AdditionallyAllowedTenants on the credential options, or add "*" to allow acquiring tokens for any tenant`, credName, specified) | ||
} | ||
// PowershellContextCredentialOptions contains optional parameters for AzureDeveloperCLICredential. | ||
type PowershellContextCredentialOptions struct { | ||
// TenantID identifies the tenant the credential should authenticate in. Defaults to the azd environment, | ||
// which is the tenant of the selected Azure subscription. | ||
TenantID string | ||
|
||
tokenProvider PSTokenProvider | ||
} | ||
|
||
// PowershellContextCredential authenticates as the identity logged in to the [Azure Developer CLI]. | ||
// | ||
// [Azure Developer CLI]: https://learn.microsoft.com/azure/developer/azure-developer-cli/overview | ||
type PowershellContextCredential struct { | ||
mu *sync.Mutex | ||
opts PowershellContextCredentialOptions | ||
} | ||
|
||
func NewPowershellContextCredential(options *PowershellContextCredentialOptions) (*PowershellContextCredential, error) { | ||
cp := PowershellContextCredentialOptions{} | ||
if options != nil { | ||
cp = *options | ||
} | ||
if cp.TenantID != "" && !validTenantID(cp.TenantID) { | ||
return nil, errors.New("invalid tenant id") | ||
} | ||
if cp.tokenProvider == nil { | ||
cp.tokenProvider = defaultAzdTokenProvider | ||
} | ||
return &PowershellContextCredential{mu: &sync.Mutex{}, opts: cp}, nil | ||
} | ||
|
||
// GetToken requests a token from the Azure Developer CLI. This credential doesn't cache tokens, so every call invokes azd. | ||
// This method is called automatically by Azure SDK clients. | ||
func (c *PowershellContextCredential) GetToken(ctx context.Context, opts policy.TokenRequestOptions) (azcore.AccessToken, error) { | ||
at := azcore.AccessToken{} | ||
if len(opts.Scopes) != 1 { | ||
return at, errors.New(credNamePSContext + ": GetToken() exactly one scope") | ||
} | ||
|
||
tenant, err := resolveTenant(c.opts.TenantID, opts.TenantID, credNamePSContext, nil) | ||
if err != nil { | ||
return at, err | ||
} | ||
c.mu.Lock() | ||
defer c.mu.Unlock() | ||
b, err := c.opts.tokenProvider(ctx, opts.Scopes[0], tenant) | ||
if err == nil { | ||
at, err = c.createAccessToken(b) | ||
} | ||
if err != nil { | ||
return at, err | ||
} | ||
//msg := fmt.Sprintf("%s.GetToken() acquired a token for scope %q", credNamePSContext, strings.Join(opts.Scopes, ", ")) | ||
return at, nil | ||
} | ||
|
||
// We ignore resource because PS does not support all Resources. Disk scope is not supported | ||
// and we are here only with Storage scope | ||
var defaultAzdTokenProvider PSTokenProvider = func(ctx context.Context, _ string, tenantID string) ([]byte, error) { | ||
// set a default timeout for this authentication iff the application hasn't done so already | ||
var cancel context.CancelFunc | ||
if _, hasDeadline := ctx.Deadline(); !hasDeadline { | ||
ctx, cancel = context.WithTimeout(ctx, 10 * time.Minute) | ||
defer cancel() | ||
} | ||
|
||
const StorageResourceName = "https://storage.azure.com" | ||
commandLine := "Get-AzAccessToken -ResourceUrl " + StorageResourceName + " | ConvertTo-Json" | ||
if tenantID != "" { | ||
commandLine += " -TenantId " + tenantID | ||
} | ||
cliCmd := exec.CommandContext(ctx, "powershell", commandLine) | ||
cliCmd.Env = os.Environ() | ||
var stderr bytes.Buffer | ||
cliCmd.Stderr = &stderr | ||
|
||
output, err := cliCmd.Output() | ||
if err != nil { | ||
msg := stderr.String() | ||
var exErr *exec.ExitError | ||
if errors.As(err, &exErr) && exErr.ExitCode() == 127 || strings.HasPrefix(msg, "Not found") { | ||
msg = "Powershell path not found" | ||
} | ||
if msg == "" { | ||
msg = err.Error() | ||
} | ||
return nil, errors.New(credNamePSContext + msg) | ||
} | ||
|
||
return output, nil | ||
} | ||
|
||
func (c *PowershellContextCredential) createAccessToken(tk []byte) (azcore.AccessToken, error) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. can we add UT for some of these methods? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Did you add tests for some of these methods? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The test covers entire workflow. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Which test? I only see some changes to runner, how does that get picked up in CI? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. KMN. Thanks for getting this. I had not checked in the file. I've checked in now, need to get some environment vars correct in CI. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. great! thanks for double checking that. I thought I was going crazy for a minute |
||
t := struct { | ||
AccessToken string `json:"token"` | ||
ExpiresOn string `json:"expiresOn"` | ||
}{} | ||
|
||
err := json.Unmarshal(tk, &t) | ||
if err != nil { | ||
return azcore.AccessToken{}, err | ||
} | ||
|
||
parseErr := "error parsing token expiration time %q: %v" | ||
exp, err := time.Parse("2006-01-02T15:04:05Z", t.ExpiresOn) | ||
if err != nil { | ||
// In some environments time is a unix stamp of format 'Date(<unixtime>)' | ||
rgx := regexp.MustCompile(`\((.*?)\)`) | ||
if rgx.Match([]byte(t.ExpiresOn)) { | ||
rs := rgx.FindStringSubmatch(t.ExpiresOn) | ||
expTime, err := strconv.ParseInt(rs[1], 10, 64) | ||
if err != nil { | ||
return azcore.AccessToken{}, fmt.Errorf(parseErr, t.ExpiresOn, err) | ||
} | ||
exp = time.Unix(expTime, 0) | ||
} else { | ||
return azcore.AccessToken{}, fmt.Errorf(parseErr, t.ExpiresOn, err) | ||
} | ||
} | ||
return azcore.AccessToken{ | ||
ExpiresOn: exp.UTC(), | ||
Token: t.AccessToken, | ||
}, nil | ||
} | ||
|
||
var _ azcore.TokenCredential = (*PowershellContextCredential)(nil) | ||
|
||
|
||
// NewPowershellContextCredential constructs an AzureDeveloperCLICredential. Pass nil to accept default options. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
in the future we might be able to improve this by creating an enum for oauth cred type instead of having a bunch of booleans