From 8fb35c65699fe22c9c019f1c37e7641d80f3def9 Mon Sep 17 00:00:00 2001 From: Margo Crawford Date: Tue, 6 Jul 2021 11:34:54 -0700 Subject: [PATCH] Active Directory cli options --- .../types_activedirectoryidentityprovider.go.tmpl | 5 +++-- cmd/pinniped/cmd/kubeconfig.go | 2 +- cmd/pinniped/cmd/kubeconfig_test.go | 2 +- cmd/pinniped/cmd/login_oidc.go | 6 ++++-- cmd/pinniped/cmd/login_oidc_test.go | 15 +++++++++++++-- ...iped.dev_activedirectoryidentityproviders.yaml | 7 ++++--- generated/1.17/README.adoc | 2 +- .../types_activedirectoryidentityprovider.go | 5 +++-- ...iped.dev_activedirectoryidentityproviders.yaml | 7 ++++--- generated/1.18/README.adoc | 2 +- .../types_activedirectoryidentityprovider.go | 5 +++-- ...iped.dev_activedirectoryidentityproviders.yaml | 7 ++++--- generated/1.19/README.adoc | 2 +- .../types_activedirectoryidentityprovider.go | 5 +++-- ...iped.dev_activedirectoryidentityproviders.yaml | 7 ++++--- generated/1.20/README.adoc | 2 +- .../types_activedirectoryidentityprovider.go | 5 +++-- ...iped.dev_activedirectoryidentityproviders.yaml | 7 ++++--- .../types_activedirectoryidentityprovider.go | 5 +++-- internal/upstreamad/upstreamad.go | 14 +------------- internal/upstreamad/upstreamad_test.go | 2 +- pkg/oidcclient/login.go | 3 ++- test/integration/supervisor_login_test.go | 3 +++ 23 files changed, 68 insertions(+), 52 deletions(-) diff --git a/apis/supervisor/idp/v1alpha1/types_activedirectoryidentityprovider.go.tmpl b/apis/supervisor/idp/v1alpha1/types_activedirectoryidentityprovider.go.tmpl index 18726e7b..dc108466 100644 --- a/apis/supervisor/idp/v1alpha1/types_activedirectoryidentityprovider.go.tmpl +++ b/apis/supervisor/idp/v1alpha1/types_activedirectoryidentityprovider.go.tmpl @@ -72,8 +72,9 @@ type ActiveDirectoryIdentityProviderGroupSearchAttributes struct { type ActiveDirectoryIdentityProviderUserSearch struct { // Base is the dn (distinguished name) that should be used as the search base when searching for users. // E.g. "ou=users,dc=example,dc=com". - // Optional, when not specified it will be defaulted based on the host, for example if your active directory host is - // "activedirectory.example.com:636", it will be "dc=activedirectory,dc=example,dc=com". + // Optional, when not specified it will search the whole directory tree. + // Note that if your bind user only has permission to search a subtree, this must be specified. + // Search a subtree will also be faster. // +optional Base string `json:"base,omitempty"` diff --git a/cmd/pinniped/cmd/kubeconfig.go b/cmd/pinniped/cmd/kubeconfig.go index 54042b34..7eb70031 100644 --- a/cmd/pinniped/cmd/kubeconfig.go +++ b/cmd/pinniped/cmd/kubeconfig.go @@ -153,7 +153,7 @@ func kubeconfigCommand(deps kubeconfigDeps) *cobra.Command { f.BoolVar(&flags.oidc.debugSessionCache, "oidc-debug-session-cache", false, "Print debug logs related to the OpenID Connect session cache") f.StringVar(&flags.oidc.requestAudience, "oidc-request-audience", "", "Request a token with an alternate audience using RFC8693 token exchange") f.StringVar(&flags.oidc.upstreamIDPName, "upstream-identity-provider-name", "", "The name of the upstream identity provider used during login with a Supervisor") - f.StringVar(&flags.oidc.upstreamIDPType, "upstream-identity-provider-type", "", "The type of the upstream identity provider used during login with a Supervisor (e.g. 'oidc', 'ldap')") + f.StringVar(&flags.oidc.upstreamIDPType, "upstream-identity-provider-type", "", "The type of the upstream identity provider used during login with a Supervisor (e.g. 'oidc', 'ldap', 'activedirectory')") f.StringVar(&flags.kubeconfigPath, "kubeconfig", os.Getenv("KUBECONFIG"), "Path to kubeconfig file") f.StringVar(&flags.kubeconfigContextOverride, "kubeconfig-context", "", "Kubeconfig context name (default: current active context)") f.BoolVar(&flags.skipValidate, "skip-validation", false, "Skip final validation of the kubeconfig (default: false)") diff --git a/cmd/pinniped/cmd/kubeconfig_test.go b/cmd/pinniped/cmd/kubeconfig_test.go index cb9f7b83..464271b3 100644 --- a/cmd/pinniped/cmd/kubeconfig_test.go +++ b/cmd/pinniped/cmd/kubeconfig_test.go @@ -150,7 +150,7 @@ func TestGetKubeconfig(t *testing.T) { --static-token-env string Instead of doing an OIDC-based login, read a static token from the environment --timeout duration Timeout for autodiscovery and validation (default 10m0s) --upstream-identity-provider-name string The name of the upstream identity provider used during login with a Supervisor - --upstream-identity-provider-type string The type of the upstream identity provider used during login with a Supervisor (e.g. 'oidc', 'ldap') + --upstream-identity-provider-type string The type of the upstream identity provider used during login with a Supervisor (e.g. 'oidc', 'ldap', 'activedirectory') `) }, }, diff --git a/cmd/pinniped/cmd/login_oidc.go b/cmd/pinniped/cmd/login_oidc.go index 9141d26a..c1d2c321 100644 --- a/cmd/pinniped/cmd/login_oidc.go +++ b/cmd/pinniped/cmd/login_oidc.go @@ -107,7 +107,7 @@ func oidcLoginCommand(deps oidcLoginCommandDeps) *cobra.Command { cmd.Flags().StringVar(&flags.conciergeAPIGroupSuffix, "concierge-api-group-suffix", groupsuffix.PinnipedDefaultSuffix, "Concierge API group suffix") cmd.Flags().StringVar(&flags.credentialCachePath, "credential-cache", filepath.Join(mustGetConfigDir(), "credentials.yaml"), "Path to cluster-specific credentials cache (\"\" disables the cache)") cmd.Flags().StringVar(&flags.upstreamIdentityProviderName, "upstream-identity-provider-name", "", "The name of the upstream identity provider used during login with a Supervisor") - cmd.Flags().StringVar(&flags.upstreamIdentityProviderType, "upstream-identity-provider-type", "oidc", "The type of the upstream identity provider used during login with a Supervisor (e.g. 'oidc', 'ldap')") + cmd.Flags().StringVar(&flags.upstreamIdentityProviderType, "upstream-identity-provider-type", "oidc", "The type of the upstream identity provider used during login with a Supervisor (e.g. 'oidc', 'ldap', 'activedirectory')") // --skip-listen is mainly needed for testing. We'll leave it hidden until we have a non-testing use case. mustMarkHidden(cmd, "skip-listen") @@ -165,10 +165,12 @@ func runOIDCLogin(cmd *cobra.Command, deps oidcLoginCommandDeps, flags oidcLogin // this is the default, so don't need to do anything case "ldap": opts = append(opts, oidcclient.WithCLISendingCredentials()) + case "activedirectory": + opts = append(opts, oidcclient.WithCLISendingCredentials()) default: // Surprisingly cobra does not support this kind of flag validation. See https://github.com/spf13/pflag/issues/236 return fmt.Errorf( - "--upstream-identity-provider-type value not recognized: %s (supported values: oidc, ldap)", + "--upstream-identity-provider-type value not recognized: %s (supported values: oidc, ldap, activedirectory)", flags.upstreamIdentityProviderType) } diff --git a/cmd/pinniped/cmd/login_oidc_test.go b/cmd/pinniped/cmd/login_oidc_test.go index 055dcec6..e888599e 100644 --- a/cmd/pinniped/cmd/login_oidc_test.go +++ b/cmd/pinniped/cmd/login_oidc_test.go @@ -78,7 +78,7 @@ func TestLoginOIDCCommand(t *testing.T) { --session-cache string Path to session cache file (default "` + cfgDir + `/sessions.yaml") --skip-browser Skip opening the browser (just print the URL) --upstream-identity-provider-name string The name of the upstream identity provider used during login with a Supervisor - --upstream-identity-provider-type string The type of the upstream identity provider used during login with a Supervisor (e.g. 'oidc', 'ldap') (default "oidc") + --upstream-identity-provider-type string The type of the upstream identity provider used during login with a Supervisor (e.g. 'oidc', 'ldap', 'activedirectory') (default "oidc") `), }, { @@ -148,7 +148,7 @@ func TestLoginOIDCCommand(t *testing.T) { }, wantError: true, wantStderr: here.Doc(` - Error: --upstream-identity-provider-type value not recognized: invalid (supported values: oidc, ldap) + Error: --upstream-identity-provider-type value not recognized: invalid (supported values: oidc, ldap, activedirectory) `), }, { @@ -173,6 +173,17 @@ func TestLoginOIDCCommand(t *testing.T) { wantOptionsCount: 5, wantStdout: `{"kind":"ExecCredential","apiVersion":"client.authentication.k8s.io/v1beta1","spec":{},"status":{"expirationTimestamp":"3020-10-12T13:14:15Z","token":"test-id-token"}}` + "\n", }, + { + name: "activedirectory upstream type is allowed", + args: []string{ + "--issuer", "test-issuer", + "--client-id", "test-client-id", + "--upstream-identity-provider-type", "activedirectory", + "--credential-cache", "", // must specify --credential-cache or else the cache file on disk causes test pollution + }, + wantOptionsCount: 5, + wantStdout: `{"kind":"ExecCredential","apiVersion":"client.authentication.k8s.io/v1beta1","spec":{},"status":{"expirationTimestamp":"3020-10-12T13:14:15Z","token":"test-id-token"}}` + "\n", + }, { name: "login error", args: []string{ diff --git a/deploy/supervisor/idp.supervisor.pinniped.dev_activedirectoryidentityproviders.yaml b/deploy/supervisor/idp.supervisor.pinniped.dev_activedirectoryidentityproviders.yaml index 03fbcd08..0f94c50a 100644 --- a/deploy/supervisor/idp.supervisor.pinniped.dev_activedirectoryidentityproviders.yaml +++ b/deploy/supervisor/idp.supervisor.pinniped.dev_activedirectoryidentityproviders.yaml @@ -155,9 +155,10 @@ spec: base: description: Base is the dn (distinguished name) that should be used as the search base when searching for users. E.g. "ou=users,dc=example,dc=com". - Optional, when not specified it will be defaulted based on the - host, for example if your active directory host is "activedirectory.example.com:636", - it will be "dc=activedirectory,dc=example,dc=com". + Optional, when not specified it will search the whole directory + tree. Note that if your bind user only has permission to search + a subtree, this must be specified. Search a subtree will also + be faster. type: string filter: description: Filter is the ActiveDirectory search filter which diff --git a/generated/1.17/README.adoc b/generated/1.17/README.adoc index d0ba80df..288c802d 100644 --- a/generated/1.17/README.adoc +++ b/generated/1.17/README.adoc @@ -875,7 +875,7 @@ Status of an Active Directory identity provider. [cols="25a,75a", options="header"] |=== | Field | Description -| *`base`* __string__ | Base is the dn (distinguished name) that should be used as the search base when searching for users. E.g. "ou=users,dc=example,dc=com". Optional, when not specified it will be defaulted based on the host, for example if your active directory host is "activedirectory.example.com:636", it will be "dc=activedirectory,dc=example,dc=com". +| *`base`* __string__ | Base is the dn (distinguished name) that should be used as the search base when searching for users. E.g. "ou=users,dc=example,dc=com". Optional, when not specified it will search the whole directory tree. Note that if your bind user only has permission to search a subtree, this must be specified. Search a subtree will also be faster. | *`filter`* __string__ | Filter is the ActiveDirectory search filter which should be applied when searching for users. The pattern "{}" must occur in the filter at least once and will be dynamically replaced by the username for which the search is being run. E.g. "mail={}" or "&(objectClass=person)(uid={})". For more information about ActiveDirectory filters, see https://ldap.com/ldap-filters. Note that the dn (distinguished name) is not an attribute of an entry, so "dn={}" cannot be used. Optional. When not specified, the default will act as if the Filter were specified as the value from Attributes.Username appended by "={}". When the Attributes.Username is set to "dn" then the Filter must be explicitly specified, since the default value of "dn={}" would not work. | *`attributes`* __xref:{anchor_prefix}-go-pinniped-dev-generated-1-17-apis-supervisor-idp-v1alpha1-activedirectoryidentityproviderusersearchattributes[$$ActiveDirectoryIdentityProviderUserSearchAttributes$$]__ | Attributes specifies how the user's information should be read from the ActiveDirectory entry which was found as the result of the user search. |=== diff --git a/generated/1.17/apis/supervisor/idp/v1alpha1/types_activedirectoryidentityprovider.go b/generated/1.17/apis/supervisor/idp/v1alpha1/types_activedirectoryidentityprovider.go index 18726e7b..dc108466 100644 --- a/generated/1.17/apis/supervisor/idp/v1alpha1/types_activedirectoryidentityprovider.go +++ b/generated/1.17/apis/supervisor/idp/v1alpha1/types_activedirectoryidentityprovider.go @@ -72,8 +72,9 @@ type ActiveDirectoryIdentityProviderGroupSearchAttributes struct { type ActiveDirectoryIdentityProviderUserSearch struct { // Base is the dn (distinguished name) that should be used as the search base when searching for users. // E.g. "ou=users,dc=example,dc=com". - // Optional, when not specified it will be defaulted based on the host, for example if your active directory host is - // "activedirectory.example.com:636", it will be "dc=activedirectory,dc=example,dc=com". + // Optional, when not specified it will search the whole directory tree. + // Note that if your bind user only has permission to search a subtree, this must be specified. + // Search a subtree will also be faster. // +optional Base string `json:"base,omitempty"` diff --git a/generated/1.17/crds/idp.supervisor.pinniped.dev_activedirectoryidentityproviders.yaml b/generated/1.17/crds/idp.supervisor.pinniped.dev_activedirectoryidentityproviders.yaml index 03fbcd08..0f94c50a 100644 --- a/generated/1.17/crds/idp.supervisor.pinniped.dev_activedirectoryidentityproviders.yaml +++ b/generated/1.17/crds/idp.supervisor.pinniped.dev_activedirectoryidentityproviders.yaml @@ -155,9 +155,10 @@ spec: base: description: Base is the dn (distinguished name) that should be used as the search base when searching for users. E.g. "ou=users,dc=example,dc=com". - Optional, when not specified it will be defaulted based on the - host, for example if your active directory host is "activedirectory.example.com:636", - it will be "dc=activedirectory,dc=example,dc=com". + Optional, when not specified it will search the whole directory + tree. Note that if your bind user only has permission to search + a subtree, this must be specified. Search a subtree will also + be faster. type: string filter: description: Filter is the ActiveDirectory search filter which diff --git a/generated/1.18/README.adoc b/generated/1.18/README.adoc index 12946f74..8d99ecc4 100644 --- a/generated/1.18/README.adoc +++ b/generated/1.18/README.adoc @@ -875,7 +875,7 @@ Status of an Active Directory identity provider. [cols="25a,75a", options="header"] |=== | Field | Description -| *`base`* __string__ | Base is the dn (distinguished name) that should be used as the search base when searching for users. E.g. "ou=users,dc=example,dc=com". Optional, when not specified it will be defaulted based on the host, for example if your active directory host is "activedirectory.example.com:636", it will be "dc=activedirectory,dc=example,dc=com". +| *`base`* __string__ | Base is the dn (distinguished name) that should be used as the search base when searching for users. E.g. "ou=users,dc=example,dc=com". Optional, when not specified it will search the whole directory tree. Note that if your bind user only has permission to search a subtree, this must be specified. Search a subtree will also be faster. | *`filter`* __string__ | Filter is the ActiveDirectory search filter which should be applied when searching for users. The pattern "{}" must occur in the filter at least once and will be dynamically replaced by the username for which the search is being run. E.g. "mail={}" or "&(objectClass=person)(uid={})". For more information about ActiveDirectory filters, see https://ldap.com/ldap-filters. Note that the dn (distinguished name) is not an attribute of an entry, so "dn={}" cannot be used. Optional. When not specified, the default will act as if the Filter were specified as the value from Attributes.Username appended by "={}". When the Attributes.Username is set to "dn" then the Filter must be explicitly specified, since the default value of "dn={}" would not work. | *`attributes`* __xref:{anchor_prefix}-go-pinniped-dev-generated-1-18-apis-supervisor-idp-v1alpha1-activedirectoryidentityproviderusersearchattributes[$$ActiveDirectoryIdentityProviderUserSearchAttributes$$]__ | Attributes specifies how the user's information should be read from the ActiveDirectory entry which was found as the result of the user search. |=== diff --git a/generated/1.18/apis/supervisor/idp/v1alpha1/types_activedirectoryidentityprovider.go b/generated/1.18/apis/supervisor/idp/v1alpha1/types_activedirectoryidentityprovider.go index 18726e7b..dc108466 100644 --- a/generated/1.18/apis/supervisor/idp/v1alpha1/types_activedirectoryidentityprovider.go +++ b/generated/1.18/apis/supervisor/idp/v1alpha1/types_activedirectoryidentityprovider.go @@ -72,8 +72,9 @@ type ActiveDirectoryIdentityProviderGroupSearchAttributes struct { type ActiveDirectoryIdentityProviderUserSearch struct { // Base is the dn (distinguished name) that should be used as the search base when searching for users. // E.g. "ou=users,dc=example,dc=com". - // Optional, when not specified it will be defaulted based on the host, for example if your active directory host is - // "activedirectory.example.com:636", it will be "dc=activedirectory,dc=example,dc=com". + // Optional, when not specified it will search the whole directory tree. + // Note that if your bind user only has permission to search a subtree, this must be specified. + // Search a subtree will also be faster. // +optional Base string `json:"base,omitempty"` diff --git a/generated/1.18/crds/idp.supervisor.pinniped.dev_activedirectoryidentityproviders.yaml b/generated/1.18/crds/idp.supervisor.pinniped.dev_activedirectoryidentityproviders.yaml index 03fbcd08..0f94c50a 100644 --- a/generated/1.18/crds/idp.supervisor.pinniped.dev_activedirectoryidentityproviders.yaml +++ b/generated/1.18/crds/idp.supervisor.pinniped.dev_activedirectoryidentityproviders.yaml @@ -155,9 +155,10 @@ spec: base: description: Base is the dn (distinguished name) that should be used as the search base when searching for users. E.g. "ou=users,dc=example,dc=com". - Optional, when not specified it will be defaulted based on the - host, for example if your active directory host is "activedirectory.example.com:636", - it will be "dc=activedirectory,dc=example,dc=com". + Optional, when not specified it will search the whole directory + tree. Note that if your bind user only has permission to search + a subtree, this must be specified. Search a subtree will also + be faster. type: string filter: description: Filter is the ActiveDirectory search filter which diff --git a/generated/1.19/README.adoc b/generated/1.19/README.adoc index eadb1b33..e999bf69 100644 --- a/generated/1.19/README.adoc +++ b/generated/1.19/README.adoc @@ -875,7 +875,7 @@ Status of an Active Directory identity provider. [cols="25a,75a", options="header"] |=== | Field | Description -| *`base`* __string__ | Base is the dn (distinguished name) that should be used as the search base when searching for users. E.g. "ou=users,dc=example,dc=com". Optional, when not specified it will be defaulted based on the host, for example if your active directory host is "activedirectory.example.com:636", it will be "dc=activedirectory,dc=example,dc=com". +| *`base`* __string__ | Base is the dn (distinguished name) that should be used as the search base when searching for users. E.g. "ou=users,dc=example,dc=com". Optional, when not specified it will search the whole directory tree. Note that if your bind user only has permission to search a subtree, this must be specified. Search a subtree will also be faster. | *`filter`* __string__ | Filter is the ActiveDirectory search filter which should be applied when searching for users. The pattern "{}" must occur in the filter at least once and will be dynamically replaced by the username for which the search is being run. E.g. "mail={}" or "&(objectClass=person)(uid={})". For more information about ActiveDirectory filters, see https://ldap.com/ldap-filters. Note that the dn (distinguished name) is not an attribute of an entry, so "dn={}" cannot be used. Optional. When not specified, the default will act as if the Filter were specified as the value from Attributes.Username appended by "={}". When the Attributes.Username is set to "dn" then the Filter must be explicitly specified, since the default value of "dn={}" would not work. | *`attributes`* __xref:{anchor_prefix}-go-pinniped-dev-generated-1-19-apis-supervisor-idp-v1alpha1-activedirectoryidentityproviderusersearchattributes[$$ActiveDirectoryIdentityProviderUserSearchAttributes$$]__ | Attributes specifies how the user's information should be read from the ActiveDirectory entry which was found as the result of the user search. |=== diff --git a/generated/1.19/apis/supervisor/idp/v1alpha1/types_activedirectoryidentityprovider.go b/generated/1.19/apis/supervisor/idp/v1alpha1/types_activedirectoryidentityprovider.go index 18726e7b..dc108466 100644 --- a/generated/1.19/apis/supervisor/idp/v1alpha1/types_activedirectoryidentityprovider.go +++ b/generated/1.19/apis/supervisor/idp/v1alpha1/types_activedirectoryidentityprovider.go @@ -72,8 +72,9 @@ type ActiveDirectoryIdentityProviderGroupSearchAttributes struct { type ActiveDirectoryIdentityProviderUserSearch struct { // Base is the dn (distinguished name) that should be used as the search base when searching for users. // E.g. "ou=users,dc=example,dc=com". - // Optional, when not specified it will be defaulted based on the host, for example if your active directory host is - // "activedirectory.example.com:636", it will be "dc=activedirectory,dc=example,dc=com". + // Optional, when not specified it will search the whole directory tree. + // Note that if your bind user only has permission to search a subtree, this must be specified. + // Search a subtree will also be faster. // +optional Base string `json:"base,omitempty"` diff --git a/generated/1.19/crds/idp.supervisor.pinniped.dev_activedirectoryidentityproviders.yaml b/generated/1.19/crds/idp.supervisor.pinniped.dev_activedirectoryidentityproviders.yaml index 03fbcd08..0f94c50a 100644 --- a/generated/1.19/crds/idp.supervisor.pinniped.dev_activedirectoryidentityproviders.yaml +++ b/generated/1.19/crds/idp.supervisor.pinniped.dev_activedirectoryidentityproviders.yaml @@ -155,9 +155,10 @@ spec: base: description: Base is the dn (distinguished name) that should be used as the search base when searching for users. E.g. "ou=users,dc=example,dc=com". - Optional, when not specified it will be defaulted based on the - host, for example if your active directory host is "activedirectory.example.com:636", - it will be "dc=activedirectory,dc=example,dc=com". + Optional, when not specified it will search the whole directory + tree. Note that if your bind user only has permission to search + a subtree, this must be specified. Search a subtree will also + be faster. type: string filter: description: Filter is the ActiveDirectory search filter which diff --git a/generated/1.20/README.adoc b/generated/1.20/README.adoc index 13051452..2eccb623 100644 --- a/generated/1.20/README.adoc +++ b/generated/1.20/README.adoc @@ -875,7 +875,7 @@ Status of an Active Directory identity provider. [cols="25a,75a", options="header"] |=== | Field | Description -| *`base`* __string__ | Base is the dn (distinguished name) that should be used as the search base when searching for users. E.g. "ou=users,dc=example,dc=com". Optional, when not specified it will be defaulted based on the host, for example if your active directory host is "activedirectory.example.com:636", it will be "dc=activedirectory,dc=example,dc=com". +| *`base`* __string__ | Base is the dn (distinguished name) that should be used as the search base when searching for users. E.g. "ou=users,dc=example,dc=com". Optional, when not specified it will search the whole directory tree. Note that if your bind user only has permission to search a subtree, this must be specified. Search a subtree will also be faster. | *`filter`* __string__ | Filter is the ActiveDirectory search filter which should be applied when searching for users. The pattern "{}" must occur in the filter at least once and will be dynamically replaced by the username for which the search is being run. E.g. "mail={}" or "&(objectClass=person)(uid={})". For more information about ActiveDirectory filters, see https://ldap.com/ldap-filters. Note that the dn (distinguished name) is not an attribute of an entry, so "dn={}" cannot be used. Optional. When not specified, the default will act as if the Filter were specified as the value from Attributes.Username appended by "={}". When the Attributes.Username is set to "dn" then the Filter must be explicitly specified, since the default value of "dn={}" would not work. | *`attributes`* __xref:{anchor_prefix}-go-pinniped-dev-generated-1-20-apis-supervisor-idp-v1alpha1-activedirectoryidentityproviderusersearchattributes[$$ActiveDirectoryIdentityProviderUserSearchAttributes$$]__ | Attributes specifies how the user's information should be read from the ActiveDirectory entry which was found as the result of the user search. |=== diff --git a/generated/1.20/apis/supervisor/idp/v1alpha1/types_activedirectoryidentityprovider.go b/generated/1.20/apis/supervisor/idp/v1alpha1/types_activedirectoryidentityprovider.go index 18726e7b..dc108466 100644 --- a/generated/1.20/apis/supervisor/idp/v1alpha1/types_activedirectoryidentityprovider.go +++ b/generated/1.20/apis/supervisor/idp/v1alpha1/types_activedirectoryidentityprovider.go @@ -72,8 +72,9 @@ type ActiveDirectoryIdentityProviderGroupSearchAttributes struct { type ActiveDirectoryIdentityProviderUserSearch struct { // Base is the dn (distinguished name) that should be used as the search base when searching for users. // E.g. "ou=users,dc=example,dc=com". - // Optional, when not specified it will be defaulted based on the host, for example if your active directory host is - // "activedirectory.example.com:636", it will be "dc=activedirectory,dc=example,dc=com". + // Optional, when not specified it will search the whole directory tree. + // Note that if your bind user only has permission to search a subtree, this must be specified. + // Search a subtree will also be faster. // +optional Base string `json:"base,omitempty"` diff --git a/generated/1.20/crds/idp.supervisor.pinniped.dev_activedirectoryidentityproviders.yaml b/generated/1.20/crds/idp.supervisor.pinniped.dev_activedirectoryidentityproviders.yaml index 03fbcd08..0f94c50a 100644 --- a/generated/1.20/crds/idp.supervisor.pinniped.dev_activedirectoryidentityproviders.yaml +++ b/generated/1.20/crds/idp.supervisor.pinniped.dev_activedirectoryidentityproviders.yaml @@ -155,9 +155,10 @@ spec: base: description: Base is the dn (distinguished name) that should be used as the search base when searching for users. E.g. "ou=users,dc=example,dc=com". - Optional, when not specified it will be defaulted based on the - host, for example if your active directory host is "activedirectory.example.com:636", - it will be "dc=activedirectory,dc=example,dc=com". + Optional, when not specified it will search the whole directory + tree. Note that if your bind user only has permission to search + a subtree, this must be specified. Search a subtree will also + be faster. type: string filter: description: Filter is the ActiveDirectory search filter which diff --git a/generated/latest/apis/supervisor/idp/v1alpha1/types_activedirectoryidentityprovider.go b/generated/latest/apis/supervisor/idp/v1alpha1/types_activedirectoryidentityprovider.go index 18726e7b..dc108466 100644 --- a/generated/latest/apis/supervisor/idp/v1alpha1/types_activedirectoryidentityprovider.go +++ b/generated/latest/apis/supervisor/idp/v1alpha1/types_activedirectoryidentityprovider.go @@ -72,8 +72,9 @@ type ActiveDirectoryIdentityProviderGroupSearchAttributes struct { type ActiveDirectoryIdentityProviderUserSearch struct { // Base is the dn (distinguished name) that should be used as the search base when searching for users. // E.g. "ou=users,dc=example,dc=com". - // Optional, when not specified it will be defaulted based on the host, for example if your active directory host is - // "activedirectory.example.com:636", it will be "dc=activedirectory,dc=example,dc=com". + // Optional, when not specified it will search the whole directory tree. + // Note that if your bind user only has permission to search a subtree, this must be specified. + // Search a subtree will also be faster. // +optional Base string `json:"base,omitempty"` diff --git a/internal/upstreamad/upstreamad.go b/internal/upstreamad/upstreamad.go index 37fe3912..c64be5bb 100644 --- a/internal/upstreamad/upstreamad.go +++ b/internal/upstreamad/upstreamad.go @@ -417,19 +417,7 @@ func (p *Provider) userSearchRequest(username string) *ldap.SearchRequest { func (p *Provider) userSearchBase() string { if len(p.c.UserSearch.Base) == 0 { - parsed, err := endpointaddr.Parse(p.c.Host, 636) - if err != nil { - return "" - } - dcParts := strings.Split(parsed.Host, ".") - base := "" - for i, dcPart := range dcParts { - base += "dc=" + dcPart - if i < len(dcParts)-1 { - base += "," - } - } - return base + return "" } return p.c.UserSearch.Base } diff --git a/internal/upstreamad/upstreamad_test.go b/internal/upstreamad/upstreamad_test.go index 1fe0fca8..9c0c2a0b 100644 --- a/internal/upstreamad/upstreamad_test.go +++ b/internal/upstreamad/upstreamad_test.go @@ -218,7 +218,7 @@ func TestEndUserAuthentication(t *testing.T) { conn.EXPECT().Search(expectedUserSearch(func(r *ldap.SearchRequest) { r.Filter = "(" + sAMAccountNameAttributeName + "=" + testUpstreamUsername + ")" r.Attributes = []string{sAMAccountNameAttributeName, testUserSearchUIDAttribute} - r.BaseDN = "dc=activedirectory,dc=example,dc=com" + r.BaseDN = "" })).Return(&ldap.SearchResult{ Entries: []*ldap.Entry{ { diff --git a/pkg/oidcclient/login.go b/pkg/oidcclient/login.go index ffd827e9..b036eb14 100644 --- a/pkg/oidcclient/login.go +++ b/pkg/oidcclient/login.go @@ -228,7 +228,8 @@ func WithRequestAudience(audience string) Option { // WithCLISendingCredentials causes the login flow to use CLI-based prompts for username and password and causes the // call to the Issuer's authorize endpoint to be made directly (no web browser) with the username and password on custom // HTTP headers. This is only intended to be used when the issuer is a Pinniped Supervisor and the upstream identity -// provider type supports this style of authentication. Currently this is supported by LDAPIdentityProviders. +// provider type supports this style of authentication. Currently this is supported by LDAPIdentityProviders and +// ActiveDirectoryIdentityProviders. // This should never be used with non-Supervisor issuers because it will send the user's password to the authorization // endpoint as a custom header, which would be ignored but could potentially get logged somewhere by the issuer. func WithCLISendingCredentials() Option { diff --git a/test/integration/supervisor_login_test.go b/test/integration/supervisor_login_test.go index b747beaf..e1d1049f 100644 --- a/test/integration/supervisor_login_test.go +++ b/test/integration/supervisor_login_test.go @@ -230,6 +230,9 @@ func TestSupervisorLogin(t *testing.T) { wantDownstreamIDTokenUsernameToMatch: regexp.QuoteMeta(env.SupervisorUpstreamLDAP.TestUserDN), wantDownstreamIDTokenGroups: env.SupervisorUpstreamLDAP.TestUserDirectGroupsCNs, }, + { + name: "activedirectory with all default options", + }, } for _, test := range tests { tt := test