-
Notifications
You must be signed in to change notification settings - Fork 315
Add unix: and npipe: transports to AZD external authentication #8371
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
Draft
Copilot
wants to merge
13
commits into
main
Choose a base branch
from
copilot/add-unix-domain-socket-windows-named-pipe-support
base: main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Draft
Changes from 12 commits
Commits
Show all changes
13 commits
Select commit
Hold shift + click to select a range
0e07e61
Add UDS and Windows named pipe transports for external auth
Copilot 5f53437
Address review feedback: tidy imports and docs spacing
Copilot 4750628
Replace SDDL regex parsing with structured ACE walk
Copilot 333bd2b
Clarify ACE walk: rename, document unsafe cast, improve error
Copilot fb68c22
Merge branch 'main' into copilot/add-unix-domain-socket-windows-named…
bwateratmsft 78d32a9
Merge branch 'main' into copilot/add-unix-domain-socket-windows-named…
bwateratmsft 1d27979
Fix spelling issues
bwateratmsft 2e9c0f2
Missed one
bwateratmsft 850e8a9
Fix gosec findings in unix auth transport checks
Copilot 27fcf13
fix: document audited windows ACE SID cast
Copilot 36e2167
Require AZD_AUTH_KEY universally, require cert for https, reject syml…
Copilot 1e12c1c
CSpell yet again...
bwateratmsft 40199b4
Fix gosec G703 in unix auth transport socket check
Copilot File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,122 @@ | ||
| // Copyright (c) Microsoft Corporation. All rights reserved. | ||
| // Licensed under the MIT License. | ||
|
|
||
| package cmd | ||
|
|
||
| import ( | ||
| "fmt" | ||
| "net/http" | ||
| "net/url" | ||
|
|
||
| "github.com/azure/azure-dev/cli/azd/pkg/auth" | ||
| "github.com/azure/azure-dev/cli/azd/pkg/httputil" | ||
| ) | ||
|
|
||
| // rewrittenAuthEndpoint is the canonical placeholder URL used as the | ||
| // AZD_AUTH_ENDPOINT after rewriting unix:/npipe: schemes. RemoteCredential | ||
| // formats the request URL as "<endpoint>/token?api-version=..." so this | ||
| // placeholder produces a syntactically valid URL whose host/path are | ||
| // irrelevant because the transport dials a fixed socket/pipe. | ||
| const rewrittenAuthEndpoint = "http://azd-auth" | ||
|
|
||
| // buildExternalAuthConfiguration constructs the auth.ExternalAuthConfiguration | ||
| // from the raw AZD_AUTH_* env values. It dispatches on the scheme of the | ||
| // endpoint URL: | ||
| // | ||
| // - "" or "https": existing loopback HTTPS behavior. AZD_AUTH_CERT is | ||
| // required for "https". AZD_AUTH_KEY is required. | ||
| // - "unix": POSIX-only Unix domain socket transport. Cert MUST NOT be set. | ||
| // AZD_AUTH_KEY is required. | ||
| // - "npipe": Windows-only named pipe transport. Cert MUST NOT be set. | ||
| // AZD_AUTH_KEY is required. | ||
| // | ||
| // The "" and "http" schemes are accepted only to preserve the existing | ||
| // loopback test harness; production hosts use "https", "unix", or "npipe". | ||
| // | ||
| // Any other scheme yields an error that lists the supported schemes. | ||
| func buildExternalAuthConfiguration(endpoint, key, cert string) (auth.ExternalAuthConfiguration, error) { | ||
| // Parse the endpoint up front so we can dispatch on its scheme. An empty | ||
| // endpoint string parses successfully with an empty scheme, which is the | ||
| // historical "no external auth configured" / "implicit http for tests" | ||
| // case. | ||
| endpointUrl, err := url.Parse(endpoint) | ||
| if err != nil { | ||
| return auth.ExternalAuthConfiguration{}, | ||
| fmt.Errorf("invalid AZD_AUTH_ENDPOINT value '%s': %w", endpoint, err) | ||
| } | ||
|
|
||
| switch endpointUrl.Scheme { | ||
| case "", "http", "https": | ||
| return buildHTTPSExternalAuth(endpoint, key, cert, endpointUrl.Scheme) | ||
| case "unix": | ||
| return buildLocalIPCExternalAuth(endpoint, key, cert, newSocketTransport) | ||
| case "npipe": | ||
| return buildLocalIPCExternalAuth(endpoint, key, cert, newPipeTransport) | ||
| default: | ||
| return auth.ExternalAuthConfiguration{}, fmt.Errorf( | ||
| "invalid AZD_AUTH_ENDPOINT value '%s': unsupported scheme %q "+ | ||
| "(supported schemes: https, unix, npipe; http and no-scheme are accepted for local testing only)", | ||
| endpoint, endpointUrl.Scheme) | ||
| } | ||
| } | ||
|
|
||
| // buildHTTPSExternalAuth implements the historical HTTPS / no-scheme path. | ||
| // The "https" scheme requires AZD_AUTH_CERT; the "" and "http" schemes are | ||
| // retained only for the loopback test harness. When a cert is provided, the | ||
| // scheme MUST be "https". | ||
| func buildHTTPSExternalAuth(endpoint, key, cert, scheme string) (auth.ExternalAuthConfiguration, error) { | ||
| if scheme == "https" && len(cert) == 0 { | ||
| return auth.ExternalAuthConfiguration{}, fmt.Errorf( | ||
| "invalid AZD_AUTH_ENDPOINT value '%s': AZD_AUTH_CERT is required when using the 'https' scheme", | ||
| endpoint) | ||
| } | ||
| client := &http.Client{} | ||
| if len(cert) > 0 { | ||
| transport, err := httputil.TlsEnabledTransport(cert) | ||
|
bwateratmsft marked this conversation as resolved.
|
||
| if err != nil { | ||
| return auth.ExternalAuthConfiguration{}, | ||
| fmt.Errorf("parsing AZD_AUTH_CERT: %w", err) | ||
| } | ||
| client.Transport = transport | ||
|
|
||
| if scheme != "https" { | ||
| return auth.ExternalAuthConfiguration{}, | ||
| fmt.Errorf( | ||
| "invalid AZD_AUTH_ENDPOINT value '%s': scheme must be 'https' when certificate is provided", | ||
| endpoint) | ||
| } | ||
| } | ||
| return auth.ExternalAuthConfiguration{ | ||
| Endpoint: endpoint, | ||
| Transporter: client, | ||
| Key: key, | ||
| }, nil | ||
| } | ||
|
|
||
| // buildLocalIPCExternalAuth implements the unix: / npipe: paths. Both share | ||
| // the same shape: cert is forbidden, key is required, the transport is built | ||
| // by the platform-specific factory, and the endpoint is rewritten to a | ||
| // canonical placeholder so RemoteCredential can format request URLs. | ||
| func buildLocalIPCExternalAuth( | ||
| endpoint, key, cert string, | ||
| newTransport func(string) (http.RoundTripper, string, error), | ||
| ) (auth.ExternalAuthConfiguration, error) { | ||
| if len(cert) > 0 { | ||
| return auth.ExternalAuthConfiguration{}, fmt.Errorf( | ||
| "AZD_AUTH_CERT must not be set when AZD_AUTH_ENDPOINT uses a local IPC scheme " + | ||
| "(unix:, npipe:); the OS enforces caller identity") | ||
| } | ||
| if len(key) == 0 { | ||
| return auth.ExternalAuthConfiguration{}, fmt.Errorf( | ||
| "AZD_AUTH_KEY is required when AZD_AUTH_ENDPOINT is set") | ||
| } | ||
| transport, rewritten, err := newTransport(endpoint) | ||
| if err != nil { | ||
| return auth.ExternalAuthConfiguration{}, err | ||
| } | ||
| return auth.ExternalAuthConfiguration{ | ||
| Endpoint: rewritten, | ||
| Transporter: &http.Client{Transport: transport}, | ||
| Key: key, | ||
| }, nil | ||
| } | ||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,23 @@ | ||
| // Copyright (c) Microsoft Corporation. All rights reserved. | ||
| // Licensed under the MIT License. | ||
|
|
||
| //go:build !unix && !windows | ||
|
|
||
| package cmd | ||
|
|
||
| import ( | ||
| "fmt" | ||
| "net/http" | ||
| ) | ||
|
|
||
| // newSocketTransport is not supported on this platform. | ||
| func newSocketTransport(rawURL string) (http.RoundTripper, string, error) { | ||
| return nil, "", fmt.Errorf( | ||
| "AZD_AUTH_ENDPOINT scheme 'unix' is not supported on this platform") | ||
| } | ||
|
|
||
| // newPipeTransport is not supported on this platform. | ||
| func newPipeTransport(rawURL string) (http.RoundTripper, string, error) { | ||
| return nil, "", fmt.Errorf( | ||
| "AZD_AUTH_ENDPOINT scheme 'npipe' is not supported on this platform") | ||
| } |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,117 @@ | ||
| // Copyright (c) Microsoft Corporation. All rights reserved. | ||
| // Licensed under the MIT License. | ||
|
|
||
| package cmd | ||
|
|
||
| import ( | ||
| "strings" | ||
| "testing" | ||
|
|
||
| "github.com/stretchr/testify/require" | ||
| ) | ||
|
|
||
| // TestBuildExternalAuthConfiguration_Schemes exercises the scheme dispatch in | ||
| // buildExternalAuthConfiguration. Per-scheme transport construction (unix | ||
| // permission checks, Windows pipe SD checks) is covered by the platform- | ||
| // specific tests in auth_transport_unix_test.go / auth_transport_windows_test.go. | ||
| func TestBuildExternalAuthConfiguration_Schemes(t *testing.T) { | ||
| t.Parallel() | ||
|
|
||
| tests := []struct { | ||
| name string | ||
| endpoint string | ||
| key string | ||
| cert string | ||
| wantErrSub string // substring expected in error message; empty means no error expected | ||
| wantRewrite string // expected Endpoint on success; empty to skip | ||
| }{ | ||
| { | ||
| name: "empty endpoint, no cert preserves backward compat", | ||
| endpoint: "", | ||
| key: "k", | ||
| cert: "", | ||
| }, | ||
| { | ||
| name: "https without cert is rejected because cert is required", | ||
| endpoint: "https://127.0.0.1:1234", | ||
| key: "k", | ||
| cert: "", | ||
| wantErrSub: "AZD_AUTH_CERT is required when using the 'https' scheme", | ||
| }, | ||
|
bwateratmsft marked this conversation as resolved.
|
||
| { | ||
| name: "http with cert is rejected because cert requires https", | ||
| endpoint: "http://127.0.0.1:1234", | ||
| key: "k", | ||
| cert: "not-a-real-cert", | ||
| wantErrSub: "AZD_AUTH_CERT", // cert parse failure fires first | ||
| }, | ||
| { | ||
| name: "http without cert is preserved for backward compat", | ||
| endpoint: "http://127.0.0.1:1234", | ||
| key: "k", | ||
| cert: "", | ||
| }, | ||
| { | ||
| name: "unix scheme rejects cert", | ||
| endpoint: "unix:/tmp/some.sock", | ||
| key: "k", | ||
| cert: "anything", | ||
| wantErrSub: "AZD_AUTH_CERT must not be set", | ||
| }, | ||
| { | ||
| name: "npipe scheme rejects cert", | ||
| endpoint: "npipe:azd-auth-x", | ||
| key: "k", | ||
| cert: "anything", | ||
| wantErrSub: "AZD_AUTH_CERT must not be set", | ||
| }, | ||
| { | ||
| name: "unix scheme requires a key", | ||
| endpoint: "unix:/tmp/some.sock", | ||
| key: "", | ||
| cert: "", | ||
| wantErrSub: "AZD_AUTH_KEY is required", | ||
| }, | ||
| { | ||
| name: "unknown scheme is refused with a list of supported schemes", | ||
| endpoint: "ftp://nope", | ||
| key: "k", | ||
| cert: "", | ||
| wantErrSub: "supported schemes: https, unix, npipe", | ||
| }, | ||
| { | ||
| name: "malformed url is reported", | ||
| endpoint: "://broken", | ||
| wantErrSub: "invalid AZD_AUTH_ENDPOINT", | ||
| }, | ||
| } | ||
|
|
||
| for _, tt := range tests { | ||
| t.Run(tt.name, func(t *testing.T) { | ||
| cfg, err := buildExternalAuthConfiguration(tt.endpoint, tt.key, tt.cert) | ||
| if tt.wantErrSub != "" { | ||
| require.Error(t, err) | ||
| require.Contains(t, err.Error(), tt.wantErrSub) | ||
| return | ||
| } | ||
| require.NoError(t, err) | ||
| require.NotNil(t, cfg.Transporter) | ||
| require.Equal(t, tt.key, cfg.Key) | ||
| if tt.wantRewrite != "" { | ||
| require.Equal(t, tt.wantRewrite, cfg.Endpoint) | ||
| } else { | ||
| require.Equal(t, tt.endpoint, cfg.Endpoint) | ||
| } | ||
| }) | ||
| } | ||
| } | ||
|
|
||
| // TestRewrittenAuthEndpoint_FormatsValidURL verifies that the placeholder | ||
| // endpoint produces a syntactically valid request URL when concatenated by | ||
| // RemoteCredential with "/token?api-version=...". | ||
| func TestRewrittenAuthEndpoint_FormatsValidURL(t *testing.T) { | ||
| t.Parallel() | ||
| require.True(t, strings.HasPrefix(rewrittenAuthEndpoint, "http://"), | ||
| "placeholder must be an absolute URL so net/http accepts it") | ||
| require.NotContains(t, rewrittenAuthEndpoint, " ") | ||
| } | ||
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Uh oh!
There was an error while loading. Please reload this page.