Skip to content

Commit

Permalink
WIP: Support OIDC in Azure Pipelines
Browse files Browse the repository at this point in the history
This change teaches `azd` how to login using a service connection for
an OIDC like experience when running in Azure Pipelines using service
connections and then updates our pipelines to use this authentication
strategy.

Contributes To Azure#4341
  • Loading branch information
ellismg committed Sep 18, 2024
1 parent 6992411 commit d340e85
Show file tree
Hide file tree
Showing 10 changed files with 173 additions and 86 deletions.
3 changes: 3 additions & 0 deletions cli/azd/.vscode/cspell.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,9 @@ dictionaryDefinitions:
dictionaries:
- azdProjectDictionary
overrides:
- filename: cmd/auth_login.go
words:
- ACCESSTOKEN
- filename: internal/tracing/fields/domains.go
words:
- azmk
Expand Down
43 changes: 31 additions & 12 deletions cli/azd/cmd/auth_login.go
Original file line number Diff line number Diff line change
Expand Up @@ -46,18 +46,20 @@ func newAuthLoginFlags(cmd *cobra.Command, global *internal.GlobalCommandOptions
}

type loginFlags struct {
onlyCheckStatus bool
browser bool
managedIdentity bool
useDeviceCode boolPtr
tenantID string
clientID string
clientSecret stringPtr
clientCertificate string
federatedTokenProvider string
scopes []string
redirectPort int
global *internal.GlobalCommandOptions
onlyCheckStatus bool
browser bool
managedIdentity bool
useDeviceCode boolPtr
tenantID string
clientID string
clientSecret stringPtr
clientCertificate string
serviceConnectionID string
systemAccessTokenEnvVar string
federatedTokenProvider string
scopes []string
redirectPort int
global *internal.GlobalCommandOptions
}

