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 #4341
  • Loading branch information
ellismg committed Sep 18, 2024
1 parent 6992411 commit f22a1ca
Show file tree
Hide file tree
Showing 11 changed files with 186 additions and 89 deletions.
6 changes: 6 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 Expand Up @@ -110,6 +113,9 @@ overrides:
- filename: test/functional/testdata/samples/funcapp/getting_started.md
words:
- funcignore
- filename: test/recording/recording.go
words:
- oidctoken
- filename: internal/vsrpc/handler.go
words:
- arity
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
4 changes: 3 additions & 1 deletion cli/azd/test/recording/recording.go
Original file line number Diff line number Diff line change
Expand Up @@ -306,7 +306,9 @@ func Start(t *testing.T, opts ...Options) *Session {
strings.Contains(req.URL.Path, "/azure-dev")) ||
strings.Contains(req.URL.Host, "azure-dev.azureedge.net") ||
strings.Contains(req.URL.Host, "azdrelease.azureedge.net") ||
strings.Contains(req.URL.Host, "default.exp-tas.com")
strings.Contains(req.URL.Host, "default.exp-tas.com") ||
(strings.Contains(req.URL.Host, "dev.azure.com") &&
strings.Contains(req.URL.Path, "/oidctoken"))
})

proxy := &connectHandler{
Expand Down
9 changes: 7 additions & 2 deletions eng/pipelines/templates/jobs/build-cli.yml
Original file line number Diff line number Diff line change
Expand Up @@ -121,9 +121,14 @@ jobs:
AZD_TEST_AZURE_SUBSCRIPTION_ID: $(SubscriptionId)
AZD_TEST_AZURE_LOCATION: eastus2
AZURE_RECORD_MODE: $(AZURE_RECORD_MODE)
# AZD Live Test: Terraform authentication via `az`
ARM_USE_CLI: true
SYSTEM_ACCESSTOKEN: $(System.AccessToken)
# AZD Live Test: Terraform authentication via OIDC.
ARM_CLIENT_ID: $(arm-client-id)
ARM_SUBSCRIPTION_ID: $(SubscriptionId)
ARM_TENANT_ID: $(arm-tenant-id)
ARM_USE_OIDC: true
ARM_OIDC_REQUEST_URL: $(arm-oidc-request-url)
ARM_OIDC_REQUEST_TOKEN: $(System.AccessToken)
# Code Coverage: Generate junit report to publish results
GOTESTSUM_JUNITFILE: junitTestReport.xml

Expand Down
10 changes: 8 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 @@ -27,8 +30,11 @@ steps:
Write-Host "##vso[task.setvariable variable=SubscriptionId]$($subscriptionConfiguration.SubscriptionId)"
# Export service principal auth information for terraform testing
Write-Host "##vso[task.setvariable variable=arm-oidc-request-url;issecret=false]$($env:SYSTEM_OIDCREQUESTURI)?api-version=7.1&serviceConnectionId=${{ parameters.ServiceConnectionId }}"
Write-Host "##vso[task.setvariable variable=arm-client-id;issecret=false]$($subscriptionConfiguration.TestApplicationId)"
Write-Host "##vso[task.setvariable variable=arm-tenant-id;issecret=false]$($subscriptionConfiguration.TenantId)"
condition: and(succeeded(), ne(variables['Skip.LiveTest'], 'true'))
displayName: Azure Dev Login
env:
SYSTEM_ACCESSTOKEN: $(System.AccessToken)
Loading

0 comments on commit f22a1ca

Please sign in to comment.