From e9d5743845cfd1a2264e5bb1e97e6a4c831534f2 Mon Sep 17 00:00:00 2001 From: Ryan Richard Date: Fri, 16 Apr 2021 14:04:05 -0700 Subject: [PATCH] Add authentication dry run validation to LDAPIdentityProvider Also force the LDAP server pod to restart whenever the LDIF file changes, so whenever you redeploy the tools deployment with a new test user password the server will be updated. --- .../types_ldapidentityprovider.go.tmpl | 22 ++ ...or.pinniped.dev_ldapidentityproviders.yaml | 32 +++ generated/1.17/README.adoc | 1 + .../v1alpha1/types_ldapidentityprovider.go | 22 ++ ...or.pinniped.dev_ldapidentityproviders.yaml | 32 +++ generated/1.18/README.adoc | 1 + .../v1alpha1/types_ldapidentityprovider.go | 22 ++ ...or.pinniped.dev_ldapidentityproviders.yaml | 32 +++ generated/1.19/README.adoc | 1 + .../v1alpha1/types_ldapidentityprovider.go | 22 ++ ...or.pinniped.dev_ldapidentityproviders.yaml | 32 +++ generated/1.20/README.adoc | 1 + .../v1alpha1/types_ldapidentityprovider.go | 22 ++ ...or.pinniped.dev_ldapidentityproviders.yaml | 32 +++ .../v1alpha1/types_ldapidentityprovider.go | 22 ++ .../upstreamwatcher/ldap_upstream_watcher.go | 70 +++++- .../ldap_upstream_watcher_test.go | 120 +++++++++ internal/upstreamldap/upstreamldap.go | 32 ++- internal/upstreamldap/upstreamldap_test.go | 140 +++++++---- test/deploy/tools/ldap.yaml | 231 +++++++++--------- test/integration/supervisor_login_test.go | 88 ++++++- 21 files changed, 802 insertions(+), 175 deletions(-) diff --git a/apis/supervisor/idp/v1alpha1/types_ldapidentityprovider.go.tmpl b/apis/supervisor/idp/v1alpha1/types_ldapidentityprovider.go.tmpl index 0861c0de..c7a6b747 100644 --- a/apis/supervisor/idp/v1alpha1/types_ldapidentityprovider.go.tmpl +++ b/apis/supervisor/idp/v1alpha1/types_ldapidentityprovider.go.tmpl @@ -106,6 +106,28 @@ type LDAPIdentityProviderSpec struct { // UserSearch contains the configuration for searching for a user by name in the LDAP provider. UserSearch LDAPIdentityProviderUserSearchSpec `json:"userSearch,omitempty"` + + // DryRunAuthenticationUsername influences how the LDAPIdentityProvider's configuration is validated. + // When DryRunAuthenticationUsername is blank, the LDAPIdentityProvider will be validated by opening a connection + // to the LDAP server using the Host and TLS settings and also will bind using the Bind settings. The success + // or failure of the connect and bind will be reflected in the LDAPIdentityProvider's status conditions array. + // When DryRunAuthenticationUsername is not blank, the LDAPIdentityProvider will be validated by opening a + // connection to the LDAP server and performing a full dry run of authenticating as the end user with the username + // specified by DryRunAuthenticationUsername. The dry run will act as if the correct password were specified for + // that end user during the authentication. This will test all of the configuration options of the + // LDAPIdentityProvider. The success or failure of the authentication dry run will be reflected in the + // LDAPIdentityProvider's status conditions array, along with details of what username, UID, and group memberships + // were selected for the specified user. If the dry run fails, then that user would not be able to authenticate + // in a real authentication situation either, so the LDAPIdentityProvider's Status.Phase will be set to "Error". + // Therefore, the specified DryRunAuthenticationUsername must be a valid username of a real user who should be able + // to authenticate given all of the LDAPIdentityProvider's configuration. For example, if the UserSearch + // configuration were set up such that an end user should log in using their email address as their username, then + // the DryRunAuthenticationUsername should be the actual email address of a valid user who will be found in the LDAP + // server by the UserSearch criteria. Once you have used DryRunAuthenticationUsername to validate your + // LDAPIdentityProvider's configuration, you might choose to remove the DryRunAuthenticationUsername configuration + // if you are concerned that the user's LDAP account could change in the future, e.g. if the account could become + // disabled in the future. + DryRunAuthenticationUsername string `json:"dryRunAuthenticationUsername,omitempty"` } // LDAPIdentityProvider describes the configuration of an upstream Lightweight Directory Access diff --git a/deploy/supervisor/idp.supervisor.pinniped.dev_ldapidentityproviders.yaml b/deploy/supervisor/idp.supervisor.pinniped.dev_ldapidentityproviders.yaml index d262f36f..c9924a16 100644 --- a/deploy/supervisor/idp.supervisor.pinniped.dev_ldapidentityproviders.yaml +++ b/deploy/supervisor/idp.supervisor.pinniped.dev_ldapidentityproviders.yaml @@ -69,6 +69,38 @@ spec: required: - secretName type: object + dryRunAuthenticationUsername: + description: DryRunAuthenticationUsername influences how the LDAPIdentityProvider's + configuration is validated. When DryRunAuthenticationUsername is + blank, the LDAPIdentityProvider will be validated by opening a connection + to the LDAP server using the Host and TLS settings and also will + bind using the Bind settings. The success or failure of the connect + and bind will be reflected in the LDAPIdentityProvider's status + conditions array. When DryRunAuthenticationUsername is not blank, + the LDAPIdentityProvider will be validated by opening a connection + to the LDAP server and performing a full dry run of authenticating + as the end user with the username specified by DryRunAuthenticationUsername. + The dry run will act as if the correct password were specified for + that end user during the authentication. This will test all of the + configuration options of the LDAPIdentityProvider. The success or + failure of the authentication dry run will be reflected in the LDAPIdentityProvider's + status conditions array, along with details of what username, UID, + and group memberships were selected for the specified user. If the + dry run fails, then that user would not be able to authenticate + in a real authentication situation either, so the LDAPIdentityProvider's + Status.Phase will be set to "Error". Therefore, the specified DryRunAuthenticationUsername + must be a valid username of a real user who should be able to authenticate + given all of the LDAPIdentityProvider's configuration. For example, + if the UserSearch configuration were set up such that an end user + should log in using their email address as their username, then + the DryRunAuthenticationUsername should be the actual email address + of a valid user who will be found in the LDAP server by the UserSearch + criteria. Once you have used DryRunAuthenticationUsername to validate + your LDAPIdentityProvider's configuration, you might choose to remove + the DryRunAuthenticationUsername configuration if you are concerned + that the user's LDAP account could change in the future, e.g. if + the account could become disabled in the future. + type: string host: description: 'Host is the hostname of this LDAP identity provider, i.e., where to connect. For example: ldap.example.com:636.' diff --git a/generated/1.17/README.adoc b/generated/1.17/README.adoc index 8de20adc..27df8cb7 100644 --- a/generated/1.17/README.adoc +++ b/generated/1.17/README.adoc @@ -757,6 +757,7 @@ Spec for configuring an LDAP identity provider. | *`tls`* __xref:{anchor_prefix}-go-pinniped-dev-generated-1-17-apis-supervisor-idp-v1alpha1-ldapidentityprovidertlsspec[$$LDAPIdentityProviderTLSSpec$$]__ | TLS contains the connection settings for how to establish the connection to the Host. | *`bind`* __xref:{anchor_prefix}-go-pinniped-dev-generated-1-17-apis-supervisor-idp-v1alpha1-ldapidentityproviderbindspec[$$LDAPIdentityProviderBindSpec$$]__ | Bind contains the configuration for how to provide access credentials during an initial bind to the LDAP server to be allowed to perform searches and binds to validate a user's credentials during a user's authentication attempt. | *`userSearch`* __xref:{anchor_prefix}-go-pinniped-dev-generated-1-17-apis-supervisor-idp-v1alpha1-ldapidentityproviderusersearchspec[$$LDAPIdentityProviderUserSearchSpec$$]__ | UserSearch contains the configuration for searching for a user by name in the LDAP provider. +| *`dryRunAuthenticationUsername`* __string__ | DryRunAuthenticationUsername influences how the LDAPIdentityProvider's configuration is validated. When DryRunAuthenticationUsername is blank, the LDAPIdentityProvider will be validated by opening a connection to the LDAP server using the Host and TLS settings and also will bind using the Bind settings. The success or failure of the connect and bind will be reflected in the LDAPIdentityProvider's status conditions array. When DryRunAuthenticationUsername is not blank, the LDAPIdentityProvider will be validated by opening a connection to the LDAP server and performing a full dry run of authenticating as the end user with the username specified by DryRunAuthenticationUsername. The dry run will act as if the correct password were specified for that end user during the authentication. This will test all of the configuration options of the LDAPIdentityProvider. The success or failure of the authentication dry run will be reflected in the LDAPIdentityProvider's status conditions array, along with details of what username, UID, and group memberships were selected for the specified user. If the dry run fails, then that user would not be able to authenticate in a real authentication situation either, so the LDAPIdentityProvider's Status.Phase will be set to "Error". Therefore, the specified DryRunAuthenticationUsername must be a valid username of a real user who should be able to authenticate given all of the LDAPIdentityProvider's configuration. For example, if the UserSearch configuration were set up such that an end user should log in using their email address as their username, then the DryRunAuthenticationUsername should be the actual email address of a valid user who will be found in the LDAP server by the UserSearch criteria. Once you have used DryRunAuthenticationUsername to validate your LDAPIdentityProvider's configuration, you might choose to remove the DryRunAuthenticationUsername configuration if you are concerned that the user's LDAP account could change in the future, e.g. if the account could become disabled in the future. |=== diff --git a/generated/1.17/apis/supervisor/idp/v1alpha1/types_ldapidentityprovider.go b/generated/1.17/apis/supervisor/idp/v1alpha1/types_ldapidentityprovider.go index 0861c0de..c7a6b747 100644 --- a/generated/1.17/apis/supervisor/idp/v1alpha1/types_ldapidentityprovider.go +++ b/generated/1.17/apis/supervisor/idp/v1alpha1/types_ldapidentityprovider.go @@ -106,6 +106,28 @@ type LDAPIdentityProviderSpec struct { // UserSearch contains the configuration for searching for a user by name in the LDAP provider. UserSearch LDAPIdentityProviderUserSearchSpec `json:"userSearch,omitempty"` + + // DryRunAuthenticationUsername influences how the LDAPIdentityProvider's configuration is validated. + // When DryRunAuthenticationUsername is blank, the LDAPIdentityProvider will be validated by opening a connection + // to the LDAP server using the Host and TLS settings and also will bind using the Bind settings. The success + // or failure of the connect and bind will be reflected in the LDAPIdentityProvider's status conditions array. + // When DryRunAuthenticationUsername is not blank, the LDAPIdentityProvider will be validated by opening a + // connection to the LDAP server and performing a full dry run of authenticating as the end user with the username + // specified by DryRunAuthenticationUsername. The dry run will act as if the correct password were specified for + // that end user during the authentication. This will test all of the configuration options of the + // LDAPIdentityProvider. The success or failure of the authentication dry run will be reflected in the + // LDAPIdentityProvider's status conditions array, along with details of what username, UID, and group memberships + // were selected for the specified user. If the dry run fails, then that user would not be able to authenticate + // in a real authentication situation either, so the LDAPIdentityProvider's Status.Phase will be set to "Error". + // Therefore, the specified DryRunAuthenticationUsername must be a valid username of a real user who should be able + // to authenticate given all of the LDAPIdentityProvider's configuration. For example, if the UserSearch + // configuration were set up such that an end user should log in using their email address as their username, then + // the DryRunAuthenticationUsername should be the actual email address of a valid user who will be found in the LDAP + // server by the UserSearch criteria. Once you have used DryRunAuthenticationUsername to validate your + // LDAPIdentityProvider's configuration, you might choose to remove the DryRunAuthenticationUsername configuration + // if you are concerned that the user's LDAP account could change in the future, e.g. if the account could become + // disabled in the future. + DryRunAuthenticationUsername string `json:"dryRunAuthenticationUsername,omitempty"` } // LDAPIdentityProvider describes the configuration of an upstream Lightweight Directory Access diff --git a/generated/1.17/crds/idp.supervisor.pinniped.dev_ldapidentityproviders.yaml b/generated/1.17/crds/idp.supervisor.pinniped.dev_ldapidentityproviders.yaml index d262f36f..c9924a16 100644 --- a/generated/1.17/crds/idp.supervisor.pinniped.dev_ldapidentityproviders.yaml +++ b/generated/1.17/crds/idp.supervisor.pinniped.dev_ldapidentityproviders.yaml @@ -69,6 +69,38 @@ spec: required: - secretName type: object + dryRunAuthenticationUsername: + description: DryRunAuthenticationUsername influences how the LDAPIdentityProvider's + configuration is validated. When DryRunAuthenticationUsername is + blank, the LDAPIdentityProvider will be validated by opening a connection + to the LDAP server using the Host and TLS settings and also will + bind using the Bind settings. The success or failure of the connect + and bind will be reflected in the LDAPIdentityProvider's status + conditions array. When DryRunAuthenticationUsername is not blank, + the LDAPIdentityProvider will be validated by opening a connection + to the LDAP server and performing a full dry run of authenticating + as the end user with the username specified by DryRunAuthenticationUsername. + The dry run will act as if the correct password were specified for + that end user during the authentication. This will test all of the + configuration options of the LDAPIdentityProvider. The success or + failure of the authentication dry run will be reflected in the LDAPIdentityProvider's + status conditions array, along with details of what username, UID, + and group memberships were selected for the specified user. If the + dry run fails, then that user would not be able to authenticate + in a real authentication situation either, so the LDAPIdentityProvider's + Status.Phase will be set to "Error". Therefore, the specified DryRunAuthenticationUsername + must be a valid username of a real user who should be able to authenticate + given all of the LDAPIdentityProvider's configuration. For example, + if the UserSearch configuration were set up such that an end user + should log in using their email address as their username, then + the DryRunAuthenticationUsername should be the actual email address + of a valid user who will be found in the LDAP server by the UserSearch + criteria. Once you have used DryRunAuthenticationUsername to validate + your LDAPIdentityProvider's configuration, you might choose to remove + the DryRunAuthenticationUsername configuration if you are concerned + that the user's LDAP account could change in the future, e.g. if + the account could become disabled in the future. + type: string host: description: 'Host is the hostname of this LDAP identity provider, i.e., where to connect. For example: ldap.example.com:636.' diff --git a/generated/1.18/README.adoc b/generated/1.18/README.adoc index 6de19496..2671cc14 100644 --- a/generated/1.18/README.adoc +++ b/generated/1.18/README.adoc @@ -757,6 +757,7 @@ Spec for configuring an LDAP identity provider. | *`tls`* __xref:{anchor_prefix}-go-pinniped-dev-generated-1-18-apis-supervisor-idp-v1alpha1-ldapidentityprovidertlsspec[$$LDAPIdentityProviderTLSSpec$$]__ | TLS contains the connection settings for how to establish the connection to the Host. | *`bind`* __xref:{anchor_prefix}-go-pinniped-dev-generated-1-18-apis-supervisor-idp-v1alpha1-ldapidentityproviderbindspec[$$LDAPIdentityProviderBindSpec$$]__ | Bind contains the configuration for how to provide access credentials during an initial bind to the LDAP server to be allowed to perform searches and binds to validate a user's credentials during a user's authentication attempt. | *`userSearch`* __xref:{anchor_prefix}-go-pinniped-dev-generated-1-18-apis-supervisor-idp-v1alpha1-ldapidentityproviderusersearchspec[$$LDAPIdentityProviderUserSearchSpec$$]__ | UserSearch contains the configuration for searching for a user by name in the LDAP provider. +| *`dryRunAuthenticationUsername`* __string__ | DryRunAuthenticationUsername influences how the LDAPIdentityProvider's configuration is validated. When DryRunAuthenticationUsername is blank, the LDAPIdentityProvider will be validated by opening a connection to the LDAP server using the Host and TLS settings and also will bind using the Bind settings. The success or failure of the connect and bind will be reflected in the LDAPIdentityProvider's status conditions array. When DryRunAuthenticationUsername is not blank, the LDAPIdentityProvider will be validated by opening a connection to the LDAP server and performing a full dry run of authenticating as the end user with the username specified by DryRunAuthenticationUsername. The dry run will act as if the correct password were specified for that end user during the authentication. This will test all of the configuration options of the LDAPIdentityProvider. The success or failure of the authentication dry run will be reflected in the LDAPIdentityProvider's status conditions array, along with details of what username, UID, and group memberships were selected for the specified user. If the dry run fails, then that user would not be able to authenticate in a real authentication situation either, so the LDAPIdentityProvider's Status.Phase will be set to "Error". Therefore, the specified DryRunAuthenticationUsername must be a valid username of a real user who should be able to authenticate given all of the LDAPIdentityProvider's configuration. For example, if the UserSearch configuration were set up such that an end user should log in using their email address as their username, then the DryRunAuthenticationUsername should be the actual email address of a valid user who will be found in the LDAP server by the UserSearch criteria. Once you have used DryRunAuthenticationUsername to validate your LDAPIdentityProvider's configuration, you might choose to remove the DryRunAuthenticationUsername configuration if you are concerned that the user's LDAP account could change in the future, e.g. if the account could become disabled in the future. |=== diff --git a/generated/1.18/apis/supervisor/idp/v1alpha1/types_ldapidentityprovider.go b/generated/1.18/apis/supervisor/idp/v1alpha1/types_ldapidentityprovider.go index 0861c0de..c7a6b747 100644 --- a/generated/1.18/apis/supervisor/idp/v1alpha1/types_ldapidentityprovider.go +++ b/generated/1.18/apis/supervisor/idp/v1alpha1/types_ldapidentityprovider.go @@ -106,6 +106,28 @@ type LDAPIdentityProviderSpec struct { // UserSearch contains the configuration for searching for a user by name in the LDAP provider. UserSearch LDAPIdentityProviderUserSearchSpec `json:"userSearch,omitempty"` + + // DryRunAuthenticationUsername influences how the LDAPIdentityProvider's configuration is validated. + // When DryRunAuthenticationUsername is blank, the LDAPIdentityProvider will be validated by opening a connection + // to the LDAP server using the Host and TLS settings and also will bind using the Bind settings. The success + // or failure of the connect and bind will be reflected in the LDAPIdentityProvider's status conditions array. + // When DryRunAuthenticationUsername is not blank, the LDAPIdentityProvider will be validated by opening a + // connection to the LDAP server and performing a full dry run of authenticating as the end user with the username + // specified by DryRunAuthenticationUsername. The dry run will act as if the correct password were specified for + // that end user during the authentication. This will test all of the configuration options of the + // LDAPIdentityProvider. The success or failure of the authentication dry run will be reflected in the + // LDAPIdentityProvider's status conditions array, along with details of what username, UID, and group memberships + // were selected for the specified user. If the dry run fails, then that user would not be able to authenticate + // in a real authentication situation either, so the LDAPIdentityProvider's Status.Phase will be set to "Error". + // Therefore, the specified DryRunAuthenticationUsername must be a valid username of a real user who should be able + // to authenticate given all of the LDAPIdentityProvider's configuration. For example, if the UserSearch + // configuration were set up such that an end user should log in using their email address as their username, then + // the DryRunAuthenticationUsername should be the actual email address of a valid user who will be found in the LDAP + // server by the UserSearch criteria. Once you have used DryRunAuthenticationUsername to validate your + // LDAPIdentityProvider's configuration, you might choose to remove the DryRunAuthenticationUsername configuration + // if you are concerned that the user's LDAP account could change in the future, e.g. if the account could become + // disabled in the future. + DryRunAuthenticationUsername string `json:"dryRunAuthenticationUsername,omitempty"` } // LDAPIdentityProvider describes the configuration of an upstream Lightweight Directory Access diff --git a/generated/1.18/crds/idp.supervisor.pinniped.dev_ldapidentityproviders.yaml b/generated/1.18/crds/idp.supervisor.pinniped.dev_ldapidentityproviders.yaml index d262f36f..c9924a16 100644 --- a/generated/1.18/crds/idp.supervisor.pinniped.dev_ldapidentityproviders.yaml +++ b/generated/1.18/crds/idp.supervisor.pinniped.dev_ldapidentityproviders.yaml @@ -69,6 +69,38 @@ spec: required: - secretName type: object + dryRunAuthenticationUsername: + description: DryRunAuthenticationUsername influences how the LDAPIdentityProvider's + configuration is validated. When DryRunAuthenticationUsername is + blank, the LDAPIdentityProvider will be validated by opening a connection + to the LDAP server using the Host and TLS settings and also will + bind using the Bind settings. The success or failure of the connect + and bind will be reflected in the LDAPIdentityProvider's status + conditions array. When DryRunAuthenticationUsername is not blank, + the LDAPIdentityProvider will be validated by opening a connection + to the LDAP server and performing a full dry run of authenticating + as the end user with the username specified by DryRunAuthenticationUsername. + The dry run will act as if the correct password were specified for + that end user during the authentication. This will test all of the + configuration options of the LDAPIdentityProvider. The success or + failure of the authentication dry run will be reflected in the LDAPIdentityProvider's + status conditions array, along with details of what username, UID, + and group memberships were selected for the specified user. If the + dry run fails, then that user would not be able to authenticate + in a real authentication situation either, so the LDAPIdentityProvider's + Status.Phase will be set to "Error". Therefore, the specified DryRunAuthenticationUsername + must be a valid username of a real user who should be able to authenticate + given all of the LDAPIdentityProvider's configuration. For example, + if the UserSearch configuration were set up such that an end user + should log in using their email address as their username, then + the DryRunAuthenticationUsername should be the actual email address + of a valid user who will be found in the LDAP server by the UserSearch + criteria. Once you have used DryRunAuthenticationUsername to validate + your LDAPIdentityProvider's configuration, you might choose to remove + the DryRunAuthenticationUsername configuration if you are concerned + that the user's LDAP account could change in the future, e.g. if + the account could become disabled in the future. + type: string host: description: 'Host is the hostname of this LDAP identity provider, i.e., where to connect. For example: ldap.example.com:636.' diff --git a/generated/1.19/README.adoc b/generated/1.19/README.adoc index 37d2251c..00302c8c 100644 --- a/generated/1.19/README.adoc +++ b/generated/1.19/README.adoc @@ -757,6 +757,7 @@ Spec for configuring an LDAP identity provider. | *`tls`* __xref:{anchor_prefix}-go-pinniped-dev-generated-1-19-apis-supervisor-idp-v1alpha1-ldapidentityprovidertlsspec[$$LDAPIdentityProviderTLSSpec$$]__ | TLS contains the connection settings for how to establish the connection to the Host. | *`bind`* __xref:{anchor_prefix}-go-pinniped-dev-generated-1-19-apis-supervisor-idp-v1alpha1-ldapidentityproviderbindspec[$$LDAPIdentityProviderBindSpec$$]__ | Bind contains the configuration for how to provide access credentials during an initial bind to the LDAP server to be allowed to perform searches and binds to validate a user's credentials during a user's authentication attempt. | *`userSearch`* __xref:{anchor_prefix}-go-pinniped-dev-generated-1-19-apis-supervisor-idp-v1alpha1-ldapidentityproviderusersearchspec[$$LDAPIdentityProviderUserSearchSpec$$]__ | UserSearch contains the configuration for searching for a user by name in the LDAP provider. +| *`dryRunAuthenticationUsername`* __string__ | DryRunAuthenticationUsername influences how the LDAPIdentityProvider's configuration is validated. When DryRunAuthenticationUsername is blank, the LDAPIdentityProvider will be validated by opening a connection to the LDAP server using the Host and TLS settings and also will bind using the Bind settings. The success or failure of the connect and bind will be reflected in the LDAPIdentityProvider's status conditions array. When DryRunAuthenticationUsername is not blank, the LDAPIdentityProvider will be validated by opening a connection to the LDAP server and performing a full dry run of authenticating as the end user with the username specified by DryRunAuthenticationUsername. The dry run will act as if the correct password were specified for that end user during the authentication. This will test all of the configuration options of the LDAPIdentityProvider. The success or failure of the authentication dry run will be reflected in the LDAPIdentityProvider's status conditions array, along with details of what username, UID, and group memberships were selected for the specified user. If the dry run fails, then that user would not be able to authenticate in a real authentication situation either, so the LDAPIdentityProvider's Status.Phase will be set to "Error". Therefore, the specified DryRunAuthenticationUsername must be a valid username of a real user who should be able to authenticate given all of the LDAPIdentityProvider's configuration. For example, if the UserSearch configuration were set up such that an end user should log in using their email address as their username, then the DryRunAuthenticationUsername should be the actual email address of a valid user who will be found in the LDAP server by the UserSearch criteria. Once you have used DryRunAuthenticationUsername to validate your LDAPIdentityProvider's configuration, you might choose to remove the DryRunAuthenticationUsername configuration if you are concerned that the user's LDAP account could change in the future, e.g. if the account could become disabled in the future. |=== diff --git a/generated/1.19/apis/supervisor/idp/v1alpha1/types_ldapidentityprovider.go b/generated/1.19/apis/supervisor/idp/v1alpha1/types_ldapidentityprovider.go index 0861c0de..c7a6b747 100644 --- a/generated/1.19/apis/supervisor/idp/v1alpha1/types_ldapidentityprovider.go +++ b/generated/1.19/apis/supervisor/idp/v1alpha1/types_ldapidentityprovider.go @@ -106,6 +106,28 @@ type LDAPIdentityProviderSpec struct { // UserSearch contains the configuration for searching for a user by name in the LDAP provider. UserSearch LDAPIdentityProviderUserSearchSpec `json:"userSearch,omitempty"` + + // DryRunAuthenticationUsername influences how the LDAPIdentityProvider's configuration is validated. + // When DryRunAuthenticationUsername is blank, the LDAPIdentityProvider will be validated by opening a connection + // to the LDAP server using the Host and TLS settings and also will bind using the Bind settings. The success + // or failure of the connect and bind will be reflected in the LDAPIdentityProvider's status conditions array. + // When DryRunAuthenticationUsername is not blank, the LDAPIdentityProvider will be validated by opening a + // connection to the LDAP server and performing a full dry run of authenticating as the end user with the username + // specified by DryRunAuthenticationUsername. The dry run will act as if the correct password were specified for + // that end user during the authentication. This will test all of the configuration options of the + // LDAPIdentityProvider. The success or failure of the authentication dry run will be reflected in the + // LDAPIdentityProvider's status conditions array, along with details of what username, UID, and group memberships + // were selected for the specified user. If the dry run fails, then that user would not be able to authenticate + // in a real authentication situation either, so the LDAPIdentityProvider's Status.Phase will be set to "Error". + // Therefore, the specified DryRunAuthenticationUsername must be a valid username of a real user who should be able + // to authenticate given all of the LDAPIdentityProvider's configuration. For example, if the UserSearch + // configuration were set up such that an end user should log in using their email address as their username, then + // the DryRunAuthenticationUsername should be the actual email address of a valid user who will be found in the LDAP + // server by the UserSearch criteria. Once you have used DryRunAuthenticationUsername to validate your + // LDAPIdentityProvider's configuration, you might choose to remove the DryRunAuthenticationUsername configuration + // if you are concerned that the user's LDAP account could change in the future, e.g. if the account could become + // disabled in the future. + DryRunAuthenticationUsername string `json:"dryRunAuthenticationUsername,omitempty"` } // LDAPIdentityProvider describes the configuration of an upstream Lightweight Directory Access diff --git a/generated/1.19/crds/idp.supervisor.pinniped.dev_ldapidentityproviders.yaml b/generated/1.19/crds/idp.supervisor.pinniped.dev_ldapidentityproviders.yaml index d262f36f..c9924a16 100644 --- a/generated/1.19/crds/idp.supervisor.pinniped.dev_ldapidentityproviders.yaml +++ b/generated/1.19/crds/idp.supervisor.pinniped.dev_ldapidentityproviders.yaml @@ -69,6 +69,38 @@ spec: required: - secretName type: object + dryRunAuthenticationUsername: + description: DryRunAuthenticationUsername influences how the LDAPIdentityProvider's + configuration is validated. When DryRunAuthenticationUsername is + blank, the LDAPIdentityProvider will be validated by opening a connection + to the LDAP server using the Host and TLS settings and also will + bind using the Bind settings. The success or failure of the connect + and bind will be reflected in the LDAPIdentityProvider's status + conditions array. When DryRunAuthenticationUsername is not blank, + the LDAPIdentityProvider will be validated by opening a connection + to the LDAP server and performing a full dry run of authenticating + as the end user with the username specified by DryRunAuthenticationUsername. + The dry run will act as if the correct password were specified for + that end user during the authentication. This will test all of the + configuration options of the LDAPIdentityProvider. The success or + failure of the authentication dry run will be reflected in the LDAPIdentityProvider's + status conditions array, along with details of what username, UID, + and group memberships were selected for the specified user. If the + dry run fails, then that user would not be able to authenticate + in a real authentication situation either, so the LDAPIdentityProvider's + Status.Phase will be set to "Error". Therefore, the specified DryRunAuthenticationUsername + must be a valid username of a real user who should be able to authenticate + given all of the LDAPIdentityProvider's configuration. For example, + if the UserSearch configuration were set up such that an end user + should log in using their email address as their username, then + the DryRunAuthenticationUsername should be the actual email address + of a valid user who will be found in the LDAP server by the UserSearch + criteria. Once you have used DryRunAuthenticationUsername to validate + your LDAPIdentityProvider's configuration, you might choose to remove + the DryRunAuthenticationUsername configuration if you are concerned + that the user's LDAP account could change in the future, e.g. if + the account could become disabled in the future. + type: string host: description: 'Host is the hostname of this LDAP identity provider, i.e., where to connect. For example: ldap.example.com:636.' diff --git a/generated/1.20/README.adoc b/generated/1.20/README.adoc index a24ce0b5..fe6e5796 100644 --- a/generated/1.20/README.adoc +++ b/generated/1.20/README.adoc @@ -757,6 +757,7 @@ Spec for configuring an LDAP identity provider. | *`tls`* __xref:{anchor_prefix}-go-pinniped-dev-generated-1-20-apis-supervisor-idp-v1alpha1-ldapidentityprovidertlsspec[$$LDAPIdentityProviderTLSSpec$$]__ | TLS contains the connection settings for how to establish the connection to the Host. | *`bind`* __xref:{anchor_prefix}-go-pinniped-dev-generated-1-20-apis-supervisor-idp-v1alpha1-ldapidentityproviderbindspec[$$LDAPIdentityProviderBindSpec$$]__ | Bind contains the configuration for how to provide access credentials during an initial bind to the LDAP server to be allowed to perform searches and binds to validate a user's credentials during a user's authentication attempt. | *`userSearch`* __xref:{anchor_prefix}-go-pinniped-dev-generated-1-20-apis-supervisor-idp-v1alpha1-ldapidentityproviderusersearchspec[$$LDAPIdentityProviderUserSearchSpec$$]__ | UserSearch contains the configuration for searching for a user by name in the LDAP provider. +| *`dryRunAuthenticationUsername`* __string__ | DryRunAuthenticationUsername influences how the LDAPIdentityProvider's configuration is validated. When DryRunAuthenticationUsername is blank, the LDAPIdentityProvider will be validated by opening a connection to the LDAP server using the Host and TLS settings and also will bind using the Bind settings. The success or failure of the connect and bind will be reflected in the LDAPIdentityProvider's status conditions array. When DryRunAuthenticationUsername is not blank, the LDAPIdentityProvider will be validated by opening a connection to the LDAP server and performing a full dry run of authenticating as the end user with the username specified by DryRunAuthenticationUsername. The dry run will act as if the correct password were specified for that end user during the authentication. This will test all of the configuration options of the LDAPIdentityProvider. The success or failure of the authentication dry run will be reflected in the LDAPIdentityProvider's status conditions array, along with details of what username, UID, and group memberships were selected for the specified user. If the dry run fails, then that user would not be able to authenticate in a real authentication situation either, so the LDAPIdentityProvider's Status.Phase will be set to "Error". Therefore, the specified DryRunAuthenticationUsername must be a valid username of a real user who should be able to authenticate given all of the LDAPIdentityProvider's configuration. For example, if the UserSearch configuration were set up such that an end user should log in using their email address as their username, then the DryRunAuthenticationUsername should be the actual email address of a valid user who will be found in the LDAP server by the UserSearch criteria. Once you have used DryRunAuthenticationUsername to validate your LDAPIdentityProvider's configuration, you might choose to remove the DryRunAuthenticationUsername configuration if you are concerned that the user's LDAP account could change in the future, e.g. if the account could become disabled in the future. |=== diff --git a/generated/1.20/apis/supervisor/idp/v1alpha1/types_ldapidentityprovider.go b/generated/1.20/apis/supervisor/idp/v1alpha1/types_ldapidentityprovider.go index 0861c0de..c7a6b747 100644 --- a/generated/1.20/apis/supervisor/idp/v1alpha1/types_ldapidentityprovider.go +++ b/generated/1.20/apis/supervisor/idp/v1alpha1/types_ldapidentityprovider.go @@ -106,6 +106,28 @@ type LDAPIdentityProviderSpec struct { // UserSearch contains the configuration for searching for a user by name in the LDAP provider. UserSearch LDAPIdentityProviderUserSearchSpec `json:"userSearch,omitempty"` + + // DryRunAuthenticationUsername influences how the LDAPIdentityProvider's configuration is validated. + // When DryRunAuthenticationUsername is blank, the LDAPIdentityProvider will be validated by opening a connection + // to the LDAP server using the Host and TLS settings and also will bind using the Bind settings. The success + // or failure of the connect and bind will be reflected in the LDAPIdentityProvider's status conditions array. + // When DryRunAuthenticationUsername is not blank, the LDAPIdentityProvider will be validated by opening a + // connection to the LDAP server and performing a full dry run of authenticating as the end user with the username + // specified by DryRunAuthenticationUsername. The dry run will act as if the correct password were specified for + // that end user during the authentication. This will test all of the configuration options of the + // LDAPIdentityProvider. The success or failure of the authentication dry run will be reflected in the + // LDAPIdentityProvider's status conditions array, along with details of what username, UID, and group memberships + // were selected for the specified user. If the dry run fails, then that user would not be able to authenticate + // in a real authentication situation either, so the LDAPIdentityProvider's Status.Phase will be set to "Error". + // Therefore, the specified DryRunAuthenticationUsername must be a valid username of a real user who should be able + // to authenticate given all of the LDAPIdentityProvider's configuration. For example, if the UserSearch + // configuration were set up such that an end user should log in using their email address as their username, then + // the DryRunAuthenticationUsername should be the actual email address of a valid user who will be found in the LDAP + // server by the UserSearch criteria. Once you have used DryRunAuthenticationUsername to validate your + // LDAPIdentityProvider's configuration, you might choose to remove the DryRunAuthenticationUsername configuration + // if you are concerned that the user's LDAP account could change in the future, e.g. if the account could become + // disabled in the future. + DryRunAuthenticationUsername string `json:"dryRunAuthenticationUsername,omitempty"` } // LDAPIdentityProvider describes the configuration of an upstream Lightweight Directory Access diff --git a/generated/1.20/crds/idp.supervisor.pinniped.dev_ldapidentityproviders.yaml b/generated/1.20/crds/idp.supervisor.pinniped.dev_ldapidentityproviders.yaml index d262f36f..c9924a16 100644 --- a/generated/1.20/crds/idp.supervisor.pinniped.dev_ldapidentityproviders.yaml +++ b/generated/1.20/crds/idp.supervisor.pinniped.dev_ldapidentityproviders.yaml @@ -69,6 +69,38 @@ spec: required: - secretName type: object + dryRunAuthenticationUsername: + description: DryRunAuthenticationUsername influences how the LDAPIdentityProvider's + configuration is validated. When DryRunAuthenticationUsername is + blank, the LDAPIdentityProvider will be validated by opening a connection + to the LDAP server using the Host and TLS settings and also will + bind using the Bind settings. The success or failure of the connect + and bind will be reflected in the LDAPIdentityProvider's status + conditions array. When DryRunAuthenticationUsername is not blank, + the LDAPIdentityProvider will be validated by opening a connection + to the LDAP server and performing a full dry run of authenticating + as the end user with the username specified by DryRunAuthenticationUsername. + The dry run will act as if the correct password were specified for + that end user during the authentication. This will test all of the + configuration options of the LDAPIdentityProvider. The success or + failure of the authentication dry run will be reflected in the LDAPIdentityProvider's + status conditions array, along with details of what username, UID, + and group memberships were selected for the specified user. If the + dry run fails, then that user would not be able to authenticate + in a real authentication situation either, so the LDAPIdentityProvider's + Status.Phase will be set to "Error". Therefore, the specified DryRunAuthenticationUsername + must be a valid username of a real user who should be able to authenticate + given all of the LDAPIdentityProvider's configuration. For example, + if the UserSearch configuration were set up such that an end user + should log in using their email address as their username, then + the DryRunAuthenticationUsername should be the actual email address + of a valid user who will be found in the LDAP server by the UserSearch + criteria. Once you have used DryRunAuthenticationUsername to validate + your LDAPIdentityProvider's configuration, you might choose to remove + the DryRunAuthenticationUsername configuration if you are concerned + that the user's LDAP account could change in the future, e.g. if + the account could become disabled in the future. + type: string host: description: 'Host is the hostname of this LDAP identity provider, i.e., where to connect. For example: ldap.example.com:636.' diff --git a/generated/latest/apis/supervisor/idp/v1alpha1/types_ldapidentityprovider.go b/generated/latest/apis/supervisor/idp/v1alpha1/types_ldapidentityprovider.go index 0861c0de..c7a6b747 100644 --- a/generated/latest/apis/supervisor/idp/v1alpha1/types_ldapidentityprovider.go +++ b/generated/latest/apis/supervisor/idp/v1alpha1/types_ldapidentityprovider.go @@ -106,6 +106,28 @@ type LDAPIdentityProviderSpec struct { // UserSearch contains the configuration for searching for a user by name in the LDAP provider. UserSearch LDAPIdentityProviderUserSearchSpec `json:"userSearch,omitempty"` + + // DryRunAuthenticationUsername influences how the LDAPIdentityProvider's configuration is validated. + // When DryRunAuthenticationUsername is blank, the LDAPIdentityProvider will be validated by opening a connection + // to the LDAP server using the Host and TLS settings and also will bind using the Bind settings. The success + // or failure of the connect and bind will be reflected in the LDAPIdentityProvider's status conditions array. + // When DryRunAuthenticationUsername is not blank, the LDAPIdentityProvider will be validated by opening a + // connection to the LDAP server and performing a full dry run of authenticating as the end user with the username + // specified by DryRunAuthenticationUsername. The dry run will act as if the correct password were specified for + // that end user during the authentication. This will test all of the configuration options of the + // LDAPIdentityProvider. The success or failure of the authentication dry run will be reflected in the + // LDAPIdentityProvider's status conditions array, along with details of what username, UID, and group memberships + // were selected for the specified user. If the dry run fails, then that user would not be able to authenticate + // in a real authentication situation either, so the LDAPIdentityProvider's Status.Phase will be set to "Error". + // Therefore, the specified DryRunAuthenticationUsername must be a valid username of a real user who should be able + // to authenticate given all of the LDAPIdentityProvider's configuration. For example, if the UserSearch + // configuration were set up such that an end user should log in using their email address as their username, then + // the DryRunAuthenticationUsername should be the actual email address of a valid user who will be found in the LDAP + // server by the UserSearch criteria. Once you have used DryRunAuthenticationUsername to validate your + // LDAPIdentityProvider's configuration, you might choose to remove the DryRunAuthenticationUsername configuration + // if you are concerned that the user's LDAP account could change in the future, e.g. if the account could become + // disabled in the future. + DryRunAuthenticationUsername string `json:"dryRunAuthenticationUsername,omitempty"` } // LDAPIdentityProvider describes the configuration of an upstream Lightweight Directory Access diff --git a/internal/controller/supervisorconfig/upstreamwatcher/ldap_upstream_watcher.go b/internal/controller/supervisorconfig/upstreamwatcher/ldap_upstream_watcher.go index 6a0796eb..dd0bd73f 100644 --- a/internal/controller/supervisorconfig/upstreamwatcher/ldap_upstream_watcher.go +++ b/internal/controller/supervisorconfig/upstreamwatcher/ldap_upstream_watcher.go @@ -33,12 +33,13 @@ const ( testLDAPConnectionTimeout = 90 * time.Second // Constants related to conditions. - typeBindSecretValid = "BindSecretValid" - typeTLSConfigurationValid = "TLSConfigurationValid" - typeLDAPConnectionValid = "LDAPConnectionValid" - reasonLDAPConnectionError = "LDAPConnectionError" - noTLSConfigurationMessage = "no TLS configuration provided" - loadedTLSConfigurationMessage = "loaded TLS configuration" + typeBindSecretValid = "BindSecretValid" + typeTLSConfigurationValid = "TLSConfigurationValid" + typeLDAPConnectionValid = "LDAPConnectionValid" + reasonLDAPConnectionError = "LDAPConnectionError" + reasonAuthenticationDryRunError = "AuthenticationDryRunError" + noTLSConfigurationMessage = "no TLS configuration provided" + loadedTLSConfigurationMessage = "loaded TLS configuration" ) var ( @@ -184,7 +185,21 @@ func (c *ldapWatcherController) validateFinishedConfig(ctx context.Context, upst testConnectionTimeout, cancelFunc := context.WithTimeout(ctx, testLDAPConnectionTimeout) defer cancelFunc() - err := ldapProvider.TestConnection(testConnectionTimeout) + if len(upstream.Spec.DryRunAuthenticationUsername) > 0 { + return c.dryRunAuthentication(testConnectionTimeout, upstream, ldapProvider, currentSecretVersion) + } + + return c.testConnection(testConnectionTimeout, upstream, config, ldapProvider, currentSecretVersion) +} + +func (c *ldapWatcherController) testConnection( + ctx context.Context, + upstream *v1alpha1.LDAPIdentityProvider, + config *upstreamldap.ProviderConfig, + ldapProvider *upstreamldap.Provider, + currentSecretVersion string, +) *v1alpha1.Condition { + err := ldapProvider.TestConnection(ctx) if err != nil { return &v1alpha1.Condition{ Type: typeLDAPConnectionValid, @@ -204,6 +219,47 @@ func (c *ldapWatcherController) validateFinishedConfig(ctx context.Context, upst } } +func (c *ldapWatcherController) dryRunAuthentication( + ctx context.Context, + upstream *v1alpha1.LDAPIdentityProvider, + ldapProvider *upstreamldap.Provider, + currentSecretVersion string, +) *v1alpha1.Condition { + authResponse, authenticated, err := ldapProvider.DryRunAuthenticateUser(ctx, upstream.Spec.DryRunAuthenticationUsername) + if err != nil { + return &v1alpha1.Condition{ + Type: typeLDAPConnectionValid, + Status: v1alpha1.ConditionFalse, + Reason: reasonAuthenticationDryRunError, + Message: fmt.Sprintf(`failed authentication dry run for end user "%s": %s`, + upstream.Spec.DryRunAuthenticationUsername, err.Error()), + } + } + + if !authenticated { + // Since we aren't doing a real auth with a password that could be wrong, the only reason we should get + // an unauthenticated response without an error is when the username was wrong. + return &v1alpha1.Condition{ + Type: typeLDAPConnectionValid, + Status: v1alpha1.ConditionFalse, + Reason: reasonAuthenticationDryRunError, + Message: fmt.Sprintf(`failed authentication dry run for end user "%s": user not found`, + upstream.Spec.DryRunAuthenticationUsername), + } + } + + return &v1alpha1.Condition{ + Type: typeLDAPConnectionValid, + Status: v1alpha1.ConditionTrue, + Reason: reasonSuccess, + Message: fmt.Sprintf( + `successful authentication dry run for end user "%s": selected username "%s" and UID "%s" [validated with Secret "%s" at version "%s"]`, + upstream.Spec.DryRunAuthenticationUsername, + authResponse.User.GetName(), authResponse.User.GetUID(), + upstream.Spec.Bind.SecretName, currentSecretVersion), + } +} + func hasPreviousSuccessfulConditionForCurrentSpecGenerationAndSecretVersion(upstream *v1alpha1.LDAPIdentityProvider, currentSecretVersion string) bool { currentGeneration := upstream.Generation for _, c := range upstream.Status.Conditions { diff --git a/internal/controller/supervisorconfig/upstreamwatcher/ldap_upstream_watcher_test.go b/internal/controller/supervisorconfig/upstreamwatcher/ldap_upstream_watcher_test.go index b8af6d0c..e904f2a4 100644 --- a/internal/controller/supervisorconfig/upstreamwatcher/ldap_upstream_watcher_test.go +++ b/internal/controller/supervisorconfig/upstreamwatcher/ldap_upstream_watcher_test.go @@ -12,6 +12,8 @@ import ( "testing" "time" + "github.com/go-ldap/ldap/v3" + "github.com/golang/mock/gomock" "github.com/stretchr/testify/require" corev1 "k8s.io/api/core/v1" @@ -665,7 +667,125 @@ func TestLDAPUpstreamWatcherControllerSync(t *testing.T) { }, }}, }, + { + name: "when DryRunAuthenticationUsername is specified and a successful dry run authentication is performed", + inputUpstreams: []runtime.Object{editedValidUpstream(func(upstream *v1alpha1.LDAPIdentityProvider) { + upstream.Spec.DryRunAuthenticationUsername = "endUserUsername" + })}, + inputSecrets: []runtime.Object{validBindUserSecret("4242")}, + setupMocks: func(conn *mockldapconn.MockConn) { + // Should perform a full auth dry run. + conn.EXPECT().Bind(testBindUsername, testBindPassword).Times(1) + conn.EXPECT().Search(gomock.Any()).Return(&ldap.SearchResult{ + Entries: []*ldap.Entry{ + { + DN: "testFoundUserDN", + Attributes: []*ldap.EntryAttribute{ + ldap.NewEntryAttribute(testUsernameAttrName, []string{"testDownstreamUsername"}), + ldap.NewEntryAttribute(testUIDAttrName, []string{"testDownstreamUID"}), + }, + }, + }, + }, nil).Times(1) + conn.EXPECT().Close().Times(1) + }, + wantResultingCache: []*upstreamldap.ProviderConfig{providerConfigForValidUpstream}, + wantResultingUpstreams: []v1alpha1.LDAPIdentityProvider{{ + ObjectMeta: metav1.ObjectMeta{Namespace: testNamespace, Name: testName, Generation: 1234}, + Status: v1alpha1.LDAPIdentityProviderStatus{ + Phase: "Ready", + Conditions: []v1alpha1.Condition{ + bindSecretValidTrueCondition(1234), + { + Type: "LDAPConnectionValid", + Status: "True", + LastTransitionTime: now, + Reason: "Success", + Message: fmt.Sprintf( + `successful authentication dry run for end user "%s": selected username "%s" and UID "%s" [validated with Secret "%s" at version "%s"]`, + "endUserUsername", "testDownstreamUsername", "testDownstreamUID", testSecretName, "4242"), + ObservedGeneration: 1234, + }, + tlsConfigurationValidLoadedTrueCondition(1234), + }, + }, + }}, + }, + { + name: "when DryRunAuthenticationUsername is specified and the dry run authentication returns an error", + inputUpstreams: []runtime.Object{editedValidUpstream(func(upstream *v1alpha1.LDAPIdentityProvider) { + upstream.Spec.DryRunAuthenticationUsername = "endUserUsername" + })}, + inputSecrets: []runtime.Object{validBindUserSecret("4242")}, + setupMocks: func(conn *mockldapconn.MockConn) { + // Failure during a full auth dry run. + conn.EXPECT().Bind(testBindUsername, testBindPassword).Times(1) + conn.EXPECT().Search(gomock.Any()).Return(nil, errors.New("some dry run error")).Times(1) + conn.EXPECT().Close().Times(1) + }, + wantErr: controllerlib.ErrSyntheticRequeue.Error(), + wantResultingCache: []*upstreamldap.ProviderConfig{}, + wantResultingUpstreams: []v1alpha1.LDAPIdentityProvider{{ + ObjectMeta: metav1.ObjectMeta{Namespace: testNamespace, Name: testName, Generation: 1234}, + Status: v1alpha1.LDAPIdentityProviderStatus{ + Phase: "Error", + Conditions: []v1alpha1.Condition{ + bindSecretValidTrueCondition(1234), + { + Type: "LDAPConnectionValid", + Status: "False", + LastTransitionTime: now, + Reason: "AuthenticationDryRunError", + Message: fmt.Sprintf( + `failed authentication dry run for end user "%s": error searching for user "%s": some dry run error`, + "endUserUsername", "endUserUsername"), + ObservedGeneration: 1234, + }, + tlsConfigurationValidLoadedTrueCondition(1234), + }, + }, + }}, + }, + { + name: "when DryRunAuthenticationUsername is specified and the dry run authentication returns unauthenticated without an error", + inputUpstreams: []runtime.Object{editedValidUpstream(func(upstream *v1alpha1.LDAPIdentityProvider) { + upstream.Spec.DryRunAuthenticationUsername = "endUserUsername" + })}, + inputSecrets: []runtime.Object{validBindUserSecret("4242")}, + setupMocks: func(conn *mockldapconn.MockConn) { + // Failure during full auth dry run which will cause it to return unauthenticated instead of error. + conn.EXPECT().Bind(testBindUsername, testBindPassword).Times(1) + conn.EXPECT().Search(gomock.Any()).Return(&ldap.SearchResult{ + // No search results means the user did not enter a valid username, which is unauthenticated instead of error. + Entries: []*ldap.Entry{}, + }, nil).Times(1) + conn.EXPECT().Close().Times(1) + }, + wantErr: controllerlib.ErrSyntheticRequeue.Error(), + wantResultingCache: []*upstreamldap.ProviderConfig{}, + wantResultingUpstreams: []v1alpha1.LDAPIdentityProvider{{ + ObjectMeta: metav1.ObjectMeta{Namespace: testNamespace, Name: testName, Generation: 1234}, + Status: v1alpha1.LDAPIdentityProviderStatus{ + Phase: "Error", + Conditions: []v1alpha1.Condition{ + bindSecretValidTrueCondition(1234), + { + Type: "LDAPConnectionValid", + Status: "False", + LastTransitionTime: now, + Reason: "AuthenticationDryRunError", + Message: fmt.Sprintf( + `failed authentication dry run for end user "%s": user not found`, + "endUserUsername"), + ObservedGeneration: 1234, + }, + tlsConfigurationValidLoadedTrueCondition(1234), + }, + }, + }}, + }, } + for _, tt := range tests { tt := tt t.Run(tt.name, func(t *testing.T) { diff --git a/internal/upstreamldap/upstreamldap.go b/internal/upstreamldap/upstreamldap.go index e7694cf5..ae296db5 100644 --- a/internal/upstreamldap/upstreamldap.go +++ b/internal/upstreamldap/upstreamldap.go @@ -202,17 +202,27 @@ func (p *Provider) TestConnection(ctx context.Context) error { return nil } -// TestAuthenticateUser provides a method for testing all of the Provider settings in a kind of dry run of -// authentication. It runs the same logic as AuthenticateUser except it does not bind as that user, so it does not test -// their password. It returns the same authenticator.Response values and the same errors that a real call to +// DryRunAuthenticateUser provides a method for testing all of the Provider settings in a kind of dry run of +// authentication for a given end user's username. It runs the same logic as AuthenticateUser except it does +// not bind as that user, so it does not test their password. It returns the same values that a real call to // AuthenticateUser with the correct password would return. -func (p *Provider) TestAuthenticateUser(ctx context.Context, testUsername string) (*authenticator.Response, error) { - // TODO implement me - return nil, nil +func (p *Provider) DryRunAuthenticateUser(ctx context.Context, username string) (*authenticator.Response, bool, error) { + endUserBindFunc := func(conn Conn, foundUserDN string) error { + // Act as if the end user bind always succeeds. + return nil + } + return p.authenticateUserImpl(ctx, username, endUserBindFunc) } -// Authenticate a user and return their mapped username, groups, and UID. Implements authenticators.UserAuthenticator. +// Authenticate an end user and return their mapped username, groups, and UID. Implements authenticators.UserAuthenticator. func (p *Provider) AuthenticateUser(ctx context.Context, username, password string) (*authenticator.Response, bool, error) { + endUserBindFunc := func(conn Conn, foundUserDN string) error { + return conn.Bind(foundUserDN, password) + } + return p.authenticateUserImpl(ctx, username, endUserBindFunc) +} + +func (p *Provider) authenticateUserImpl(ctx context.Context, username string, bindFunc func(conn Conn, foundUserDN string) error) (*authenticator.Response, bool, error) { err := p.validateConfig() if err != nil { return nil, false, err @@ -234,7 +244,7 @@ func (p *Provider) AuthenticateUser(ctx context.Context, username, password stri return nil, false, fmt.Errorf(`error binding as "%s" before user search: %w`, p.c.BindUsername, err) } - mappedUsername, mappedUID, err := p.searchAndBindUser(conn, username, password) + mappedUsername, mappedUID, err := p.searchAndBindUser(conn, username, bindFunc) if err != nil { return nil, false, err } @@ -261,7 +271,7 @@ func (p *Provider) validateConfig() error { return nil } -func (p *Provider) searchAndBindUser(conn Conn, username string, password string) (string, string, error) { +func (p *Provider) searchAndBindUser(conn Conn, username string, bindFunc func(conn Conn, foundUserDN string) error) (string, string, error) { searchResult, err := conn.Search(p.userSearchRequest(username)) if err != nil { return "", "", fmt.Errorf(`error searching for user "%s": %w`, username, err) @@ -292,7 +302,7 @@ func (p *Provider) searchAndBindUser(conn Conn, username string, password string } // Caution: Note that any other LDAP commands after this bind will be run as this user instead of as the configured BindUsername! - err = conn.Bind(userEntry.DN, password) + err = bindFunc(conn, userEntry.DN) if err != nil { plog.DebugErr("error binding for user (if this is not the expected dn for this username, please check the user search configuration)", err, "upstreamName", p.GetName(), "username", username, "dn", userEntry.DN) @@ -344,7 +354,7 @@ func (p *Provider) userSearchFilter(username string) string { } func (p *Provider) escapeUsernameForSearchFilter(username string) string { - // The username is end-user input, so it should be escaped before being included in a search to prevent query injection. + // The username is end user input, so it should be escaped before being included in a search to prevent query injection. return ldap.EscapeFilter(username) } diff --git a/internal/upstreamldap/upstreamldap_test.go b/internal/upstreamldap/upstreamldap_test.go index c2cb7de6..73a42e08 100644 --- a/internal/upstreamldap/upstreamldap_test.go +++ b/internal/upstreamldap/upstreamldap_test.go @@ -42,7 +42,7 @@ var ( testUserSearchFilterInterpolated = fmt.Sprintf("(some-filter=%s-and-more-filter=%s)", testUpstreamUsername, testUpstreamUsername) ) -func TestAuthenticateUser(t *testing.T) { +func TestEndUserAuthentication(t *testing.T) { providerConfig := func(editFunc func(p *ProviderConfig)) *ProviderConfig { config := &ProviderConfig{ Name: "some-provider-name", @@ -82,23 +82,25 @@ func TestAuthenticateUser(t *testing.T) { } tests := []struct { - name string - username string - password string - providerConfig *ProviderConfig - setupMocks func(conn *mockldapconn.MockConn) - dialError error - wantError string - wantToSkipDial bool - wantAuthResponse *authenticator.Response - wantUnauthenticated bool + name string + username string + password string + providerConfig *ProviderConfig + searchMocks func(conn *mockldapconn.MockConn) + bindEndUserMocks func(conn *mockldapconn.MockConn) + dialError error + wantError string + wantToSkipDial bool + wantAuthResponse *authenticator.Response + wantUnauthenticated bool + skipDryRunAuthenticateUser bool // tests about when the end user bind fails don't make sense for DryRunAuthenticateUser() }{ { name: "happy path", username: testUpstreamUsername, password: testUpstreamPassword, providerConfig: providerConfig(nil), - setupMocks: func(conn *mockldapconn.MockConn) { + searchMocks: func(conn *mockldapconn.MockConn) { conn.EXPECT().Bind(testBindUsername, testBindPassword).Times(1) conn.EXPECT().Search(expectedSearch(nil)).Return(&ldap.SearchResult{ Entries: []*ldap.Entry{ @@ -113,9 +115,11 @@ func TestAuthenticateUser(t *testing.T) { Referrals: []string{}, // note that we are not following referrals at this time Controls: []ldap.Control{}, // TODO are there any response controls that we need to be able to handle? }, nil).Times(1) - conn.EXPECT().Bind(testSearchResultDNValue, testUpstreamPassword).Times(1) conn.EXPECT().Close().Times(1) }, + bindEndUserMocks: func(conn *mockldapconn.MockConn) { + conn.EXPECT().Bind(testSearchResultDNValue, testUpstreamPassword).Times(1) + }, wantAuthResponse: &authenticator.Response{ User: &user.DefaultInfo{ Name: testSearchResultUsernameAttributeValue, @@ -131,7 +135,7 @@ func TestAuthenticateUser(t *testing.T) { providerConfig: providerConfig(func(p *ProviderConfig) { p.UserSearch.Filter = "(" + testUserSearchFilter + ")" }), - setupMocks: func(conn *mockldapconn.MockConn) { + searchMocks: func(conn *mockldapconn.MockConn) { conn.EXPECT().Bind(testBindUsername, testBindPassword).Times(1) conn.EXPECT().Search(expectedSearch(nil)).Return(&ldap.SearchResult{ Entries: []*ldap.Entry{ @@ -144,9 +148,11 @@ func TestAuthenticateUser(t *testing.T) { }, }, }, nil).Times(1) - conn.EXPECT().Bind(testSearchResultDNValue, testUpstreamPassword).Times(1) conn.EXPECT().Close().Times(1) }, + bindEndUserMocks: func(conn *mockldapconn.MockConn) { + conn.EXPECT().Bind(testSearchResultDNValue, testUpstreamPassword).Times(1) + }, wantAuthResponse: &authenticator.Response{ User: &user.DefaultInfo{ Name: testSearchResultUsernameAttributeValue, @@ -162,7 +168,7 @@ func TestAuthenticateUser(t *testing.T) { providerConfig: providerConfig(func(p *ProviderConfig) { p.UserSearch.UsernameAttribute = "dn" }), - setupMocks: func(conn *mockldapconn.MockConn) { + searchMocks: func(conn *mockldapconn.MockConn) { conn.EXPECT().Bind(testBindUsername, testBindPassword).Times(1) conn.EXPECT().Search(expectedSearch(func(r *ldap.SearchRequest) { r.Attributes = []string{testUserSearchUIDAttribute} @@ -176,9 +182,11 @@ func TestAuthenticateUser(t *testing.T) { }, }, }, nil).Times(1) - conn.EXPECT().Bind(testSearchResultDNValue, testUpstreamPassword).Times(1) conn.EXPECT().Close().Times(1) }, + bindEndUserMocks: func(conn *mockldapconn.MockConn) { + conn.EXPECT().Bind(testSearchResultDNValue, testUpstreamPassword).Times(1) + }, wantAuthResponse: &authenticator.Response{ User: &user.DefaultInfo{ Name: testSearchResultDNValue, @@ -194,7 +202,7 @@ func TestAuthenticateUser(t *testing.T) { providerConfig: providerConfig(func(p *ProviderConfig) { p.UserSearch.UIDAttribute = "dn" }), - setupMocks: func(conn *mockldapconn.MockConn) { + searchMocks: func(conn *mockldapconn.MockConn) { conn.EXPECT().Bind(testBindUsername, testBindPassword).Times(1) conn.EXPECT().Search(expectedSearch(func(r *ldap.SearchRequest) { r.Attributes = []string{testUserSearchUsernameAttribute} @@ -208,9 +216,11 @@ func TestAuthenticateUser(t *testing.T) { }, }, }, nil).Times(1) - conn.EXPECT().Bind(testSearchResultDNValue, testUpstreamPassword).Times(1) conn.EXPECT().Close().Times(1) }, + bindEndUserMocks: func(conn *mockldapconn.MockConn) { + conn.EXPECT().Bind(testSearchResultDNValue, testUpstreamPassword).Times(1) + }, wantAuthResponse: &authenticator.Response{ User: &user.DefaultInfo{ Name: testSearchResultUsernameAttributeValue, @@ -226,7 +236,7 @@ func TestAuthenticateUser(t *testing.T) { providerConfig: providerConfig(func(p *ProviderConfig) { p.UserSearch.Filter = "" }), - setupMocks: func(conn *mockldapconn.MockConn) { + searchMocks: func(conn *mockldapconn.MockConn) { conn.EXPECT().Bind(testBindUsername, testBindPassword).Times(1) conn.EXPECT().Search(expectedSearch(func(r *ldap.SearchRequest) { r.Filter = "(" + testUserSearchUsernameAttribute + "=" + testUpstreamUsername + ")" @@ -241,9 +251,11 @@ func TestAuthenticateUser(t *testing.T) { }, }, }, nil).Times(1) - conn.EXPECT().Bind(testSearchResultDNValue, testUpstreamPassword).Times(1) conn.EXPECT().Close().Times(1) }, + bindEndUserMocks: func(conn *mockldapconn.MockConn) { + conn.EXPECT().Bind(testSearchResultDNValue, testUpstreamPassword).Times(1) + }, wantAuthResponse: &authenticator.Response{ User: &user.DefaultInfo{ Name: testSearchResultUsernameAttributeValue, @@ -257,7 +269,7 @@ func TestAuthenticateUser(t *testing.T) { username: `a&b|c(d)e\f*g`, password: testUpstreamPassword, providerConfig: providerConfig(nil), - setupMocks: func(conn *mockldapconn.MockConn) { + searchMocks: func(conn *mockldapconn.MockConn) { conn.EXPECT().Bind(testBindUsername, testBindPassword).Times(1) conn.EXPECT().Search(expectedSearch(func(r *ldap.SearchRequest) { r.Filter = fmt.Sprintf("(some-filter=%s-and-more-filter=%s)", `a&b|c\28d\29e\5cf\2ag`, `a&b|c\28d\29e\5cf\2ag`) @@ -272,9 +284,11 @@ func TestAuthenticateUser(t *testing.T) { }, }, }, nil).Times(1) - conn.EXPECT().Bind(testSearchResultDNValue, testUpstreamPassword).Times(1) conn.EXPECT().Close().Times(1) }, + bindEndUserMocks: func(conn *mockldapconn.MockConn) { + conn.EXPECT().Bind(testSearchResultDNValue, testUpstreamPassword).Times(1) + }, wantAuthResponse: &authenticator.Response{ User: &user.DefaultInfo{ Name: testSearchResultUsernameAttributeValue, @@ -307,7 +321,7 @@ func TestAuthenticateUser(t *testing.T) { username: testUpstreamUsername, password: testUpstreamPassword, providerConfig: providerConfig(nil), - setupMocks: func(conn *mockldapconn.MockConn) { + searchMocks: func(conn *mockldapconn.MockConn) { conn.EXPECT().Bind(testBindUsername, testBindPassword).Return(errors.New("some bind error")).Times(1) conn.EXPECT().Close().Times(1) }, @@ -318,7 +332,7 @@ func TestAuthenticateUser(t *testing.T) { username: testUpstreamUsername, password: testUpstreamPassword, providerConfig: providerConfig(nil), - setupMocks: func(conn *mockldapconn.MockConn) { + searchMocks: func(conn *mockldapconn.MockConn) { conn.EXPECT().Bind(testBindUsername, testBindPassword).Times(1) conn.EXPECT().Search(expectedSearch(nil)).Return(nil, errors.New("some search error")).Times(1) conn.EXPECT().Close().Times(1) @@ -330,7 +344,7 @@ func TestAuthenticateUser(t *testing.T) { username: testUpstreamUsername, password: testUpstreamPassword, providerConfig: providerConfig(nil), - setupMocks: func(conn *mockldapconn.MockConn) { + searchMocks: func(conn *mockldapconn.MockConn) { conn.EXPECT().Bind(testBindUsername, testBindPassword).Times(1) conn.EXPECT().Search(expectedSearch(nil)).Return(&ldap.SearchResult{ Entries: []*ldap.Entry{}, @@ -344,7 +358,7 @@ func TestAuthenticateUser(t *testing.T) { username: testUpstreamUsername, password: testUpstreamPassword, providerConfig: providerConfig(nil), - setupMocks: func(conn *mockldapconn.MockConn) { + searchMocks: func(conn *mockldapconn.MockConn) { conn.EXPECT().Bind(testBindUsername, testBindPassword).Times(1) conn.EXPECT().Search(expectedSearch(nil)).Return(&ldap.SearchResult{ Entries: []*ldap.Entry{ @@ -361,7 +375,7 @@ func TestAuthenticateUser(t *testing.T) { username: testUpstreamUsername, password: testUpstreamPassword, providerConfig: providerConfig(nil), - setupMocks: func(conn *mockldapconn.MockConn) { + searchMocks: func(conn *mockldapconn.MockConn) { conn.EXPECT().Bind(testBindUsername, testBindPassword).Times(1) conn.EXPECT().Search(expectedSearch(nil)).Return(&ldap.SearchResult{ Entries: []*ldap.Entry{ @@ -377,7 +391,7 @@ func TestAuthenticateUser(t *testing.T) { username: testUpstreamUsername, password: testUpstreamPassword, providerConfig: providerConfig(nil), - setupMocks: func(conn *mockldapconn.MockConn) { + searchMocks: func(conn *mockldapconn.MockConn) { conn.EXPECT().Bind(testBindUsername, testBindPassword).Times(1) conn.EXPECT().Search(expectedSearch(nil)).Return(&ldap.SearchResult{ Entries: []*ldap.Entry{ @@ -398,7 +412,7 @@ func TestAuthenticateUser(t *testing.T) { username: testUpstreamUsername, password: testUpstreamPassword, providerConfig: providerConfig(nil), - setupMocks: func(conn *mockldapconn.MockConn) { + searchMocks: func(conn *mockldapconn.MockConn) { conn.EXPECT().Bind(testBindUsername, testBindPassword).Times(1) conn.EXPECT().Search(expectedSearch(nil)).Return(&ldap.SearchResult{ Entries: []*ldap.Entry{ @@ -423,7 +437,7 @@ func TestAuthenticateUser(t *testing.T) { username: testUpstreamUsername, password: testUpstreamPassword, providerConfig: providerConfig(nil), - setupMocks: func(conn *mockldapconn.MockConn) { + searchMocks: func(conn *mockldapconn.MockConn) { conn.EXPECT().Bind(testBindUsername, testBindPassword).Times(1) conn.EXPECT().Search(expectedSearch(nil)).Return(&ldap.SearchResult{ Entries: []*ldap.Entry{ @@ -445,7 +459,7 @@ func TestAuthenticateUser(t *testing.T) { username: testUpstreamUsername, password: testUpstreamPassword, providerConfig: providerConfig(nil), - setupMocks: func(conn *mockldapconn.MockConn) { + searchMocks: func(conn *mockldapconn.MockConn) { conn.EXPECT().Bind(testBindUsername, testBindPassword).Times(1) conn.EXPECT().Search(expectedSearch(nil)).Return(&ldap.SearchResult{ Entries: []*ldap.Entry{ @@ -466,7 +480,7 @@ func TestAuthenticateUser(t *testing.T) { username: testUpstreamUsername, password: testUpstreamPassword, providerConfig: providerConfig(nil), - setupMocks: func(conn *mockldapconn.MockConn) { + searchMocks: func(conn *mockldapconn.MockConn) { conn.EXPECT().Bind(testBindUsername, testBindPassword).Times(1) conn.EXPECT().Search(expectedSearch(nil)).Return(&ldap.SearchResult{ Entries: []*ldap.Entry{ @@ -491,7 +505,7 @@ func TestAuthenticateUser(t *testing.T) { username: testUpstreamUsername, password: testUpstreamPassword, providerConfig: providerConfig(nil), - setupMocks: func(conn *mockldapconn.MockConn) { + searchMocks: func(conn *mockldapconn.MockConn) { conn.EXPECT().Bind(testBindUsername, testBindPassword).Times(1) conn.EXPECT().Search(expectedSearch(nil)).Return(&ldap.SearchResult{ Entries: []*ldap.Entry{ @@ -513,7 +527,7 @@ func TestAuthenticateUser(t *testing.T) { username: testUpstreamUsername, password: testUpstreamPassword, providerConfig: providerConfig(nil), - setupMocks: func(conn *mockldapconn.MockConn) { + searchMocks: func(conn *mockldapconn.MockConn) { conn.EXPECT().Bind(testBindUsername, testBindPassword).Times(1) conn.EXPECT().Search(expectedSearch(nil)).Return(&ldap.SearchResult{ Entries: []*ldap.Entry{ @@ -526,17 +540,20 @@ func TestAuthenticateUser(t *testing.T) { }, }, }, nil).Times(1) - conn.EXPECT().Bind(testSearchResultDNValue, testUpstreamPassword).Return(errors.New("some bind error")).Times(1) conn.EXPECT().Close().Times(1) }, - wantError: fmt.Sprintf(`error binding for user "%s" using provided password against DN "%s": some bind error`, testUpstreamUsername, testSearchResultDNValue), + bindEndUserMocks: func(conn *mockldapconn.MockConn) { + conn.EXPECT().Bind(testSearchResultDNValue, testUpstreamPassword).Return(errors.New("some bind error")).Times(1) + }, + skipDryRunAuthenticateUser: true, + wantError: fmt.Sprintf(`error binding for user "%s" using provided password against DN "%s": some bind error`, testUpstreamUsername, testSearchResultDNValue), }, { name: "when binding as the found user returns a specific invalid credentials error", username: testUpstreamUsername, password: testUpstreamPassword, providerConfig: providerConfig(nil), - setupMocks: func(conn *mockldapconn.MockConn) { + searchMocks: func(conn *mockldapconn.MockConn) { conn.EXPECT().Bind(testBindUsername, testBindPassword).Times(1) conn.EXPECT().Search(expectedSearch(nil)).Return(&ldap.SearchResult{ Entries: []*ldap.Entry{ @@ -549,10 +566,13 @@ func TestAuthenticateUser(t *testing.T) { }, }, }, nil).Times(1) - conn.EXPECT().Bind(testSearchResultDNValue, testUpstreamPassword).Return(errors.New(`LDAP Result Code 49 "Invalid Credentials": some bind error`)).Times(1) conn.EXPECT().Close().Times(1) }, - wantUnauthenticated: true, + wantUnauthenticated: true, + skipDryRunAuthenticateUser: true, + bindEndUserMocks: func(conn *mockldapconn.MockConn) { + conn.EXPECT().Bind(testSearchResultDNValue, testUpstreamPassword).Return(errors.New(`LDAP Result Code 49 "Invalid Credentials": some bind error`)).Times(1) + }, }, { name: "when no username is specified", @@ -571,8 +591,11 @@ func TestAuthenticateUser(t *testing.T) { t.Cleanup(ctrl.Finish) conn := mockldapconn.NewMockConn(ctrl) - if tt.setupMocks != nil { - tt.setupMocks(conn) + if tt.searchMocks != nil { + tt.searchMocks(conn) + } + if tt.bindEndUserMocks != nil { + tt.bindEndUserMocks(conn) } dialWasAttempted := false @@ -586,10 +609,41 @@ func TestAuthenticateUser(t *testing.T) { }) provider := New(*tt.providerConfig) + authResponse, authenticated, err := provider.AuthenticateUser(context.Background(), tt.username, tt.password) - require.Equal(t, !tt.wantToSkipDial, dialWasAttempted) + switch { + case tt.wantError != "": + require.EqualError(t, err, tt.wantError) + require.False(t, authenticated) + require.Nil(t, authResponse) + case tt.wantUnauthenticated: + require.NoError(t, err) + require.False(t, authenticated) + require.Nil(t, authResponse) + default: + require.NoError(t, err) + require.True(t, authenticated) + require.Equal(t, tt.wantAuthResponse, authResponse) + } + // DryRunAuthenticateUser() should have the same behavior as AuthenticateUser() except that it does not bind + // as the end user to confirm their password. Since it should behave the same, all of the same test cases + // apply, except for those which are specifically testing what happens when the end user bind fails. + if tt.skipDryRunAuthenticateUser { + return // move on to the next test + } + + // Reset some variables to get ready to call DryRunAuthenticateUser(). + dialWasAttempted = false + conn = mockldapconn.NewMockConn(ctrl) + if tt.searchMocks != nil { + tt.searchMocks(conn) + } + // Skip tt.bindEndUserMocks since DryRunAuthenticateUser() never binds as the end user. + + authResponse, authenticated, err = provider.DryRunAuthenticateUser(context.Background(), tt.username) + require.Equal(t, !tt.wantToSkipDial, dialWasAttempted) switch { case tt.wantError != "": require.EqualError(t, err, tt.wantError) diff --git a/test/deploy/tools/ldap.yaml b/test/deploy/tools/ldap.yaml index e98abccb..affb7c61 100644 --- a/test/deploy/tools/ldap.yaml +++ b/test/deploy/tools/ldap.yaml @@ -2,6 +2,122 @@ #! SPDX-License-Identifier: Apache-2.0 #@ load("@ytt:data", "data") +#@ load("@ytt:sha256", "sha256") +#@ load("@ytt:yaml", "yaml") + +#@ def ldapLIDIF(): +#@yaml/text-templated-strings +ldap.ldif: | + # ** CAUTION: Blank lines separate entries in the LDIF format! Do not remove them! *** + # Here's a good explanation of LDIF: + # https://www.digitalocean.com/community/tutorials/how-to-use-ldif-files-to-make-changes-to-an-openldap-system + + # pinniped.dev (organization, root) + dn: dc=pinniped,dc=dev + objectClass: dcObject + objectClass: organization + dc: pinniped + o: example + + # users, pinniped.dev (organization unit) + dn: ou=users,dc=pinniped,dc=dev + objectClass: organizationalUnit + ou: users + + # groups, pinniped.dev (organization unit) + dn: ou=groups,dc=pinniped,dc=dev + objectClass: organizationalUnit + ou: groups + + # beach-groups, groups, pinniped.dev (organization unit) + dn: ou=beach-groups,ou=groups,dc=pinniped,dc=dev + objectClass: organizationalUnit + ou: beach-groups + + # pinny, users, pinniped.dev (user) + dn: cn=pinny,ou=users,dc=pinniped,dc=dev + objectClass: inetOrgPerson + objectClass: posixAccount + objectClass: shadowAccount + cn: pinny + sn: Seal + givenName: Pinny + mail: pinny.ldap@example.com + userPassword: (@= data.values.pinny_ldap_password @) + uid: pinny + uidNumber: 1000 + gidNumber: 1000 + homeDirectory: /home/pinny + loginShell: /bin/bash + gecos: pinny-the-seal + + # wally, users, pinniped.dev (user without password) + dn: cn=wally,ou=users,dc=pinniped,dc=dev + objectClass: inetOrgPerson + objectClass: posixAccount + objectClass: shadowAccount + cn: wally + sn: Walrus + givenName: Wally + mail: wally.ldap@example.com + mail: wally.alternate@example.com + uid: wally + uidNumber: 1001 + gidNumber: 1001 + homeDirectory: /home/wally + loginShell: /bin/bash + gecos: wally-the-walrus + + # olive, users, pinniped.dev (user without password) + dn: cn=olive,ou=users,dc=pinniped,dc=dev + objectClass: inetOrgPerson + objectClass: posixAccount + objectClass: shadowAccount + cn: olive + sn: Boston Terrier + givenName: Olive + mail: olive.ldap@example.com + uid: olive + uidNumber: 1002 + gidNumber: 1002 + homeDirectory: /home/olive + loginShell: /bin/bash + gecos: olive-the-dog + + # ball-game-players, beach-groups, groups, pinniped.dev (group of users) + dn: cn=ball-game-players,ou=beach-groups,ou=groups,dc=pinniped,dc=dev + cn: ball-game-players + objectClass: groupOfNames + member: cn=pinny,ou=users,dc=pinniped,dc=dev + member: cn=olive,ou=users,dc=pinniped,dc=dev + + # seals, groups, pinniped.dev (group of users) + dn: cn=seals,ou=groups,dc=pinniped,dc=dev + cn: seals + objectClass: groupOfNames + member: cn=pinny,ou=users,dc=pinniped,dc=dev + + # walruses, groups, pinniped.dev (group of users) + dn: cn=walruses,ou=groups,dc=pinniped,dc=dev + cn: walruses + objectClass: groupOfNames + member: cn=wally,ou=users,dc=pinniped,dc=dev + + # pinnipeds, users, pinniped.dev (group of groups) + dn: cn=pinnipeds,ou=groups,dc=pinniped,dc=dev + cn: pinnipeds + objectClass: groupOfNames + member: cn=seals,ou=groups,dc=pinniped,dc=dev + member: cn=walruses,ou=groups,dc=pinniped,dc=dev + + # mammals, groups, pinniped.dev (group of both groups and users) + dn: cn=mammals,ou=groups,dc=pinniped,dc=dev + cn: mammals + objectClass: groupOfNames + member: cn=pinninpeds,ou=groups,dc=pinniped,dc=dev + member: cn=olive,ou=users,dc=pinniped,dc=dev +#@ end + --- apiVersion: v1 kind: Secret @@ -9,117 +125,7 @@ metadata: name: ldap-ldif-files namespace: tools type: Opaque -stringData: - #@yaml/text-templated-strings - ldap.ldif: | - # ** CAUTION: Blank lines separate entries in the LDIF format! Do not remove them! *** - # Here's a good explanation of LDIF: - # https://www.digitalocean.com/community/tutorials/how-to-use-ldif-files-to-make-changes-to-an-openldap-system - - # pinniped.dev (organization, root) - dn: dc=pinniped,dc=dev - objectClass: dcObject - objectClass: organization - dc: pinniped - o: example - - # users, pinniped.dev (organization unit) - dn: ou=users,dc=pinniped,dc=dev - objectClass: organizationalUnit - ou: users - - # groups, pinniped.dev (organization unit) - dn: ou=groups,dc=pinniped,dc=dev - objectClass: organizationalUnit - ou: groups - - # beach-groups, groups, pinniped.dev (organization unit) - dn: ou=beach-groups,ou=groups,dc=pinniped,dc=dev - objectClass: organizationalUnit - ou: beach-groups - - # pinny, users, pinniped.dev (user) - dn: cn=pinny,ou=users,dc=pinniped,dc=dev - objectClass: inetOrgPerson - objectClass: posixAccount - objectClass: shadowAccount - cn: pinny - sn: Seal - givenName: Pinny - mail: pinny.ldap@example.com - userPassword: (@= data.values.pinny_ldap_password @) - uid: pinny - uidNumber: 1000 - gidNumber: 1000 - homeDirectory: /home/pinny - loginShell: /bin/bash - gecos: pinny-the-seal - - # wally, users, pinniped.dev (user without password) - dn: cn=wally,ou=users,dc=pinniped,dc=dev - objectClass: inetOrgPerson - objectClass: posixAccount - objectClass: shadowAccount - cn: wally - sn: Walrus - givenName: Wally - mail: wally.ldap@example.com - mail: wally.alternate@example.com - uid: wally - uidNumber: 1001 - gidNumber: 1001 - homeDirectory: /home/wally - loginShell: /bin/bash - gecos: wally-the-walrus - - # olive, users, pinniped.dev (user without password) - dn: cn=olive,ou=users,dc=pinniped,dc=dev - objectClass: inetOrgPerson - objectClass: posixAccount - objectClass: shadowAccount - cn: olive - sn: Boston Terrier - givenName: Olive - mail: olive.ldap@example.com - uid: olive - uidNumber: 1002 - gidNumber: 1002 - homeDirectory: /home/olive - loginShell: /bin/bash - gecos: olive-the-dog - - # ball-game-players, beach-groups, groups, pinniped.dev (group of users) - dn: cn=ball-game-players,ou=beach-groups,ou=groups,dc=pinniped,dc=dev - cn: ball-game-players - objectClass: groupOfNames - member: cn=pinny,ou=users,dc=pinniped,dc=dev - member: cn=olive,ou=users,dc=pinniped,dc=dev - - # seals, groups, pinniped.dev (group of users) - dn: cn=seals,ou=groups,dc=pinniped,dc=dev - cn: seals - objectClass: groupOfNames - member: cn=pinny,ou=users,dc=pinniped,dc=dev - - # walruses, groups, pinniped.dev (group of users) - dn: cn=walruses,ou=groups,dc=pinniped,dc=dev - cn: walruses - objectClass: groupOfNames - member: cn=wally,ou=users,dc=pinniped,dc=dev - - # pinnipeds, users, pinniped.dev (group of groups) - dn: cn=pinnipeds,ou=groups,dc=pinniped,dc=dev - cn: pinnipeds - objectClass: groupOfNames - member: cn=seals,ou=groups,dc=pinniped,dc=dev - member: cn=walruses,ou=groups,dc=pinniped,dc=dev - - # mammals, groups, pinniped.dev (group of both groups and users) - dn: cn=mammals,ou=groups,dc=pinniped,dc=dev - cn: mammals - objectClass: groupOfNames - member: cn=pinninpeds,ou=groups,dc=pinniped,dc=dev - member: cn=olive,ou=users,dc=pinniped,dc=dev +stringData: #@ ldapLIDIF() --- apiVersion: apps/v1 kind: Deployment @@ -137,6 +143,9 @@ spec: metadata: labels: app: ldap + annotations: + #! Cause the pod to get recreated whenever the LDIF file changes. + ldifConfigHash: #@ sha256.sum(yaml.encode(ldapLIDIF())) spec: containers: - name: ldap diff --git a/test/integration/supervisor_login_test.go b/test/integration/supervisor_login_test.go index 2ee9f8b4..ba47ed0d 100644 --- a/test/integration/supervisor_login_test.go +++ b/test/integration/supervisor_login_test.go @@ -66,9 +66,8 @@ func TestSupervisorLogin(t *testing.T) { // the ID token Username should include the upstream user ID after the upstream issuer name wantDownstreamIDTokenUsernameToMatch: regexp.QuoteMeta(env.SupervisorUpstreamOIDC.Issuer+"?sub=") + ".+", }, - // TODO add more variations of this LDAP test to try using different user search filters and attributes { - name: "ldap", + name: "ldap with email as username and with dry run", createIDP: func(t *testing.T) { t.Helper() secret := library.CreateTestSecret(t, env.SupervisorNamespace, "ldap-service-account", v1.SecretTypeBasicAuth, @@ -77,7 +76,7 @@ func TestSupervisorLogin(t *testing.T) { v1.BasicAuthPasswordKey: env.SupervisorUpstreamLDAP.BindPassword, }, ) - library.CreateTestLDAPIdentityProvider(t, idpv1alpha1.LDAPIdentityProviderSpec{ + ldapIDP := library.CreateTestLDAPIdentityProvider(t, idpv1alpha1.LDAPIdentityProviderSpec{ Host: env.SupervisorUpstreamLDAP.Host, TLS: &idpv1alpha1.LDAPIdentityProviderTLSSpec{ CertificateAuthorityData: base64.StdEncoding.EncodeToString([]byte(env.SupervisorUpstreamLDAP.CABundle)), @@ -93,7 +92,15 @@ func TestSupervisorLogin(t *testing.T) { UniqueID: env.SupervisorUpstreamLDAP.TestUserUniqueIDAttributeName, }, }, + DryRunAuthenticationUsername: env.SupervisorUpstreamLDAP.TestUserMailAttributeValue, }, idpv1alpha1.LDAPPhaseReady) + expectedMsg := fmt.Sprintf( + `successful authentication dry run for end user "%s": selected username "%s" and UID "%s" [validated with Secret "%s" at version "%s"]`, + env.SupervisorUpstreamLDAP.TestUserMailAttributeValue, env.SupervisorUpstreamLDAP.TestUserMailAttributeValue, + env.SupervisorUpstreamLDAP.TestUserUniqueIDAttributeValue, + secret.Name, secret.ResourceVersion, + ) + requireSuccessfulLDAPIdentityProviderConditions(t, ldapIDP, expectedMsg) }, requestAuthorization: func(t *testing.T, downstreamAuthorizeURL, _ string, httpClient *http.Client) { requestAuthorizationUsingLDAPIdentityProvider(t, @@ -110,6 +117,56 @@ func TestSupervisorLogin(t *testing.T) { // the ID token Username should have been pulled from the requested UserSearch.Attributes.Username attribute wantDownstreamIDTokenUsernameToMatch: regexp.QuoteMeta(env.SupervisorUpstreamLDAP.TestUserMailAttributeValue), }, + { + name: "ldap with CN as username and without dry run", // try another variation of configuration options + createIDP: func(t *testing.T) { + t.Helper() + secret := library.CreateTestSecret(t, env.SupervisorNamespace, "ldap-service-account", v1.SecretTypeBasicAuth, + map[string]string{ + v1.BasicAuthUsernameKey: env.SupervisorUpstreamLDAP.BindUsername, + v1.BasicAuthPasswordKey: env.SupervisorUpstreamLDAP.BindPassword, + }, + ) + ldapIDP := library.CreateTestLDAPIdentityProvider(t, idpv1alpha1.LDAPIdentityProviderSpec{ + Host: env.SupervisorUpstreamLDAP.Host, + TLS: &idpv1alpha1.LDAPIdentityProviderTLSSpec{ + CertificateAuthorityData: base64.StdEncoding.EncodeToString([]byte(env.SupervisorUpstreamLDAP.CABundle)), + }, + Bind: idpv1alpha1.LDAPIdentityProviderBindSpec{ + SecretName: secret.Name, + }, + UserSearch: idpv1alpha1.LDAPIdentityProviderUserSearchSpec{ + Base: env.SupervisorUpstreamLDAP.UserSearchBase, + Filter: "cn={}", // try using a non-default search filter + Attributes: idpv1alpha1.LDAPIdentityProviderUserSearchAttributesSpec{ + Username: "dn", // try using the user's DN as the downstream username + UniqueID: env.SupervisorUpstreamLDAP.TestUserUniqueIDAttributeName, + }, + }, + DryRunAuthenticationUsername: "", // try without dry run + }, idpv1alpha1.LDAPPhaseReady) + expectedMsg := fmt.Sprintf( + `successfully able to connect to "%s" and bind as user "%s" [validated with Secret "%s" at version "%s"]`, + env.SupervisorUpstreamLDAP.Host, env.SupervisorUpstreamLDAP.BindUsername, + secret.Name, secret.ResourceVersion, + ) + requireSuccessfulLDAPIdentityProviderConditions(t, ldapIDP, expectedMsg) + }, + requestAuthorization: func(t *testing.T, downstreamAuthorizeURL, _ string, httpClient *http.Client) { + requestAuthorizationUsingLDAPIdentityProvider(t, + downstreamAuthorizeURL, + env.SupervisorUpstreamLDAP.TestUserCN, // username to present to server during login + env.SupervisorUpstreamLDAP.TestUserPassword, // password to present to server during login + httpClient, + ) + }, + // the ID token Subject should be the Host URL plus the value pulled from the requested UserSearch.Attributes.UID attribute + wantDownstreamIDTokenSubjectToMatch: regexp.QuoteMeta( + "ldaps://" + env.SupervisorUpstreamLDAP.Host + "?sub=" + env.SupervisorUpstreamLDAP.TestUserUniqueIDAttributeValue, + ), + // the ID token Username should have been pulled from the requested UserSearch.Attributes.Username attribute + wantDownstreamIDTokenUsernameToMatch: regexp.QuoteMeta(env.SupervisorUpstreamLDAP.TestUserDN), + }, } for _, test := range tests { test := test @@ -124,6 +181,31 @@ func TestSupervisorLogin(t *testing.T) { } } +func requireSuccessfulLDAPIdentityProviderConditions(t *testing.T, ldapIDP *idpv1alpha1.LDAPIdentityProvider, expectedLDAPConnectionValidMessage string) { + require.Len(t, ldapIDP.Status.Conditions, 3) + + conditionsSummary := [][]string{} + for _, condition := range ldapIDP.Status.Conditions { + conditionsSummary = append(conditionsSummary, []string{condition.Type, string(condition.Status), condition.Reason}) + t.Logf("Saw LDAPIdentityProvider Status.Condition Type=%s Status=%s Reason=%s Message=%s", + condition.Type, string(condition.Status), condition.Reason, condition.Message) + switch condition.Type { + case "BindSecretValid": + require.Equal(t, "loaded bind secret", condition.Message) + case "TLSConfigurationValid": + require.Equal(t, "loaded TLS configuration", condition.Message) + case "LDAPConnectionValid": + require.Equal(t, expectedLDAPConnectionValidMessage, condition.Message) + } + } + + require.ElementsMatch(t, [][]string{ + {"BindSecretValid", "True", "Success"}, + {"TLSConfigurationValid", "True", "Success"}, + {"LDAPConnectionValid", "True", "Success"}, + }, conditionsSummary) +} + func testSupervisorLogin( t *testing.T, createIDP func(t *testing.T),