// stringPtr implements a pflag.Value and allows us to distinguish between a flag value being explicitly set to the empty
Expand Down Expand Up @@ -139,6 +141,16 @@ func (lf *loginFlags) Bind(local *pflag.FlagSet, global *internal.GlobalCommandO
cClientCertificateFlagName,
"",
"The path to the client certificate for the service principal to authenticate with.")
local.StringVar(
&lf.serviceConnectionID,
"service-connection-id",
"",
"The service connection id to authenticate with.")
local.StringVar(
&lf.systemAccessTokenEnvVar,
"system-access-token-environment-variable-name",
"SYSTEM_ACCESSTOKEN",
"The name of the environment variable that contains the system access token to authenticate with.")
local.StringVar(
&lf.federatedTokenProvider,
cFederatedCredentialProviderFlagName,
Expand Down Expand Up @@ -407,6 +419,7 @@ func (la *loginAction) login(ctx context.Context) error {
if countTrue(
la.flags.clientSecret.ptr != nil,
la.flags.clientCertificate != "",
la.flags.serviceConnectionID != "",
la.flags.federatedTokenProvider != "",
) != 1 {
return fmt.Errorf(
Expand Down Expand Up @@ -457,6 +470,12 @@ func (la *loginAction) login(ctx context.Context) error {
); err != nil {
return fmt.Errorf("logging in: %w", err)
}
case la.flags.serviceConnectionID != "":
if _, err := la.authManager.LoginWithServiceConnection(
ctx, la.flags.tenantID, la.flags.clientID, la.flags.serviceConnectionID, la.flags.systemAccessTokenEnvVar,
); err != nil {
return fmt.Errorf("logging in: %w", err)
}
}

return nil
Expand Down
24 changes: 13 additions & 11 deletions cli/azd/cmd/testdata/TestUsage-azd-auth-login.snap
Original file line number Diff line number Diff line change
Expand Up @@ -5,17 +5,19 @@ Usage
azd auth login [flags]

Flags
--check-status : Checks the log-in status instead of logging in.
--client-certificate string : The path to the client certificate for the service principal to authenticate with.
--client-id string : The client id for the service principal to authenticate with.
--client-secret string : The client secret for the service principal to authenticate with. Set to the empty string to read the value from the console.
--docs : Opens the documentation for azd auth login in your web browser.
--federated-credential-provider string : The provider to use to acquire a federated token to authenticate with.
-h, --help : Gets help for login.
--managed-identity : Use a managed identity to authenticate.
--redirect-port int : Choose the port to be used as part of the redirect URI during interactive login.
--tenant-id string : The tenant id or domain name to authenticate with.
--use-device-code : When true, log in by using a device code instead of a browser.
--check-status : Checks the log-in status instead of logging in.
--client-certificate string : The path to the client certificate for the service principal to authenticate with.
--client-id string : The client id for the service principal to authenticate with.
--client-secret string : The client secret for the service principal to authenticate with. Set to the empty string to read the value from the console.
--docs : Opens the documentation for azd auth login in your web browser.
--federated-credential-provider string : The provider to use to acquire a federated token to authenticate with.
-h, --help : Gets help for login.
--managed-identity : Use a managed identity to authenticate.
--redirect-port int : Choose the port to be used as part of the redirect URI during interactive login.
--service-connection-id string : The service connection id to authenticate with.
--system-access-token-environment-variable-name string : The name of the environment variable that contains the system access token to authenticate with.
--tenant-id string : The tenant id or domain name to authenticate with.
--use-device-code : When true, log in by using a device code instead of a browser.

Global Flags
-C, --cwd string : Sets the current working directory.
Expand Down
104 changes: 94 additions & 10 deletions cli/azd/pkg/auth/manager.go
Original file line number Diff line number Diff line change
Expand Up @@ -312,11 +312,6 @@ func (m *Manager) CredentialForCurrentUser(
}
return m.newCredentialFromManagedIdentity(clientID)
} else if currentUser.TenantID != nil && currentUser.ClientID != nil {
ps, err := m.loadSecret(*currentUser.TenantID, *currentUser.ClientID)
if err != nil {
return nil, fmt.Errorf("loading secret: %w: %w", err, ErrNoCurrentUser)
}

// by default we used the stored tenant (i.e. the one provided with the tenant id parameter when a user ran
// `azd auth login`), but we allow an override using the options bag, when
// TenantID is non-empty and PreferFallbackTenant is not true.
Expand All @@ -326,6 +321,16 @@ func (m *Manager) CredentialForCurrentUser(
tenantID = options.TenantID
}

if currentUser.ServiceConnectionID != nil && currentUser.SystemAccessTokenName != nil {
return m.newCredentialFromServiceConnection(
tenantID, *currentUser.ClientID, *currentUser.ServiceConnectionID, *currentUser.SystemAccessTokenName)
}

ps, err := m.loadSecret(*currentUser.TenantID, *currentUser.ClientID)
if err != nil {
return nil, fmt.Errorf("loading secret: %w: %w", err, ErrNoCurrentUser)
}

if ps.ClientSecret != nil {
return m.newCredentialFromClientSecret(tenantID, *currentUser.ClientID, *ps.ClientSecret)
} else if ps.ClientCertificate != nil {
Expand Down Expand Up @@ -481,6 +486,36 @@ func (m *Manager) newCredentialFromClientSecret(
return cred, nil
}

func (m *Manager) newCredentialFromServiceConnection(
tenantID string,
clientID string,
serviceConnectionID string,
systemAccessTokenEnvVar string,
) (azcore.TokenCredential, error) {
options := &azidentity.AzurePipelinesCredentialOptions{
ClientOptions: azcore.ClientOptions{
Transport: m.httpClient,
// TODO: Inject client options instead? this can be done if we're OK
// using the default user agent string.
Cloud: m.cloud.Configuration,
},
}

systemAccessToken := os.Getenv(systemAccessTokenEnvVar)
if systemAccessToken == "" {
// nolint:lll
return nil, fmt.Errorf("system access token not found, ensure the System.AccessToken value is mapped to an environment variable named %s", systemAccessTokenEnvVar)
}

cred, err := azidentity.NewAzurePipelinesCredential(
tenantID, clientID, serviceConnectionID, systemAccessToken, options)
if err != nil {
return nil, fmt.Errorf("creating credential: %w: %w", err, ErrNoCurrentUser)
}

return cred, nil
}

func (m *Manager) newCredentialFromClientCertificate(
tenantID string,
clientID string,
Expand Down Expand Up @@ -688,6 +723,37 @@ func (m *Manager) LoginWithDeviceCode(

}

func (m *Manager) LoginWithServiceConnection(
ctx context.Context, tenantID string, clientID string, serviceConnectionID string, systemAccessTokenEnvVar string,
) (azcore.TokenCredential, error) {
systemAccessToken := os.Getenv(systemAccessTokenEnvVar)

if systemAccessToken == "" {
// nolint:lll
return nil, fmt.Errorf("system access token not found, ensure the System.AccessToken value is mapped to an environment variable named %s", systemAccessTokenEnvVar)
}

options := &azidentity.AzurePipelinesCredentialOptions{
ClientOptions: azcore.ClientOptions{
Transport: m.httpClient,
// TODO: Inject client options instead? this can be done if we're OK
// using the default user agent string.
Cloud: m.cloud.Configuration,
},
}

cred, err := azidentity.NewAzurePipelinesCredential(tenantID, clientID, serviceConnectionID, systemAccessToken, options)
if err != nil {
return nil, fmt.Errorf("creating credential: %w", err)
}

if err := m.saveLoginForServiceConnection(tenantID, clientID, serviceConnectionID, systemAccessTokenEnvVar); err != nil {
return nil, err
}

return cred, nil
}

func (m *Manager) LoginWithManagedIdentity(ctx context.Context, clientID string) (azcore.TokenCredential, error) {
options := &azidentity.ManagedIdentityCredentialOptions{}
if clientID != "" {
Expand Down Expand Up @@ -848,6 +914,22 @@ func (m *Manager) saveLoginForManagedIdentity(clientID string) error {
return nil
}

func (m *Manager) saveLoginForServiceConnection(
tenantID, clientID, serviceConnectionID, systemAccessTokenEnvVar string,
) error {
props := &userProperties{
ClientID: &clientID,
TenantID: &tenantID,
ServiceConnectionID: &serviceConnectionID,
SystemAccessTokenName: &systemAccessTokenEnvVar,
}
if err := m.saveUserProperties(props); err != nil {
return err
}

return nil
}

func (m *Manager) saveLoginForServicePrincipal(tenantId, clientId string, secret *persistedSecret) error {
if err := m.saveSecret(tenantId, clientId, secret); err != nil {
return err
Expand Down Expand Up @@ -1033,11 +1115,13 @@ type federatedAuth struct {
// either an home account id (when logging in using a public client) or a client and tenant id (when using a confidential
// client).
type userProperties struct {
ManagedIdentity bool `json:"managedIdentity,omitempty"`
HomeAccountID *string `json:"homeAccountId,omitempty"`
FromOneAuth bool `json:"fromOneAuth,omitempty"`
ClientID *string `json:"clientId,omitempty"`
TenantID *string `json:"tenantId,omitempty"`
ManagedIdentity bool `json:"managedIdentity,omitempty"`
HomeAccountID *string `json:"homeAccountId,omitempty"`
FromOneAuth bool `json:"fromOneAuth,omitempty"`
ClientID *string `json:"clientId,omitempty"`
TenantID *string `json:"tenantId,omitempty"`
ServiceConnectionID *string `json:"serviceConnectionId,omitempty"`
SystemAccessTokenName *string `json:"systemAccessTokenName,omitempty"`
}

func readUserProperties(cfg config.Config) (*userProperties, error) {
Expand Down
2 changes: 0 additions & 2 deletions cli/azd/test/functional/deployment_stacks_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,8 +18,6 @@ import (
)

func Test_DeploymentStacks(t *testing.T) {
t.Skip("azure/azure-dev#4341")

t.Run("Subscription_Scope_Up_Down", func(t *testing.T) {
t.Parallel()
ctx, cancel := newTestContext(t)
Expand Down
41 changes: 0 additions & 41 deletions cli/azd/test/functional/login_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -53,47 +53,6 @@ func Test_CLI_AuthLoginStatus(t *testing.T) {
}
}

func Test_CLI_LoginServicePrincipal(t *testing.T) {
t.Skip("azure/azure-dev#4341")

ctx, cancel := newTestContext(t)
defer cancel()

dir := t.TempDir()

cli := azdcli.NewCLI(t)
// Isolate login to a separate configuration directory
cli.Env = append(cli.Env, "AZD_CONFIG_DIR="+dir)
if cfg.ClientID == "" || cfg.TenantID == "" /* || cfg.ClientSecret == "" */ {
if cfg.CI {
panic("Service principal is not configured. AZD_TEST_* variables are required to be set for live testing.")
}

t.Skip("Skipping test because service principal is not configured. " +
"Set the relevant AZD_TEST_* variables to run this test.")
return
}

loginState := loginStatus(t, ctx, cli)
require.Equal(t, contracts.LoginStatusUnauthenticated, loginState.Status)

_, err := cli.RunCommand(ctx,
"auth", "login",
"--client-id", cfg.ClientID,
// "--client-secret", cfg.ClientSecret,
"--tenant-id", cfg.TenantID)
require.NoError(t, err)

loginState = loginStatus(t, ctx, cli)
require.Equal(t, contracts.LoginStatusSuccess, loginState.Status)

_, err = cli.RunCommand(ctx, "auth", "logout")
require.NoError(t, err)

loginState = loginStatus(t, ctx, cli)
require.Equal(t, contracts.LoginStatusUnauthenticated, loginState.Status)
}

func loginStatus(t *testing.T, ctx context.Context, cli *azdcli.CLI) contracts.LoginResult {
result, err := cli.RunCommand(ctx, "auth", "login", "--check-status", "--output", "json")
require.NoError(t, err)
Expand Down
1 change: 1 addition & 0 deletions eng/pipelines/templates/jobs/build-cli.yml
Original file line number Diff line number Diff line change
Expand Up @@ -126,6 +126,7 @@ jobs:
ARM_TENANT_ID: $(arm-tenant-id)
# Code Coverage: Generate junit report to publish results
GOTESTSUM_JUNITFILE: junitTestReport.xml
SYSTEM_ACCESSTOKEN: $(System.AccessToken)

- task: PublishTestResults@2
inputs:
Expand Down
9 changes: 7 additions & 2 deletions eng/pipelines/templates/steps/azd-login.yml
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
parameters:
SubscriptionConfiguration: $(sub-config-azure-cloud-test-resources)
AzdDirectory: ""
ServiceConnectionId: "3d79cc98-46f2-428c-bdd5-861414f85602"

steps:
- pwsh: |
Expand All @@ -14,8 +15,10 @@ steps:
${{ parameters.SubscriptionConfiguration }}
'@ | ConvertFrom-Json -AsHashtable;
# Delegate auth to az CLI which supports federated auth in AzDo
& $azdCmd config set auth.useAzCliAuth true
& $azdCmd login `
--client-id "$($subscriptionConfiguration.TestApplicationId)" `
--tenant-id "$($subscriptionConfiguration.TenantId)" `
--service-connection-id "${{ parameters.ServiceConnectionId }}"
if ($LASTEXITCODE -ne 0) { exit $LASTEXITCODE }
Expand All @@ -32,3 +35,5 @@ steps:
condition: and(succeeded(), ne(variables['Skip.LiveTest'], 'true'))
displayName: Azure Dev Login
env:
SYSTEM_ACCESSTOKEN: $(System.AccessToken)
16 changes: 8 additions & 8 deletions go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -4,8 +4,8 @@ go 1.23

require (
github.com/AlecAivazis/survey/v2 v2.3.2
github.com/Azure/azure-sdk-for-go/sdk/azcore v1.12.0
github.com/Azure/azure-sdk-for-go/sdk/azidentity v1.6.0
github.com/Azure/azure-sdk-for-go/sdk/azcore v1.14.0
github.com/Azure/azure-sdk-for-go/sdk/azidentity v1.7.0
github.com/Azure/azure-sdk-for-go/sdk/resourcemanager/apimanagement/armapimanagement v1.0.0
github.com/Azure/azure-sdk-for-go/sdk/resourcemanager/appconfiguration/armappconfiguration v1.0.0
github.com/Azure/azure-sdk-for-go/sdk/resourcemanager/appcontainers/armappcontainers/v3 v3.0.0-beta.1
Expand Down Expand Up @@ -67,14 +67,14 @@ require (
go.opentelemetry.io/otel/trace v1.8.0
go.uber.org/atomic v1.9.0
go.uber.org/multierr v1.8.0
golang.org/x/sys v0.21.0
golang.org/x/sys v0.25.0
gopkg.in/dnaeon/go-vcr.v3 v3.1.2
gopkg.in/yaml.v3 v3.0.1
)

require (
github.com/Azure/azure-pipeline-go v0.2.1 // indirect
github.com/Azure/azure-sdk-for-go/sdk/internal v1.9.0 // indirect
github.com/Azure/azure-sdk-for-go/sdk/internal v1.10.0 // indirect
github.com/Azure/azure-sdk-for-go/sdk/security/keyvault/internal v0.8.0 // indirect
github.com/cenkalti/backoff/v4 v4.1.3 // indirect
github.com/davecgh/go-spew v1.1.1 // indirect
Expand All @@ -99,10 +99,10 @@ require (
go.opentelemetry.io/otel/exporters/otlp/internal/retry v1.8.0 // indirect
go.opentelemetry.io/otel/exporters/otlp/otlptrace v1.8.0 // indirect
go.opentelemetry.io/proto/otlp v0.18.0 // indirect
golang.org/x/crypto v0.24.0 // indirect
golang.org/x/net v0.26.0 // indirect
golang.org/x/term v0.21.0 // indirect
golang.org/x/text v0.16.0 // indirect
golang.org/x/crypto v0.27.0 // indirect
golang.org/x/net v0.29.0 // indirect
golang.org/x/term v0.24.0 // indirect
golang.org/x/text v0.18.0 // indirect
google.golang.org/genproto v0.0.0-20230410155749-daa745c078e1 // indirect
google.golang.org/grpc v1.56.3 // indirect
google.golang.org/protobuf v1.33.0 // indirect
Expand Down
Loading

0 comments on commit d340e85

Please sign in to comment.