From f0b6a961fdfdafab2a9e8134fca6ef8493c60d54 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Maciej=20Kara=C5=9B?= Date: Fri, 25 Apr 2025 11:45:31 +0200 Subject: [PATCH 01/36] CRD changes --- api/v1/mdb/mongodb_types.go | 85 ++++++- api/v1/mdb/mongodb_validation.go | 2 +- api/v1/mdbmulti/mongodb_multi_types.go | 10 +- config/crd/bases/mongodb.com_mongodb.yaml | 74 ++++++ .../mongodb.com_mongodbmulticluster.yaml | 74 ++++++ config/crd/bases/mongodb.com_opsmanagers.yaml | 74 ++++++ helm_chart/crds/mongodb.com_mongodb.yaml | 74 ++++++ .../crds/mongodb.com_mongodbmulticluster.yaml | 74 ++++++ helm_chart/crds/mongodb.com_opsmanagers.yaml | 74 ++++++ pkg/util/constants.go | 1 + public/crds.yaml | 222 ++++++++++++++++++ 11 files changed, 757 insertions(+), 7 deletions(-) diff --git a/api/v1/mdb/mongodb_types.go b/api/v1/mdb/mongodb_types.go index 9cf672d4e..21929bb90 100644 --- a/api/v1/mdb/mongodb_types.go +++ b/api/v1/mdb/mongodb_types.go @@ -57,6 +57,12 @@ const ( ClusterTopologySingleCluster = "SingleCluster" ClusterTopologyMultiCluster = "MultiCluster" + OIDCAuthorizationTypeGroupMembership = "GroupMembership" + OIDCAuthorizationTypeUserID = "UserID" + + OIDCAuthorizationMethodWorkforceIdentityFederation = "WorkforceIdentityFederation" + OIDCAuthorizationMethodWorkloadIdentityFederation = "WorkloadIdentityFederation" + LabelResourceOwner = "mongodb.com/v1.mongodbResourceOwner" ) @@ -878,7 +884,7 @@ func (s Security) RequiresClientTLSAuthentication() bool { return false } - if len(s.Authentication.Modes) == 1 && IsAuthPresent(s.Authentication.Modes, util.X509) { + if len(s.Authentication.Modes) == 1 && s.Authentication.IsX509Enabled() { return true } @@ -912,6 +918,8 @@ type Authentication struct { // +optional Ldap *Ldap `json:"ldap,omitempty"` + OIDCProviderConfigs []OIDCProviderConfig `json:"oidcProviderConfigs,omitempty"` + // Agents contains authentication configuration properties for the agents // +optional Agents AgentAuthentication `json:"agents,omitempty"` @@ -920,7 +928,7 @@ type Authentication struct { RequiresClientTLSAuthentication bool `json:"requireClientTLSAuthentication,omitempty"` } -// +kubebuilder:validation:Enum=X509;SCRAM;SCRAM-SHA-1;MONGODB-CR;SCRAM-SHA-256;LDAP +// +kubebuilder:validation:Enum=X509;SCRAM;SCRAM-SHA-1;MONGODB-CR;SCRAM-SHA-256;LDAP;OIDC type AuthMode string func ConvertAuthModesToStrings(authModes []AuthMode) []string { @@ -993,10 +1001,15 @@ func (a *Authentication) IsX509Enabled() bool { } // IsLDAPEnabled determines if LDAP is to be enabled at the project level -func (a *Authentication) isLDAPEnabled() bool { +func (a *Authentication) IsLDAPEnabled() bool { return stringutil.Contains(a.GetModes(), util.LDAP) } +// IsOIDCEnabled determines if OIDC is to be enabled at the project level +func (a *Authentication) IsOIDCEnabled() bool { + return stringutil.Contains(a.GetModes(), util.OIDC) +} + // GetModes returns the modes of the Authentication instance of an empty // list if it is nil func (a *Authentication) GetModes() []string { @@ -1033,6 +1046,63 @@ type Ldap struct { UserCacheInvalidationInterval int `json:"userCacheInvalidationInterval"` } +type OIDCProviderConfig struct { + // TODO add proper validation + // Unique label that identifies this configuration. This label is visible to your Ops Manager users and is used when + // creating users and roles for authorization. It is case-sensitive and can only contain the following characters: + // - alphanumeric characters (combination of a to z and 0 to 9) + // - hyphens (-) + // - underscores (_) + ConfigurationName string `json:"configurationName"` + + // TODO add URI validation + // Issuer value provided by your registered IdP application. Using this URI, MongoDB finds an OpenID Provider + // Configuration Document, which should be available in the /.wellknown/open-id-configuration endpoint. + IssuerURI string `json:"issuerURI"` + + // Entity that your external identity provider intends the token for. + // Enter the audience value from the app you registered with external Identity Provider. + Audience string `json:"audience"` + + // Select GroupMembership to grant authorization based on IdP user group membership, or select UserID to grant + // an individual user authorization. + AuthorizationType OIDCAuthorizationType `json:"authorizationType"` + + // The identifier of the claim that includes the user principal identity. + // Accept the default value unless your IdP uses a different claim. + // +default:value:sub + UserClaim string `json:"userClaim"` + + // The identifier of the claim that includes the principal's IdP user group membership information. + // Accept the default value unless your IdP uses a different claim, or you need a custom claim. + // Required when selected GroupMembership as the authorization type, ignored otherwise + // +default:value:groups + // +optional + GroupsClaim string `json:"groupsClaim"` + + // Configure single-sign-on for human user access to Ops Manager deployments with Workforce Identity Federation. + // For programmatic, application access to Ops Manager deployments use Workload Identity Federation. + // Only one Workforce Identity Federation IdP can be configured per MongoDB resource + AuthorizationMethod OIDCAuthorizationMethod `json:"authorizationMethod"` + + // Unique identifier for your registered application. Enter the clientId value from the app you + // registered with an external Identity Provider. + // Required when selected Workforce Identity Federation authorization method + // +optional + ClientId string `json:"clientId"` + + // Tokens that give users permission to request data from the authorization endpoint. + // Only used for Workforce Identity Federation authorization method + // +optional + RequestedScopes []string `json:"requestedScopes"` +} + +// +kubebuilder:validation:Enum=GroupMembership;UserID +type OIDCAuthorizationType string + +// +kubebuilder:validation:Enum=WorkforceIdentityFederation;WorkloadIdentityFederation +type OIDCAuthorizationMethod string + type SecretRef struct { // +kubebuilder:validation:Required Name string `json:"name"` @@ -1142,7 +1212,14 @@ func (m *MongoDB) IsLDAPEnabled() bool { if m.Spec.Security == nil || m.Spec.Security.Authentication == nil { return false } - return IsAuthPresent(m.Spec.Security.Authentication.Modes, util.LDAP) + return m.Spec.Security.Authentication.IsLDAPEnabled() +} + +func (m *MongoDB) IsOIDCEnabled() bool { + if m.Spec.Security == nil || m.Spec.Security.Authentication == nil { + return false + } + return m.Spec.Security.Authentication.IsOIDCEnabled() } func (m *MongoDB) UpdateStatus(phase status.Phase, statusOptions ...status.Option) { diff --git a/api/v1/mdb/mongodb_validation.go b/api/v1/mdb/mongodb_validation.go index e0d16dfdd..9b811cf48 100644 --- a/api/v1/mdb/mongodb_validation.go +++ b/api/v1/mdb/mongodb_validation.go @@ -107,7 +107,7 @@ func scramSha1AuthValidation(d DbCommonSpec) v1.ValidationResult { func ldapAuthRequiresEnterprise(d DbCommonSpec) v1.ValidationResult { authSpec := d.Security.Authentication - if authSpec != nil && authSpec.isLDAPEnabled() && !strings.HasSuffix(d.Version, "-ent") { + if authSpec != nil && authSpec.IsLDAPEnabled() && !strings.HasSuffix(d.Version, "-ent") { return v1.ValidationError("Cannot enable LDAP authentication with MongoDB Community Builds") } return v1.ValidationSuccess() diff --git a/api/v1/mdbmulti/mongodb_multi_types.go b/api/v1/mdbmulti/mongodb_multi_types.go index 3768b99ad..efb15c567 100644 --- a/api/v1/mdbmulti/mongodb_multi_types.go +++ b/api/v1/mdbmulti/mongodb_multi_types.go @@ -23,7 +23,6 @@ import ( "github.com/mongodb/mongodb-kubernetes/pkg/multicluster/failedcluster" "github.com/mongodb/mongodb-kubernetes/pkg/util" intp "github.com/mongodb/mongodb-kubernetes/pkg/util/int" - "github.com/mongodb/mongodb-kubernetes/pkg/util/stringutil" ) func init() { @@ -123,7 +122,14 @@ func (m *MongoDBMultiCluster) IsLDAPEnabled() bool { if m.Spec.Security == nil || m.Spec.Security.Authentication == nil { return false } - return stringutil.Contains(m.Spec.GetSecurityAuthenticationModes(), util.LDAP) + return m.Spec.Security.Authentication.IsLDAPEnabled() +} + +func (m *MongoDBMultiCluster) IsOIDCEnabled() bool { + if m.Spec.Security == nil || m.Spec.Security.Authentication == nil { + return false + } + return m.Spec.Security.Authentication.IsOIDCEnabled() } func (m *MongoDBMultiCluster) GetLDAP(password, caContents string) *ldap.Ldap { diff --git a/config/crd/bases/mongodb.com_mongodb.yaml b/config/crd/bases/mongodb.com_mongodb.yaml index a5b0287c2..d93468b13 100644 --- a/config/crd/bases/mongodb.com_mongodb.yaml +++ b/config/crd/bases/mongodb.com_mongodb.yaml @@ -1521,8 +1521,82 @@ spec: - MONGODB-CR - SCRAM-SHA-256 - LDAP + - OIDC type: string type: array + oidcProviderConfigs: + items: + properties: + audience: + description: |- + Entity that your external identity provider intends the token for. + Enter the audience value from the app you registered with external Identity Provider. + type: string + authorizationMethod: + description: |- + Configure single-sign-on for human user access to Ops Manager deployments with Workforce Identity Federation. + For programmatic, application access to Ops Manager deployments use Workload Identity Federation. + Only one Workforce Identity Federation IdP can be configured per MongoDB resource + enum: + - WorkforceIdentityFederation + - WorkloadIdentityFederation + type: string + authorizationType: + description: |- + Select GroupMembership to grant authorization based on IdP user group membership, or select UserID to grant + an individual user authorization. + enum: + - GroupMembership + - UserID + type: string + clientId: + description: |- + Unique identifier for your registered application. Enter the clientId value from the app you + registered with an external Identity Provider. + Required when selected Workforce Identity Federation authorization method + type: string + configurationName: + description: |- + TODO add proper validation + Unique label that identifies this configuration. This label is visible to your Ops Manager users and is used when + creating users and roles for authorization. It is case-sensitive and can only contain the following characters: + - alphanumeric characters (combination of a to z and 0 to 9) + - hyphens (-) + - underscores (_) + type: string + groupsClaim: + description: |- + The identifier of the claim that includes the principal's IdP user group membership information. + Accept the default value unless your IdP uses a different claim, or you need a custom claim. + Required when selected GroupMembership as the authorization type, ignored otherwise + type: string + issuerURI: + description: |- + TODO add URI validation + Issuer value provided by your registered IdP application. Using this URI, MongoDB finds an OpenID Provider + Configuration Document, which should be available in the /.wellknown/open-id-configuration endpoint. + type: string + requestedScopes: + description: |- + Tokens that give users permission to request data from the authorization endpoint. + Only used for Workforce Identity Federation authorization method + items: + type: string + type: array + userClaim: + description: |- + The identifier of the claim that includes the user principal identity. + Accept the default value unless your IdP uses a different claim. + type: string + required: + - audience + - authorizationMethod + - authorizationType + - configurationName + - issuerURI + - userClaim + type: object + type: array requireClientTLSAuthentication: description: Clients should present valid TLS certificates type: boolean diff --git a/config/crd/bases/mongodb.com_mongodbmulticluster.yaml b/config/crd/bases/mongodb.com_mongodbmulticluster.yaml index 0e1d65480..f9fed2293 100644 --- a/config/crd/bases/mongodb.com_mongodbmulticluster.yaml +++ b/config/crd/bases/mongodb.com_mongodbmulticluster.yaml @@ -781,8 +781,82 @@ spec: - MONGODB-CR - SCRAM-SHA-256 - LDAP + - OIDC type: string type: array + oidcProviderConfigs: + items: + properties: + audience: + description: |- + Entity that your external identity provider intends the token for. + Enter the audience value from the app you registered with external Identity Provider. + type: string + authorizationMethod: + description: |- + Configure single-sign-on for human user access to Ops Manager deployments with Workforce Identity Federation. + For programmatic, application access to Ops Manager deployments use Workload Identity Federation. + Only one Workforce Identity Federation IdP can be configured per MongoDB resource + enum: + - WorkforceIdentityFederation + - WorkloadIdentityFederation + type: string + authorizationType: + description: |- + Select GroupMembership to grant authorization based on IdP user group membership, or select UserID to grant + an individual user authorization. + enum: + - GroupMembership + - UserID + type: string + clientId: + description: |- + Unique identifier for your registered application. Enter the clientId value from the app you + registered with an external Identity Provider. + Required when selected Workforce Identity Federation authorization method + type: string + configurationName: + description: |- + TODO add proper validation + Unique label that identifies this configuration. This label is visible to your Ops Manager users and is used when + creating users and roles for authorization. It is case-sensitive and can only contain the following characters: + - alphanumeric characters (combination of a to z and 0 to 9) + - hyphens (-) + - underscores (_) + type: string + groupsClaim: + description: |- + The identifier of the claim that includes the principal's IdP user group membership information. + Accept the default value unless your IdP uses a different claim, or you need a custom claim. + Required when selected GroupMembership as the authorization type, ignored otherwise + type: string + issuerURI: + description: |- + TODO add URI validation + Issuer value provided by your registered IdP application. Using this URI, MongoDB finds an OpenID Provider + Configuration Document, which should be available in the /.wellknown/open-id-configuration endpoint. + type: string + requestedScopes: + description: |- + Tokens that give users permission to request data from the authorization endpoint. + Only used for Workforce Identity Federation authorization method + items: + type: string + type: array + userClaim: + description: |- + The identifier of the claim that includes the user principal identity. + Accept the default value unless your IdP uses a different claim. + type: string + required: + - audience + - authorizationMethod + - authorizationType + - configurationName + - issuerURI + - userClaim + type: object + type: array requireClientTLSAuthentication: description: Clients should present valid TLS certificates type: boolean diff --git a/config/crd/bases/mongodb.com_opsmanagers.yaml b/config/crd/bases/mongodb.com_opsmanagers.yaml index 72c2ce553..adac7f2ae 100644 --- a/config/crd/bases/mongodb.com_opsmanagers.yaml +++ b/config/crd/bases/mongodb.com_opsmanagers.yaml @@ -843,8 +843,82 @@ spec: - MONGODB-CR - SCRAM-SHA-256 - LDAP + - OIDC type: string type: array + oidcProviderConfigs: + items: + properties: + audience: + description: |- + Entity that your external identity provider intends the token for. + Enter the audience value from the app you registered with external Identity Provider. + type: string + authorizationMethod: + description: |- + Configure single-sign-on for human user access to Ops Manager deployments with Workforce Identity Federation. + For programmatic, application access to Ops Manager deployments use Workload Identity Federation. + Only one Workforce Identity Federation IdP can be configured per MongoDB resource + enum: + - WorkforceIdentityFederation + - WorkloadIdentityFederation + type: string + authorizationType: + description: |- + Select GroupMembership to grant authorization based on IdP user group membership, or select UserID to grant + an individual user authorization. + enum: + - GroupMembership + - UserID + type: string + clientId: + description: |- + Unique identifier for your registered application. Enter the clientId value from the app you + registered with an external Identity Provider. + Required when selected Workforce Identity Federation authorization method + type: string + configurationName: + description: |- + TODO add proper validation + Unique label that identifies this configuration. This label is visible to your Ops Manager users and is used when + creating users and roles for authorization. It is case-sensitive and can only contain the following characters: + - alphanumeric characters (combination of a to z and 0 to 9) + - hyphens (-) + - underscores (_) + type: string + groupsClaim: + description: |- + The identifier of the claim that includes the principal's IdP user group membership information. + Accept the default value unless your IdP uses a different claim, or you need a custom claim. + Required when selected GroupMembership as the authorization type, ignored otherwise + type: string + issuerURI: + description: |- + TODO add URI validation + Issuer value provided by your registered IdP application. Using this URI, MongoDB finds an OpenID Provider + Configuration Document, which should be available in the /.wellknown/open-id-configuration endpoint. + type: string + requestedScopes: + description: |- + Tokens that give users permission to request data from the authorization endpoint. + Only used for Workforce Identity Federation authorization method + items: + type: string + type: array + userClaim: + description: |- + The identifier of the claim that includes the user principal identity. + Accept the default value unless your IdP uses a different claim. + type: string + required: + - audience + - authorizationMethod + - authorizationType + - configurationName + - issuerURI + - userClaim + type: object + type: array requireClientTLSAuthentication: description: Clients should present valid TLS certificates type: boolean diff --git a/helm_chart/crds/mongodb.com_mongodb.yaml b/helm_chart/crds/mongodb.com_mongodb.yaml index a5b0287c2..d93468b13 100644 --- a/helm_chart/crds/mongodb.com_mongodb.yaml +++ b/helm_chart/crds/mongodb.com_mongodb.yaml @@ -1521,8 +1521,82 @@ spec: - MONGODB-CR - SCRAM-SHA-256 - LDAP + - OIDC type: string type: array + oidcProviderConfigs: + items: + properties: + audience: + description: |- + Entity that your external identity provider intends the token for. + Enter the audience value from the app you registered with external Identity Provider. + type: string + authorizationMethod: + description: |- + Configure single-sign-on for human user access to Ops Manager deployments with Workforce Identity Federation. + For programmatic, application access to Ops Manager deployments use Workload Identity Federation. + Only one Workforce Identity Federation IdP can be configured per MongoDB resource + enum: + - WorkforceIdentityFederation + - WorkloadIdentityFederation + type: string + authorizationType: + description: |- + Select GroupMembership to grant authorization based on IdP user group membership, or select UserID to grant + an individual user authorization. + enum: + - GroupMembership + - UserID + type: string + clientId: + description: |- + Unique identifier for your registered application. Enter the clientId value from the app you + registered with an external Identity Provider. + Required when selected Workforce Identity Federation authorization method + type: string + configurationName: + description: |- + TODO add proper validation + Unique label that identifies this configuration. This label is visible to your Ops Manager users and is used when + creating users and roles for authorization. It is case-sensitive and can only contain the following characters: + - alphanumeric characters (combination of a to z and 0 to 9) + - hyphens (-) + - underscores (_) + type: string + groupsClaim: + description: |- + The identifier of the claim that includes the principal's IdP user group membership information. + Accept the default value unless your IdP uses a different claim, or you need a custom claim. + Required when selected GroupMembership as the authorization type, ignored otherwise + type: string + issuerURI: + description: |- + TODO add URI validation + Issuer value provided by your registered IdP application. Using this URI, MongoDB finds an OpenID Provider + Configuration Document, which should be available in the /.wellknown/open-id-configuration endpoint. + type: string + requestedScopes: + description: |- + Tokens that give users permission to request data from the authorization endpoint. + Only used for Workforce Identity Federation authorization method + items: + type: string + type: array + userClaim: + description: |- + The identifier of the claim that includes the user principal identity. + Accept the default value unless your IdP uses a different claim. + type: string + required: + - audience + - authorizationMethod + - authorizationType + - configurationName + - issuerURI + - userClaim + type: object + type: array requireClientTLSAuthentication: description: Clients should present valid TLS certificates type: boolean diff --git a/helm_chart/crds/mongodb.com_mongodbmulticluster.yaml b/helm_chart/crds/mongodb.com_mongodbmulticluster.yaml index 0e1d65480..f9fed2293 100644 --- a/helm_chart/crds/mongodb.com_mongodbmulticluster.yaml +++ b/helm_chart/crds/mongodb.com_mongodbmulticluster.yaml @@ -781,8 +781,82 @@ spec: - MONGODB-CR - SCRAM-SHA-256 - LDAP + - OIDC type: string type: array + oidcProviderConfigs: + items: + properties: + audience: + description: |- + Entity that your external identity provider intends the token for. + Enter the audience value from the app you registered with external Identity Provider. + type: string + authorizationMethod: + description: |- + Configure single-sign-on for human user access to Ops Manager deployments with Workforce Identity Federation. + For programmatic, application access to Ops Manager deployments use Workload Identity Federation. + Only one Workforce Identity Federation IdP can be configured per MongoDB resource + enum: + - WorkforceIdentityFederation + - WorkloadIdentityFederation + type: string + authorizationType: + description: |- + Select GroupMembership to grant authorization based on IdP user group membership, or select UserID to grant + an individual user authorization. + enum: + - GroupMembership + - UserID + type: string + clientId: + description: |- + Unique identifier for your registered application. Enter the clientId value from the app you + registered with an external Identity Provider. + Required when selected Workforce Identity Federation authorization method + type: string + configurationName: + description: |- + TODO add proper validation + Unique label that identifies this configuration. This label is visible to your Ops Manager users and is used when + creating users and roles for authorization. It is case-sensitive and can only contain the following characters: + - alphanumeric characters (combination of a to z and 0 to 9) + - hyphens (-) + - underscores (_) + type: string + groupsClaim: + description: |- + The identifier of the claim that includes the principal's IdP user group membership information. + Accept the default value unless your IdP uses a different claim, or you need a custom claim. + Required when selected GroupMembership as the authorization type, ignored otherwise + type: string + issuerURI: + description: |- + TODO add URI validation + Issuer value provided by your registered IdP application. Using this URI, MongoDB finds an OpenID Provider + Configuration Document, which should be available in the /.wellknown/open-id-configuration endpoint. + type: string + requestedScopes: + description: |- + Tokens that give users permission to request data from the authorization endpoint. + Only used for Workforce Identity Federation authorization method + items: + type: string + type: array + userClaim: + description: |- + The identifier of the claim that includes the user principal identity. + Accept the default value unless your IdP uses a different claim. + type: string + required: + - audience + - authorizationMethod + - authorizationType + - configurationName + - issuerURI + - userClaim + type: object + type: array requireClientTLSAuthentication: description: Clients should present valid TLS certificates type: boolean diff --git a/helm_chart/crds/mongodb.com_opsmanagers.yaml b/helm_chart/crds/mongodb.com_opsmanagers.yaml index 72c2ce553..adac7f2ae 100644 --- a/helm_chart/crds/mongodb.com_opsmanagers.yaml +++ b/helm_chart/crds/mongodb.com_opsmanagers.yaml @@ -843,8 +843,82 @@ spec: - MONGODB-CR - SCRAM-SHA-256 - LDAP + - OIDC type: string type: array + oidcProviderConfigs: + items: + properties: + audience: + description: |- + Entity that your external identity provider intends the token for. + Enter the audience value from the app you registered with external Identity Provider. + type: string + authorizationMethod: + description: |- + Configure single-sign-on for human user access to Ops Manager deployments with Workforce Identity Federation. + For programmatic, application access to Ops Manager deployments use Workload Identity Federation. + Only one Workforce Identity Federation IdP can be configured per MongoDB resource + enum: + - WorkforceIdentityFederation + - WorkloadIdentityFederation + type: string + authorizationType: + description: |- + Select GroupMembership to grant authorization based on IdP user group membership, or select UserID to grant + an individual user authorization. + enum: + - GroupMembership + - UserID + type: string + clientId: + description: |- + Unique identifier for your registered application. Enter the clientId value from the app you + registered with an external Identity Provider. + Required when selected Workforce Identity Federation authorization method + type: string + configurationName: + description: |- + TODO add proper validation + Unique label that identifies this configuration. This label is visible to your Ops Manager users and is used when + creating users and roles for authorization. It is case-sensitive and can only contain the following characters: + - alphanumeric characters (combination of a to z and 0 to 9) + - hyphens (-) + - underscores (_) + type: string + groupsClaim: + description: |- + The identifier of the claim that includes the principal's IdP user group membership information. + Accept the default value unless your IdP uses a different claim, or you need a custom claim. + Required when selected GroupMembership as the authorization type, ignored otherwise + type: string + issuerURI: + description: |- + TODO add URI validation + Issuer value provided by your registered IdP application. Using this URI, MongoDB finds an OpenID Provider + Configuration Document, which should be available in the /.wellknown/open-id-configuration endpoint. + type: string + requestedScopes: + description: |- + Tokens that give users permission to request data from the authorization endpoint. + Only used for Workforce Identity Federation authorization method + items: + type: string + type: array + userClaim: + description: |- + The identifier of the claim that includes the user principal identity. + Accept the default value unless your IdP uses a different claim. + type: string + required: + - audience + - authorizationMethod + - authorizationType + - configurationName + - issuerURI + - userClaim + type: object + type: array requireClientTLSAuthentication: description: Clients should present valid TLS certificates type: boolean diff --git a/pkg/util/constants.go b/pkg/util/constants.go index 77cfbb57c..415b334ff 100644 --- a/pkg/util/constants.go +++ b/pkg/util/constants.go @@ -147,6 +147,7 @@ const ( MONGODBCR = "MONGODB-CR" SCRAMSHA256 = "SCRAM-SHA-256" LDAP = "LDAP" + OIDC = "OIDC" MinimumScramSha256MdbVersion = "4.0.0" // these were historically used and constituted a security issue—if set they should be changed diff --git a/public/crds.yaml b/public/crds.yaml index 37c001ac6..68ee6ff03 100644 --- a/public/crds.yaml +++ b/public/crds.yaml @@ -1521,8 +1521,82 @@ spec: - MONGODB-CR - SCRAM-SHA-256 - LDAP + - OIDC type: string type: array + oidcProviderConfigs: + items: + properties: + audience: + description: |- + Entity that your external identity provider intends the token for. + Enter the audience value from the app you registered with external Identity Provider. + type: string + authorizationMethod: + description: |- + Configure single-sign-on for human user access to Ops Manager deployments with Workforce Identity Federation. + For programmatic, application access to Ops Manager deployments use Workload Identity Federation. + Only one Workforce Identity Federation IdP can be configured per MongoDB resource + enum: + - WorkforceIdentityFederation + - WorkloadIdentityFederation + type: string + authorizationType: + description: |- + Select GroupMembership to grant authorization based on IdP user group membership, or select UserID to grant + an individual user authorization. + enum: + - GroupMembership + - UserID + type: string + clientId: + description: |- + Unique identifier for your registered application. Enter the clientId value from the app you + registered with an external Identity Provider. + Required when selected Workforce Identity Federation authorization method + type: string + configurationName: + description: |- + TODO add proper validation + Unique label that identifies this configuration. This label is visible to your Ops Manager users and is used when + creating users and roles for authorization. It is case-sensitive and can only contain the following characters: + - alphanumeric characters (combination of a to z and 0 to 9) + - hyphens (-) + - underscores (_) + type: string + groupsClaim: + description: |- + The identifier of the claim that includes the principal's IdP user group membership information. + Accept the default value unless your IdP uses a different claim, or you need a custom claim. + Required when selected GroupMembership as the authorization type, ignored otherwise + type: string + issuerURI: + description: |- + TODO add URI validation + Issuer value provided by your registered IdP application. Using this URI, MongoDB finds an OpenID Provider + Configuration Document, which should be available in the /.wellknown/open-id-configuration endpoint. + type: string + requestedScopes: + description: |- + Tokens that give users permission to request data from the authorization endpoint. + Only used for Workforce Identity Federation authorization method + items: + type: string + type: array + userClaim: + description: |- + The identifier of the claim that includes the user principal identity. + Accept the default value unless your IdP uses a different claim. + type: string + required: + - audience + - authorizationMethod + - authorizationType + - configurationName + - issuerURI + - userClaim + type: object + type: array requireClientTLSAuthentication: description: Clients should present valid TLS certificates type: boolean @@ -4081,8 +4155,82 @@ spec: - MONGODB-CR - SCRAM-SHA-256 - LDAP + - OIDC type: string type: array + oidcProviderConfigs: + items: + properties: + audience: + description: |- + Entity that your external identity provider intends the token for. + Enter the audience value from the app you registered with external Identity Provider. + type: string + authorizationMethod: + description: |- + Configure single-sign-on for human user access to Ops Manager deployments with Workforce Identity Federation. + For programmatic, application access to Ops Manager deployments use Workload Identity Federation. + Only one Workforce Identity Federation IdP can be configured per MongoDB resource + enum: + - WorkforceIdentityFederation + - WorkloadIdentityFederation + type: string + authorizationType: + description: |- + Select GroupMembership to grant authorization based on IdP user group membership, or select UserID to grant + an individual user authorization. + enum: + - GroupMembership + - UserID + type: string + clientId: + description: |- + Unique identifier for your registered application. Enter the clientId value from the app you + registered with an external Identity Provider. + Required when selected Workforce Identity Federation authorization method + type: string + configurationName: + description: |- + TODO add proper validation + Unique label that identifies this configuration. This label is visible to your Ops Manager users and is used when + creating users and roles for authorization. It is case-sensitive and can only contain the following characters: + - alphanumeric characters (combination of a to z and 0 to 9) + - hyphens (-) + - underscores (_) + type: string + groupsClaim: + description: |- + The identifier of the claim that includes the principal's IdP user group membership information. + Accept the default value unless your IdP uses a different claim, or you need a custom claim. + Required when selected GroupMembership as the authorization type, ignored otherwise + type: string + issuerURI: + description: |- + TODO add URI validation + Issuer value provided by your registered IdP application. Using this URI, MongoDB finds an OpenID Provider + Configuration Document, which should be available in the /.wellknown/open-id-configuration endpoint. + type: string + requestedScopes: + description: |- + Tokens that give users permission to request data from the authorization endpoint. + Only used for Workforce Identity Federation authorization method + items: + type: string + type: array + userClaim: + description: |- + The identifier of the claim that includes the user principal identity. + Accept the default value unless your IdP uses a different claim. + type: string + required: + - audience + - authorizationMethod + - authorizationType + - configurationName + - issuerURI + - userClaim + type: object + type: array requireClientTLSAuthentication: description: Clients should present valid TLS certificates type: boolean @@ -5396,8 +5544,82 @@ spec: - MONGODB-CR - SCRAM-SHA-256 - LDAP + - OIDC type: string type: array + oidcProviderConfigs: + items: + properties: + audience: + description: |- + Entity that your external identity provider intends the token for. + Enter the audience value from the app you registered with external Identity Provider. + type: string + authorizationMethod: + description: |- + Configure single-sign-on for human user access to Ops Manager deployments with Workforce Identity Federation. + For programmatic, application access to Ops Manager deployments use Workload Identity Federation. + Only one Workforce Identity Federation IdP can be configured per MongoDB resource + enum: + - WorkforceIdentityFederation + - WorkloadIdentityFederation + type: string + authorizationType: + description: |- + Select GroupMembership to grant authorization based on IdP user group membership, or select UserID to grant + an individual user authorization. + enum: + - GroupMembership + - UserID + type: string + clientId: + description: |- + Unique identifier for your registered application. Enter the clientId value from the app you + registered with an external Identity Provider. + Required when selected Workforce Identity Federation authorization method + type: string + configurationName: + description: |- + TODO add proper validation + Unique label that identifies this configuration. This label is visible to your Ops Manager users and is used when + creating users and roles for authorization. It is case-sensitive and can only contain the following characters: + - alphanumeric characters (combination of a to z and 0 to 9) + - hyphens (-) + - underscores (_) + type: string + groupsClaim: + description: |- + The identifier of the claim that includes the principal's IdP user group membership information. + Accept the default value unless your IdP uses a different claim, or you need a custom claim. + Required when selected GroupMembership as the authorization type, ignored otherwise + type: string + issuerURI: + description: |- + TODO add URI validation + Issuer value provided by your registered IdP application. Using this URI, MongoDB finds an OpenID Provider + Configuration Document, which should be available in the /.wellknown/open-id-configuration endpoint. + type: string + requestedScopes: + description: |- + Tokens that give users permission to request data from the authorization endpoint. + Only used for Workforce Identity Federation authorization method + items: + type: string + type: array + userClaim: + description: |- + The identifier of the claim that includes the user principal identity. + Accept the default value unless your IdP uses a different claim. + type: string + required: + - audience + - authorizationMethod + - authorizationType + - configurationName + - issuerURI + - userClaim + type: object + type: array requireClientTLSAuthentication: description: Clients should present valid TLS certificates type: boolean From d344589983b1ef1b69169c5594c78290ee8b0813 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Maciej=20Kara=C5=9B?= Date: Thu, 24 Apr 2025 14:18:04 +0200 Subject: [PATCH 02/36] Authorization package refactor - part 1 --- .../operator/authentication/authentication.go | 136 ++++++++-------- .../configure_authentication_test.go | 38 +++-- controllers/operator/authentication/ldap.go | 72 ++++----- .../operator/authentication/ldap_test.go | 47 +++--- .../operator/authentication/scramsha.go | 146 ++++++------------ .../operator/authentication/scramsha_test.go | 55 +++---- controllers/operator/authentication/x509.go | 69 ++++----- .../operator/authentication/x509_test.go | 24 ++- 8 files changed, 268 insertions(+), 319 deletions(-) diff --git a/controllers/operator/authentication/authentication.go b/controllers/operator/authentication/authentication.go index 6c4955a6e..d92f7abc8 100644 --- a/controllers/operator/authentication/authentication.go +++ b/controllers/operator/authentication/authentication.go @@ -120,17 +120,17 @@ func Configure(conn om.Connection, opts Options, isRecovering bool, log *zap.Sug // once we have successfully enabled auth for the agents, we need to remove mechanisms we don't need. // this ensures we don't have mechanisms enabled that have not been configured. - if err := removeUnusedAuthenticationMechanisms(conn, opts, log); err != nil { + if err := removeUnsupportedAgentMechanisms(conn, opts, log); err != nil { return xerrors.Errorf("error removing unused authentication mechanisms %w", err) } if err := waitForReadyStateIfNeeded(); err != nil { return err } - // we remove any unrequired deployment auth mechanisms. This will generally be mechanisms + // we remove any unsupported deployment auth mechanisms. This will generally be mechanisms // that we are disabling. - if err := removeUnrequiredDeploymentMechanisms(conn, opts, log); err != nil { - return xerrors.Errorf("error removing unrequired deployment mechanisms: %w", err) + if err := removeUnsupportedDeploymentMechanisms(conn, opts, log); err != nil { + return xerrors.Errorf("error removing unsupported deployment mechanisms: %w", err) } if err := waitForReadyStateIfNeeded(); err != nil { return err @@ -281,28 +281,22 @@ func getMechanismName(mongodbResourceMode string, ac *om.AutomationConfig) Mecha panic(fmt.Sprintf("unknown mechanism name %s", mongodbResourceMode)) } -// mechanism is an interface that needs to be implemented for any Ops Manager authentication mechanism +// Mechanism is an interface that needs to be implemented for any Ops Manager authentication mechanism type Mechanism interface { - EnableAgentAuthentication(opts Options, log *zap.SugaredLogger) error - DisableAgentAuthentication(log *zap.SugaredLogger) error - EnableDeploymentAuthentication(opts Options) error - DisableDeploymentAuthentication() error + EnableAgentAuthentication(conn om.Connection, opts Options, log *zap.SugaredLogger) error + DisableAgentAuthentication(conn om.Connection, log *zap.SugaredLogger) error + EnableDeploymentAuthentication(conn om.Connection, opts Options, log *zap.SugaredLogger) error + DisableDeploymentAuthentication(conn om.Connection, log *zap.SugaredLogger) error // IsAgentAuthenticationConfigured should not rely on util.MergoDelete since the method is always // called directly after deserializing the response from OM which should not contain the util.MergoDelete value in any field. - IsAgentAuthenticationConfigured() bool - IsDeploymentAuthenticationConfigured() bool + IsAgentAuthenticationConfigured(ac *om.AutomationConfig, opts Options) bool + IsDeploymentAuthenticationConfigured(ac *om.AutomationConfig, opts Options) bool + GetName() MechanismName } -var ( - _ Mechanism = ConnectionScramSha{} - _ Mechanism = AutomationConfigScramSha{} - _ Mechanism = ConnectionX509{} - _ Mechanism = &ldapAuthMechanism{} -) - -// removeUnusedAuthenticationMechanisms removes authentication mechanism that were previously enabled, or were required +// removeUnsupportedAgentMechanisms removes authentication mechanism that were previously enabled, or were required // as part of the transition process. -func removeUnusedAuthenticationMechanisms(conn om.Connection, opts Options, log *zap.SugaredLogger) error { +func removeUnsupportedAgentMechanisms(conn om.Connection, opts Options, log *zap.SugaredLogger) error { ac, err := conn.ReadAutomationConfig() if err != nil { return xerrors.Errorf("error reading automation config: %w", err) @@ -310,20 +304,21 @@ func removeUnusedAuthenticationMechanisms(conn om.Connection, opts Options, log automationConfigAuthMechanismNames := getMechanismNames(ac, opts.Mechanisms) - unrequiredMechanisms := mechanismsToDisable(automationConfigAuthMechanismNames) + unsupportedMechanisms := mechanismsToDisable(automationConfigAuthMechanismNames) - log.Infow("configuring agent authentication mechanisms", "enabled", opts.AgentMechanism, "disabling", unrequiredMechanisms) - for _, mn := range unrequiredMechanisms { - m := fromName(mn, ac, conn, opts) - if m.IsAgentAuthenticationConfigured() { - log.Infof("disabling authentication mechanism %s", mn) - if err := m.DisableAgentAuthentication(log); err != nil { + log.Infow("configuring agent authentication mechanisms", "enabled", opts.AgentMechanism, "disabling", unsupportedMechanisms) + for _, mechanismName := range unsupportedMechanisms { + mechanism := fromName(mechanismName) + if mechanism.IsAgentAuthenticationConfigured(ac, opts) { + log.Infof("disabling authentication mechanism %s", mechanismName) + if err := mechanism.DisableAgentAuthentication(conn, log); err != nil { return xerrors.Errorf("error disabling agent authentication: %w", err) } } else { - log.Infof("mechanism %s is already disabled", mn) + log.Infof("mechanism %s is already disabled", mechanismName) } } + return nil } @@ -383,9 +378,9 @@ func ensureDeploymentsMechanismsExist(conn om.Connection, opts Options, log *zap return nil } -// removeUnrequiredDeploymentMechanisms updates the given AutomationConfig struct to enable all the given +// removeUnsupportedDeploymentMechanisms updates the given AutomationConfig struct to enable all the given // authentication mechanisms. -func removeUnrequiredDeploymentMechanisms(conn om.Connection, opts Options, log *zap.SugaredLogger) error { +func removeUnsupportedDeploymentMechanisms(conn om.Connection, opts Options, log *zap.SugaredLogger) error { ac, err := conn.ReadAutomationConfig() if err != nil { return xerrors.Errorf("error reading automation config: %w", err) @@ -395,9 +390,10 @@ func removeUnrequiredDeploymentMechanisms(conn om.Connection, opts Options, log // We need to convert this to the list of strings the automation config expects. automationConfigAuthMechanismNames := getMechanismNames(ac, opts.Mechanisms) - toDisable := mechanismsToDisable(automationConfigAuthMechanismNames) - log.Infow("Removing unrequired deployment authentication mechanisms", "Mechanisms", toDisable) - if err := ensureDeploymentMechanismsAreDisabled(conn, ac, toDisable, opts, log); err != nil { + unsupportedMechanisms := mechanismsToDisable(automationConfigAuthMechanismNames) + + log.Infow("Removing unsupported deployment authentication mechanisms", "Mechanisms", unsupportedMechanisms) + if err := ensureDeploymentMechanismsAreDisabled(conn, ac, unsupportedMechanisms, opts, log); err != nil { return xerrors.Errorf("error ensuring deployment mechanisms are disabled: %w", err) } @@ -465,19 +461,20 @@ func supportedMechanisms() []MechanismName { // fromName returns an implementation of mechanism from the string value // used in the AutomationConfig. All supported fields are in supportedMechanisms -func fromName(name MechanismName, ac *om.AutomationConfig, conn om.Connection, opts Options) Mechanism { +func fromName(name MechanismName) Mechanism { switch name { case MongoDBCR: - return NewConnectionCR(conn, ac) + return MongoDBCRMechanism case ScramSha1: - return NewConnectionScramSha1(conn, ac) + return ScramSha1Mechanism case ScramSha256: - return NewConnectionScramSha256(conn, ac) + return ScramSha256Mechanism case MongoDBX509: - return NewConnectionX509(conn, ac, opts) + return MongoDBX509Mechanism case LDAPPlain: - return NewLdap(conn, ac, opts) + return LDAPPlainMechanism } + panic(xerrors.Errorf("unknown authentication mechanism %s. Supported mechanisms are %+v", name, supportedMechanisms())) } @@ -495,67 +492,68 @@ func mechanismsToDisable(desiredMechanisms []MechanismName) []MechanismName { // ensureAgentAuthenticationIsConfigured will configure the agent authentication settings based on the desiredAgentAuthMechanism func ensureAgentAuthenticationIsConfigured(conn om.Connection, opts Options, ac *om.AutomationConfig, desiredAgentAuthMechanismName MechanismName, log *zap.SugaredLogger) error { - m := fromName(desiredAgentAuthMechanismName, ac, conn, opts) - if m.IsAgentAuthenticationConfigured() { + m := fromName(desiredAgentAuthMechanismName) + if m.IsAgentAuthenticationConfigured(ac, opts) { log.Infof("Agent authentication mechanism %s is already configured", desiredAgentAuthMechanismName) return nil } log.Infof("Enabling %s agent authentication", desiredAgentAuthMechanismName) - return m.EnableAgentAuthentication(opts, log) + return m.EnableAgentAuthentication(conn, opts, log) } // ensureDeploymentMechanisms configures the given AutomationConfig to allow deployments to // authenticate using the specified mechanisms func ensureDeploymentMechanisms(conn om.Connection, ac *om.AutomationConfig, desiredDeploymentAuthMechanisms []MechanismName, opts Options, log *zap.SugaredLogger) error { - allRequiredDeploymentMechanismsAreConfigured := true - for _, mn := range desiredDeploymentAuthMechanisms { - if !fromName(mn, ac, conn, opts).IsDeploymentAuthenticationConfigured() { - allRequiredDeploymentMechanismsAreConfigured = false + deploymentMechanismsToEnable := make(map[MechanismName]Mechanism) + for _, mechanismName := range desiredDeploymentAuthMechanisms { + mechanism := fromName(mechanismName) + if !mechanism.IsDeploymentAuthenticationConfigured(ac, opts) { + deploymentMechanismsToEnable[mechanismName] = mechanism } else { - log.Debugf("Deployment mechanism %s is already configured", mn) + log.Debugf("Deployment mechanism %s is already configured", mechanismName) } } - if allRequiredDeploymentMechanismsAreConfigured { + if len(deploymentMechanismsToEnable) == 0 { log.Info("All required deployment authentication mechanisms are configured") return nil } - return conn.ReadUpdateAutomationConfig(func(ac *om.AutomationConfig) error { - for _, mechanismName := range desiredDeploymentAuthMechanisms { - log.Debugf("Enabling deployment mechanism %s", mechanismName) - if err := fromName(mechanismName, ac, conn, opts).EnableDeploymentAuthentication(opts); err != nil { - return xerrors.Errorf("error enabling deployment authentication: %w", err) - } + for mechanismName, mechanism := range deploymentMechanismsToEnable { + log.Debugf("Enabling deployment mechanism %s", mechanismName) + if err := mechanism.EnableDeploymentAuthentication(conn, opts, log); err != nil { + return xerrors.Errorf("error enabling deployment authentication: %w", err) } - return nil - }, log) + } + + return nil } // ensureDeploymentMechanismsAreDisabled configures the given AutomationConfig to allow deployments to // authenticate using the specified mechanisms func ensureDeploymentMechanismsAreDisabled(conn om.Connection, ac *om.AutomationConfig, mechanismsToDisable []MechanismName, opts Options, log *zap.SugaredLogger) error { - allDeploymentMechanismsAreDisabled := true - for _, mn := range mechanismsToDisable { - if fromName(mn, ac, conn, opts).IsDeploymentAuthenticationConfigured() { - allDeploymentMechanismsAreDisabled = false + deploymentMechanismsToDisable := make([]Mechanism, 0) + for _, mechanismName := range mechanismsToDisable { + mechanism := fromName(mechanismName) + if mechanism.IsDeploymentAuthenticationConfigured(ac, opts) { + deploymentMechanismsToDisable = append(deploymentMechanismsToDisable, mechanism) } } - if allDeploymentMechanismsAreDisabled { + if len(deploymentMechanismsToDisable) == 0 { log.Infof("Mechanisms %+v are all already disabled", mechanismsToDisable) return nil } - return conn.ReadUpdateAutomationConfig(func(ac *om.AutomationConfig) error { - for _, mechanismName := range mechanismsToDisable { - log.Debugf("disabling deployment mechanism %s", mechanismName) - if err := fromName(mechanismName, ac, conn, opts).DisableDeploymentAuthentication(); err != nil { - return xerrors.Errorf("error disabling deployment authentication: %w", err) - } + + for _, mechanism := range deploymentMechanismsToDisable { + log.Debugf("disabling deployment mechanism %s", mechanism.GetName()) + if err := mechanism.DisableDeploymentAuthentication(conn, log); err != nil { + return xerrors.Errorf("error disabling deployment authentication: %w", err) } - return nil - }, log) + } + + return nil } // containsMechanismName returns true if there is at least one MechanismName in `slice` diff --git a/controllers/operator/authentication/configure_authentication_test.go b/controllers/operator/authentication/configure_authentication_test.go index 917ed4f6d..79694fa93 100644 --- a/controllers/operator/authentication/configure_authentication_test.go +++ b/controllers/operator/authentication/configure_authentication_test.go @@ -4,6 +4,7 @@ import ( "testing" "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" "go.uber.org/zap" "github.com/mongodb/mongodb-kubernetes/controllers/om" @@ -146,7 +147,8 @@ func TestDisableAuthentication(t *testing.T) { func TestGetCorrectAuthMechanismFromVersion(t *testing.T) { conn := om.NewMockedOmConnection(om.NewDeployment()) - ac, _ := conn.ReadAutomationConfig() + ac, err := conn.ReadAutomationConfig() + require.NoError(t, err) mechanismNames := getMechanismNames(ac, []string{"X509"}) @@ -200,17 +202,29 @@ func assertAuthenticationMechanism(t *testing.T, auth *om.Auth, mechanism string assert.Contains(t, auth.AutoAuthMechanisms, mechanism) } -func assertDeploymentMechanismsConfigured(t *testing.T, authMechanism Mechanism) { - _ = authMechanism.EnableDeploymentAuthentication(Options{CAFilePath: util.CAFilePathInContainer}) - assert.True(t, authMechanism.IsDeploymentAuthenticationConfigured()) +func assertDeploymentMechanismsConfigured(t *testing.T, authMechanism Mechanism, conn om.Connection, opts Options) { + err := authMechanism.EnableDeploymentAuthentication(conn, opts, zap.S()) + require.NoError(t, err) + + ac, err := conn.ReadAutomationConfig() + require.NoError(t, err) + assert.True(t, authMechanism.IsDeploymentAuthenticationConfigured(ac, opts)) } -func assertAgentAuthenticationDisabled(t *testing.T, authMechanism Mechanism, opts Options) { - _ = authMechanism.EnableAgentAuthentication(opts, zap.S()) - assert.True(t, authMechanism.IsAgentAuthenticationConfigured()) +func assertAgentAuthenticationDisabled(t *testing.T, authMechanism Mechanism, conn om.Connection, opts Options) { + err := authMechanism.EnableAgentAuthentication(conn, opts, zap.S()) + require.NoError(t, err) + + ac, err := conn.ReadAutomationConfig() + require.NoError(t, err) + assert.True(t, authMechanism.IsAgentAuthenticationConfigured(ac, opts)) - _ = authMechanism.DisableAgentAuthentication(zap.S()) - assert.False(t, authMechanism.IsAgentAuthenticationConfigured()) + err = authMechanism.DisableAgentAuthentication(conn, zap.S()) + require.NoError(t, err) + + ac, err = conn.ReadAutomationConfig() + require.NoError(t, err) + assert.False(t, authMechanism.IsAgentAuthenticationConfigured(ac, opts)) } func noneNil(users []*om.MongoDBUser) bool { @@ -230,9 +244,3 @@ func allNil(users []*om.MongoDBUser) bool { } return true } - -func createConnectionAndAutomationConfig() (om.Connection, *om.AutomationConfig) { - conn := om.NewMockedOmConnection(om.NewDeployment()) - ac, _ := conn.ReadAutomationConfig() - return conn, ac -} diff --git a/controllers/operator/authentication/ldap.go b/controllers/operator/authentication/ldap.go index 74d11ea16..94faf0ce8 100644 --- a/controllers/operator/authentication/ldap.go +++ b/controllers/operator/authentication/ldap.go @@ -9,23 +9,17 @@ import ( "github.com/mongodb/mongodb-kubernetes/pkg/util/stringutil" ) -type ldapAuthMechanism struct { - AutomationConfig *om.AutomationConfig - Conn om.Connection - Options Options -} +var LDAPPlainMechanism = &ldapAuthMechanism{} -func NewLdap(conn om.Connection, ac *om.AutomationConfig, opts Options) Mechanism { - return &ldapAuthMechanism{ - AutomationConfig: ac, - Conn: conn, - Options: opts, - } +type ldapAuthMechanism struct{} + +func (l *ldapAuthMechanism) GetName() MechanismName { + return LDAPPlain } -func (l *ldapAuthMechanism) EnableAgentAuthentication(opts Options, log *zap.SugaredLogger) error { +func (l *ldapAuthMechanism) EnableAgentAuthentication(conn om.Connection, opts Options, log *zap.SugaredLogger) error { log.Info("Configuring LDAP authentication") - err := l.Conn.ReadUpdateAutomationConfig(func(ac *om.AutomationConfig) error { + err := conn.ReadUpdateAutomationConfig(func(ac *om.AutomationConfig) error { if err := ac.EnsureKeyFileContents(); err != nil { return err } @@ -36,7 +30,7 @@ func (l *ldapAuthMechanism) EnableAgentAuthentication(opts Options, log *zap.Sug auth.KeyFile = util.AutomationAgentKeyFilePathInContainer auth.KeyFileWindows = util.AutomationAgentWindowsKeyFilePath - auth.AutoUser = l.Options.AutomationSubject + auth.AutoUser = opts.AutomationSubject auth.LdapGroupDN = opts.AutoLdapGroupDN auth.AutoAuthMechanisms = []string{string(LDAPPlain)} return nil @@ -46,8 +40,8 @@ func (l *ldapAuthMechanism) EnableAgentAuthentication(opts Options, log *zap.Sug } log.Info("Configuring backup agent user") - err = l.Conn.ReadUpdateBackupAgentConfig(func(config *om.BackupAgentConfig) error { - config.EnableLdapAuthentication(l.Options.AutomationSubject, opts.AutoPwd) + err = conn.ReadUpdateBackupAgentConfig(func(config *om.BackupAgentConfig) error { + config.EnableLdapAuthentication(opts.AutomationSubject, opts.AutoPwd) config.SetLdapGroupDN(opts.AutoLdapGroupDN) return nil }, log) @@ -56,15 +50,15 @@ func (l *ldapAuthMechanism) EnableAgentAuthentication(opts Options, log *zap.Sug } log.Info("Configuring monitoring agent user") - return l.Conn.ReadUpdateMonitoringAgentConfig(func(config *om.MonitoringAgentConfig) error { - config.EnableLdapAuthentication(l.Options.AutomationSubject, opts.AutoPwd) + return conn.ReadUpdateMonitoringAgentConfig(func(config *om.MonitoringAgentConfig) error { + config.EnableLdapAuthentication(opts.AutomationSubject, opts.AutoPwd) config.SetLdapGroupDN(opts.AutoLdapGroupDN) return nil }, log) } -func (l *ldapAuthMechanism) DisableAgentAuthentication(log *zap.SugaredLogger) error { - err := l.Conn.ReadUpdateAutomationConfig(func(ac *om.AutomationConfig) error { +func (l *ldapAuthMechanism) DisableAgentAuthentication(conn om.Connection, log *zap.SugaredLogger) error { + err := conn.ReadUpdateAutomationConfig(func(ac *om.AutomationConfig) error { if stringutil.Contains(ac.Auth.AutoAuthMechanisms, string(LDAPPlain)) { ac.Auth.AutoAuthMechanisms = stringutil.Remove(ac.Auth.AutoAuthMechanisms, string(LDAPPlain)) } @@ -74,7 +68,7 @@ func (l *ldapAuthMechanism) DisableAgentAuthentication(log *zap.SugaredLogger) e return err } - err = l.Conn.ReadUpdateMonitoringAgentConfig(func(config *om.MonitoringAgentConfig) error { + err = conn.ReadUpdateMonitoringAgentConfig(func(config *om.MonitoringAgentConfig) error { config.DisableLdapAuthentication() return nil }, log) @@ -82,31 +76,32 @@ func (l *ldapAuthMechanism) DisableAgentAuthentication(log *zap.SugaredLogger) e return err } - return l.Conn.ReadUpdateBackupAgentConfig(func(config *om.BackupAgentConfig) error { + return conn.ReadUpdateBackupAgentConfig(func(config *om.BackupAgentConfig) error { config.DisableLdapAuthentication() return nil }, log) } -func (l *ldapAuthMechanism) EnableDeploymentAuthentication(opts Options) error { - ac := l.AutomationConfig - ac.Ldap = opts.Ldap - if !stringutil.Contains(ac.Auth.DeploymentAuthMechanisms, string(LDAPPlain)) { - ac.Auth.DeploymentAuthMechanisms = append(ac.Auth.DeploymentAuthMechanisms, string(LDAPPlain)) - } +func (l *ldapAuthMechanism) EnableDeploymentAuthentication(conn om.Connection, opts Options, log *zap.SugaredLogger) error { + return conn.ReadUpdateAutomationConfig(func(ac *om.AutomationConfig) error { + ac.Ldap = opts.Ldap + if !stringutil.Contains(ac.Auth.DeploymentAuthMechanisms, string(LDAPPlain)) { + ac.Auth.DeploymentAuthMechanisms = append(ac.Auth.DeploymentAuthMechanisms, string(LDAPPlain)) + } - return nil + return nil + }, log) } -func (l *ldapAuthMechanism) DisableDeploymentAuthentication() error { - ac := l.AutomationConfig - ac.Ldap = nil - ac.Auth.DeploymentAuthMechanisms = stringutil.Remove(ac.Auth.DeploymentAuthMechanisms, string(LDAPPlain)) - return nil +func (l *ldapAuthMechanism) DisableDeploymentAuthentication(conn om.Connection, log *zap.SugaredLogger) error { + return conn.ReadUpdateAutomationConfig(func(ac *om.AutomationConfig) error { + ac.Ldap = nil + ac.Auth.DeploymentAuthMechanisms = stringutil.Remove(ac.Auth.DeploymentAuthMechanisms, string(LDAPPlain)) + return nil + }, log) } -func (l *ldapAuthMechanism) IsAgentAuthenticationConfigured() bool { - ac := l.AutomationConfig +func (l *ldapAuthMechanism) IsAgentAuthenticationConfigured(ac *om.AutomationConfig, _ Options) bool { if ac.Auth.Disabled { return false } @@ -122,9 +117,8 @@ func (l *ldapAuthMechanism) IsAgentAuthenticationConfigured() bool { return true } -func (l *ldapAuthMechanism) IsDeploymentAuthenticationConfigured() bool { - ac := l.AutomationConfig - return stringutil.Contains(ac.Auth.DeploymentAuthMechanisms, string(LDAPPlain)) && ldapObjectsEqual(ac.Ldap, l.Options.Ldap) +func (l *ldapAuthMechanism) IsDeploymentAuthenticationConfigured(ac *om.AutomationConfig, opts Options) bool { + return stringutil.Contains(ac.Auth.DeploymentAuthMechanisms, string(LDAPPlain)) && ldapObjectsEqual(ac.Ldap, opts.Ldap) } func ldapObjectsEqual(lhs *ldap.Ldap, rhs *ldap.Ldap) bool { diff --git a/controllers/operator/authentication/ldap_test.go b/controllers/operator/authentication/ldap_test.go index 0ebc2134b..12779faad 100644 --- a/controllers/operator/authentication/ldap_test.go +++ b/controllers/operator/authentication/ldap_test.go @@ -4,6 +4,7 @@ import ( "testing" "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" "go.uber.org/zap" "github.com/mongodb/mongodb-kubernetes/controllers/om" @@ -12,7 +13,7 @@ import ( func TestLdapDeploymentMechanism(t *testing.T) { conn := om.NewMockedOmConnection(om.NewDeployment()) - ac, _ := conn.ReadAutomationConfig() + opts := Options{ Ldap: &ldap.Ldap{ BindMethod: "BindMethod", @@ -20,45 +21,45 @@ func TestLdapDeploymentMechanism(t *testing.T) { Servers: "Servers", }, } - l := NewLdap(conn, ac, opts) - if err := l.EnableDeploymentAuthentication(opts); err != nil { - t.Fatal(err) - } + err := LDAPPlainMechanism.EnableDeploymentAuthentication(conn, opts, zap.S()) + require.NoError(t, err) + + ac, err := conn.ReadAutomationConfig() + require.NoError(t, err) assert.Contains(t, ac.Auth.DeploymentAuthMechanisms, string(LDAPPlain)) assert.Equal(t, "BindQueryUser", ac.Ldap.BindQueryUser) assert.Equal(t, "Servers", ac.Ldap.Servers) assert.Equal(t, "BindMethod", ac.Ldap.BindMethod) - if err := l.DisableDeploymentAuthentication(); err != nil { - t.Fatal(err) - } + err = LDAPPlainMechanism.DisableDeploymentAuthentication(conn, zap.S()) + require.NoError(t, err) + + ac, err = conn.ReadAutomationConfig() + require.NoError(t, err) assert.NotContains(t, ac.Auth.DeploymentAuthMechanisms, string(LDAPPlain)) assert.Nil(t, ac.Ldap) } func TestLdapEnableAgentAuthentication(t *testing.T) { - conn, ac := createConnectionAndAutomationConfig() - options := Options{ + conn := om.NewMockedOmConnection(om.NewDeployment()) + opts := Options{ AgentMechanism: "LDAP", UserOptions: UserOptions{ - AutomationSubject: ("mms-automation"), + AutomationSubject: "mms-automation", }, + AuthoritativeSet: true, + AutoPwd: "LDAPPassword.", } - l := NewLdap(conn, ac, options) - - if err := l.EnableAgentAuthentication(Options{AuthoritativeSet: true, AutoPwd: "LDAPPassword."}, zap.S()); err != nil { - t.Fatal(err) - } + err := LDAPPlainMechanism.EnableAgentAuthentication(conn, opts, zap.S()) + require.NoError(t, err) ac, err := conn.ReadAutomationConfig() - if err != nil { - t.Fatal(err) - } + require.NoError(t, err) - assert.Equal(t, ac.Auth.AutoUser, options.AutomationSubject) + assert.Equal(t, ac.Auth.AutoUser, opts.AutomationSubject) assert.Len(t, ac.Auth.AutoAuthMechanisms, 1) assert.Contains(t, ac.Auth.AutoAuthMechanisms, string(LDAPPlain)) assert.Equal(t, "LDAPPassword.", ac.Auth.AutoPwd) @@ -68,13 +69,13 @@ func TestLdapEnableAgentAuthentication(t *testing.T) { } func TestLDAP_DisableAgentAuthentication(t *testing.T) { - conn, ac := createConnectionAndAutomationConfig() + conn := om.NewMockedOmConnection(om.NewDeployment()) + opts := Options{ AutoPwd: "LDAPPassword.", UserOptions: UserOptions{ AutomationSubject: validSubject("automation"), }, } - ldap := NewLdap(conn, ac, opts) - assertAgentAuthenticationDisabled(t, ldap, opts) + assertAgentAuthenticationDisabled(t, LDAPPlainMechanism, conn, opts) } diff --git a/controllers/operator/authentication/scramsha.go b/controllers/operator/authentication/scramsha.go index 211ef23c7..9a3cb6ded 100644 --- a/controllers/operator/authentication/scramsha.go +++ b/controllers/operator/authentication/scramsha.go @@ -8,101 +8,72 @@ import ( "github.com/mongodb/mongodb-kubernetes/pkg/util/stringutil" ) -func NewConnectionScramSha256(conn om.Connection, ac *om.AutomationConfig) ConnectionScramSha { - return ConnectionScramSha{ - Conn: conn, - AutomationConfigScramSha: AutomationConfigScramSha{ - automationConfig: ac, - mechanismName: ScramSha256, - }, - } -} - -func NewConnectionCR(conn om.Connection, ac *om.AutomationConfig) ConnectionScramSha { - return ConnectionScramSha{ - Conn: conn, - AutomationConfigScramSha: AutomationConfigScramSha{ - automationConfig: ac, - mechanismName: MongoDBCR, - }, - } -} - -func NewConnectionScramSha1(conn om.Connection, ac *om.AutomationConfig) ConnectionScramSha { - return ConnectionScramSha{ - Conn: conn, - AutomationConfigScramSha: AutomationConfigScramSha{ - automationConfig: ac, - mechanismName: ScramSha1, - }, - } -} - -func NewAutomationConfigScramSha1(ac *om.AutomationConfig) AutomationConfigScramSha { - return AutomationConfigScramSha{ - automationConfig: ac, - mechanismName: MongoDBCR, - } -} - -func NewAutomationConfigScramSha256(ac *om.AutomationConfig) AutomationConfigScramSha { - return AutomationConfigScramSha{ - automationConfig: ac, - mechanismName: ScramSha256, - } -} +var ( + MongoDBCRMechanism = AutomationConfigScramSha{MechanismName: MongoDBCR} + ScramSha1Mechanism = AutomationConfigScramSha{MechanismName: ScramSha1} + ScramSha256Mechanism = AutomationConfigScramSha{MechanismName: ScramSha256} +) // AutomationConfigScramSha applies all the changes required to configure SCRAM-SHA authentication // directly to an AutomationConfig struct. This implementation does not communicate with Ops Manager in any way. type AutomationConfigScramSha struct { - mechanismName MechanismName - automationConfig *om.AutomationConfig + MechanismName MechanismName } -func (s AutomationConfigScramSha) EnableAgentAuthentication(opts Options, log *zap.SugaredLogger) error { - if err := configureScramAgentUsers(s.automationConfig, opts); err != nil { - return err - } - if err := s.automationConfig.EnsureKeyFileContents(); err != nil { - return err - } +func (s AutomationConfigScramSha) GetName() MechanismName { + return s.MechanismName +} - auth := s.automationConfig.Auth - auth.Disabled = false - auth.AuthoritativeSet = opts.AuthoritativeSet - auth.KeyFile = util.AutomationAgentKeyFilePathInContainer - auth.KeyFileWindows = util.AutomationAgentWindowsKeyFilePath +func (s AutomationConfigScramSha) EnableAgentAuthentication(conn om.Connection, opts Options, log *zap.SugaredLogger) error { + return conn.ReadUpdateAutomationConfig(func(ac *om.AutomationConfig) error { + if err := configureScramAgentUsers(ac, opts); err != nil { + return err + } + if err := ac.EnsureKeyFileContents(); err != nil { + return err + } - // We can only have a single agent authentication mechanism specified at a given time - auth.AutoAuthMechanisms = []string{string(s.mechanismName)} - return nil + auth := ac.Auth + auth.Disabled = false + auth.AuthoritativeSet = opts.AuthoritativeSet + auth.KeyFile = util.AutomationAgentKeyFilePathInContainer + auth.KeyFileWindows = util.AutomationAgentWindowsKeyFilePath + + // We can only have a single agent authentication mechanism specified at a given time + auth.AutoAuthMechanisms = []string{string(s.MechanismName)} + return nil + }, log) } -func (s AutomationConfigScramSha) DisableAgentAuthentication(log *zap.SugaredLogger) error { - s.automationConfig.Auth.AutoAuthMechanisms = stringutil.Remove(s.automationConfig.Auth.AutoAuthMechanisms, string(s.mechanismName)) - return nil +func (s AutomationConfigScramSha) DisableAgentAuthentication(conn om.Connection, log *zap.SugaredLogger) error { + return conn.ReadUpdateAutomationConfig(func(ac *om.AutomationConfig) error { + ac.Auth.AutoAuthMechanisms = stringutil.Remove(ac.Auth.AutoAuthMechanisms, string(s.MechanismName)) + return nil + }, log) } -func (s AutomationConfigScramSha) DisableDeploymentAuthentication() error { - s.automationConfig.Auth.DeploymentAuthMechanisms = stringutil.Remove(s.automationConfig.Auth.DeploymentAuthMechanisms, string(s.mechanismName)) - return nil +func (s AutomationConfigScramSha) DisableDeploymentAuthentication(conn om.Connection, log *zap.SugaredLogger) error { + return conn.ReadUpdateAutomationConfig(func(ac *om.AutomationConfig) error { + ac.Auth.DeploymentAuthMechanisms = stringutil.Remove(ac.Auth.DeploymentAuthMechanisms, string(s.MechanismName)) + return nil + }, log) } -func (s AutomationConfigScramSha) EnableDeploymentAuthentication(Options) error { - auth := s.automationConfig.Auth - if !stringutil.Contains(auth.DeploymentAuthMechanisms, string(s.mechanismName)) { - auth.DeploymentAuthMechanisms = append(auth.DeploymentAuthMechanisms, string(s.mechanismName)) - } - return nil +func (s AutomationConfigScramSha) EnableDeploymentAuthentication(conn om.Connection, _ Options, log *zap.SugaredLogger) error { + return conn.ReadUpdateAutomationConfig(func(ac *om.AutomationConfig) error { + if !stringutil.Contains(ac.Auth.DeploymentAuthMechanisms, string(s.MechanismName)) { + ac.Auth.DeploymentAuthMechanisms = append(ac.Auth.DeploymentAuthMechanisms, string(s.MechanismName)) + } + return nil + }, log) } -func (s AutomationConfigScramSha) IsAgentAuthenticationConfigured() bool { - ac := s.automationConfig +func (s AutomationConfigScramSha) IsAgentAuthenticationConfigured(ac *om.AutomationConfig, _ Options) bool { if ac.Auth.Disabled { return false } - if !stringutil.Contains(ac.Auth.AutoAuthMechanisms, string(s.mechanismName)) { + if !stringutil.Contains(ac.Auth.AutoAuthMechanisms, string(s.MechanismName)) { return false } @@ -117,29 +88,8 @@ func (s AutomationConfigScramSha) IsAgentAuthenticationConfigured() bool { return true } -func (s AutomationConfigScramSha) IsDeploymentAuthenticationConfigured() bool { - return stringutil.Contains(s.automationConfig.Auth.DeploymentAuthMechanisms, string(s.mechanismName)) -} - -// ConnectionScramSha is a wrapper around AutomationConfigScramSha which pulls the AutomationConfig -// from Ops Manager and sends back the AutomationConfig which has been configured for to enabled SCRAM-SHA -type ConnectionScramSha struct { - AutomationConfigScramSha - Conn om.Connection -} - -func (s ConnectionScramSha) EnableAgentAuthentication(opts Options, log *zap.SugaredLogger) error { - return s.Conn.ReadUpdateAutomationConfig(func(ac *om.AutomationConfig) error { - s.automationConfig = ac - return s.AutomationConfigScramSha.EnableAgentAuthentication(opts, log) - }, log) -} - -func (s ConnectionScramSha) DisableAgentAuthentication(log *zap.SugaredLogger) error { - return s.Conn.ReadUpdateAutomationConfig(func(ac *om.AutomationConfig) error { - s.automationConfig = ac - return s.AutomationConfigScramSha.DisableAgentAuthentication(log) - }, log) +func (s AutomationConfigScramSha) IsDeploymentAuthenticationConfigured(ac *om.AutomationConfig, _ Options) bool { + return stringutil.Contains(ac.Auth.DeploymentAuthMechanisms, string(s.MechanismName)) } // configureScramAgentUsers makes sure that the given automation config always has the correct SCRAM-SHA users diff --git a/controllers/operator/authentication/scramsha_test.go b/controllers/operator/authentication/scramsha_test.go index 467263898..175bfbfc7 100644 --- a/controllers/operator/authentication/scramsha_test.go +++ b/controllers/operator/authentication/scramsha_test.go @@ -4,6 +4,7 @@ import ( "testing" "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" "go.uber.org/zap" "github.com/mongodb/mongodb-kubernetes/controllers/om" @@ -11,65 +12,59 @@ import ( ) func TestAgentsAuthentication(t *testing.T) { - type ConnectionFunction func(om.Connection, *om.AutomationConfig) Mechanism type TestConfig struct { - connection ConnectionFunction - mechanismsUsed []MechanismName + mechanism AutomationConfigScramSha } tests := map[string]TestConfig{ "SCRAM-SHA-1": { - connection: func(connection om.Connection, config *om.AutomationConfig) Mechanism { - return NewConnectionScramSha1(connection, config) - }, - mechanismsUsed: []MechanismName{ScramSha1}, + mechanism: ScramSha1Mechanism, }, "SCRAM-SHA-256": { - connection: func(connection om.Connection, config *om.AutomationConfig) Mechanism { - return NewConnectionScramSha256(connection, config) - }, - mechanismsUsed: []MechanismName{ScramSha256}, + mechanism: ScramSha256Mechanism, }, "CR": { - connection: func(connection om.Connection, config *om.AutomationConfig) Mechanism { - return NewConnectionCR(connection, config) - }, - mechanismsUsed: []MechanismName{MongoDBCR}, + mechanism: MongoDBCRMechanism, }, } for testName, testConfig := range tests { t.Run(testName, func(t *testing.T) { - conn, ac := createConnectionAndAutomationConfig() + conn := om.NewMockedOmConnection(om.NewDeployment()) - s := testConfig.connection(conn, ac) + s := testConfig.mechanism - err := s.EnableAgentAuthentication(Options{AuthoritativeSet: true}, zap.S()) - assert.NoError(t, err) + opts := Options{ + AuthoritativeSet: true, + CAFilePath: util.CAFilePathInContainer, + } + + err := s.EnableAgentAuthentication(conn, opts, zap.S()) + require.NoError(t, err) - err = s.EnableDeploymentAuthentication(Options{CAFilePath: util.CAFilePathInContainer}) - assert.NoError(t, err) + err = s.EnableDeploymentAuthentication(conn, opts, zap.S()) + require.NoError(t, err) - ac, err = conn.ReadAutomationConfig() - assert.NoError(t, err) + ac, err := conn.ReadAutomationConfig() + require.NoError(t, err) assertAuthenticationEnabled(t, ac.Auth) assert.Equal(t, ac.Auth.AutoUser, util.AutomationAgentName) assert.Len(t, ac.Auth.AutoAuthMechanisms, 1) - for _, mech := range testConfig.mechanismsUsed { + for _, mech := range testConfig.mechanism.GetName() { assert.Contains(t, ac.Auth.AutoAuthMechanisms, string(mech)) } assert.NotEmpty(t, ac.Auth.AutoPwd) - assert.True(t, s.IsAgentAuthenticationConfigured()) - assert.True(t, s.IsDeploymentAuthenticationConfigured()) + assert.True(t, s.IsAgentAuthenticationConfigured(ac, opts)) + assert.True(t, s.IsDeploymentAuthenticationConfigured(ac, opts)) }) } } func TestScramSha1_DisableAgentAuthentication(t *testing.T) { - conn, ac := createConnectionAndAutomationConfig() - assertAgentAuthenticationDisabled(t, NewConnectionScramSha1(conn, ac), Options{}) + conn := om.NewMockedOmConnection(om.NewDeployment()) + assertAgentAuthenticationDisabled(t, ScramSha1Mechanism, conn, Options{}) } func TestScramSha256_DisableAgentAuthentication(t *testing.T) { - conn, ac := createConnectionAndAutomationConfig() - assertAgentAuthenticationDisabled(t, NewConnectionScramSha256(conn, ac), Options{}) + conn := om.NewMockedOmConnection(om.NewDeployment()) + assertAgentAuthenticationDisabled(t, ScramSha256Mechanism, conn, Options{}) } diff --git a/controllers/operator/authentication/x509.go b/controllers/operator/authentication/x509.go index 73992ea95..efcd8b6e6 100644 --- a/controllers/operator/authentication/x509.go +++ b/controllers/operator/authentication/x509.go @@ -12,23 +12,17 @@ import ( const ExternalDB = "$external" -func NewConnectionX509(conn om.Connection, ac *om.AutomationConfig, opts Options) ConnectionX509 { - return ConnectionX509{ - AutomationConfig: ac, - Conn: conn, - Options: opts, - } -} +var MongoDBX509Mechanism = ConnectionX509{} + +type ConnectionX509 struct{} -type ConnectionX509 struct { - AutomationConfig *om.AutomationConfig - Conn om.Connection - Options Options +func (x ConnectionX509) GetName() MechanismName { + return MongoDBX509 } -func (x ConnectionX509) EnableAgentAuthentication(opts Options, log *zap.SugaredLogger) error { +func (x ConnectionX509) EnableAgentAuthentication(conn om.Connection, opts Options, log *zap.SugaredLogger) error { log.Info("Configuring x509 authentication") - err := x.Conn.ReadUpdateAutomationConfig(func(ac *om.AutomationConfig) error { + err := conn.ReadUpdateAutomationConfig(func(ac *om.AutomationConfig) error { if err := ac.EnsureKeyFileContents(); err != nil { return err } @@ -44,7 +38,7 @@ func (x ConnectionX509) EnableAgentAuthentication(opts Options, log *zap.Sugared ClientCertificateMode: opts.ClientCertificates, } - auth.AutoUser = x.Options.AutomationSubject + auth.AutoUser = opts.AutomationSubject auth.LdapGroupDN = opts.AutoLdapGroupDN auth.AutoAuthMechanisms = []string{string(MongoDBX509)} @@ -55,7 +49,7 @@ func (x ConnectionX509) EnableAgentAuthentication(opts Options, log *zap.Sugared } log.Info("Configuring backup agent user") - err = x.Conn.ReadUpdateBackupAgentConfig(func(config *om.BackupAgentConfig) error { + err = conn.ReadUpdateBackupAgentConfig(func(config *om.BackupAgentConfig) error { config.EnableX509Authentication(opts.AutomationSubject) config.SetLdapGroupDN(opts.AutoLdapGroupDN) return nil @@ -65,15 +59,15 @@ func (x ConnectionX509) EnableAgentAuthentication(opts Options, log *zap.Sugared } log.Info("Configuring monitoring agent user") - return x.Conn.ReadUpdateMonitoringAgentConfig(func(config *om.MonitoringAgentConfig) error { + return conn.ReadUpdateMonitoringAgentConfig(func(config *om.MonitoringAgentConfig) error { config.EnableX509Authentication(opts.AutomationSubject) config.SetLdapGroupDN(opts.AutoLdapGroupDN) return nil }, log) } -func (x ConnectionX509) DisableAgentAuthentication(log *zap.SugaredLogger) error { - err := x.Conn.ReadUpdateAutomationConfig(func(ac *om.AutomationConfig) error { +func (x ConnectionX509) DisableAgentAuthentication(conn om.Connection, log *zap.SugaredLogger) error { + err := conn.ReadUpdateAutomationConfig(func(ac *om.AutomationConfig) error { ac.AgentSSL = &om.AgentSSL{ AutoPEMKeyFilePath: util.MergoDelete, ClientCertificateMode: util.OptionalClientCertficates, @@ -87,7 +81,7 @@ func (x ConnectionX509) DisableAgentAuthentication(log *zap.SugaredLogger) error if err != nil { return err } - err = x.Conn.ReadUpdateMonitoringAgentConfig(func(config *om.MonitoringAgentConfig) error { + err = conn.ReadUpdateMonitoringAgentConfig(func(config *om.MonitoringAgentConfig) error { config.DisableX509Authentication() return nil }, log) @@ -95,31 +89,32 @@ func (x ConnectionX509) DisableAgentAuthentication(log *zap.SugaredLogger) error return err } - return x.Conn.ReadUpdateBackupAgentConfig(func(config *om.BackupAgentConfig) error { + return conn.ReadUpdateBackupAgentConfig(func(config *om.BackupAgentConfig) error { config.DisableX509Authentication() return nil }, log) } -func (x ConnectionX509) EnableDeploymentAuthentication(opts Options) error { - ac := x.AutomationConfig - if !stringutil.Contains(ac.Auth.DeploymentAuthMechanisms, util.AutomationConfigX509Option) { - ac.Auth.DeploymentAuthMechanisms = append(ac.Auth.DeploymentAuthMechanisms, string(MongoDBX509)) - } - // AutomationConfig validation requires the CAFile path to be specified in the case of multiple auth - // mechanisms enabled. This is not required if only X509 is being configured - ac.AgentSSL.CAFilePath = opts.CAFilePath - return nil +func (x ConnectionX509) EnableDeploymentAuthentication(conn om.Connection, opts Options, log *zap.SugaredLogger) error { + return conn.ReadUpdateAutomationConfig(func(ac *om.AutomationConfig) error { + if !stringutil.Contains(ac.Auth.DeploymentAuthMechanisms, util.AutomationConfigX509Option) { + ac.Auth.DeploymentAuthMechanisms = append(ac.Auth.DeploymentAuthMechanisms, string(MongoDBX509)) + } + // AutomationConfig validation requires the CAFile path to be specified in the case of multiple auth + // mechanisms enabled. This is not required if only X509 is being configured + ac.AgentSSL.CAFilePath = opts.CAFilePath + return nil + }, log) } -func (x ConnectionX509) DisableDeploymentAuthentication() error { - ac := x.AutomationConfig - ac.Auth.DeploymentAuthMechanisms = stringutil.Remove(ac.Auth.DeploymentAuthMechanisms, string(MongoDBX509)) - return nil +func (x ConnectionX509) DisableDeploymentAuthentication(conn om.Connection, log *zap.SugaredLogger) error { + return conn.ReadUpdateAutomationConfig(func(ac *om.AutomationConfig) error { + ac.Auth.DeploymentAuthMechanisms = stringutil.Remove(ac.Auth.DeploymentAuthMechanisms, string(MongoDBX509)) + return nil + }, log) } -func (x ConnectionX509) IsAgentAuthenticationConfigured() bool { - ac := x.AutomationConfig +func (x ConnectionX509) IsAgentAuthenticationConfigured(ac *om.AutomationConfig, _ Options) bool { if ac.Auth.Disabled { return false } @@ -139,8 +134,8 @@ func (x ConnectionX509) IsAgentAuthenticationConfigured() bool { return true } -func (x ConnectionX509) IsDeploymentAuthenticationConfigured() bool { - return stringutil.Contains(x.AutomationConfig.Auth.DeploymentAuthMechanisms, string(MongoDBX509)) +func (x ConnectionX509) IsDeploymentAuthenticationConfigured(ac *om.AutomationConfig, _ Options) bool { + return stringutil.Contains(ac.Auth.DeploymentAuthMechanisms, string(MongoDBX509)) } // isValidX509Subject checks the subject contains CommonName, Country and Organizational Unit, Location and State. diff --git a/controllers/operator/authentication/x509_test.go b/controllers/operator/authentication/x509_test.go index b8996e5fa..098204459 100644 --- a/controllers/operator/authentication/x509_test.go +++ b/controllers/operator/authentication/x509_test.go @@ -5,22 +5,25 @@ import ( "testing" "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" "go.uber.org/zap" + "github.com/mongodb/mongodb-kubernetes/controllers/om" "github.com/mongodb/mongodb-kubernetes/pkg/util" ) func TestX509EnableAgentAuthentication(t *testing.T) { - conn, ac := createConnectionAndAutomationConfig() + conn := om.NewMockedOmConnection(om.NewDeployment()) + options := Options{ AgentMechanism: "X509", ClientCertificates: util.RequireClientCertificates, UserOptions: UserOptions{ AutomationSubject: validSubject("automation"), }, + AuthoritativeSet: true, } - x := NewConnectionX509(conn, ac, options) - if err := x.EnableAgentAuthentication(Options{AuthoritativeSet: true}, zap.S()); err != nil { + if err := MongoDBX509Mechanism.EnableAgentAuthentication(conn, options, zap.S()); err != nil { t.Fatal(err) } @@ -43,19 +46,24 @@ func TestX509EnableAgentAuthentication(t *testing.T) { } func TestX509_DisableAgentAuthentication(t *testing.T) { - conn, ac := createConnectionAndAutomationConfig() + conn := om.NewMockedOmConnection(om.NewDeployment()) + opts := Options{ UserOptions: UserOptions{ AutomationSubject: validSubject("automation"), }, } - x509 := NewConnectionX509(conn, ac, opts) - assertAgentAuthenticationDisabled(t, x509, opts) + assertAgentAuthenticationDisabled(t, MongoDBX509Mechanism, conn, opts) } func TestX509_DeploymentConfigured(t *testing.T) { - conn, ac := createConnectionAndAutomationConfig() - assertDeploymentMechanismsConfigured(t, NewConnectionX509(conn, ac, Options{AgentMechanism: "SCRAM"})) + conn := om.NewMockedOmConnection(om.NewDeployment()) + opts := Options{AgentMechanism: "SCRAM", CAFilePath: util.CAFilePathInContainer} + + assertDeploymentMechanismsConfigured(t, MongoDBX509Mechanism, conn, opts) + + ac, err := conn.ReadAutomationConfig() + require.NoError(t, err) assert.Equal(t, ac.AgentSSL.CAFilePath, util.CAFilePathInContainer) } From 99479f1818ceefb0e9bdc58be853baf11f6c1cdf Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Maciej=20Kara=C5=9B?= Date: Thu, 24 Apr 2025 16:09:22 +0200 Subject: [PATCH 03/36] Authorization package refactor - part 2 --- .../operator/authentication/authentication.go | 203 +++--------------- .../authentication_mechanism.go | 118 ++++++++++ .../configure_authentication_test.go | 18 +- .../authentication/scramsha_credentials.go | 45 +++- .../operator/authentication/scramsha_test.go | 4 +- controllers/operator/authentication/x509.go | 2 - 6 files changed, 193 insertions(+), 197 deletions(-) create mode 100644 controllers/operator/authentication/authentication_mechanism.go diff --git a/controllers/operator/authentication/authentication.go b/controllers/operator/authentication/authentication.go index d92f7abc8..bd9cc1258 100644 --- a/controllers/operator/authentication/authentication.go +++ b/controllers/operator/authentication/authentication.go @@ -1,10 +1,6 @@ package authentication import ( - "crypto/sha1" //nolint //Part of the algorithm - "crypto/sha256" - "fmt" - "go.uber.org/zap" "golang.org/x/xerrors" @@ -147,33 +143,6 @@ func Configure(conn om.Connection, opts Options, isRecovering bool, log *zap.Sug return nil } -// ConfigureScramCredentials creates both SCRAM-SHA-1 and SCRAM-SHA-256 credentials. This ensures -// that changes to the authentication settings on the MongoDB resources won't leave MongoDBUsers without -// the correct credentials. -func ConfigureScramCredentials(user *om.MongoDBUser, password string) error { - scram256Salt, err := GenerateSalt(sha256.New) - if err != nil { - return xerrors.Errorf("error generating scramSha256 salt: %w", err) - } - - scram1Salt, err := GenerateSalt(sha1.New) - if err != nil { - return xerrors.Errorf("error generating scramSha1 salt: %w", err) - } - - scram256Creds, err := ComputeScramShaCreds(user.Username, password, scram256Salt, ScramSha256) - if err != nil { - return xerrors.Errorf("error generating scramSha256 creds: %w", err) - } - scram1Creds, err := ComputeScramShaCreds(user.Username, password, scram1Salt, MongoDBCR) - if err != nil { - return xerrors.Errorf("error generating scramSha1Creds: %w", err) - } - user.ScramSha256Creds = scram256Creds - user.ScramSha1Creds = scram1Creds - return nil -} - // Disable disables all authentication mechanisms, and waits for the agents to reach goal state. It is still required to provide // automation agent username, password and keyfile contents to ensure a valid Automation Config. func Disable(conn om.Connection, opts Options, deleteUsers bool, log *zap.SugaredLogger) error { @@ -254,46 +223,6 @@ func Disable(conn om.Connection, opts Options, deleteUsers bool, log *zap.Sugare return nil } -func getMechanismName(mongodbResourceMode string, ac *om.AutomationConfig) MechanismName { - switch mongodbResourceMode { - case util.X509: - return MongoDBX509 - case util.LDAP: - return LDAPPlain - case util.SCRAMSHA1: - return ScramSha1 - case util.MONGODBCR: - return MongoDBCR - case util.SCRAMSHA256: - return ScramSha256 - case util.SCRAM: - // if we have already configured authentication and it has been set to MONGODB-CR/SCRAM-SHA-1 - // we can not transition. This needs to be done in the UI - - // if no authentication has been configured, the default value for "AutoAuthMechanism" is "MONGODB-CR" - // even if authentication is disabled, so we need to ensure that auth has been enabled. - if ac.Auth.AutoAuthMechanism == string(MongoDBCR) && ac.Auth.IsEnabled() { - return MongoDBCR - } - return ScramSha256 - } - // this should never be reached as validation of this string happens at the CR level - panic(fmt.Sprintf("unknown mechanism name %s", mongodbResourceMode)) -} - -// Mechanism is an interface that needs to be implemented for any Ops Manager authentication mechanism -type Mechanism interface { - EnableAgentAuthentication(conn om.Connection, opts Options, log *zap.SugaredLogger) error - DisableAgentAuthentication(conn om.Connection, log *zap.SugaredLogger) error - EnableDeploymentAuthentication(conn om.Connection, opts Options, log *zap.SugaredLogger) error - DisableDeploymentAuthentication(conn om.Connection, log *zap.SugaredLogger) error - // IsAgentAuthenticationConfigured should not rely on util.MergoDelete since the method is always - // called directly after deserializing the response from OM which should not contain the util.MergoDelete value in any field. - IsAgentAuthenticationConfigured(ac *om.AutomationConfig, opts Options) bool - IsDeploymentAuthenticationConfigured(ac *om.AutomationConfig, opts Options) bool - GetName() MechanismName -} - // removeUnsupportedAgentMechanisms removes authentication mechanism that were previously enabled, or were required // as part of the transition process. func removeUnsupportedAgentMechanisms(conn om.Connection, opts Options, log *zap.SugaredLogger) error { @@ -302,20 +231,19 @@ func removeUnsupportedAgentMechanisms(conn om.Connection, opts Options, log *zap return xerrors.Errorf("error reading automation config: %w", err) } - automationConfigAuthMechanismNames := getMechanismNames(ac, opts.Mechanisms) + automationConfigAuthMechanismNames := convertToMechanismList(opts.Mechanisms, ac) unsupportedMechanisms := mechanismsToDisable(automationConfigAuthMechanismNames) log.Infow("configuring agent authentication mechanisms", "enabled", opts.AgentMechanism, "disabling", unsupportedMechanisms) - for _, mechanismName := range unsupportedMechanisms { - mechanism := fromName(mechanismName) + for _, mechanism := range unsupportedMechanisms { if mechanism.IsAgentAuthenticationConfigured(ac, opts) { - log.Infof("disabling authentication mechanism %s", mechanismName) + log.Infof("disabling authentication mechanism %s", mechanism.GetName()) if err := mechanism.DisableAgentAuthentication(conn, log); err != nil { return xerrors.Errorf("error disabling agent authentication: %w", err) } } else { - log.Infof("mechanism %s is already disabled", mechanismName) + log.Infof("mechanism %s is already disabled", mechanism.GetName()) } } @@ -331,8 +259,8 @@ func enableAgentAuthentication(conn om.Connection, opts Options, log *zap.Sugare } // we then configure the agent authentication for that type - agentAuthMechanism := getMechanismName(opts.AgentMechanism, ac) - if err := ensureAgentAuthenticationIsConfigured(conn, opts, ac, agentAuthMechanism, log); err != nil { + mechanism := convertToMechanism(opts.AgentMechanism, ac) + if err := ensureAgentAuthenticationIsConfigured(conn, opts, ac, mechanism, log); err != nil { return xerrors.Errorf("error ensuring agent authentication is configured: %w", err) } @@ -368,10 +296,10 @@ func ensureDeploymentsMechanismsExist(conn om.Connection, opts Options, log *zap // "opts.Mechanisms" is the list of mechanism names passed through from the MongoDB resource. // We need to convert this to the list of strings the automation config expects. - automationConfigMechanismNames := getMechanismNames(ac, opts.Mechanisms) + automationConfigMechanisms := convertToMechanismList(opts.Mechanisms, ac) - log.Debugf("Automation config authentication mechanisms: %+v", automationConfigMechanismNames) - if err := ensureDeploymentMechanisms(conn, ac, automationConfigMechanismNames, opts, log); err != nil { + log.Debugf("Automation config authentication mechanisms: %+v", automationConfigMechanisms) + if err := ensureDeploymentMechanisms(conn, ac, automationConfigMechanisms, opts, log); err != nil { return xerrors.Errorf("error ensuring deployment mechanisms: %w", err) } @@ -387,10 +315,9 @@ func removeUnsupportedDeploymentMechanisms(conn om.Connection, opts Options, log } // "opts.Mechanisms" is the list of mechanism names passed through from the MongoDB resource. - // We need to convert this to the list of strings the automation config expects. - automationConfigAuthMechanismNames := getMechanismNames(ac, opts.Mechanisms) + automationConfigAuthMechanisms := convertToMechanismList(opts.Mechanisms, ac) - unsupportedMechanisms := mechanismsToDisable(automationConfigAuthMechanismNames) + unsupportedMechanisms := mechanismsToDisable(automationConfigAuthMechanisms) log.Infow("Removing unsupported deployment authentication mechanisms", "Mechanisms", unsupportedMechanisms) if err := ensureDeploymentMechanismsAreDisabled(conn, ac, unsupportedMechanisms, opts, log); err != nil { @@ -408,7 +335,7 @@ func addOrRemoveAgentClientCertificate(conn om.Connection, opts Options, log *za // If x509 is not enabled but still Client Certificates are, this automation config update // will add the required configuration. return conn.ReadUpdateAutomationConfig(func(ac *om.AutomationConfig) error { - if getMechanismName(opts.AgentMechanism, ac) == MongoDBX509 { + if convertToMechanism(opts.AgentMechanism, ac).GetName() == MongoDBX509 { // If TLS client authentication is managed by x509, we won't disable or enable it // in here. return nil @@ -430,98 +357,36 @@ func addOrRemoveAgentClientCertificate(conn om.Connection, opts Options, log *za }, log) } -func getMechanismNames(ac *om.AutomationConfig, mechanisms []string) []MechanismName { - automationConfigMechanismNames := make([]MechanismName, 0) - for _, m := range mechanisms { - automationConfigMechanismNames = append(automationConfigMechanismNames, getMechanismName(m, ac)) - } - return automationConfigMechanismNames -} - -// MechanismName corresponds to the string used in the automation config representing -// a particular type of authentication -type MechanismName string - -const ( - ScramSha256 MechanismName = "SCRAM-SHA-256" - ScramSha1 MechanismName = "SCRAM-SHA-1" - MongoDBX509 MechanismName = "MONGODB-X509" - LDAPPlain MechanismName = "PLAIN" - - // MongoDBCR is an umbrella term for SCRAM-SHA-1 and MONGODB-CR for legacy reasons, once MONGODB-CR - // is enabled, users can auth with SCRAM-SHA-1 credentials - MongoDBCR MechanismName = "MONGODB-CR" -) - -// supportedMechanisms returns a list of all the authentication mechanisms -// that can be configured by the Operator -func supportedMechanisms() []MechanismName { - return []MechanismName{ScramSha256, MongoDBCR, MongoDBX509, LDAPPlain} -} - -// fromName returns an implementation of mechanism from the string value -// used in the AutomationConfig. All supported fields are in supportedMechanisms -func fromName(name MechanismName) Mechanism { - switch name { - case MongoDBCR: - return MongoDBCRMechanism - case ScramSha1: - return ScramSha1Mechanism - case ScramSha256: - return ScramSha256Mechanism - case MongoDBX509: - return MongoDBX509Mechanism - case LDAPPlain: - return LDAPPlainMechanism - } - - panic(xerrors.Errorf("unknown authentication mechanism %s. Supported mechanisms are %+v", name, supportedMechanisms())) -} - -// mechanismsToDisable returns a list of mechanisms which need to be disabled -// based on the currently supported authentication mechanisms and the desiredMechanisms -func mechanismsToDisable(desiredMechanisms []MechanismName) []MechanismName { - toDisable := make([]MechanismName, 0) - for _, m := range supportedMechanisms() { - if !containsMechanismName(desiredMechanisms, m) { - toDisable = append(toDisable, m) - } - } - return toDisable -} - // ensureAgentAuthenticationIsConfigured will configure the agent authentication settings based on the desiredAgentAuthMechanism -func ensureAgentAuthenticationIsConfigured(conn om.Connection, opts Options, ac *om.AutomationConfig, desiredAgentAuthMechanismName MechanismName, log *zap.SugaredLogger) error { - m := fromName(desiredAgentAuthMechanismName) - if m.IsAgentAuthenticationConfigured(ac, opts) { - log.Infof("Agent authentication mechanism %s is already configured", desiredAgentAuthMechanismName) +func ensureAgentAuthenticationIsConfigured(conn om.Connection, opts Options, ac *om.AutomationConfig, mechanism Mechanism, log *zap.SugaredLogger) error { + if mechanism.IsAgentAuthenticationConfigured(ac, opts) { + log.Infof("Agent authentication mechanism %s is already configured", mechanism.GetName()) return nil } - log.Infof("Enabling %s agent authentication", desiredAgentAuthMechanismName) - return m.EnableAgentAuthentication(conn, opts, log) + log.Infof("Enabling %s agent authentication", mechanism.GetName()) + return mechanism.EnableAgentAuthentication(conn, opts, log) } // ensureDeploymentMechanisms configures the given AutomationConfig to allow deployments to // authenticate using the specified mechanisms -func ensureDeploymentMechanisms(conn om.Connection, ac *om.AutomationConfig, desiredDeploymentAuthMechanisms []MechanismName, opts Options, log *zap.SugaredLogger) error { - deploymentMechanismsToEnable := make(map[MechanismName]Mechanism) - for _, mechanismName := range desiredDeploymentAuthMechanisms { - mechanism := fromName(mechanismName) +func ensureDeploymentMechanisms(conn om.Connection, ac *om.AutomationConfig, mechanisms MechanismList, opts Options, log *zap.SugaredLogger) error { + mechanismsToEnable := make([]Mechanism, 0) + for _, mechanism := range mechanisms { if !mechanism.IsDeploymentAuthenticationConfigured(ac, opts) { - deploymentMechanismsToEnable[mechanismName] = mechanism + mechanismsToEnable = append(mechanismsToEnable, mechanism) } else { - log.Debugf("Deployment mechanism %s is already configured", mechanismName) + log.Debugf("Deployment mechanism %s is already configured", mechanism.GetName()) } } - if len(deploymentMechanismsToEnable) == 0 { + if len(mechanismsToEnable) == 0 { log.Info("All required deployment authentication mechanisms are configured") return nil } - for mechanismName, mechanism := range deploymentMechanismsToEnable { - log.Debugf("Enabling deployment mechanism %s", mechanismName) + for _, mechanism := range mechanismsToEnable { + log.Debugf("Enabling deployment mechanism %s", mechanism.GetName()) if err := mechanism.EnableDeploymentAuthentication(conn, opts, log); err != nil { return xerrors.Errorf("error enabling deployment authentication: %w", err) } @@ -532,17 +397,16 @@ func ensureDeploymentMechanisms(conn om.Connection, ac *om.AutomationConfig, des // ensureDeploymentMechanismsAreDisabled configures the given AutomationConfig to allow deployments to // authenticate using the specified mechanisms -func ensureDeploymentMechanismsAreDisabled(conn om.Connection, ac *om.AutomationConfig, mechanismsToDisable []MechanismName, opts Options, log *zap.SugaredLogger) error { +func ensureDeploymentMechanismsAreDisabled(conn om.Connection, ac *om.AutomationConfig, mechanismsToDisable MechanismList, opts Options, log *zap.SugaredLogger) error { deploymentMechanismsToDisable := make([]Mechanism, 0) - for _, mechanismName := range mechanismsToDisable { - mechanism := fromName(mechanismName) + for _, mechanism := range mechanismsToDisable { if mechanism.IsDeploymentAuthenticationConfigured(ac, opts) { deploymentMechanismsToDisable = append(deploymentMechanismsToDisable, mechanism) } } if len(deploymentMechanismsToDisable) == 0 { - log.Infof("Mechanisms %+v are all already disabled", mechanismsToDisable) + log.Infof("Mechanisms [%s] are all already disabled", mechanismsToDisable) return nil } @@ -555,14 +419,3 @@ func ensureDeploymentMechanismsAreDisabled(conn om.Connection, ac *om.Automation return nil } - -// containsMechanismName returns true if there is at least one MechanismName in `slice` -// that is equal to `mn`. -func containsMechanismName(slice []MechanismName, mn MechanismName) bool { - for _, item := range slice { - if item == mn { - return true - } - } - return false -} diff --git a/controllers/operator/authentication/authentication_mechanism.go b/controllers/operator/authentication/authentication_mechanism.go new file mode 100644 index 000000000..bc5521045 --- /dev/null +++ b/controllers/operator/authentication/authentication_mechanism.go @@ -0,0 +1,118 @@ +package authentication + +import ( + "slices" + "strings" + + "go.uber.org/zap" + "golang.org/x/xerrors" + + "github.com/mongodb/mongodb-kubernetes/controllers/om" + "github.com/mongodb/mongodb-kubernetes/pkg/util" +) + +// Mechanism is an interface that needs to be implemented for any Ops Manager authentication mechanism +type Mechanism interface { + EnableAgentAuthentication(conn om.Connection, opts Options, log *zap.SugaredLogger) error + DisableAgentAuthentication(conn om.Connection, log *zap.SugaredLogger) error + EnableDeploymentAuthentication(conn om.Connection, opts Options, log *zap.SugaredLogger) error + DisableDeploymentAuthentication(conn om.Connection, log *zap.SugaredLogger) error + // IsAgentAuthenticationConfigured should not rely on util.MergoDelete since the method is always + // called directly after deserializing the response from OM which should not contain the util.MergoDelete value in any field. + IsAgentAuthenticationConfigured(ac *om.AutomationConfig, opts Options) bool + IsDeploymentAuthenticationConfigured(ac *om.AutomationConfig, opts Options) bool + GetName() MechanismName +} + +// MechanismName corresponds to the string used in the automation config representing +// a particular type of authentication +type MechanismName string + +const ( + ScramSha256 MechanismName = "SCRAM-SHA-256" + ScramSha1 MechanismName = "SCRAM-SHA-1" + MongoDBX509 MechanismName = "MONGODB-X509" + LDAPPlain MechanismName = "PLAIN" + + // MongoDBCR is an umbrella term for SCRAM-SHA-1 and MONGODB-CR for legacy reasons, once MONGODB-CR + // is enabled, users can auth with SCRAM-SHA-1 credentials + MongoDBCR MechanismName = "MONGODB-CR" +) + +type MechanismList []Mechanism + +func (m MechanismList) String() string { + names := make([]string, 0) + for _, mechanism := range m { + names = append(names, string(mechanism.GetName())) + } + + slices.Sort(names) + + return strings.Join(names, ", ") +} + +func (m MechanismList) Contains(mechanism Mechanism) bool { + for _, m := range m { + if m.GetName() == mechanism.GetName() { + return true + } + } + + return false +} + +// supportedMechanisms returns a list of all supported authentication mechanisms +// that can be configured by the Operator +var supportedMechanisms = []Mechanism{ScramSha256Mechanism, MongoDBCRMechanism, MongoDBX509Mechanism, LDAPPlainMechanism} + +// mechanismsToDisable returns mechanisms which need to be disabled +// based on the currently supported authentication mechanisms and the desiredMechanisms +func mechanismsToDisable(desiredMechanisms MechanismList) MechanismList { + toDisable := make([]Mechanism, 0) + for _, mechanism := range supportedMechanisms { + if !desiredMechanisms.Contains(mechanism) { + toDisable = append(toDisable, mechanism) + } + } + + return toDisable +} + +func convertToMechanismList(mechanismModesInCR []string, ac *om.AutomationConfig) MechanismList { + result := make([]Mechanism, len(mechanismModesInCR)) + for i, mechanismModeInCR := range mechanismModesInCR { + result[i] = convertToMechanism(mechanismModeInCR, ac) + } + + return result +} + +// convertToMechanism returns an implementation of mechanism from the CR value +func convertToMechanism(mechanismModeInCR string, ac *om.AutomationConfig) Mechanism { + switch mechanismModeInCR { + case util.X509: + return MongoDBX509Mechanism + case util.LDAP: + return LDAPPlainMechanism + case util.SCRAMSHA1: + return ScramSha1Mechanism + case util.MONGODBCR: + return MongoDBCRMechanism + case util.SCRAMSHA256: + return ScramSha256Mechanism + case util.SCRAM: + // if we have already configured authentication, and it has been set to MONGODB-CR/SCRAM-SHA-1 + // we can not transition. This needs to be done in the UI + + // if no authentication has been configured, the default value for "AutoAuthMechanism" is "MONGODB-CR" + // even if authentication is disabled, so we need to ensure that auth has been enabled. + if ac.Auth.AutoAuthMechanism == string(MongoDBCR) && ac.Auth.IsEnabled() { + return MongoDBCRMechanism + } + return ScramSha256Mechanism + } + + // this should never be reached as validation of this string happens at the CR level + panic(xerrors.Errorf("unknown mechanism name %s", mechanismModeInCR)) +} diff --git a/controllers/operator/authentication/configure_authentication_test.go b/controllers/operator/authentication/configure_authentication_test.go index 79694fa93..ae21a6611 100644 --- a/controllers/operator/authentication/configure_authentication_test.go +++ b/controllers/operator/authentication/configure_authentication_test.go @@ -150,24 +150,24 @@ func TestGetCorrectAuthMechanismFromVersion(t *testing.T) { ac, err := conn.ReadAutomationConfig() require.NoError(t, err) - mechanismNames := getMechanismNames(ac, []string{"X509"}) + mechanismList := convertToMechanismList([]string{"X509"}, ac) - assert.Len(t, mechanismNames, 1) - assert.Contains(t, mechanismNames, MechanismName("MONGODB-X509")) + assert.Len(t, mechanismList, 1) + assert.Contains(t, mechanismList, MongoDBX509Mechanism) - mechanismNames = getMechanismNames(ac, []string{"SCRAM", "X509"}) + mechanismList = convertToMechanismList([]string{"SCRAM", "X509"}, ac) - assert.Contains(t, mechanismNames, MechanismName("SCRAM-SHA-256")) - assert.Contains(t, mechanismNames, MechanismName("MONGODB-X509")) + assert.Contains(t, mechanismList, ScramSha256Mechanism) + assert.Contains(t, mechanismList, MongoDBX509Mechanism) // enable MONGODB-CR ac.Auth.AutoAuthMechanism = "MONGODB-CR" ac.Auth.Enable() - mechanismNames = getMechanismNames(ac, []string{"SCRAM", "X509"}) + mechanismList = convertToMechanismList([]string{"SCRAM", "X509"}, ac) - assert.Contains(t, mechanismNames, MechanismName("MONGODB-CR")) - assert.Contains(t, mechanismNames, MechanismName("MONGODB-X509")) + assert.Contains(t, mechanismList, MongoDBCRMechanism) + assert.Contains(t, mechanismList, MongoDBX509Mechanism) } func assertAuthenticationEnabled(t *testing.T, auth *om.Auth) { diff --git a/controllers/operator/authentication/scramsha_credentials.go b/controllers/operator/authentication/scramsha_credentials.go index 3e60949ca..8ff7fe89b 100644 --- a/controllers/operator/authentication/scramsha_credentials.go +++ b/controllers/operator/authentication/scramsha_credentials.go @@ -16,6 +16,8 @@ import ( ) const ( + ExternalDB = "$external" + clientKeyInput = "Client Key" // specified in RFC 5802 serverKeyInput = "Server Key" // specified in RFC 5802 @@ -23,15 +25,42 @@ const ( scramSha1Iterations = 10000 scramSha256Iterations = 15000 - RFC5802MandatedSaltSize = 4 + rfc5802MandatedSaltSize = 4 ) +// ConfigureScramCredentials creates both SCRAM-SHA-1 and SCRAM-SHA-256 credentials. This ensures +// that changes to the authentication settings on the MongoDB resources won't leave MongoDBUsers without +// the correct credentials. +func ConfigureScramCredentials(user *om.MongoDBUser, password string) error { + scram256Salt, err := generateSalt(sha256.New) + if err != nil { + return xerrors.Errorf("error generating scramSha256 salt: %w", err) + } + + scram1Salt, err := generateSalt(sha1.New) + if err != nil { + return xerrors.Errorf("error generating scramSha1 salt: %w", err) + } + + scram256Creds, err := computeScramShaCreds(user.Username, password, scram256Salt, ScramSha256) + if err != nil { + return xerrors.Errorf("error generating scramSha256 creds: %w", err) + } + scram1Creds, err := computeScramShaCreds(user.Username, password, scram1Salt, MongoDBCR) + if err != nil { + return xerrors.Errorf("error generating scramSha1Creds: %w", err) + } + user.ScramSha256Creds = scram256Creds + user.ScramSha1Creds = scram1Creds + return nil +} + // The code in this file is largely adapted from the Automation Agent codebase. // https://github.com/10gen/mms-automation/blob/c108e0319cc05c0d8719ceea91a0424a016db583/go_planner/src/com.tengen/cm/crypto/scram.go -// ComputeScramShaCreds takes a plain text password and a specified mechanism name and generates +// computeScramShaCreds takes a plain text password and a specified mechanism name and generates // the ScramShaCreds which will be embedded into a MongoDBUser. -func ComputeScramShaCreds(username, password string, salt []byte, name MechanismName) (*om.ScramShaCreds, error) { +func computeScramShaCreds(username, password string, salt []byte, name MechanismName) (*om.ScramShaCreds, error) { var hashConstructor func() hash.Hash iterations := 0 if name == ScramSha256 { @@ -52,10 +81,10 @@ func ComputeScramShaCreds(username, password string, salt []byte, name Mechanism return computeScramCredentials(hashConstructor, iterations, base64EncodedSalt, password) } -// GenerateSalt will create a salt for use with ComputeScramShaCreds based on the given hashConstructor. +// generateSalt will create a salt for use with computeScramShaCreds based on the given hashConstructor. // sha1.New should be used for MONGODB-CR/SCRAM-SHA-1 and sha256.New should be used for SCRAM-SHA-256 -func GenerateSalt(hashConstructor func() hash.Hash) ([]byte, error) { - saltSize := hashConstructor().Size() - RFC5802MandatedSaltSize +func generateSalt(hashConstructor func() hash.Hash) ([]byte, error) { + saltSize := hashConstructor().Size() - rfc5802MandatedSaltSize salt, err := generate.RandomFixedLengthStringOfSize(saltSize) if err != nil { return nil, err @@ -81,8 +110,8 @@ func hmacIteration(hashConstructor func() hash.Hash, input, salt []byte, iterati // incorrect salt size will pass validation, but the credentials will be invalid. i.e. it will not // be possible to auth with the password provided to create the credentials. - if len(salt) != hashSize-RFC5802MandatedSaltSize { - return nil, xerrors.Errorf("salt should have a size of %v bytes, but instead has a size of %v bytes", hashSize-RFC5802MandatedSaltSize, len(salt)) + if len(salt) != hashSize-rfc5802MandatedSaltSize { + return nil, xerrors.Errorf("salt should have a size of %v bytes, but instead has a size of %v bytes", hashSize-rfc5802MandatedSaltSize, len(salt)) } startKey := append(salt, 0, 0, 0, 1) diff --git a/controllers/operator/authentication/scramsha_test.go b/controllers/operator/authentication/scramsha_test.go index 175bfbfc7..21a0dd61c 100644 --- a/controllers/operator/authentication/scramsha_test.go +++ b/controllers/operator/authentication/scramsha_test.go @@ -49,9 +49,7 @@ func TestAgentsAuthentication(t *testing.T) { assertAuthenticationEnabled(t, ac.Auth) assert.Equal(t, ac.Auth.AutoUser, util.AutomationAgentName) assert.Len(t, ac.Auth.AutoAuthMechanisms, 1) - for _, mech := range testConfig.mechanism.GetName() { - assert.Contains(t, ac.Auth.AutoAuthMechanisms, string(mech)) - } + assert.Contains(t, ac.Auth.AutoAuthMechanisms, string(testConfig.mechanism.GetName())) assert.NotEmpty(t, ac.Auth.AutoPwd) assert.True(t, s.IsAgentAuthenticationConfigured(ac, opts)) assert.True(t, s.IsDeploymentAuthenticationConfigured(ac, opts)) diff --git a/controllers/operator/authentication/x509.go b/controllers/operator/authentication/x509.go index efcd8b6e6..61d7ff2ce 100644 --- a/controllers/operator/authentication/x509.go +++ b/controllers/operator/authentication/x509.go @@ -10,8 +10,6 @@ import ( "github.com/mongodb/mongodb-kubernetes/pkg/util/stringutil" ) -const ExternalDB = "$external" - var MongoDBX509Mechanism = ConnectionX509{} type ConnectionX509 struct{} From 7c231436177bb5b2c7eb2aee4adbb4f4e515994f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Maciej=20Kara=C5=9B?= Date: Fri, 25 Apr 2025 12:54:46 +0200 Subject: [PATCH 04/36] Added validation logic + tests --- api/v1/mdb/mongodb_types.go | 23 +- api/v1/mdb/mongodb_types_test.go | 69 +++++ api/v1/mdb/mongodb_validation.go | 151 +++++++++- api/v1/mdb/mongodb_validation_test.go | 271 ++++++++++++++++++ config/crd/bases/mongodb.com_mongodb.yaml | 2 +- .../mongodb.com_mongodbmulticluster.yaml | 2 +- config/crd/bases/mongodb.com_opsmanagers.yaml | 2 +- helm_chart/crds/mongodb.com_mongodb.yaml | 2 +- .../crds/mongodb.com_mongodbmulticluster.yaml | 2 +- helm_chart/crds/mongodb.com_opsmanagers.yaml | 2 +- pkg/util/util.go | 7 + public/crds.yaml | 6 +- 12 files changed, 526 insertions(+), 13 deletions(-) diff --git a/api/v1/mdb/mongodb_types.go b/api/v1/mdb/mongodb_types.go index 21929bb90..31c3ee3f9 100644 --- a/api/v1/mdb/mongodb_types.go +++ b/api/v1/mdb/mongodb_types.go @@ -807,6 +807,22 @@ func (s *Security) IsTLSEnabled() bool { return s.CertificatesSecretsPrefix != "" } +func (s *Security) IsOIDCEnabled() bool { + if s == nil { + return false + } + + if s.Authentication == nil { + return false + } + + if !s.Authentication.Enabled { + return false + } + + return s.Authentication.IsOIDCEnabled() +} + // GetAgentMechanism returns the authentication mechanism that the agents will be using. // The agents will use X509 if it is the only mechanism specified, otherwise they will use SCRAM if specified // and no auth if no mechanisms exist. @@ -1047,12 +1063,13 @@ type Ldap struct { } type OIDCProviderConfig struct { - // TODO add proper validation + // TODO add proper validation and test it // Unique label that identifies this configuration. This label is visible to your Ops Manager users and is used when // creating users and roles for authorization. It is case-sensitive and can only contain the following characters: // - alphanumeric characters (combination of a to z and 0 to 9) // - hyphens (-) // - underscores (_) + // +kubebuilder:validation:Pattern:"^[a-zA-Z0-9-_]+$" ConfigurationName string `json:"configurationName"` // TODO add URI validation @@ -1280,6 +1297,10 @@ func (m *MongoDB) GetStatus(...status.Option) interface{} { return m.Status } +func (m *MongoDB) GetStatusWarnings() []status.Warning { + return m.Status.Warnings +} + func (m *MongoDB) GetCommonStatus(...status.Option) *status.Common { return &m.Status.Common } diff --git a/api/v1/mdb/mongodb_types_test.go b/api/v1/mdb/mongodb_types_test.go index b17bbc04e..8566d6f71 100644 --- a/api/v1/mdb/mongodb_types_test.go +++ b/api/v1/mdb/mongodb_types_test.go @@ -61,6 +61,75 @@ func TestGetAgentAuthentication(t *testing.T) { assert.Equal(t, util.X509, sec.GetAgentMechanism("SCRAM-SHA-256"), "transitioning from SCRAM -> X509 is allowed") } +func TestGetAuthenticationIsEnabledMethods(t *testing.T) { + tests := []struct { + name string + authentication *Authentication + expectedX509 bool + expectedLDAP bool + expectedOIDC bool + }{ + { + name: "Nil authentication", + authentication: nil, + expectedX509: false, + expectedLDAP: false, + expectedOIDC: false, + }, + { + name: "Empty authentication", + authentication: newAuthentication(), + expectedX509: false, + expectedLDAP: false, + expectedOIDC: false, + }, + { + name: "Authentication with x509 only", + authentication: &Authentication{ + Modes: []AuthMode{util.X509}, + }, + expectedX509: true, + expectedLDAP: false, + expectedOIDC: false, + }, + { + name: "Authentication with LDAP only", + authentication: &Authentication{ + Modes: []AuthMode{util.LDAP}, + }, + expectedX509: false, + expectedLDAP: true, + expectedOIDC: false, + }, + { + name: "Authentication with OIDC only", + authentication: &Authentication{ + Modes: []AuthMode{util.OIDC}, + }, + expectedX509: false, + expectedLDAP: false, + expectedOIDC: true, + }, + { + name: "Authentication with multiple modes", + authentication: &Authentication{ + Modes: []AuthMode{util.X509, util.LDAP, util.OIDC, util.SCRAM}, + }, + expectedX509: true, + expectedLDAP: true, + expectedOIDC: true, + }, + } + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + auth := test.authentication + assert.Equal(t, test.expectedX509, auth.IsX509Enabled()) + assert.Equal(t, test.expectedLDAP, auth.IsLDAPEnabled()) + assert.Equal(t, test.expectedOIDC, auth.IsOIDCEnabled()) + }) + } +} + func TestMinimumMajorVersion(t *testing.T) { mdbSpec := MongoDbSpec{ DbCommonSpec: DbCommonSpec{ diff --git a/api/v1/mdb/mongodb_validation.go b/api/v1/mdb/mongodb_validation.go index 9b811cf48..a91f983bf 100644 --- a/api/v1/mdb/mongodb_validation.go +++ b/api/v1/mdb/mongodb_validation.go @@ -2,6 +2,7 @@ package mdb import ( "errors" + "fmt" "strings" "k8s.io/apimachinery/pkg/runtime" @@ -105,6 +106,146 @@ func scramSha1AuthValidation(d DbCommonSpec) v1.ValidationResult { return v1.ValidationSuccess() } +func oidcAuthValidators(db DbCommonSpec) []func(DbCommonSpec) v1.ValidationResult { + validators := make([]func(DbCommonSpec) v1.ValidationResult, 0) + if !db.Security.IsOIDCEnabled() { + return validators + } + + authentication := db.Security.Authentication + validators = append(validators, oidcAuthModeValidator(authentication)) + + providerConfigs := authentication.OIDCProviderConfigs + if len(providerConfigs) == 0 { + return validators + } + + validators = append(validators, + oidcProviderConfigsUniqueNameValidation(providerConfigs), + oidcProviderConfigsSingleWorkforceIdentityFederationValidation(providerConfigs), + ) + + for _, config := range providerConfigs { + validators = append(validators, + oidcProviderConfigIssuerURIValidator(config), + oidcProviderConfigClientIdValidator(config), + oidcProviderConfigRequestedScopesValidator(config), + oidcProviderConfigAuthorizationTypeValidator(config), + ) + } + + return validators +} + +func oidcAuthModeValidator(authentication *Authentication) func(DbCommonSpec) v1.ValidationResult { + return func(spec DbCommonSpec) v1.ValidationResult { + // OIDC cannot be used for agent authentication so other auth mode has to enabled as well + if len(authentication.Modes) == 1 { + return v1.ValidationError("OIDC authentication cannot be used as the only authentication mechanism") + } + + oidcProviderConfigs := authentication.OIDCProviderConfigs + if len(oidcProviderConfigs) == 0 { + return v1.ValidationError("At least one OIDC provider config needs to be specified when OIDC authentication is enabled") + } + + return v1.ValidationSuccess() + } +} + +func oidcProviderConfigsUniqueNameValidation(configs []OIDCProviderConfig) func(DbCommonSpec) v1.ValidationResult { + return func(spec DbCommonSpec) v1.ValidationResult { + configNames := make(map[string]bool) + for _, config := range configs { + if _, ok := configNames[config.ConfigurationName]; ok { + return v1.ValidationError("OIDC provider config name %s is not unique", config.ConfigurationName) + } + + configNames[config.ConfigurationName] = true + } + + return v1.ValidationSuccess() + } +} + +func oidcProviderConfigsSingleWorkforceIdentityFederationValidation(configs []OIDCProviderConfig) func(DbCommonSpec) v1.ValidationResult { + return func(spec DbCommonSpec) v1.ValidationResult { + workforceIdentityFederationConfigs := make([]string, 0) + for _, config := range configs { + if config.AuthorizationMethod == OIDCAuthorizationMethodWorkforceIdentityFederation { + workforceIdentityFederationConfigs = append(workforceIdentityFederationConfigs, config.ConfigurationName) + } + } + + if len(workforceIdentityFederationConfigs) > 1 { + msg := fmt.Sprintf("Only one OIDC provider config can be configured with Workforce Identity Federation. "+ + "The following configs are configured with Workforce Identity Federation: %s", strings.Join(workforceIdentityFederationConfigs, ", ")) + return v1.ValidationError("%s", msg) + } + + return v1.ValidationSuccess() + } +} + +func oidcProviderConfigIssuerURIValidator(config OIDCProviderConfig) func(DbCommonSpec) v1.ValidationResult { + return func(_ DbCommonSpec) v1.ValidationResult { + ok, url := util.IsURL(config.IssuerURI) + if !ok { + return v1.ValidationError("Invalid IssuerURI in OIDC provider config %q", config.ConfigurationName) + } + + if url.Scheme != "https" { + return v1.ValidationWarning("IssuerURI in OIDC provider config %q in not secure endpoint", config.ConfigurationName) + } + + return v1.ValidationSuccess() + } +} + +func oidcProviderConfigClientIdValidator(config OIDCProviderConfig) func(DbCommonSpec) v1.ValidationResult { + return func(_ DbCommonSpec) v1.ValidationResult { + if config.AuthorizationMethod == OIDCAuthorizationMethodWorkforceIdentityFederation { + if config.ClientId == "" { + return v1.ValidationError("ClientId has to be specified in OIDC provider config %q with Workforce Identity Federation", config.ConfigurationName) + } + } else if config.AuthorizationMethod == OIDCAuthorizationMethodWorkloadIdentityFederation { + if config.ClientId != "" { + return v1.ValidationWarning("ClientId will be ignored in OIDC provider config %q with Workload Identity Federation", config.ConfigurationName) + } + } + + return v1.ValidationSuccess() + } +} + +func oidcProviderConfigRequestedScopesValidator(config OIDCProviderConfig) func(DbCommonSpec) v1.ValidationResult { + return func(_ DbCommonSpec) v1.ValidationResult { + if config.AuthorizationMethod == OIDCAuthorizationMethodWorkloadIdentityFederation { + if len(config.RequestedScopes) > 0 { + return v1.ValidationWarning("RequestedScopes will be ignored in OIDC provider config %q with Workload Identity Federation", config.ConfigurationName) + } + } + + return v1.ValidationSuccess() + } +} + +func oidcProviderConfigAuthorizationTypeValidator(config OIDCProviderConfig) func(DbCommonSpec) v1.ValidationResult { + return func(_ DbCommonSpec) v1.ValidationResult { + if config.AuthorizationType == OIDCAuthorizationTypeGroupMembership { + if config.GroupsClaim == "" { + return v1.ValidationError("GroupsClaim has to be specified in OIDC provider config %q when using Group Membership authorization", config.ConfigurationName) + } + } else if config.AuthorizationType == OIDCAuthorizationTypeUserID { + if config.GroupsClaim != "" { + return v1.ValidationWarning("GroupsClaim will be ignored in OIDC provider config %q when using User ID authorization", config.ConfigurationName) + } + } + + return v1.ValidationSuccess() + } +} + func ldapAuthRequiresEnterprise(d DbCommonSpec) v1.ValidationResult { authSpec := d.Security.Authentication if authSpec != nil && authSpec.IsLDAPEnabled() && !strings.HasSuffix(d.Version, "-ent") { @@ -187,8 +328,8 @@ func specWithExactlyOneSchema(d DbCommonSpec) v1.ValidationResult { return v1.ValidationSuccess() } -func CommonValidators() []func(d DbCommonSpec) v1.ValidationResult { - return []func(d DbCommonSpec) v1.ValidationResult{ +func CommonValidators(db DbCommonSpec) []func(d DbCommonSpec) v1.ValidationResult { + validators := []func(d DbCommonSpec) v1.ValidationResult{ replicaSetHorizonsRequireTLS, deploymentsMustHaveTLSInX509Env, deploymentsMustHaveAtLeastOneAuthModeIfAuthIsEnabled, @@ -201,6 +342,10 @@ func CommonValidators() []func(d DbCommonSpec) v1.ValidationResult { specWithExactlyOneSchema, featureCompatibilityVersionValidation, } + + validators = append(validators, oidcAuthValidators(db)...) + + return validators } func featureCompatibilityVersionValidation(d DbCommonSpec) v1.ValidationResult { @@ -245,7 +390,7 @@ func (m *MongoDB) RunValidations(old *MongoDB) []v1.ValidationResult { } } - for _, validator := range CommonValidators() { + for _, validator := range CommonValidators(m.Spec.DbCommonSpec) { res := validator(m.Spec.DbCommonSpec) if res.Level > 0 { validationResults = append(validationResults, res) diff --git a/api/v1/mdb/mongodb_validation_test.go b/api/v1/mdb/mongodb_validation_test.go index 21f2263d1..4a3db9da4 100644 --- a/api/v1/mdb/mongodb_validation_test.go +++ b/api/v1/mdb/mongodb_validation_test.go @@ -8,6 +8,7 @@ import ( "k8s.io/utils/ptr" v1 "github.com/mongodb/mongodb-kubernetes/api/v1" + "github.com/mongodb/mongodb-kubernetes/api/v1/status" "github.com/mongodb/mongodb-kubernetes/pkg/util" ) @@ -200,3 +201,273 @@ func TestReplicasetFCV(t *testing.T) { }) } } + +func TestOIDCAuthValidation(t *testing.T) { + tests := []struct { + name string + auth *Authentication + expectedErrorMessage string + expectedWarning status.Warning + }{ + { + name: "Authentication disabled", + auth: &Authentication{ + Enabled: false, + }, + }, + { + name: "OIDC not enabled", + auth: &Authentication{ + Enabled: true, + Modes: []AuthMode{util.SCRAMSHA256}, + }, + }, + { + name: "OIDC cannot be only authentication mode enabled", + auth: &Authentication{ + Enabled: true, + Modes: []AuthMode{util.OIDC}, + }, + expectedErrorMessage: "OIDC authentication cannot be used as the only authentication mechanism", + }, + { + name: "Agent authentication mode not specified, but required", + auth: &Authentication{ + Enabled: true, + Modes: []AuthMode{util.OIDC, util.SCRAMSHA256}, + }, + expectedErrorMessage: "spec.security.authentication.agents.mode must be specified if more than one entry is present in spec.security.authentication.modes", + }, + { + name: "OIDC enabled but without provider configs", + auth: &Authentication{ + Enabled: true, + Agents: AgentAuthentication{Mode: util.SCRAMSHA256}, + Modes: []AuthMode{util.OIDC, util.SCRAMSHA256}, + }, + expectedErrorMessage: "At least one OIDC provider config needs to be specified when OIDC authentication is enabled", + }, + { + name: "Multiple non unique configuration names", + auth: &Authentication{ + Enabled: true, + Agents: AgentAuthentication{Mode: util.SCRAMSHA256}, + Modes: []AuthMode{util.OIDC, util.SCRAMSHA256}, + OIDCProviderConfigs: []OIDCProviderConfig{ + { + ConfigurationName: "provider", + IssuerURI: "https://example1.com", + AuthorizationMethod: OIDCAuthorizationMethodWorkforceIdentityFederation, + ClientId: "clientId1", + }, + { + ConfigurationName: "provider", + IssuerURI: "https://example2.com", + AuthorizationMethod: OIDCAuthorizationMethodWorkforceIdentityFederation, + ClientId: "clientId2", + }, + }, + }, + expectedErrorMessage: "OIDC provider config name provider is not unique", + }, + { + name: "Multiple Workforce Identity Federation configs", + auth: &Authentication{ + Enabled: true, + Agents: AgentAuthentication{Mode: util.SCRAMSHA256}, + Modes: []AuthMode{util.OIDC, util.SCRAMSHA256}, + OIDCProviderConfigs: []OIDCProviderConfig{ + { + ConfigurationName: "test-provider1", + IssuerURI: "https://example1.com", + AuthorizationMethod: OIDCAuthorizationMethodWorkforceIdentityFederation, + ClientId: "clientId1", + }, + { + ConfigurationName: "test-provider2", + IssuerURI: "https://example2.com", + AuthorizationMethod: OIDCAuthorizationMethodWorkforceIdentityFederation, + ClientId: "clientId2", + }, + }, + }, + expectedErrorMessage: "Only one OIDC provider config can be configured with Workforce Identity Federation. The following configs are configured with Workforce Identity Federation: test-provider1, test-provider2", + }, + { + name: "Invalid issuer URI", + auth: &Authentication{ + Enabled: true, + Agents: AgentAuthentication{Mode: util.SCRAMSHA256}, + Modes: []AuthMode{util.OIDC, util.SCRAMSHA256}, + OIDCProviderConfigs: []OIDCProviderConfig{ + { + ConfigurationName: "test-provider", + IssuerURI: "invalid-uri", + }, + }, + }, + expectedErrorMessage: "Invalid IssuerURI in OIDC provider config \"test-provider\"", + }, + // TODO fix validation, we should not return warnings immediately, only add them to the result + { + name: "Non-HTTPS issuer URI - warning", + auth: &Authentication{ + Enabled: true, + Agents: AgentAuthentication{Mode: util.SCRAMSHA256}, + Modes: []AuthMode{util.OIDC, util.SCRAMSHA256}, + OIDCProviderConfigs: []OIDCProviderConfig{ + { + ConfigurationName: "test-provider", + IssuerURI: "http://example.com", + }, + }, + }, + expectedWarning: "IssuerURI in OIDC provider config \"test-provider\" in not secure endpoint", + }, + { + name: "Workforce Identity Federation without ClientId", + auth: &Authentication{ + Enabled: true, + Agents: AgentAuthentication{Mode: util.SCRAMSHA256}, + Modes: []AuthMode{util.OIDC, util.SCRAMSHA256}, + OIDCProviderConfigs: []OIDCProviderConfig{ + { + ConfigurationName: "test-provider", + IssuerURI: "https://example.com", + AuthorizationMethod: OIDCAuthorizationMethodWorkforceIdentityFederation, + }, + }, + }, + expectedErrorMessage: "ClientId has to be specified in OIDC provider config \"test-provider\" with Workforce Identity Federation", + }, + { + name: "Workload Identity Federation with ClientId - warning", + auth: &Authentication{ + Enabled: true, + Agents: AgentAuthentication{Mode: util.SCRAMSHA256}, + Modes: []AuthMode{util.OIDC, util.SCRAMSHA256}, + OIDCProviderConfigs: []OIDCProviderConfig{ + { + ConfigurationName: "test-provider", + IssuerURI: "https://example.com", + AuthorizationMethod: OIDCAuthorizationMethodWorkloadIdentityFederation, + ClientId: "clientId", + }, + }, + }, + expectedWarning: "ClientId will be ignored in OIDC provider config \"test-provider\" with Workload Identity Federation", + }, + { + name: "Workload Identity Federation with RequestedScopes - warning", + auth: &Authentication{ + Enabled: true, + Agents: AgentAuthentication{Mode: util.SCRAMSHA256}, + Modes: []AuthMode{util.OIDC, util.SCRAMSHA256}, + OIDCProviderConfigs: []OIDCProviderConfig{ + { + ConfigurationName: "test-provider", + IssuerURI: "https://example.com", + AuthorizationMethod: OIDCAuthorizationMethodWorkloadIdentityFederation, + RequestedScopes: []string{"openid", "email"}, + }, + }, + }, + expectedWarning: "RequestedScopes will be ignored in OIDC provider config \"test-provider\" with Workload Identity Federation", + }, + { + name: "Group Membership authorization without GroupsClaim", + auth: &Authentication{ + Enabled: true, + Agents: AgentAuthentication{Mode: util.SCRAMSHA256}, + Modes: []AuthMode{util.OIDC, util.SCRAMSHA256}, + OIDCProviderConfigs: []OIDCProviderConfig{ + { + ConfigurationName: "test-provider1", + IssuerURI: "https://example.com", + AuthorizationType: OIDCAuthorizationTypeGroupMembership, + GroupsClaim: "groups", + }, + { + ConfigurationName: "test-provider2", + IssuerURI: "https://example.com", + AuthorizationType: OIDCAuthorizationTypeGroupMembership, + }, + }, + }, + expectedErrorMessage: "GroupsClaim has to be specified in OIDC provider config \"test-provider2\" when using Group Membership authorization", + }, + { + name: "User ID authorization with GroupsClaim - warning", + auth: &Authentication{ + Enabled: true, + Agents: AgentAuthentication{Mode: util.SCRAMSHA256}, + Modes: []AuthMode{util.OIDC, util.SCRAMSHA256}, + OIDCProviderConfigs: []OIDCProviderConfig{ + { + ConfigurationName: "test-provider1", + IssuerURI: "https://example.com", + AuthorizationType: OIDCAuthorizationTypeUserID, + GroupsClaim: "groups", + UserClaim: "sub", + }, + { + ConfigurationName: "test-provider2", + IssuerURI: "https://example.com", + AuthorizationType: OIDCAuthorizationTypeUserID, + UserClaim: "sub", + }, + }, + }, + expectedWarning: "GroupsClaim will be ignored in OIDC provider config \"test-provider1\" when using User ID authorization", + }, + { + name: "Valid OIDC configuration", + auth: &Authentication{ + Enabled: true, + Agents: AgentAuthentication{Mode: util.MONGODBCR}, + Modes: []AuthMode{util.OIDC, util.MONGODBCR}, + OIDCProviderConfigs: []OIDCProviderConfig{ + { + ConfigurationName: "test-provider1", + IssuerURI: "https://example.com", + AuthorizationType: OIDCAuthorizationTypeGroupMembership, + GroupsClaim: "groups", + }, + { + ConfigurationName: "test-provider2", + IssuerURI: "https://example.com", + AuthorizationType: OIDCAuthorizationTypeGroupMembership, + GroupsClaim: "groups", + }, + }, + }, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + rs := NewReplicaSetBuilder(). + SetSecurityTLSEnabled(). + Build() + + rs.Spec.CloudManagerConfig = &PrivateCloudConfig{ + ConfigMapRef: ConfigMapRef{Name: "cloud-manager"}, + } + rs.Spec.Security.Authentication = tt.auth + + err := rs.ProcessValidationsOnReconcile(nil) + + if tt.expectedErrorMessage != "" { + assert.NotNil(t, err) + assert.Equal(t, tt.expectedErrorMessage, err.Error()) + } else { + assert.Nil(t, err) + } + + if tt.expectedWarning != "" { + warnings := rs.GetStatusWarnings() + assert.Contains(t, warnings, tt.expectedWarning) + } + }) + } +} diff --git a/config/crd/bases/mongodb.com_mongodb.yaml b/config/crd/bases/mongodb.com_mongodb.yaml index d93468b13..33115441b 100644 --- a/config/crd/bases/mongodb.com_mongodb.yaml +++ b/config/crd/bases/mongodb.com_mongodb.yaml @@ -1557,7 +1557,7 @@ spec: type: string configurationName: description: |- - TODO add proper validation + TODO add proper validation and test it Unique label that identifies this configuration. This label is visible to your Ops Manager users and is used when creating users and roles for authorization. It is case-sensitive and can only contain the following characters: - alphanumeric characters (combination of a to z and 0 to 9) diff --git a/config/crd/bases/mongodb.com_mongodbmulticluster.yaml b/config/crd/bases/mongodb.com_mongodbmulticluster.yaml index f9fed2293..28087ec50 100644 --- a/config/crd/bases/mongodb.com_mongodbmulticluster.yaml +++ b/config/crd/bases/mongodb.com_mongodbmulticluster.yaml @@ -817,7 +817,7 @@ spec: type: string configurationName: description: |- - TODO add proper validation + TODO add proper validation and test it Unique label that identifies this configuration. This label is visible to your Ops Manager users and is used when creating users and roles for authorization. It is case-sensitive and can only contain the following characters: - alphanumeric characters (combination of a to z and 0 to 9) diff --git a/config/crd/bases/mongodb.com_opsmanagers.yaml b/config/crd/bases/mongodb.com_opsmanagers.yaml index adac7f2ae..e54059ce5 100644 --- a/config/crd/bases/mongodb.com_opsmanagers.yaml +++ b/config/crd/bases/mongodb.com_opsmanagers.yaml @@ -879,7 +879,7 @@ spec: type: string configurationName: description: |- - TODO add proper validation + TODO add proper validation and test it Unique label that identifies this configuration. This label is visible to your Ops Manager users and is used when creating users and roles for authorization. It is case-sensitive and can only contain the following characters: - alphanumeric characters (combination of a to z and 0 to 9) diff --git a/helm_chart/crds/mongodb.com_mongodb.yaml b/helm_chart/crds/mongodb.com_mongodb.yaml index d93468b13..33115441b 100644 --- a/helm_chart/crds/mongodb.com_mongodb.yaml +++ b/helm_chart/crds/mongodb.com_mongodb.yaml @@ -1557,7 +1557,7 @@ spec: type: string configurationName: description: |- - TODO add proper validation + TODO add proper validation and test it Unique label that identifies this configuration. This label is visible to your Ops Manager users and is used when creating users and roles for authorization. It is case-sensitive and can only contain the following characters: - alphanumeric characters (combination of a to z and 0 to 9) diff --git a/helm_chart/crds/mongodb.com_mongodbmulticluster.yaml b/helm_chart/crds/mongodb.com_mongodbmulticluster.yaml index f9fed2293..28087ec50 100644 --- a/helm_chart/crds/mongodb.com_mongodbmulticluster.yaml +++ b/helm_chart/crds/mongodb.com_mongodbmulticluster.yaml @@ -817,7 +817,7 @@ spec: type: string configurationName: description: |- - TODO add proper validation + TODO add proper validation and test it Unique label that identifies this configuration. This label is visible to your Ops Manager users and is used when creating users and roles for authorization. It is case-sensitive and can only contain the following characters: - alphanumeric characters (combination of a to z and 0 to 9) diff --git a/helm_chart/crds/mongodb.com_opsmanagers.yaml b/helm_chart/crds/mongodb.com_opsmanagers.yaml index adac7f2ae..e54059ce5 100644 --- a/helm_chart/crds/mongodb.com_opsmanagers.yaml +++ b/helm_chart/crds/mongodb.com_opsmanagers.yaml @@ -879,7 +879,7 @@ spec: type: string configurationName: description: |- - TODO add proper validation + TODO add proper validation and test it Unique label that identifies this configuration. This label is visible to your Ops Manager users and is used when creating users and roles for authorization. It is case-sensitive and can only contain the following characters: - alphanumeric characters (combination of a to z and 0 to 9) diff --git a/pkg/util/util.go b/pkg/util/util.go index ef2a8a654..f941b4bb7 100644 --- a/pkg/util/util.go +++ b/pkg/util/util.go @@ -6,6 +6,7 @@ import ( "encoding/gob" "encoding/hex" "fmt" + "net/url" "regexp" "strings" "time" @@ -184,3 +185,9 @@ func TransformToMap[T any, K comparable, V any](objs []T, f func(obj T, idx int) } return result } + +// IsURL checks if the given string is a valid URL and returns the parsed URL if valid. +func IsURL(str string) (bool, *url.URL) { + u, err := url.Parse(str) + return err == nil && u.Scheme != "" && u.Host != "", u +} diff --git a/public/crds.yaml b/public/crds.yaml index 68ee6ff03..ca76be53b 100644 --- a/public/crds.yaml +++ b/public/crds.yaml @@ -1557,7 +1557,7 @@ spec: type: string configurationName: description: |- - TODO add proper validation + TODO add proper validation and test it Unique label that identifies this configuration. This label is visible to your Ops Manager users and is used when creating users and roles for authorization. It is case-sensitive and can only contain the following characters: - alphanumeric characters (combination of a to z and 0 to 9) @@ -4191,7 +4191,7 @@ spec: type: string configurationName: description: |- - TODO add proper validation + TODO add proper validation and test it Unique label that identifies this configuration. This label is visible to your Ops Manager users and is used when creating users and roles for authorization. It is case-sensitive and can only contain the following characters: - alphanumeric characters (combination of a to z and 0 to 9) @@ -5580,7 +5580,7 @@ spec: type: string configurationName: description: |- - TODO add proper validation + TODO add proper validation and test it Unique label that identifies this configuration. This label is visible to your Ops Manager users and is used when creating users and roles for authorization. It is case-sensitive and can only contain the following characters: - alphanumeric characters (combination of a to z and 0 to 9) From 8cba1c1a7ad5d2b543999e221519f1c31ef1ca6e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Maciej=20Kara=C5=9B?= Date: Sun, 27 Apr 2025 16:40:23 +0200 Subject: [PATCH 05/36] Add URL test validation --- api/v1/mdb/mongodb_types.go | 1 - pkg/util/util_test.go | 80 +++++++++++++++++++++++++++++++++++++ 2 files changed, 80 insertions(+), 1 deletion(-) diff --git a/api/v1/mdb/mongodb_types.go b/api/v1/mdb/mongodb_types.go index 31c3ee3f9..a12933875 100644 --- a/api/v1/mdb/mongodb_types.go +++ b/api/v1/mdb/mongodb_types.go @@ -1072,7 +1072,6 @@ type OIDCProviderConfig struct { // +kubebuilder:validation:Pattern:"^[a-zA-Z0-9-_]+$" ConfigurationName string `json:"configurationName"` - // TODO add URI validation // Issuer value provided by your registered IdP application. Using this URI, MongoDB finds an OpenID Provider // Configuration Document, which should be available in the /.wellknown/open-id-configuration endpoint. IssuerURI string `json:"issuerURI"` diff --git a/pkg/util/util_test.go b/pkg/util/util_test.go index 00548660b..7ffe59f80 100644 --- a/pkg/util/util_test.go +++ b/pkg/util/util_test.go @@ -2,6 +2,7 @@ package util import ( "fmt" + "net/url" "testing" "github.com/stretchr/testify/assert" @@ -197,6 +198,85 @@ func TestTransformToMap(t *testing.T) { })) } +// TestIsURL tests the IsURL function with various inputs. +// +//goland:noinspection HttpUrlsUsage +func TestIsURL(t *testing.T) { + tests := []struct { + name string + input string + isValid bool + checkURL func(*url.URL) bool + }{ + { + name: "valid http URL", + input: "http://example.com", + isValid: true, + checkURL: func(u *url.URL) bool { + return u.Scheme == "http" && u.Host == "example.com" + }, + }, + { + name: "valid https URL with path", + input: "https://example.com/path", + isValid: true, + checkURL: func(u *url.URL) bool { + return u.Scheme == "https" && u.Host == "example.com" && u.Path == "/path" + }, + }, + { + name: "valid URL with port", + input: "http://example.com:8080", + isValid: true, + checkURL: func(u *url.URL) bool { + return u.Host == "example.com:8080" + }, + }, + { + name: "missing scheme", + input: "example.com", + isValid: false, + }, + { + name: "missing host", + input: "http://", + isValid: false, + }, + { + name: "empty string", + input: "", + isValid: false, + }, + { + name: "invalid URL", + input: ":invalid-url", + isValid: false, + }, + { + name: "file scheme", + input: "file:///path/to/file", + isValid: true, + checkURL: func(u *url.URL) bool { + return u.Scheme == "file" && u.Path == "/path/to/file" + }, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + valid, u := IsURL(tt.input) + assert.Equal(t, tt.isValid, valid) + + if tt.isValid { + assert.NotNil(t, u) + if tt.checkURL != nil { + assert.True(t, tt.checkURL(u)) + } + } + }) + } +} + func pair(left, right identifiable.Identifiable) []identifiable.Identifiable { return []identifiable.Identifiable{left, right} } From 25288928c1103f4c5555ac313142e96adb0f8b3b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Maciej=20Kara=C5=9B?= Date: Sun, 27 Apr 2025 16:48:22 +0200 Subject: [PATCH 06/36] Fixed MDB Multi code --- api/v1/mdbmulti/mongodbmulti_validation.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/api/v1/mdbmulti/mongodbmulti_validation.go b/api/v1/mdbmulti/mongodbmulti_validation.go index be2e3cae9..ff33ed781 100644 --- a/api/v1/mdbmulti/mongodbmulti_validation.go +++ b/api/v1/mdbmulti/mongodbmulti_validation.go @@ -53,7 +53,7 @@ func (m *MongoDBMultiCluster) RunValidations(old *MongoDBMultiCluster) []v1.Vali var validationResults []v1.ValidationResult - for _, validator := range mdbv1.CommonValidators() { + for _, validator := range mdbv1.CommonValidators(m.Spec.DbCommonSpec) { res := validator(m.Spec.DbCommonSpec) if res.Level > 0 { validationResults = append(validationResults, res) From 6d2745895e95da8ad8bb39ecb0f19f16efde4838 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Maciej=20Kara=C5=9B?= Date: Thu, 17 Apr 2025 11:43:17 +0200 Subject: [PATCH 07/36] Propagating CRD values --- controllers/om/automation_config.go | 27 ++- .../operator/authentication/authentication.go | 3 + .../authentication_mechanism.go | 3 + controllers/operator/authentication/oidc.go | 159 ++++++++++++++++++ controllers/operator/common_controller.go | 4 + controllers/operator/oidc/oidc_types.go | 14 ++ 6 files changed, 206 insertions(+), 4 deletions(-) create mode 100644 controllers/operator/authentication/oidc.go create mode 100644 controllers/operator/oidc/oidc_types.go diff --git a/controllers/om/automation_config.go b/controllers/om/automation_config.go index 9dffdffdb..ca60e6e5b 100644 --- a/controllers/om/automation_config.go +++ b/controllers/om/automation_config.go @@ -20,10 +20,11 @@ import ( // configuration which are merged into the `Deployment` object before sending it back to Ops Manager. // As of right now only support configuring LogRotate for monitoring and backup via dedicated endpoints. type AutomationConfig struct { - Auth *Auth - AgentSSL *AgentSSL - Deployment Deployment - Ldap *ldap.Ldap + Auth *Auth + AgentSSL *AgentSSL + Deployment Deployment + Ldap *ldap.Ldap + OIDCProviderConfigs []oidc.ProviderConfig } // Apply merges the state of all concrete structs into the Deployment (map[string]interface{}) @@ -58,6 +59,10 @@ func applyInto(a AutomationConfig, into *Deployment) error { } (*into)["ldap"] = mergedLdap } + + if _, ok := a.Deployment["oidcProviderConfigs"]; ok { + // TODO merge lists and their contents (should we just overwrite everything?) + } return nil } @@ -432,6 +437,20 @@ func BuildAutomationConfigFromDeployment(deployment Deployment) (*AutomationConf finalAutomationConfig.Ldap = acLdap } + oidcProviderConfigsArray, ok := deployment["oidcProviderConfigs"] + if ok { + // TODO use valid oidcProviderConfigs object + oidcProviderConfigsMarshalled, err := json.Marshal(oidcProviderConfigsArray) + if err != nil { + return nil, err + } + acLdap := &ldap.Ldap{} + if err := json.Unmarshal(oidcProviderConfigsMarshalled, acLdap); err != nil { + return nil, err + } + finalAutomationConfig.Ldap = acLdap + } + return finalAutomationConfig, nil } diff --git a/controllers/operator/authentication/authentication.go b/controllers/operator/authentication/authentication.go index bd9cc1258..603b20f3c 100644 --- a/controllers/operator/authentication/authentication.go +++ b/controllers/operator/authentication/authentication.go @@ -16,6 +16,7 @@ type AuthResource interface { GetNamespace() string GetSecurity() *mdbv1.Security IsLDAPEnabled() bool + IsOIDCEnabled() bool GetLDAP(password, caContents string) *ldap.Ldap } @@ -52,6 +53,8 @@ type Options struct { // Only required if LDAP is configured as an authentication mechanism Ldap *ldap.Ldap + OIDCProviderConfigs []oidc.ProviderConfig + AutoUser string AutoPwd string diff --git a/controllers/operator/authentication/authentication_mechanism.go b/controllers/operator/authentication/authentication_mechanism.go index bc5521045..d980b166b 100644 --- a/controllers/operator/authentication/authentication_mechanism.go +++ b/controllers/operator/authentication/authentication_mechanism.go @@ -33,6 +33,7 @@ const ( ScramSha1 MechanismName = "SCRAM-SHA-1" MongoDBX509 MechanismName = "MONGODB-X509" LDAPPlain MechanismName = "PLAIN" + MongoDBOIDC MechanismName = "MONGODB-OIDC" // MongoDBCR is an umbrella term for SCRAM-SHA-1 and MONGODB-CR for legacy reasons, once MONGODB-CR // is enabled, users can auth with SCRAM-SHA-1 credentials @@ -101,6 +102,8 @@ func convertToMechanism(mechanismModeInCR string, ac *om.AutomationConfig) Mecha return MongoDBCRMechanism case util.SCRAMSHA256: return ScramSha256Mechanism + case util.OIDC: + return MongoDBOIDCMechanism case util.SCRAM: // if we have already configured authentication, and it has been set to MONGODB-CR/SCRAM-SHA-1 // we can not transition. This needs to be done in the UI diff --git a/controllers/operator/authentication/oidc.go b/controllers/operator/authentication/oidc.go new file mode 100644 index 000000000..965559109 --- /dev/null +++ b/controllers/operator/authentication/oidc.go @@ -0,0 +1,159 @@ +package authentication + +import ( + "fmt" + "slices" + "strings" + + "go.uber.org/zap" + "golang.org/x/xerrors" + + mdbv1 "github.com/10gen/ops-manager-kubernetes/api/v1/mdb" + "github.com/10gen/ops-manager-kubernetes/controllers/om" + "github.com/10gen/ops-manager-kubernetes/controllers/operator/oidc" + "github.com/10gen/ops-manager-kubernetes/pkg/util/stringutil" +) + +var MongoDBOIDCMechanism = &oidcAuthMechanism{} + +type oidcAuthMechanism struct{} + +func (o *oidcAuthMechanism) GetName() MechanismName { + return MongoDBOIDC +} + +func (o *oidcAuthMechanism) EnableAgentAuthentication(_ om.Connection, _ Options, _ *zap.SugaredLogger) error { + return xerrors.Errorf("OIDC agent authentication is not supported") +} + +func (o *oidcAuthMechanism) DisableAgentAuthentication(_ om.Connection, _ *zap.SugaredLogger) error { + return xerrors.Errorf("OIDC agent authentication is not supported") +} + +func (o *oidcAuthMechanism) EnableDeploymentAuthentication(conn om.Connection, opts Options, log *zap.SugaredLogger) error { + return conn.ReadUpdateAutomationConfig(func(ac *om.AutomationConfig) error { + if !stringutil.Contains(ac.Auth.DeploymentAuthMechanisms, string(MongoDBOIDC)) { + ac.Auth.DeploymentAuthMechanisms = append(ac.Auth.DeploymentAuthMechanisms, string(MongoDBOIDC)) + } + // TODO merge configs with existing ones, and don't overwrite read only values + ac.OIDCProviderConfigs = opts.OIDCProviderConfigs + + return nil + }, log) +} + +func (o *oidcAuthMechanism) DisableDeploymentAuthentication(conn om.Connection, log *zap.SugaredLogger) error { + return conn.ReadUpdateAutomationConfig(func(ac *om.AutomationConfig) error { + ac.Auth.DeploymentAuthMechanisms = stringutil.Remove(ac.Auth.DeploymentAuthMechanisms, string(MongoDBOIDC)) + ac.OIDCProviderConfigs = nil + + return nil + }, log) +} + +func (o *oidcAuthMechanism) IsAgentAuthenticationConfigured(*om.AutomationConfig, Options) bool { + return false +} + +func (o *oidcAuthMechanism) IsDeploymentAuthenticationConfigured(ac *om.AutomationConfig, opts Options) bool { + return stringutil.Contains(ac.Auth.DeploymentAuthMechanisms, string(MongoDBOIDC)) && oidcProviderConfigsEqual(ac.OIDCProviderConfigs, opts.OIDCProviderConfigs) +} + +func oidcProviderConfigsEqual(lhs []oidc.ProviderConfig, rhs []oidc.ProviderConfig) bool { + if len(lhs) != len(rhs) { + return false + } + + lhsSorted := sortOIDCPProviderConfigs(lhs) + rhsSorted := sortOIDCPProviderConfigs(rhs) + + return slices.EqualFunc(lhsSorted, rhsSorted, oidcProviderConfigEqual) +} + +func sortOIDCPProviderConfigs(configs []oidc.ProviderConfig) []oidc.ProviderConfig { + configsSeq := slices.Values(configs) + return slices.SortedFunc(configsSeq, func(l, r oidc.ProviderConfig) int { + return strings.Compare(l.AuthNamePrefix, r.AuthNamePrefix) + }) +} + +func oidcProviderConfigEqual(l oidc.ProviderConfig, r oidc.ProviderConfig) bool { + if l.AuthNamePrefix != r.AuthNamePrefix { + return false + } + + if l.Audience != r.Audience { + return false + } + + if l.IssuerUri != r.IssuerUri { + return false + } + + if !slices.Equal(l.RequestedScopes, r.RequestedScopes) { + return false + } + + if l.UserClaim != r.UserClaim { + return false + } + + if l.GroupsClaim != r.GroupsClaim { + return false + } + + if l.SupportsHumanFlows != r.SupportsHumanFlows { + return false + } + + if l.UseAuthorizationClaim != r.UseAuthorizationClaim { + return false + } + + return true +} + +func MapOIDCProviderConfigs(oidcProviderConfigs []mdbv1.OIDCProviderConfig) []oidc.ProviderConfig { + if len(oidcProviderConfigs) == 0 { + return nil + } + + result := make([]oidc.ProviderConfig, len(oidcProviderConfigs)) + for i, providerConfig := range oidcProviderConfigs { + result[i] = oidc.ProviderConfig{ + AuthNamePrefix: providerConfig.ConfigurationName, + Audience: providerConfig.Audience, + IssuerUri: providerConfig.IssuerURI, + ClientId: providerConfig.ClientId, + RequestedScopes: providerConfig.RequestedScopes, + UserClaim: providerConfig.UserClaim, + GroupsClaim: providerConfig.GroupsClaim, + SupportsHumanFlows: mapToSupportHumanFlows(providerConfig.AuthorizationMethod), + UseAuthorizationClaim: mapToUseAuthorizationClaim(providerConfig.AuthorizationType), + } + } + + return result +} + +func mapToSupportHumanFlows(authMethod mdbv1.OIDCAuthorizationMethod) bool { + switch authMethod { + case mdbv1.OIDCAuthorizationMethodWorkforceIdentityFederation: + return true + case mdbv1.OIDCAuthorizationMethodWorkloadIdentityFederation: + return false + } + + panic(fmt.Sprintf("unsupported OIDC authorization method: %s", authMethod)) +} + +func mapToUseAuthorizationClaim(authType mdbv1.OIDCAuthorizationType) bool { + switch authType { + case mdbv1.OIDCAuthorizationTypeGroupMembership: + return true + case mdbv1.OIDCAuthorizationTypeUserID: + return false + } + + panic(fmt.Sprintf("unsupported OIDC authorization type: %s", authType)) +} diff --git a/controllers/operator/common_controller.go b/controllers/operator/common_controller.go index 436257e64..b4a289510 100644 --- a/controllers/operator/common_controller.go +++ b/controllers/operator/common_controller.go @@ -409,6 +409,10 @@ func (r *ReconcileCommonController) updateOmAuthentication(ctx context.Context, authOpts.Ldap = ar.GetLDAP(bindUserPassword, caContents) } + if ar.IsOIDCEnabled() { + authOpts.OIDCProviderConfigs = authentication.MapOIDCProviderConfigs(ar.GetSecurity().Authentication.OIDCProviderConfigs) + } + log.Debugf("Using authentication options %+v", authentication.Redact(authOpts)) agentSecretSelector := ar.GetSecurity().AgentClientCertificateSecretName(ar.GetName()) diff --git a/controllers/operator/oidc/oidc_types.go b/controllers/operator/oidc/oidc_types.go new file mode 100644 index 000000000..4a5a8f2c5 --- /dev/null +++ b/controllers/operator/oidc/oidc_types.go @@ -0,0 +1,14 @@ +package oidc + +type ProviderConfig struct { + AuthNamePrefix string `json:"authNamePrefix"` + Audience string `json:"audience"` + IssuerUri string `json:"issuerUri"` + ClientId string `json:"clientId"` + RequestedScopes []string `json:"requestedScopes"` + UserClaim string `json:"userClaim"` + GroupsClaim string `json:"groupsClaim"` + JWKSPollSecs int `json:"JWKSPollSecs"` // read only + SupportsHumanFlows bool `json:"supportsHumanFlows"` + UseAuthorizationClaim bool `json:"useAuthorizationClaim"` +} From 81e6107f9139cfd792c9ad26d1f2003d53bc7597 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Maciej=20Kara=C5=9B?= Date: Fri, 25 Apr 2025 16:26:06 +0200 Subject: [PATCH 08/36] Moved OIDCProviderConfigs to Deployment.Auth where it belongs --- controllers/om/automation_config.go | 28 +++++---------------- controllers/operator/authentication/oidc.go | 6 ++--- 2 files changed, 9 insertions(+), 25 deletions(-) diff --git a/controllers/om/automation_config.go b/controllers/om/automation_config.go index ca60e6e5b..1dde8aba0 100644 --- a/controllers/om/automation_config.go +++ b/controllers/om/automation_config.go @@ -20,11 +20,10 @@ import ( // configuration which are merged into the `Deployment` object before sending it back to Ops Manager. // As of right now only support configuring LogRotate for monitoring and backup via dedicated endpoints. type AutomationConfig struct { - Auth *Auth - AgentSSL *AgentSSL - Deployment Deployment - Ldap *ldap.Ldap - OIDCProviderConfigs []oidc.ProviderConfig + Auth *Auth + AgentSSL *AgentSSL + Deployment Deployment + Ldap *ldap.Ldap } // Apply merges the state of all concrete structs into the Deployment (map[string]interface{}) @@ -60,9 +59,6 @@ func applyInto(a AutomationConfig, into *Deployment) error { (*into)["ldap"] = mergedLdap } - if _, ok := a.Deployment["oidcProviderConfigs"]; ok { - // TODO merge lists and their contents (should we just overwrite everything?) - } return nil } @@ -231,6 +227,8 @@ type Auth struct { NewAutoPwd string `json:"newAutoPwd,omitempty"` // LdapGroupDN is required when enabling LDAP authz and agents authentication on $external LdapGroupDN string `json:"autoLdapGroupDN,omitempty"` + // OIDCProviderConfigs is a list of OIDC provider configurations + OIDCProviderConfigs []oidc.ProviderConfig `json:"oidcProviderConfigs,omitempty"` } // IsEnabled is a convenience function to aid readability @@ -437,20 +435,6 @@ func BuildAutomationConfigFromDeployment(deployment Deployment) (*AutomationConf finalAutomationConfig.Ldap = acLdap } - oidcProviderConfigsArray, ok := deployment["oidcProviderConfigs"] - if ok { - // TODO use valid oidcProviderConfigs object - oidcProviderConfigsMarshalled, err := json.Marshal(oidcProviderConfigsArray) - if err != nil { - return nil, err - } - acLdap := &ldap.Ldap{} - if err := json.Unmarshal(oidcProviderConfigsMarshalled, acLdap); err != nil { - return nil, err - } - finalAutomationConfig.Ldap = acLdap - } - return finalAutomationConfig, nil } diff --git a/controllers/operator/authentication/oidc.go b/controllers/operator/authentication/oidc.go index 965559109..bfc5cdb57 100644 --- a/controllers/operator/authentication/oidc.go +++ b/controllers/operator/authentication/oidc.go @@ -36,7 +36,7 @@ func (o *oidcAuthMechanism) EnableDeploymentAuthentication(conn om.Connection, o ac.Auth.DeploymentAuthMechanisms = append(ac.Auth.DeploymentAuthMechanisms, string(MongoDBOIDC)) } // TODO merge configs with existing ones, and don't overwrite read only values - ac.OIDCProviderConfigs = opts.OIDCProviderConfigs + ac.Auth.OIDCProviderConfigs = opts.OIDCProviderConfigs return nil }, log) @@ -45,7 +45,7 @@ func (o *oidcAuthMechanism) EnableDeploymentAuthentication(conn om.Connection, o func (o *oidcAuthMechanism) DisableDeploymentAuthentication(conn om.Connection, log *zap.SugaredLogger) error { return conn.ReadUpdateAutomationConfig(func(ac *om.AutomationConfig) error { ac.Auth.DeploymentAuthMechanisms = stringutil.Remove(ac.Auth.DeploymentAuthMechanisms, string(MongoDBOIDC)) - ac.OIDCProviderConfigs = nil + ac.Auth.OIDCProviderConfigs = nil return nil }, log) @@ -56,7 +56,7 @@ func (o *oidcAuthMechanism) IsAgentAuthenticationConfigured(*om.AutomationConfig } func (o *oidcAuthMechanism) IsDeploymentAuthenticationConfigured(ac *om.AutomationConfig, opts Options) bool { - return stringutil.Contains(ac.Auth.DeploymentAuthMechanisms, string(MongoDBOIDC)) && oidcProviderConfigsEqual(ac.OIDCProviderConfigs, opts.OIDCProviderConfigs) + return stringutil.Contains(ac.Auth.DeploymentAuthMechanisms, string(MongoDBOIDC)) && oidcProviderConfigsEqual(ac.Auth.OIDCProviderConfigs, opts.OIDCProviderConfigs) } func oidcProviderConfigsEqual(lhs []oidc.ProviderConfig, rhs []oidc.ProviderConfig) bool { From 024fa63a4e0f42b128761f1a8f87f272e2b938cc Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Maciej=20Kara=C5=9B?= Date: Sun, 27 Apr 2025 17:19:01 +0200 Subject: [PATCH 09/36] Fixed migrating to mongodb-kubernetes repository --- config/crd/bases/mongodb.com_mongodb.yaml | 1 - config/crd/bases/mongodb.com_mongodbmulticluster.yaml | 1 - config/crd/bases/mongodb.com_opsmanagers.yaml | 1 - controllers/om/automation_config.go | 1 + controllers/operator/authentication/authentication.go | 1 + controllers/operator/authentication/oidc.go | 8 ++++---- helm_chart/crds/mongodb.com_mongodb.yaml | 1 - helm_chart/crds/mongodb.com_mongodbmulticluster.yaml | 1 - helm_chart/crds/mongodb.com_opsmanagers.yaml | 1 - public/crds.yaml | 3 --- 10 files changed, 6 insertions(+), 13 deletions(-) diff --git a/config/crd/bases/mongodb.com_mongodb.yaml b/config/crd/bases/mongodb.com_mongodb.yaml index 33115441b..bdf8bc3f1 100644 --- a/config/crd/bases/mongodb.com_mongodb.yaml +++ b/config/crd/bases/mongodb.com_mongodb.yaml @@ -1572,7 +1572,6 @@ spec: type: string issuerURI: description: |- - TODO add URI validation Issuer value provided by your registered IdP application. Using this URI, MongoDB finds an OpenID Provider Configuration Document, which should be available in the /.wellknown/open-id-configuration endpoint. type: string diff --git a/config/crd/bases/mongodb.com_mongodbmulticluster.yaml b/config/crd/bases/mongodb.com_mongodbmulticluster.yaml index 28087ec50..f0b64535e 100644 --- a/config/crd/bases/mongodb.com_mongodbmulticluster.yaml +++ b/config/crd/bases/mongodb.com_mongodbmulticluster.yaml @@ -832,7 +832,6 @@ spec: type: string issuerURI: description: |- - TODO add URI validation Issuer value provided by your registered IdP application. Using this URI, MongoDB finds an OpenID Provider Configuration Document, which should be available in the /.wellknown/open-id-configuration endpoint. type: string diff --git a/config/crd/bases/mongodb.com_opsmanagers.yaml b/config/crd/bases/mongodb.com_opsmanagers.yaml index e54059ce5..4206080be 100644 --- a/config/crd/bases/mongodb.com_opsmanagers.yaml +++ b/config/crd/bases/mongodb.com_opsmanagers.yaml @@ -894,7 +894,6 @@ spec: type: string issuerURI: description: |- - TODO add URI validation Issuer value provided by your registered IdP application. Using this URI, MongoDB finds an OpenID Provider Configuration Document, which should be available in the /.wellknown/open-id-configuration endpoint. type: string diff --git a/controllers/om/automation_config.go b/controllers/om/automation_config.go index 1dde8aba0..41d6d04d4 100644 --- a/controllers/om/automation_config.go +++ b/controllers/om/automation_config.go @@ -8,6 +8,7 @@ import ( "k8s.io/apimachinery/pkg/api/equality" "github.com/mongodb/mongodb-kubernetes/controllers/operator/ldap" + "github.com/mongodb/mongodb-kubernetes/controllers/operator/oidc" "github.com/mongodb/mongodb-kubernetes/pkg/util" "github.com/mongodb/mongodb-kubernetes/pkg/util/generate" "github.com/mongodb/mongodb-kubernetes/pkg/util/maputil" diff --git a/controllers/operator/authentication/authentication.go b/controllers/operator/authentication/authentication.go index 603b20f3c..37829013f 100644 --- a/controllers/operator/authentication/authentication.go +++ b/controllers/operator/authentication/authentication.go @@ -7,6 +7,7 @@ import ( mdbv1 "github.com/mongodb/mongodb-kubernetes/api/v1/mdb" "github.com/mongodb/mongodb-kubernetes/controllers/om" "github.com/mongodb/mongodb-kubernetes/controllers/operator/ldap" + "github.com/mongodb/mongodb-kubernetes/controllers/operator/oidc" "github.com/mongodb/mongodb-kubernetes/pkg/util" ) diff --git a/controllers/operator/authentication/oidc.go b/controllers/operator/authentication/oidc.go index bfc5cdb57..bd953c1e8 100644 --- a/controllers/operator/authentication/oidc.go +++ b/controllers/operator/authentication/oidc.go @@ -8,10 +8,10 @@ import ( "go.uber.org/zap" "golang.org/x/xerrors" - mdbv1 "github.com/10gen/ops-manager-kubernetes/api/v1/mdb" - "github.com/10gen/ops-manager-kubernetes/controllers/om" - "github.com/10gen/ops-manager-kubernetes/controllers/operator/oidc" - "github.com/10gen/ops-manager-kubernetes/pkg/util/stringutil" + mdbv1 "github.com/mongodb/mongodb-kubernetes/api/v1/mdb" + "github.com/mongodb/mongodb-kubernetes/controllers/om" + "github.com/mongodb/mongodb-kubernetes/controllers/operator/oidc" + "github.com/mongodb/mongodb-kubernetes/pkg/util/stringutil" ) var MongoDBOIDCMechanism = &oidcAuthMechanism{} diff --git a/helm_chart/crds/mongodb.com_mongodb.yaml b/helm_chart/crds/mongodb.com_mongodb.yaml index 33115441b..bdf8bc3f1 100644 --- a/helm_chart/crds/mongodb.com_mongodb.yaml +++ b/helm_chart/crds/mongodb.com_mongodb.yaml @@ -1572,7 +1572,6 @@ spec: type: string issuerURI: description: |- - TODO add URI validation Issuer value provided by your registered IdP application. Using this URI, MongoDB finds an OpenID Provider Configuration Document, which should be available in the /.wellknown/open-id-configuration endpoint. type: string diff --git a/helm_chart/crds/mongodb.com_mongodbmulticluster.yaml b/helm_chart/crds/mongodb.com_mongodbmulticluster.yaml index 28087ec50..f0b64535e 100644 --- a/helm_chart/crds/mongodb.com_mongodbmulticluster.yaml +++ b/helm_chart/crds/mongodb.com_mongodbmulticluster.yaml @@ -832,7 +832,6 @@ spec: type: string issuerURI: description: |- - TODO add URI validation Issuer value provided by your registered IdP application. Using this URI, MongoDB finds an OpenID Provider Configuration Document, which should be available in the /.wellknown/open-id-configuration endpoint. type: string diff --git a/helm_chart/crds/mongodb.com_opsmanagers.yaml b/helm_chart/crds/mongodb.com_opsmanagers.yaml index e54059ce5..4206080be 100644 --- a/helm_chart/crds/mongodb.com_opsmanagers.yaml +++ b/helm_chart/crds/mongodb.com_opsmanagers.yaml @@ -894,7 +894,6 @@ spec: type: string issuerURI: description: |- - TODO add URI validation Issuer value provided by your registered IdP application. Using this URI, MongoDB finds an OpenID Provider Configuration Document, which should be available in the /.wellknown/open-id-configuration endpoint. type: string diff --git a/public/crds.yaml b/public/crds.yaml index ca76be53b..644df1f69 100644 --- a/public/crds.yaml +++ b/public/crds.yaml @@ -1572,7 +1572,6 @@ spec: type: string issuerURI: description: |- - TODO add URI validation Issuer value provided by your registered IdP application. Using this URI, MongoDB finds an OpenID Provider Configuration Document, which should be available in the /.wellknown/open-id-configuration endpoint. type: string @@ -4206,7 +4205,6 @@ spec: type: string issuerURI: description: |- - TODO add URI validation Issuer value provided by your registered IdP application. Using this URI, MongoDB finds an OpenID Provider Configuration Document, which should be available in the /.wellknown/open-id-configuration endpoint. type: string @@ -5595,7 +5593,6 @@ spec: type: string issuerURI: description: |- - TODO add URI validation Issuer value provided by your registered IdP application. Using this URI, MongoDB finds an OpenID Provider Configuration Document, which should be available in the /.wellknown/open-id-configuration endpoint. type: string From 7a53fd7f8d426e7ce60c524e0903708cfc3cc686 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Maciej=20Kara=C5=9B?= Date: Sun, 27 Apr 2025 17:20:54 +0200 Subject: [PATCH 10/36] Fixed unit tests + CRD generation --- config/crd/bases/mongodb.com_mongodb.yaml | 1 - config/crd/bases/mongodb.com_mongodbmulticluster.yaml | 1 - config/crd/bases/mongodb.com_opsmanagers.yaml | 1 - helm_chart/crds/mongodb.com_mongodb.yaml | 1 - helm_chart/crds/mongodb.com_mongodbmulticluster.yaml | 1 - helm_chart/crds/mongodb.com_opsmanagers.yaml | 1 - pkg/util/util_test.go | 5 +---- public/crds.yaml | 3 --- 8 files changed, 1 insertion(+), 13 deletions(-) diff --git a/config/crd/bases/mongodb.com_mongodb.yaml b/config/crd/bases/mongodb.com_mongodb.yaml index 33115441b..bdf8bc3f1 100644 --- a/config/crd/bases/mongodb.com_mongodb.yaml +++ b/config/crd/bases/mongodb.com_mongodb.yaml @@ -1572,7 +1572,6 @@ spec: type: string issuerURI: description: |- - TODO add URI validation Issuer value provided by your registered IdP application. Using this URI, MongoDB finds an OpenID Provider Configuration Document, which should be available in the /.wellknown/open-id-configuration endpoint. type: string diff --git a/config/crd/bases/mongodb.com_mongodbmulticluster.yaml b/config/crd/bases/mongodb.com_mongodbmulticluster.yaml index 28087ec50..f0b64535e 100644 --- a/config/crd/bases/mongodb.com_mongodbmulticluster.yaml +++ b/config/crd/bases/mongodb.com_mongodbmulticluster.yaml @@ -832,7 +832,6 @@ spec: type: string issuerURI: description: |- - TODO add URI validation Issuer value provided by your registered IdP application. Using this URI, MongoDB finds an OpenID Provider Configuration Document, which should be available in the /.wellknown/open-id-configuration endpoint. type: string diff --git a/config/crd/bases/mongodb.com_opsmanagers.yaml b/config/crd/bases/mongodb.com_opsmanagers.yaml index e54059ce5..4206080be 100644 --- a/config/crd/bases/mongodb.com_opsmanagers.yaml +++ b/config/crd/bases/mongodb.com_opsmanagers.yaml @@ -894,7 +894,6 @@ spec: type: string issuerURI: description: |- - TODO add URI validation Issuer value provided by your registered IdP application. Using this URI, MongoDB finds an OpenID Provider Configuration Document, which should be available in the /.wellknown/open-id-configuration endpoint. type: string diff --git a/helm_chart/crds/mongodb.com_mongodb.yaml b/helm_chart/crds/mongodb.com_mongodb.yaml index 33115441b..bdf8bc3f1 100644 --- a/helm_chart/crds/mongodb.com_mongodb.yaml +++ b/helm_chart/crds/mongodb.com_mongodb.yaml @@ -1572,7 +1572,6 @@ spec: type: string issuerURI: description: |- - TODO add URI validation Issuer value provided by your registered IdP application. Using this URI, MongoDB finds an OpenID Provider Configuration Document, which should be available in the /.wellknown/open-id-configuration endpoint. type: string diff --git a/helm_chart/crds/mongodb.com_mongodbmulticluster.yaml b/helm_chart/crds/mongodb.com_mongodbmulticluster.yaml index 28087ec50..f0b64535e 100644 --- a/helm_chart/crds/mongodb.com_mongodbmulticluster.yaml +++ b/helm_chart/crds/mongodb.com_mongodbmulticluster.yaml @@ -832,7 +832,6 @@ spec: type: string issuerURI: description: |- - TODO add URI validation Issuer value provided by your registered IdP application. Using this URI, MongoDB finds an OpenID Provider Configuration Document, which should be available in the /.wellknown/open-id-configuration endpoint. type: string diff --git a/helm_chart/crds/mongodb.com_opsmanagers.yaml b/helm_chart/crds/mongodb.com_opsmanagers.yaml index e54059ce5..4206080be 100644 --- a/helm_chart/crds/mongodb.com_opsmanagers.yaml +++ b/helm_chart/crds/mongodb.com_opsmanagers.yaml @@ -894,7 +894,6 @@ spec: type: string issuerURI: description: |- - TODO add URI validation Issuer value provided by your registered IdP application. Using this URI, MongoDB finds an OpenID Provider Configuration Document, which should be available in the /.wellknown/open-id-configuration endpoint. type: string diff --git a/pkg/util/util_test.go b/pkg/util/util_test.go index 7ffe59f80..514bee8c1 100644 --- a/pkg/util/util_test.go +++ b/pkg/util/util_test.go @@ -255,10 +255,7 @@ func TestIsURL(t *testing.T) { { name: "file scheme", input: "file:///path/to/file", - isValid: true, - checkURL: func(u *url.URL) bool { - return u.Scheme == "file" && u.Path == "/path/to/file" - }, + isValid: false, }, } diff --git a/public/crds.yaml b/public/crds.yaml index ca76be53b..644df1f69 100644 --- a/public/crds.yaml +++ b/public/crds.yaml @@ -1572,7 +1572,6 @@ spec: type: string issuerURI: description: |- - TODO add URI validation Issuer value provided by your registered IdP application. Using this URI, MongoDB finds an OpenID Provider Configuration Document, which should be available in the /.wellknown/open-id-configuration endpoint. type: string @@ -4206,7 +4205,6 @@ spec: type: string issuerURI: description: |- - TODO add URI validation Issuer value provided by your registered IdP application. Using this URI, MongoDB finds an OpenID Provider Configuration Document, which should be available in the /.wellknown/open-id-configuration endpoint. type: string @@ -5595,7 +5593,6 @@ spec: type: string issuerURI: description: |- - TODO add URI validation Issuer value provided by your registered IdP application. Using this URI, MongoDB finds an OpenID Provider Configuration Document, which should be available in the /.wellknown/open-id-configuration endpoint. type: string From 97a5c99c8230e65240056fbf493d8f439ce02ae0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Maciej=20Kara=C5=9B?= Date: Sun, 27 Apr 2025 17:42:53 +0200 Subject: [PATCH 11/36] Add unit tests --- .../operator/authentication/oidc_test.go | 93 +++++++++++++++++++ controllers/operator/oidc/oidc_types.go | 1 - 2 files changed, 93 insertions(+), 1 deletion(-) create mode 100644 controllers/operator/authentication/oidc_test.go diff --git a/controllers/operator/authentication/oidc_test.go b/controllers/operator/authentication/oidc_test.go new file mode 100644 index 000000000..56ce0292f --- /dev/null +++ b/controllers/operator/authentication/oidc_test.go @@ -0,0 +1,93 @@ +package authentication + +import ( + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + "go.uber.org/zap" + + "github.com/mongodb/mongodb-kubernetes/controllers/om" + "github.com/mongodb/mongodb-kubernetes/controllers/operator/oidc" +) + +func TestOIDC_EnableDeploymentAuthentication(t *testing.T) { + conn := om.NewMockedOmConnection(om.NewDeployment()) + ac, err := conn.ReadAutomationConfig() + require.NoError(t, err) + assert.Empty(t, ac.Auth.OIDCProviderConfigs) + assert.Empty(t, ac.Auth.DeploymentAuthMechanisms) + + providerConfigs := []oidc.ProviderConfig{ + { + AuthNamePrefix: "okta", + Audience: "aud", + IssuerUri: "https://okta.mongodb.com", + ClientId: "client1", + RequestedScopes: []string{"openid", "profile"}, + UserClaim: "sub", + SupportsHumanFlows: true, + UseAuthorizationClaim: false, + }, + { + AuthNamePrefix: "congito", + Audience: "aud", + IssuerUri: "https://congito.mongodb.com", + ClientId: "client2", + UserClaim: "sub", + GroupsClaim: "groups", + SupportsHumanFlows: false, + UseAuthorizationClaim: true, + }, + } + + opts := Options{ + Mechanisms: []string{string(MongoDBOIDC)}, + OIDCProviderConfigs: providerConfigs, + } + + configured := MongoDBOIDCMechanism.IsDeploymentAuthenticationConfigured(ac, opts) + assert.False(t, configured) + + err = MongoDBOIDCMechanism.EnableDeploymentAuthentication(conn, opts, zap.S()) + require.NoError(t, err) + + ac, err = conn.ReadAutomationConfig() + require.NoError(t, err) + assert.Contains(t, ac.Auth.DeploymentAuthMechanisms, string(MongoDBOIDC)) + assert.Equal(t, providerConfigs, ac.Auth.OIDCProviderConfigs) + + configured = MongoDBOIDCMechanism.IsDeploymentAuthenticationConfigured(ac, opts) + assert.True(t, configured) + + err = MongoDBOIDCMechanism.DisableDeploymentAuthentication(conn, zap.S()) + require.NoError(t, err) + + ac, err = conn.ReadAutomationConfig() + require.NoError(t, err) + + configured = MongoDBOIDCMechanism.IsDeploymentAuthenticationConfigured(ac, opts) + assert.False(t, configured) + + assert.NotContains(t, ac.Auth.DeploymentAuthMechanisms, string(MongoDBOIDC)) + assert.Empty(t, ac.Auth.OIDCProviderConfigs) +} + +func TestOIDC_EnableAgentAuthentication(t *testing.T) { + conn := om.NewMockedOmConnection(om.NewDeployment()) + opts := Options{ + Mechanisms: []string{string(MongoDBOIDC)}, + } + + ac, err := conn.ReadAutomationConfig() + require.NoError(t, err) + + configured := MongoDBOIDCMechanism.IsAgentAuthenticationConfigured(ac, opts) + assert.False(t, configured) + + err = MongoDBOIDCMechanism.EnableAgentAuthentication(conn, opts, zap.S()) + require.Error(t, err) + + err = MongoDBOIDCMechanism.DisableAgentAuthentication(conn, zap.S()) + require.Error(t, err) +} diff --git a/controllers/operator/oidc/oidc_types.go b/controllers/operator/oidc/oidc_types.go index 4a5a8f2c5..c5c4ef80e 100644 --- a/controllers/operator/oidc/oidc_types.go +++ b/controllers/operator/oidc/oidc_types.go @@ -8,7 +8,6 @@ type ProviderConfig struct { RequestedScopes []string `json:"requestedScopes"` UserClaim string `json:"userClaim"` GroupsClaim string `json:"groupsClaim"` - JWKSPollSecs int `json:"JWKSPollSecs"` // read only SupportsHumanFlows bool `json:"supportsHumanFlows"` UseAuthorizationClaim bool `json:"useAuthorizationClaim"` } From 6dd49768cae01ee25d5121cc19c7ed4b1d63ca06 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Maciej=20Kara=C5=9B?= Date: Mon, 28 Apr 2025 00:05:56 +0200 Subject: [PATCH 12/36] Temporal fix for AC --- controllers/om/automation_config.go | 39 ++++++++++++++++--- controllers/operator/authentication/oidc.go | 6 +-- .../operator/authentication/oidc_test.go | 6 +-- 3 files changed, 39 insertions(+), 12 deletions(-) diff --git a/controllers/om/automation_config.go b/controllers/om/automation_config.go index 41d6d04d4..1b6bbc608 100644 --- a/controllers/om/automation_config.go +++ b/controllers/om/automation_config.go @@ -21,10 +21,11 @@ import ( // configuration which are merged into the `Deployment` object before sending it back to Ops Manager. // As of right now only support configuring LogRotate for monitoring and backup via dedicated endpoints. type AutomationConfig struct { - Auth *Auth - AgentSSL *AgentSSL - Deployment Deployment - Ldap *ldap.Ldap + Auth *Auth + AgentSSL *AgentSSL + Deployment Deployment + Ldap *ldap.Ldap + OIDCProviderConfigs []oidc.ProviderConfig } // Apply merges the state of all concrete structs into the Deployment (map[string]interface{}) @@ -60,6 +61,21 @@ func applyInto(a AutomationConfig, into *Deployment) error { (*into)["ldap"] = mergedLdap } + if _, ok := a.Deployment["oidcProviderConfigs"]; ok || len(a.OIDCProviderConfigs) > 0 { + // TODO: this is not merged yet, but only overridden + bytes, err := json.Marshal(a.OIDCProviderConfigs) + if err != nil { + return err + } + + dst := make([]map[string]interface{}, 0) + err = json.Unmarshal(bytes, &dst) + if err != nil { + return err + } + (*into)["oidcProviderConfigs"] = dst + } + return nil } @@ -228,8 +244,6 @@ type Auth struct { NewAutoPwd string `json:"newAutoPwd,omitempty"` // LdapGroupDN is required when enabling LDAP authz and agents authentication on $external LdapGroupDN string `json:"autoLdapGroupDN,omitempty"` - // OIDCProviderConfigs is a list of OIDC provider configurations - OIDCProviderConfigs []oidc.ProviderConfig `json:"oidcProviderConfigs,omitempty"` } // IsEnabled is a convenience function to aid readability @@ -436,6 +450,19 @@ func BuildAutomationConfigFromDeployment(deployment Deployment) (*AutomationConf finalAutomationConfig.Ldap = acLdap } + oidcSlice, ok := deployment["oidcProviderConfigs"] + if ok { + oidcMarshalled, err := json.Marshal(oidcSlice) + if err != nil { + return nil, err + } + providerConfigs := make([]oidc.ProviderConfig, 0) + if err := json.Unmarshal(oidcMarshalled, &providerConfigs); err != nil { + return nil, err + } + finalAutomationConfig.OIDCProviderConfigs = providerConfigs + } + return finalAutomationConfig, nil } diff --git a/controllers/operator/authentication/oidc.go b/controllers/operator/authentication/oidc.go index bd953c1e8..3ffc9bdaf 100644 --- a/controllers/operator/authentication/oidc.go +++ b/controllers/operator/authentication/oidc.go @@ -36,7 +36,7 @@ func (o *oidcAuthMechanism) EnableDeploymentAuthentication(conn om.Connection, o ac.Auth.DeploymentAuthMechanisms = append(ac.Auth.DeploymentAuthMechanisms, string(MongoDBOIDC)) } // TODO merge configs with existing ones, and don't overwrite read only values - ac.Auth.OIDCProviderConfigs = opts.OIDCProviderConfigs + ac.OIDCProviderConfigs = opts.OIDCProviderConfigs return nil }, log) @@ -45,7 +45,7 @@ func (o *oidcAuthMechanism) EnableDeploymentAuthentication(conn om.Connection, o func (o *oidcAuthMechanism) DisableDeploymentAuthentication(conn om.Connection, log *zap.SugaredLogger) error { return conn.ReadUpdateAutomationConfig(func(ac *om.AutomationConfig) error { ac.Auth.DeploymentAuthMechanisms = stringutil.Remove(ac.Auth.DeploymentAuthMechanisms, string(MongoDBOIDC)) - ac.Auth.OIDCProviderConfigs = nil + ac.OIDCProviderConfigs = nil return nil }, log) @@ -56,7 +56,7 @@ func (o *oidcAuthMechanism) IsAgentAuthenticationConfigured(*om.AutomationConfig } func (o *oidcAuthMechanism) IsDeploymentAuthenticationConfigured(ac *om.AutomationConfig, opts Options) bool { - return stringutil.Contains(ac.Auth.DeploymentAuthMechanisms, string(MongoDBOIDC)) && oidcProviderConfigsEqual(ac.Auth.OIDCProviderConfigs, opts.OIDCProviderConfigs) + return stringutil.Contains(ac.Auth.DeploymentAuthMechanisms, string(MongoDBOIDC)) && oidcProviderConfigsEqual(ac.OIDCProviderConfigs, opts.OIDCProviderConfigs) } func oidcProviderConfigsEqual(lhs []oidc.ProviderConfig, rhs []oidc.ProviderConfig) bool { diff --git a/controllers/operator/authentication/oidc_test.go b/controllers/operator/authentication/oidc_test.go index 56ce0292f..bb22e1d23 100644 --- a/controllers/operator/authentication/oidc_test.go +++ b/controllers/operator/authentication/oidc_test.go @@ -15,7 +15,7 @@ func TestOIDC_EnableDeploymentAuthentication(t *testing.T) { conn := om.NewMockedOmConnection(om.NewDeployment()) ac, err := conn.ReadAutomationConfig() require.NoError(t, err) - assert.Empty(t, ac.Auth.OIDCProviderConfigs) + assert.Empty(t, ac.OIDCProviderConfigs) assert.Empty(t, ac.Auth.DeploymentAuthMechanisms) providerConfigs := []oidc.ProviderConfig{ @@ -55,7 +55,7 @@ func TestOIDC_EnableDeploymentAuthentication(t *testing.T) { ac, err = conn.ReadAutomationConfig() require.NoError(t, err) assert.Contains(t, ac.Auth.DeploymentAuthMechanisms, string(MongoDBOIDC)) - assert.Equal(t, providerConfigs, ac.Auth.OIDCProviderConfigs) + assert.Equal(t, providerConfigs, ac.OIDCProviderConfigs) configured = MongoDBOIDCMechanism.IsDeploymentAuthenticationConfigured(ac, opts) assert.True(t, configured) @@ -70,7 +70,7 @@ func TestOIDC_EnableDeploymentAuthentication(t *testing.T) { assert.False(t, configured) assert.NotContains(t, ac.Auth.DeploymentAuthMechanisms, string(MongoDBOIDC)) - assert.Empty(t, ac.Auth.OIDCProviderConfigs) + assert.Empty(t, ac.OIDCProviderConfigs) } func TestOIDC_EnableAgentAuthentication(t *testing.T) { From 8b342228ce178086aa02bea52768b8e8ecaa07ba Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Maciej=20Kara=C5=9B?= Date: Tue, 29 Apr 2025 09:49:54 +0200 Subject: [PATCH 13/36] Fix kubebuilder validation rules --- api/v1/mdb/mongodb_types.go | 27 ++++++++++++------- config/crd/bases/mongodb.com_mongodb.yaml | 5 +++- .../mongodb.com_mongodbmulticluster.yaml | 5 +++- config/crd/bases/mongodb.com_opsmanagers.yaml | 5 +++- helm_chart/crds/mongodb.com_mongodb.yaml | 5 +++- .../crds/mongodb.com_mongodbmulticluster.yaml | 5 +++- helm_chart/crds/mongodb.com_opsmanagers.yaml | 5 +++- public/crds.yaml | 15 ++++++++--- 8 files changed, 53 insertions(+), 19 deletions(-) diff --git a/api/v1/mdb/mongodb_types.go b/api/v1/mdb/mongodb_types.go index a12933875..fc2b9f54d 100644 --- a/api/v1/mdb/mongodb_types.go +++ b/api/v1/mdb/mongodb_types.go @@ -934,6 +934,8 @@ type Authentication struct { // +optional Ldap *Ldap `json:"ldap,omitempty"` + // Configuration for OIDC providers + // +optional OIDCProviderConfigs []OIDCProviderConfig `json:"oidcProviderConfigs,omitempty"` // Agents contains authentication configuration properties for the agents @@ -1063,54 +1065,59 @@ type Ldap struct { } type OIDCProviderConfig struct { - // TODO add proper validation and test it // Unique label that identifies this configuration. This label is visible to your Ops Manager users and is used when // creating users and roles for authorization. It is case-sensitive and can only contain the following characters: // - alphanumeric characters (combination of a to z and 0 to 9) // - hyphens (-) // - underscores (_) - // +kubebuilder:validation:Pattern:"^[a-zA-Z0-9-_]+$" + // +kubebuilder:validation:Pattern="^[a-zA-Z0-9-_]+$" + // +kubebuilder:validation:Required ConfigurationName string `json:"configurationName"` // Issuer value provided by your registered IdP application. Using this URI, MongoDB finds an OpenID Provider // Configuration Document, which should be available in the /.wellknown/open-id-configuration endpoint. + // +kubebuilder:validation:Required IssuerURI string `json:"issuerURI"` // Entity that your external identity provider intends the token for. // Enter the audience value from the app you registered with external Identity Provider. + // +kubebuilder:validation:Required Audience string `json:"audience"` // Select GroupMembership to grant authorization based on IdP user group membership, or select UserID to grant // an individual user authorization. + // +kubebuilder:validation:Required AuthorizationType OIDCAuthorizationType `json:"authorizationType"` // The identifier of the claim that includes the user principal identity. // Accept the default value unless your IdP uses a different claim. - // +default:value:sub + // +kubebuilder:default=sub + // +kubebuilder:validation:Required UserClaim string `json:"userClaim"` // The identifier of the claim that includes the principal's IdP user group membership information. // Accept the default value unless your IdP uses a different claim, or you need a custom claim. // Required when selected GroupMembership as the authorization type, ignored otherwise - // +default:value:groups - // +optional - GroupsClaim string `json:"groupsClaim"` + // +kubebuilder:default=groups + // +kubebuilder:validation:Optional + GroupsClaim string `json:"groupsClaim,omitempty"` // Configure single-sign-on for human user access to Ops Manager deployments with Workforce Identity Federation. // For programmatic, application access to Ops Manager deployments use Workload Identity Federation. // Only one Workforce Identity Federation IdP can be configured per MongoDB resource + // +kubebuilder:validation:Required AuthorizationMethod OIDCAuthorizationMethod `json:"authorizationMethod"` // Unique identifier for your registered application. Enter the clientId value from the app you // registered with an external Identity Provider. // Required when selected Workforce Identity Federation authorization method - // +optional - ClientId string `json:"clientId"` + // +kubebuilder:validation:Optional + ClientId string `json:"clientId,omitempty"` // Tokens that give users permission to request data from the authorization endpoint. // Only used for Workforce Identity Federation authorization method - // +optional - RequestedScopes []string `json:"requestedScopes"` + // +kubebuilder:validation:Optional + RequestedScopes []string `json:"requestedScopes,omitempty"` } // +kubebuilder:validation:Enum=GroupMembership;UserID diff --git a/config/crd/bases/mongodb.com_mongodb.yaml b/config/crd/bases/mongodb.com_mongodb.yaml index bdf8bc3f1..8b5ffc33e 100644 --- a/config/crd/bases/mongodb.com_mongodb.yaml +++ b/config/crd/bases/mongodb.com_mongodb.yaml @@ -1525,6 +1525,7 @@ spec: type: string type: array oidcProviderConfigs: + description: Configuration for OIDC providers items: properties: audience: @@ -1557,14 +1558,15 @@ spec: type: string configurationName: description: |- - TODO add proper validation and test it Unique label that identifies this configuration. This label is visible to your Ops Manager users and is used when creating users and roles for authorization. It is case-sensitive and can only contain the following characters: - alphanumeric characters (combination of a to z and 0 to 9) - hyphens (-) - underscores (_) + pattern: ^[a-zA-Z0-9-_]+$ type: string groupsClaim: + default: groups description: |- The identifier of the claim that includes the principal's IdP user group membership information. Accept the default value unless your IdP uses a different claim, or you need a custom claim. @@ -1583,6 +1585,7 @@ spec: type: string type: array userClaim: + default: sub description: |- The identifier of the claim that includes the user principal identity. Accept the default value unless your IdP uses a different claim. diff --git a/config/crd/bases/mongodb.com_mongodbmulticluster.yaml b/config/crd/bases/mongodb.com_mongodbmulticluster.yaml index f0b64535e..523238eb6 100644 --- a/config/crd/bases/mongodb.com_mongodbmulticluster.yaml +++ b/config/crd/bases/mongodb.com_mongodbmulticluster.yaml @@ -785,6 +785,7 @@ spec: type: string type: array oidcProviderConfigs: + description: Configuration for OIDC providers items: properties: audience: @@ -817,14 +818,15 @@ spec: type: string configurationName: description: |- - TODO add proper validation and test it Unique label that identifies this configuration. This label is visible to your Ops Manager users and is used when creating users and roles for authorization. It is case-sensitive and can only contain the following characters: - alphanumeric characters (combination of a to z and 0 to 9) - hyphens (-) - underscores (_) + pattern: ^[a-zA-Z0-9-_]+$ type: string groupsClaim: + default: groups description: |- The identifier of the claim that includes the principal's IdP user group membership information. Accept the default value unless your IdP uses a different claim, or you need a custom claim. @@ -843,6 +845,7 @@ spec: type: string type: array userClaim: + default: sub description: |- The identifier of the claim that includes the user principal identity. Accept the default value unless your IdP uses a different claim. diff --git a/config/crd/bases/mongodb.com_opsmanagers.yaml b/config/crd/bases/mongodb.com_opsmanagers.yaml index 4206080be..a250a4871 100644 --- a/config/crd/bases/mongodb.com_opsmanagers.yaml +++ b/config/crd/bases/mongodb.com_opsmanagers.yaml @@ -847,6 +847,7 @@ spec: type: string type: array oidcProviderConfigs: + description: Configuration for OIDC providers items: properties: audience: @@ -879,14 +880,15 @@ spec: type: string configurationName: description: |- - TODO add proper validation and test it Unique label that identifies this configuration. This label is visible to your Ops Manager users and is used when creating users and roles for authorization. It is case-sensitive and can only contain the following characters: - alphanumeric characters (combination of a to z and 0 to 9) - hyphens (-) - underscores (_) + pattern: ^[a-zA-Z0-9-_]+$ type: string groupsClaim: + default: groups description: |- The identifier of the claim that includes the principal's IdP user group membership information. Accept the default value unless your IdP uses a different claim, or you need a custom claim. @@ -905,6 +907,7 @@ spec: type: string type: array userClaim: + default: sub description: |- The identifier of the claim that includes the user principal identity. Accept the default value unless your IdP uses a different claim. diff --git a/helm_chart/crds/mongodb.com_mongodb.yaml b/helm_chart/crds/mongodb.com_mongodb.yaml index bdf8bc3f1..8b5ffc33e 100644 --- a/helm_chart/crds/mongodb.com_mongodb.yaml +++ b/helm_chart/crds/mongodb.com_mongodb.yaml @@ -1525,6 +1525,7 @@ spec: type: string type: array oidcProviderConfigs: + description: Configuration for OIDC providers items: properties: audience: @@ -1557,14 +1558,15 @@ spec: type: string configurationName: description: |- - TODO add proper validation and test it Unique label that identifies this configuration. This label is visible to your Ops Manager users and is used when creating users and roles for authorization. It is case-sensitive and can only contain the following characters: - alphanumeric characters (combination of a to z and 0 to 9) - hyphens (-) - underscores (_) + pattern: ^[a-zA-Z0-9-_]+$ type: string groupsClaim: + default: groups description: |- The identifier of the claim that includes the principal's IdP user group membership information. Accept the default value unless your IdP uses a different claim, or you need a custom claim. @@ -1583,6 +1585,7 @@ spec: type: string type: array userClaim: + default: sub description: |- The identifier of the claim that includes the user principal identity. Accept the default value unless your IdP uses a different claim. diff --git a/helm_chart/crds/mongodb.com_mongodbmulticluster.yaml b/helm_chart/crds/mongodb.com_mongodbmulticluster.yaml index f0b64535e..523238eb6 100644 --- a/helm_chart/crds/mongodb.com_mongodbmulticluster.yaml +++ b/helm_chart/crds/mongodb.com_mongodbmulticluster.yaml @@ -785,6 +785,7 @@ spec: type: string type: array oidcProviderConfigs: + description: Configuration for OIDC providers items: properties: audience: @@ -817,14 +818,15 @@ spec: type: string configurationName: description: |- - TODO add proper validation and test it Unique label that identifies this configuration. This label is visible to your Ops Manager users and is used when creating users and roles for authorization. It is case-sensitive and can only contain the following characters: - alphanumeric characters (combination of a to z and 0 to 9) - hyphens (-) - underscores (_) + pattern: ^[a-zA-Z0-9-_]+$ type: string groupsClaim: + default: groups description: |- The identifier of the claim that includes the principal's IdP user group membership information. Accept the default value unless your IdP uses a different claim, or you need a custom claim. @@ -843,6 +845,7 @@ spec: type: string type: array userClaim: + default: sub description: |- The identifier of the claim that includes the user principal identity. Accept the default value unless your IdP uses a different claim. diff --git a/helm_chart/crds/mongodb.com_opsmanagers.yaml b/helm_chart/crds/mongodb.com_opsmanagers.yaml index 4206080be..a250a4871 100644 --- a/helm_chart/crds/mongodb.com_opsmanagers.yaml +++ b/helm_chart/crds/mongodb.com_opsmanagers.yaml @@ -847,6 +847,7 @@ spec: type: string type: array oidcProviderConfigs: + description: Configuration for OIDC providers items: properties: audience: @@ -879,14 +880,15 @@ spec: type: string configurationName: description: |- - TODO add proper validation and test it Unique label that identifies this configuration. This label is visible to your Ops Manager users and is used when creating users and roles for authorization. It is case-sensitive and can only contain the following characters: - alphanumeric characters (combination of a to z and 0 to 9) - hyphens (-) - underscores (_) + pattern: ^[a-zA-Z0-9-_]+$ type: string groupsClaim: + default: groups description: |- The identifier of the claim that includes the principal's IdP user group membership information. Accept the default value unless your IdP uses a different claim, or you need a custom claim. @@ -905,6 +907,7 @@ spec: type: string type: array userClaim: + default: sub description: |- The identifier of the claim that includes the user principal identity. Accept the default value unless your IdP uses a different claim. diff --git a/public/crds.yaml b/public/crds.yaml index 644df1f69..c47fde748 100644 --- a/public/crds.yaml +++ b/public/crds.yaml @@ -1525,6 +1525,7 @@ spec: type: string type: array oidcProviderConfigs: + description: Configuration for OIDC providers items: properties: audience: @@ -1557,14 +1558,15 @@ spec: type: string configurationName: description: |- - TODO add proper validation and test it Unique label that identifies this configuration. This label is visible to your Ops Manager users and is used when creating users and roles for authorization. It is case-sensitive and can only contain the following characters: - alphanumeric characters (combination of a to z and 0 to 9) - hyphens (-) - underscores (_) + pattern: ^[a-zA-Z0-9-_]+$ type: string groupsClaim: + default: groups description: |- The identifier of the claim that includes the principal's IdP user group membership information. Accept the default value unless your IdP uses a different claim, or you need a custom claim. @@ -1583,6 +1585,7 @@ spec: type: string type: array userClaim: + default: sub description: |- The identifier of the claim that includes the user principal identity. Accept the default value unless your IdP uses a different claim. @@ -4158,6 +4161,7 @@ spec: type: string type: array oidcProviderConfigs: + description: Configuration for OIDC providers items: properties: audience: @@ -4190,14 +4194,15 @@ spec: type: string configurationName: description: |- - TODO add proper validation and test it Unique label that identifies this configuration. This label is visible to your Ops Manager users and is used when creating users and roles for authorization. It is case-sensitive and can only contain the following characters: - alphanumeric characters (combination of a to z and 0 to 9) - hyphens (-) - underscores (_) + pattern: ^[a-zA-Z0-9-_]+$ type: string groupsClaim: + default: groups description: |- The identifier of the claim that includes the principal's IdP user group membership information. Accept the default value unless your IdP uses a different claim, or you need a custom claim. @@ -4216,6 +4221,7 @@ spec: type: string type: array userClaim: + default: sub description: |- The identifier of the claim that includes the user principal identity. Accept the default value unless your IdP uses a different claim. @@ -5546,6 +5552,7 @@ spec: type: string type: array oidcProviderConfigs: + description: Configuration for OIDC providers items: properties: audience: @@ -5578,14 +5585,15 @@ spec: type: string configurationName: description: |- - TODO add proper validation and test it Unique label that identifies this configuration. This label is visible to your Ops Manager users and is used when creating users and roles for authorization. It is case-sensitive and can only contain the following characters: - alphanumeric characters (combination of a to z and 0 to 9) - hyphens (-) - underscores (_) + pattern: ^[a-zA-Z0-9-_]+$ type: string groupsClaim: + default: groups description: |- The identifier of the claim that includes the principal's IdP user group membership information. Accept the default value unless your IdP uses a different claim, or you need a custom claim. @@ -5604,6 +5612,7 @@ spec: type: string type: array userClaim: + default: sub description: |- The identifier of the claim that includes the user principal identity. Accept the default value unless your IdP uses a different claim. From 1cbe97a4586028ecd2e37b8e23549a41119222fb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Maciej=20Kara=C5=9B?= Date: Wed, 30 Apr 2025 09:16:43 +0200 Subject: [PATCH 14/36] Fixes for util.ParseURL --- api/v1/mdb/mongodb_validation.go | 8 ++-- pkg/util/util.go | 26 +++++++++-- pkg/util/util_test.go | 79 +++++++++++++------------------- 3 files changed, 60 insertions(+), 53 deletions(-) diff --git a/api/v1/mdb/mongodb_validation.go b/api/v1/mdb/mongodb_validation.go index a91f983bf..171974b79 100644 --- a/api/v1/mdb/mongodb_validation.go +++ b/api/v1/mdb/mongodb_validation.go @@ -189,13 +189,13 @@ func oidcProviderConfigsSingleWorkforceIdentityFederationValidation(configs []OI func oidcProviderConfigIssuerURIValidator(config OIDCProviderConfig) func(DbCommonSpec) v1.ValidationResult { return func(_ DbCommonSpec) v1.ValidationResult { - ok, url := util.IsURL(config.IssuerURI) - if !ok { - return v1.ValidationError("Invalid IssuerURI in OIDC provider config %q", config.ConfigurationName) + url, err := util.ParseURL(config.IssuerURI) + if err != nil { + return v1.ValidationError("Invalid IssuerURI in OIDC provider config %q: %s", config.ConfigurationName, err.Error()) } if url.Scheme != "https" { - return v1.ValidationWarning("IssuerURI in OIDC provider config %q in not secure endpoint", config.ConfigurationName) + return v1.ValidationWarning("IssuerURI %s in OIDC provider config %q in not secure endpoint", url.String(), config.ConfigurationName) } return v1.ValidationSuccess() diff --git a/pkg/util/util.go b/pkg/util/util.go index f941b4bb7..13d6b5ac0 100644 --- a/pkg/util/util.go +++ b/pkg/util/util.go @@ -186,8 +186,28 @@ func TransformToMap[T any, K comparable, V any](objs []T, f func(obj T, idx int) return result } -// IsURL checks if the given string is a valid URL and returns the parsed URL if valid. -func IsURL(str string) (bool, *url.URL) { +// ParseURL checks if the given string is a valid URL and returns the parsed URL if valid. +func ParseURL(str string) (*url.URL, error) { + if strings.TrimSpace(str) == "" { + return nil, fmt.Errorf("empty URL") + } + u, err := url.Parse(str) - return err == nil && u.Scheme != "" && u.Host != "", u + if err != nil { + return nil, fmt.Errorf("invalid URL: %w", err) + } + + if u.Scheme == "" { + return nil, fmt.Errorf("missing URL scheme: %s", str) + } + + if u.Scheme != "http" && u.Scheme != "https" { + return nil, fmt.Errorf("invalid URL scheme (http or https): %s", str) + } + + if u.Host == "" { + return nil, fmt.Errorf("missing URL host: %s", str) + } + + return u, nil } diff --git a/pkg/util/util_test.go b/pkg/util/util_test.go index 514bee8c1..cd639d41c 100644 --- a/pkg/util/util_test.go +++ b/pkg/util/util_test.go @@ -2,10 +2,10 @@ package util import ( "fmt" - "net/url" "testing" "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" "github.com/mongodb/mongodb-kubernetes/pkg/util/identifiable" ) @@ -198,77 +198,64 @@ func TestTransformToMap(t *testing.T) { })) } -// TestIsURL tests the IsURL function with various inputs. +// TestIsURL tests the ParseURL function with various inputs. // //goland:noinspection HttpUrlsUsage func TestIsURL(t *testing.T) { tests := []struct { - name string - input string - isValid bool - checkURL func(*url.URL) bool + name string + input string + expectedErrorString string }{ { - name: "valid http URL", - input: "http://example.com", - isValid: true, - checkURL: func(u *url.URL) bool { - return u.Scheme == "http" && u.Host == "example.com" - }, + name: "valid http URL", + input: "http://example.com", }, { - name: "valid https URL with path", - input: "https://example.com/path", - isValid: true, - checkURL: func(u *url.URL) bool { - return u.Scheme == "https" && u.Host == "example.com" && u.Path == "/path" - }, + name: "valid https URL with path", + input: "https://example.com/path", }, { - name: "valid URL with port", - input: "http://example.com:8080", - isValid: true, - checkURL: func(u *url.URL) bool { - return u.Host == "example.com:8080" - }, + name: "valid URL with port", + input: "http://example.com:8080", }, { - name: "missing scheme", - input: "example.com", - isValid: false, + name: "missing scheme", + input: "example.com", + expectedErrorString: "missing URL scheme: example.com", }, { - name: "missing host", - input: "http://", - isValid: false, + name: "missing host", + input: "http://", + expectedErrorString: "missing URL host: http://", }, { - name: "empty string", - input: "", - isValid: false, + name: "empty string", + input: "", + expectedErrorString: "empty URL", }, { - name: "invalid URL", - input: ":invalid-url", - isValid: false, + name: "invalid URL", + input: ":invalid-url", + expectedErrorString: "invalid URL: parse \":invalid-url\": missing protocol scheme", }, { - name: "file scheme", - input: "file:///path/to/file", - isValid: false, + name: "file scheme", + input: "file://path/to/file", + expectedErrorString: "invalid URL scheme (http or https): file://path/to/file", }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - valid, u := IsURL(tt.input) - assert.Equal(t, tt.isValid, valid) - - if tt.isValid { + u, err := ParseURL(tt.input) + if tt.expectedErrorString != "" { + require.Error(t, err) + assert.Equal(t, tt.expectedErrorString, err.Error()) + assert.Nil(t, u) + } else { + assert.NoError(t, err) assert.NotNil(t, u) - if tt.checkURL != nil { - assert.True(t, tt.checkURL(u)) - } } }) } From 0ce0874677c3aaf51078a5a046cf2aaf1b01bc6c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Maciej=20Kara=C5=9B?= Date: Tue, 29 Apr 2025 15:13:46 +0200 Subject: [PATCH 15/36] Proper OIDC AC merging --- controllers/om/automation_config.go | 66 +++- controllers/om/automation_config_test.go | 285 +++++++++++++++++- .../om/testdata/automation_config.json | 48 ++- controllers/operator/authentication/oidc.go | 16 +- 4 files changed, 383 insertions(+), 32 deletions(-) diff --git a/controllers/om/automation_config.go b/controllers/om/automation_config.go index 1b6bbc608..951382c0f 100644 --- a/controllers/om/automation_config.go +++ b/controllers/om/automation_config.go @@ -61,24 +61,66 @@ func applyInto(a AutomationConfig, into *Deployment) error { (*into)["ldap"] = mergedLdap } - if _, ok := a.Deployment["oidcProviderConfigs"]; ok || len(a.OIDCProviderConfigs) > 0 { - // TODO: this is not merged yet, but only overridden - bytes, err := json.Marshal(a.OIDCProviderConfigs) - if err != nil { - return err + if len(a.OIDCProviderConfigs) > 0 { + deploymentConfigs := make([]map[string]any, 0) + if configs, ok := a.Deployment["oidcProviderConfigs"]; ok { + configsSlice := cast.ToSlice(configs) + for _, config := range configsSlice { + deploymentConfigs = append(deploymentConfigs, config.(map[string]any)) + } } - dst := make([]map[string]interface{}, 0) - err = json.Unmarshal(bytes, &dst) - if err != nil { - return err + result := make([]map[string]any, 0) + for _, config := range a.OIDCProviderConfigs { + deploymentConfig := findOrCreateEmptyDeploymentConfig(deploymentConfigs, config.AuthNamePrefix) + + deploymentConfig["authNamePrefix"] = config.AuthNamePrefix + deploymentConfig["audience"] = config.Audience + deploymentConfig["issuerUri"] = config.IssuerUri + deploymentConfig["userClaim"] = config.UserClaim + deploymentConfig["supportsHumanFlows"] = config.SupportsHumanFlows + deploymentConfig["useAuthorizationClaim"] = config.UseAuthorizationClaim + + if config.ClientId == util.MergoDelete { + delete(deploymentConfig, "clientId") + } else { + deploymentConfig["clientId"] = config.ClientId + } + + if len(config.RequestedScopes) == 0 { + delete(deploymentConfig, "requestedScopes") + } else { + deploymentConfig["requestedScopes"] = config.RequestedScopes + } + + if config.GroupsClaim == util.MergoDelete { + delete(deploymentConfig, "groupsClaim") + } else { + deploymentConfig["groupsClaim"] = config.GroupsClaim + } + + result = append(result, deploymentConfig) } - (*into)["oidcProviderConfigs"] = dst + + (*into)["oidcProviderConfigs"] = result + } else { + // Clear oidcProviderConfigs if no configs are provided + delete(*into, "oidcProviderConfigs") } return nil } +func findOrCreateEmptyDeploymentConfig(deploymentConfigs []map[string]any, configName string) map[string]any { + for _, deploymentConfig := range deploymentConfigs { + if configName == deploymentConfig["authNamePrefix"] { + return deploymentConfig + } + } + + return make(map[string]any) +} + // EqualsWithoutDeployment returns true if two AutomationConfig objects are meaningful equal by following the following conditions: // - Not taking AutomationConfig.Deployment into consideration. // - Serializing ac A and ac B to ensure that we remove util.MergoDelete before comparing those two. @@ -450,9 +492,9 @@ func BuildAutomationConfigFromDeployment(deployment Deployment) (*AutomationConf finalAutomationConfig.Ldap = acLdap } - oidcSlice, ok := deployment["oidcProviderConfigs"] + oidcConfigsArray, ok := deployment["oidcProviderConfigs"] if ok { - oidcMarshalled, err := json.Marshal(oidcSlice) + oidcMarshalled, err := json.Marshal(oidcConfigsArray) if err != nil { return nil, err } diff --git a/controllers/om/automation_config_test.go b/controllers/om/automation_config_test.go index 0b991ef0b..fb4875636 100644 --- a/controllers/om/automation_config_test.go +++ b/controllers/om/automation_config_test.go @@ -9,6 +9,7 @@ import ( "k8s.io/apimachinery/pkg/api/equality" "github.com/mongodb/mongodb-kubernetes/controllers/operator/ldap" + "github.com/mongodb/mongodb-kubernetes/controllers/operator/oidc" "github.com/mongodb/mongodb-kubernetes/pkg/util" ) @@ -627,6 +628,54 @@ func TestAutomationConfigEquality(t *testing.T) { } ldapConfig2 := ldapConfig + oidcProviderConfigs := []oidc.ProviderConfig{ + { + AuthNamePrefix: "provider-1", + Audience: "aud", + IssuerUri: "https://provider1.okta.com", + UserClaim: "sub", + GroupsClaim: "groups", + SupportsHumanFlows: false, + UseAuthorizationClaim: true, + }, + { + AuthNamePrefix: "provider-2", + Audience: "aud", + IssuerUri: "https://provider2.okta.com", + ClientId: "provider2-clientId", + RequestedScopes: []string{"openid", "profile"}, + UserClaim: "sub", + SupportsHumanFlows: true, + UseAuthorizationClaim: false, + }, + } + + oidcProviderConfigsWithMergoDelete := []oidc.ProviderConfig{ + { + AuthNamePrefix: "provider-1", + Audience: "aud", + IssuerUri: "https://provider1.okta.com", + ClientId: util.MergoDelete, + UserClaim: "sub", + GroupsClaim: "groups", + SupportsHumanFlows: false, + UseAuthorizationClaim: true, + }, + { + AuthNamePrefix: "provider-2", + Audience: "aud", + IssuerUri: "https://provider2.okta.com", + ClientId: "provider2-clientId", + RequestedScopes: []string{"openid", "profile"}, + UserClaim: "sub", + GroupsClaim: util.MergoDelete, + SupportsHumanFlows: true, + UseAuthorizationClaim: false, + }, + } + + oidcProviderConfigs2 := oidcProviderConfigs + tests := map[string]struct { a *AutomationConfig b *AutomationConfig @@ -653,29 +702,33 @@ func TestAutomationConfigEquality(t *testing.T) { }, "Two the same configs created using the same structs are the same": { a: &AutomationConfig{ - Auth: &authConfig, - AgentSSL: &agentSSLConfig, - Deployment: deployment1, - Ldap: &ldapConfig, + Auth: &authConfig, + AgentSSL: &agentSSLConfig, + Deployment: deployment1, + Ldap: &ldapConfig, + OIDCProviderConfigs: oidcProviderConfigs, }, b: &AutomationConfig{ - Auth: &authConfig, - AgentSSL: &agentSSLConfig, - Deployment: deployment1, - Ldap: &ldapConfig, + Auth: &authConfig, + AgentSSL: &agentSSLConfig, + Deployment: deployment1, + Ldap: &ldapConfig, + OIDCProviderConfigs: oidcProviderConfigs, }, expectedEquality: true, }, "Two the same configs created using deep copy (and structs with different addresses) are the same": { a: &AutomationConfig{ - Auth: &authConfig, - AgentSSL: &agentSSLConfig, - Ldap: &ldapConfig, + Auth: &authConfig, + AgentSSL: &agentSSLConfig, + Ldap: &ldapConfig, + OIDCProviderConfigs: oidcProviderConfigs, }, b: &AutomationConfig{ - Auth: &authConfig2, - AgentSSL: &agentSSLConfig2, - Ldap: &ldapConfig2, + Auth: &authConfig2, + AgentSSL: &agentSSLConfig2, + Ldap: &ldapConfig2, + OIDCProviderConfigs: oidcProviderConfigs2, }, expectedEquality: true, }, @@ -685,7 +738,8 @@ func TestAutomationConfigEquality(t *testing.T) { NewAutoPwd: util.MergoDelete, LdapGroupDN: "abc", }, - Ldap: &ldapConfig, + Ldap: &ldapConfig, + OIDCProviderConfigs: oidcProviderConfigs, }, b: &AutomationConfig{ Auth: &Auth{ @@ -694,7 +748,8 @@ func TestAutomationConfigEquality(t *testing.T) { AgentSSL: &AgentSSL{ AutoPEMKeyFilePath: util.MergoDelete, }, - Ldap: &ldapConfig2, + Ldap: &ldapConfig2, + OIDCProviderConfigs: oidcProviderConfigsWithMergoDelete, }, expectedEquality: true, }, @@ -783,6 +838,204 @@ func TestLDAPIsMerged(t *testing.T) { assert.Contains(t, ldapMap, "CAFileContents") } +func TestOIDCProviderConfigsAreMerged(t *testing.T) { + tests := map[string]struct { + providerConfigs []oidc.ProviderConfig + expectedProviderConfigs any + }{ + "nil config list clears deployment oidcProviderConfigs": { + providerConfigs: nil, + expectedProviderConfigs: nil, + }, + "empty config list clears deployment oidcProviderConfigs": { + providerConfigs: make([]oidc.ProviderConfig, 0), + expectedProviderConfigs: nil, + }, + "single config not present in deployment oidcProviderConfigs": { + providerConfigs: []oidc.ProviderConfig{ + { + AuthNamePrefix: "provider-new", + Audience: "aud", + ClientId: util.MergoDelete, + IssuerUri: "https://provider-new.okta.com", + UserClaim: "sub", + GroupsClaim: "groups", + SupportsHumanFlows: false, + UseAuthorizationClaim: true, + }, + }, + expectedProviderConfigs: []map[string]any{ + { + "authNamePrefix": "provider-new", + "audience": "aud", + "issuerUri": "https://provider-new.okta.com", + "userClaim": "sub", + "groupsClaim": "groups", + "supportsHumanFlows": false, + "useAuthorizationClaim": true, + }, + }, + }, + "single config present in deployment oidcProviderConfig, without changes": { + providerConfigs: []oidc.ProviderConfig{ + { + AuthNamePrefix: "OIDC_WORKFORCE_USERID", + Audience: "aud", + IssuerUri: "https://provider.okta.com", + ClientId: "oktaClientId", + UserClaim: "sub", + GroupsClaim: util.MergoDelete, + RequestedScopes: []string{"openid"}, + SupportsHumanFlows: true, + UseAuthorizationClaim: false, + }, + }, + expectedProviderConfigs: []map[string]any{ + { + "authNamePrefix": "OIDC_WORKFORCE_USERID", + "audience": "aud", + "issuerUri": "https://provider.okta.com", + "clientId": "oktaClientId", + "requestedScopes": []string{ + "openid", + }, + "userClaim": "sub", + "supportsHumanFlows": true, + "useAuthorizationClaim": false, + "JWKSPollSecs": float64(360), + "additionalField": []any{ + "example.com", + }, + }, + }, + }, + "single config present in deployment oidcProviderConfig, but modified": { + providerConfigs: []oidc.ProviderConfig{ + { + AuthNamePrefix: "OIDC_WORKLOAD_USERID", + Audience: "aud", + ClientId: util.MergoDelete, + IssuerUri: "https://provider1.okta.com", + UserClaim: "sub", + GroupsClaim: "groups", + SupportsHumanFlows: false, + UseAuthorizationClaim: true, + }, + }, + expectedProviderConfigs: []map[string]any{ + { + "authNamePrefix": "OIDC_WORKLOAD_USERID", + "audience": "aud", + "issuerUri": "https://provider1.okta.com", + "userClaim": "sub", + "groupsClaim": "groups", + "supportsHumanFlows": false, + "useAuthorizationClaim": true, + "JWKSPollSecs": float64(360), + "additionalField": []interface{}{ + "example.com", + }, + }, + }, + }, + "multiple configs in deployment oidcProviderConfig": { + providerConfigs: []oidc.ProviderConfig{ + { + AuthNamePrefix: "OIDC_WORKFORCE_USERID", + Audience: "aud", + ClientId: "oktaClientId", + IssuerUri: "https://provider.okta.com", + UserClaim: "sub", + GroupsClaim: util.MergoDelete, + RequestedScopes: []string{"openid"}, + SupportsHumanFlows: true, + UseAuthorizationClaim: false, + }, + { + AuthNamePrefix: "OIDC_WORKLOAD_USERID", + Audience: "aud", + ClientId: util.MergoDelete, + IssuerUri: "https://provider.okta.com", + UserClaim: "sub", + GroupsClaim: util.MergoDelete, + RequestedScopes: []string{"openid"}, + SupportsHumanFlows: false, + UseAuthorizationClaim: false, + }, + { + AuthNamePrefix: "OIDC_WORKLOAD_GROUP_MEMBERSHIP", + Audience: "aud", + ClientId: util.MergoDelete, + IssuerUri: "https://provider.okta.com", + UserClaim: "sub", + GroupsClaim: "groups", + SupportsHumanFlows: false, + UseAuthorizationClaim: true, + }, + }, + expectedProviderConfigs: []map[string]any{ + { + "authNamePrefix": "OIDC_WORKFORCE_USERID", + "audience": "aud", + "issuerUri": "https://provider.okta.com", + "clientId": "oktaClientId", + "requestedScopes": []string{ + "openid", + }, + "userClaim": "sub", + "supportsHumanFlows": true, + "useAuthorizationClaim": false, + "JWKSPollSecs": float64(360), + "additionalField": []interface{}{ + "example.com", + }, + }, + { + "authNamePrefix": "OIDC_WORKLOAD_USERID", + "audience": "aud", + "issuerUri": "https://provider.okta.com", + "requestedScopes": []string{ + "openid", + }, + "userClaim": "sub", + "supportsHumanFlows": false, + "useAuthorizationClaim": false, + "JWKSPollSecs": float64(360), + "additionalField": []interface{}{ + "example.com", + }, + }, + { + "authNamePrefix": "OIDC_WORKLOAD_GROUP_MEMBERSHIP", + "audience": "aud", + "issuerUri": "https://provider.okta.com", + "userClaim": "sub", + "groupsClaim": "groups", + "supportsHumanFlows": false, + "useAuthorizationClaim": true, + "JWKSPollSecs": float64(360), + "additionalField": []interface{}{ + "example.com", + }, + }, + }, + }, + } + + for testName, testParameters := range tests { + t.Run(testName, func(t *testing.T) { + ac := getTestAutomationConfig() + ac.OIDCProviderConfigs = testParameters.providerConfigs + if err := ac.Apply(); err != nil { + t.Fatal(err) + } + + providerConfigs := ac.Deployment["oidcProviderConfigs"] + assert.Equal(t, testParameters.expectedProviderConfigs, providerConfigs) + }) + } +} + func TestApplyInto(t *testing.T) { config := AutomationConfig{ Auth: NewAuth(), diff --git a/controllers/om/testdata/automation_config.json b/controllers/om/testdata/automation_config.json index e2e9041e8..2ae929656 100644 --- a/controllers/om/testdata/automation_config.json +++ b/controllers/om/testdata/automation_config.json @@ -320,5 +320,51 @@ "clientCertificateMode": "OPTIONAL" }, "version": null, - "sharding": [] + "sharding": [], + "oidcProviderConfigs": [ + { + "audience": "aud", + "issuerUri": "https://provider.okta.com", + "clientId": "oktaClientId", + "requestedScopes": [ + "openid" + ], + "userClaim": "sub", + "JWKSPollSecs": 360, + "authNamePrefix": "OIDC_WORKFORCE_USERID", + "supportsHumanFlows": true, + "useAuthorizationClaim": false, + "additionalField": [ + "example.com" + ] + }, + { + "audience": "aud", + "issuerUri": "https://provider.okta.com", + "requestedScopes": [ + "openid" + ], + "userClaim": "sub", + "JWKSPollSecs": 360, + "authNamePrefix": "OIDC_WORKLOAD_USERID", + "supportsHumanFlows": false, + "useAuthorizationClaim": false, + "additionalField": [ + "example.com" + ] + }, + { + "audience": "aud", + "issuerUri": "https://provider.okta.com", + "userClaim": "sub", + "groupsClaim": "groups", + "JWKSPollSecs": 360, + "authNamePrefix": "OIDC_WORKLOAD_GROUP_MEMBERSHIP", + "supportsHumanFlows": false, + "useAuthorizationClaim": true, + "additionalField": [ + "example.com" + ] + } + ] } diff --git a/controllers/operator/authentication/oidc.go b/controllers/operator/authentication/oidc.go index 3ffc9bdaf..ef3d8d721 100644 --- a/controllers/operator/authentication/oidc.go +++ b/controllers/operator/authentication/oidc.go @@ -11,6 +11,7 @@ import ( mdbv1 "github.com/mongodb/mongodb-kubernetes/api/v1/mdb" "github.com/mongodb/mongodb-kubernetes/controllers/om" "github.com/mongodb/mongodb-kubernetes/controllers/operator/oidc" + "github.com/mongodb/mongodb-kubernetes/pkg/util" "github.com/mongodb/mongodb-kubernetes/pkg/util/stringutil" ) @@ -35,7 +36,6 @@ func (o *oidcAuthMechanism) EnableDeploymentAuthentication(conn om.Connection, o if !stringutil.Contains(ac.Auth.DeploymentAuthMechanisms, string(MongoDBOIDC)) { ac.Auth.DeploymentAuthMechanisms = append(ac.Auth.DeploymentAuthMechanisms, string(MongoDBOIDC)) } - // TODO merge configs with existing ones, and don't overwrite read only values ac.OIDCProviderConfigs = opts.OIDCProviderConfigs return nil @@ -120,14 +120,24 @@ func MapOIDCProviderConfigs(oidcProviderConfigs []mdbv1.OIDCProviderConfig) []oi result := make([]oidc.ProviderConfig, len(oidcProviderConfigs)) for i, providerConfig := range oidcProviderConfigs { + clientId := providerConfig.ClientId + if clientId == "" { + clientId = util.MergoDelete + } + + groupsClaim := providerConfig.GroupsClaim + if groupsClaim == "" { + groupsClaim = util.MergoDelete + } + result[i] = oidc.ProviderConfig{ AuthNamePrefix: providerConfig.ConfigurationName, Audience: providerConfig.Audience, IssuerUri: providerConfig.IssuerURI, - ClientId: providerConfig.ClientId, + ClientId: clientId, RequestedScopes: providerConfig.RequestedScopes, UserClaim: providerConfig.UserClaim, - GroupsClaim: providerConfig.GroupsClaim, + GroupsClaim: groupsClaim, SupportsHumanFlows: mapToSupportHumanFlows(providerConfig.AuthorizationMethod), UseAuthorizationClaim: mapToUseAuthorizationClaim(providerConfig.AuthorizationType), } From e4cfb11b677ce88cc4cd635e59990df8bbc2d641 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Maciej=20Kara=C5=9B?= Date: Wed, 30 Apr 2025 09:51:53 +0200 Subject: [PATCH 16/36] Unit test fixes --- api/v1/mdb/mongodb_validation_test.go | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/api/v1/mdb/mongodb_validation_test.go b/api/v1/mdb/mongodb_validation_test.go index 4a3db9da4..4529ef2c4 100644 --- a/api/v1/mdb/mongodb_validation_test.go +++ b/api/v1/mdb/mongodb_validation_test.go @@ -306,9 +306,8 @@ func TestOIDCAuthValidation(t *testing.T) { }, }, }, - expectedErrorMessage: "Invalid IssuerURI in OIDC provider config \"test-provider\"", + expectedErrorMessage: "Invalid IssuerURI in OIDC provider config \"test-provider\": missing URL scheme: invalid-uri", }, - // TODO fix validation, we should not return warnings immediately, only add them to the result { name: "Non-HTTPS issuer URI - warning", auth: &Authentication{ @@ -322,7 +321,7 @@ func TestOIDCAuthValidation(t *testing.T) { }, }, }, - expectedWarning: "IssuerURI in OIDC provider config \"test-provider\" in not secure endpoint", + expectedWarning: "IssuerURI http://example.com in OIDC provider config \"test-provider\" in not secure endpoint", }, { name: "Workforce Identity Federation without ClientId", From 16670455eb1fc41491abe4e7209ece6063c6a309 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Maciej=20Kara=C5=9B?= Date: Wed, 30 Apr 2025 12:35:05 +0200 Subject: [PATCH 17/36] Fixed issue with disabling OIDC --- controllers/operator/authentication/authentication_mechanism.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/controllers/operator/authentication/authentication_mechanism.go b/controllers/operator/authentication/authentication_mechanism.go index d980b166b..161e361cc 100644 --- a/controllers/operator/authentication/authentication_mechanism.go +++ b/controllers/operator/authentication/authentication_mechanism.go @@ -65,7 +65,7 @@ func (m MechanismList) Contains(mechanism Mechanism) bool { // supportedMechanisms returns a list of all supported authentication mechanisms // that can be configured by the Operator -var supportedMechanisms = []Mechanism{ScramSha256Mechanism, MongoDBCRMechanism, MongoDBX509Mechanism, LDAPPlainMechanism} +var supportedMechanisms = []Mechanism{ScramSha256Mechanism, MongoDBCRMechanism, MongoDBX509Mechanism, LDAPPlainMechanism, MongoDBOIDCMechanism} // mechanismsToDisable returns mechanisms which need to be disabled // based on the currently supported authentication mechanisms and the desiredMechanisms From e882a8c9fd41d9c80cbed95d877f8e781e7e2f17 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Maciej=20Kara=C5=9B?= Date: Wed, 30 Apr 2025 22:16:10 +0200 Subject: [PATCH 18/36] Resolve review comments --- controllers/operator/authentication/ldap.go | 2 +- controllers/operator/authentication/scramsha.go | 6 +++--- controllers/operator/authentication/scramsha_test.go | 2 +- controllers/operator/authentication/x509.go | 2 +- 4 files changed, 6 insertions(+), 6 deletions(-) diff --git a/controllers/operator/authentication/ldap.go b/controllers/operator/authentication/ldap.go index 94faf0ce8..debfe091d 100644 --- a/controllers/operator/authentication/ldap.go +++ b/controllers/operator/authentication/ldap.go @@ -9,7 +9,7 @@ import ( "github.com/mongodb/mongodb-kubernetes/pkg/util/stringutil" ) -var LDAPPlainMechanism = &ldapAuthMechanism{} +var LDAPPlainMechanism Mechanism = &ldapAuthMechanism{} type ldapAuthMechanism struct{} diff --git a/controllers/operator/authentication/scramsha.go b/controllers/operator/authentication/scramsha.go index 9a3cb6ded..e21d5aeb4 100644 --- a/controllers/operator/authentication/scramsha.go +++ b/controllers/operator/authentication/scramsha.go @@ -9,9 +9,9 @@ import ( ) var ( - MongoDBCRMechanism = AutomationConfigScramSha{MechanismName: MongoDBCR} - ScramSha1Mechanism = AutomationConfigScramSha{MechanismName: ScramSha1} - ScramSha256Mechanism = AutomationConfigScramSha{MechanismName: ScramSha256} + MongoDBCRMechanism Mechanism = AutomationConfigScramSha{MechanismName: MongoDBCR} + ScramSha1Mechanism Mechanism = AutomationConfigScramSha{MechanismName: ScramSha1} + ScramSha256Mechanism Mechanism = AutomationConfigScramSha{MechanismName: ScramSha256} ) // AutomationConfigScramSha applies all the changes required to configure SCRAM-SHA authentication diff --git a/controllers/operator/authentication/scramsha_test.go b/controllers/operator/authentication/scramsha_test.go index 21a0dd61c..81bd81ce6 100644 --- a/controllers/operator/authentication/scramsha_test.go +++ b/controllers/operator/authentication/scramsha_test.go @@ -13,7 +13,7 @@ import ( func TestAgentsAuthentication(t *testing.T) { type TestConfig struct { - mechanism AutomationConfigScramSha + mechanism Mechanism } tests := map[string]TestConfig{ "SCRAM-SHA-1": { diff --git a/controllers/operator/authentication/x509.go b/controllers/operator/authentication/x509.go index 61d7ff2ce..f221fa58d 100644 --- a/controllers/operator/authentication/x509.go +++ b/controllers/operator/authentication/x509.go @@ -10,7 +10,7 @@ import ( "github.com/mongodb/mongodb-kubernetes/pkg/util/stringutil" ) -var MongoDBX509Mechanism = ConnectionX509{} +var MongoDBX509Mechanism Mechanism = ConnectionX509{} type ConnectionX509 struct{} From 8f5ff0a72428e5fca31b81be58b6e9a020020405 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Maciej=20Kara=C5=9B?= Date: Wed, 30 Apr 2025 22:32:21 +0200 Subject: [PATCH 19/36] Added getMechanismByName() func and removed global variables --- .../authentication_mechanism.go | 43 +++++++++++++------ controllers/operator/authentication/ldap.go | 2 - .../operator/authentication/ldap_test.go | 3 ++ .../operator/authentication/scramsha.go | 24 ++++------- .../operator/authentication/scramsha_test.go | 6 +++ controllers/operator/authentication/x509.go | 18 ++++---- .../operator/authentication/x509_test.go | 2 + 7 files changed, 58 insertions(+), 40 deletions(-) diff --git a/controllers/operator/authentication/authentication_mechanism.go b/controllers/operator/authentication/authentication_mechanism.go index bc5521045..37b0fd278 100644 --- a/controllers/operator/authentication/authentication_mechanism.go +++ b/controllers/operator/authentication/authentication_mechanism.go @@ -52,9 +52,9 @@ func (m MechanismList) String() string { return strings.Join(names, ", ") } -func (m MechanismList) Contains(mechanism Mechanism) bool { +func (m MechanismList) Contains(mechanismName MechanismName) bool { for _, m := range m { - if m.GetName() == mechanism.GetName() { + if m.GetName() == mechanismName { return true } } @@ -64,15 +64,15 @@ func (m MechanismList) Contains(mechanism Mechanism) bool { // supportedMechanisms returns a list of all supported authentication mechanisms // that can be configured by the Operator -var supportedMechanisms = []Mechanism{ScramSha256Mechanism, MongoDBCRMechanism, MongoDBX509Mechanism, LDAPPlainMechanism} +var supportedMechanisms = []MechanismName{ScramSha256, MongoDBCR, MongoDBX509, LDAPPlain} // mechanismsToDisable returns mechanisms which need to be disabled // based on the currently supported authentication mechanisms and the desiredMechanisms func mechanismsToDisable(desiredMechanisms MechanismList) MechanismList { toDisable := make([]Mechanism, 0) - for _, mechanism := range supportedMechanisms { - if !desiredMechanisms.Contains(mechanism) { - toDisable = append(toDisable, mechanism) + for _, mechanismName := range supportedMechanisms { + if !desiredMechanisms.Contains(mechanismName) { + toDisable = append(toDisable, getMechanismByName(mechanismName)) } } @@ -92,15 +92,15 @@ func convertToMechanismList(mechanismModesInCR []string, ac *om.AutomationConfig func convertToMechanism(mechanismModeInCR string, ac *om.AutomationConfig) Mechanism { switch mechanismModeInCR { case util.X509: - return MongoDBX509Mechanism + return getMechanismByName(MongoDBX509) case util.LDAP: - return LDAPPlainMechanism + return getMechanismByName(LDAPPlain) case util.SCRAMSHA1: - return ScramSha1Mechanism + return getMechanismByName(ScramSha1) case util.MONGODBCR: - return MongoDBCRMechanism + return getMechanismByName(MongoDBCR) case util.SCRAMSHA256: - return ScramSha256Mechanism + return getMechanismByName(ScramSha256) case util.SCRAM: // if we have already configured authentication, and it has been set to MONGODB-CR/SCRAM-SHA-1 // we can not transition. This needs to be done in the UI @@ -108,11 +108,28 @@ func convertToMechanism(mechanismModeInCR string, ac *om.AutomationConfig) Mecha // if no authentication has been configured, the default value for "AutoAuthMechanism" is "MONGODB-CR" // even if authentication is disabled, so we need to ensure that auth has been enabled. if ac.Auth.AutoAuthMechanism == string(MongoDBCR) && ac.Auth.IsEnabled() { - return MongoDBCRMechanism + return getMechanismByName(MongoDBCR) } - return ScramSha256Mechanism + return getMechanismByName(ScramSha256) } // this should never be reached as validation of this string happens at the CR level panic(xerrors.Errorf("unknown mechanism name %s", mechanismModeInCR)) } + +func getMechanismByName(name MechanismName) Mechanism { + switch name { + case ScramSha1: + return &automationConfigScramSha{MechanismName: ScramSha1} + case ScramSha256: + return &automationConfigScramSha{MechanismName: ScramSha256} + case MongoDBCR: + return &automationConfigScramSha{MechanismName: MongoDBCR} + case MongoDBX509: + return &connectionX509{} + case LDAPPlain: + return &ldapAuthMechanism{} + } + + panic(xerrors.Errorf("unknown mechanism name %s", name)) +} diff --git a/controllers/operator/authentication/ldap.go b/controllers/operator/authentication/ldap.go index debfe091d..eea18dd34 100644 --- a/controllers/operator/authentication/ldap.go +++ b/controllers/operator/authentication/ldap.go @@ -9,8 +9,6 @@ import ( "github.com/mongodb/mongodb-kubernetes/pkg/util/stringutil" ) -var LDAPPlainMechanism Mechanism = &ldapAuthMechanism{} - type ldapAuthMechanism struct{} func (l *ldapAuthMechanism) GetName() MechanismName { diff --git a/controllers/operator/authentication/ldap_test.go b/controllers/operator/authentication/ldap_test.go index 12779faad..225e1f39c 100644 --- a/controllers/operator/authentication/ldap_test.go +++ b/controllers/operator/authentication/ldap_test.go @@ -11,6 +11,8 @@ import ( "github.com/mongodb/mongodb-kubernetes/controllers/operator/ldap" ) +var LDAPPlainMechanism = getMechanismByName(LDAPPlain) + func TestLdapDeploymentMechanism(t *testing.T) { conn := om.NewMockedOmConnection(om.NewDeployment()) @@ -77,5 +79,6 @@ func TestLDAP_DisableAgentAuthentication(t *testing.T) { AutomationSubject: validSubject("automation"), }, } + assertAgentAuthenticationDisabled(t, LDAPPlainMechanism, conn, opts) } diff --git a/controllers/operator/authentication/scramsha.go b/controllers/operator/authentication/scramsha.go index e21d5aeb4..f003015cf 100644 --- a/controllers/operator/authentication/scramsha.go +++ b/controllers/operator/authentication/scramsha.go @@ -8,23 +8,17 @@ import ( "github.com/mongodb/mongodb-kubernetes/pkg/util/stringutil" ) -var ( - MongoDBCRMechanism Mechanism = AutomationConfigScramSha{MechanismName: MongoDBCR} - ScramSha1Mechanism Mechanism = AutomationConfigScramSha{MechanismName: ScramSha1} - ScramSha256Mechanism Mechanism = AutomationConfigScramSha{MechanismName: ScramSha256} -) - -// AutomationConfigScramSha applies all the changes required to configure SCRAM-SHA authentication +// automationConfigScramSha applies all the changes required to configure SCRAM-SHA authentication // directly to an AutomationConfig struct. This implementation does not communicate with Ops Manager in any way. -type AutomationConfigScramSha struct { +type automationConfigScramSha struct { MechanismName MechanismName } -func (s AutomationConfigScramSha) GetName() MechanismName { +func (s *automationConfigScramSha) GetName() MechanismName { return s.MechanismName } -func (s AutomationConfigScramSha) EnableAgentAuthentication(conn om.Connection, opts Options, log *zap.SugaredLogger) error { +func (s *automationConfigScramSha) EnableAgentAuthentication(conn om.Connection, opts Options, log *zap.SugaredLogger) error { return conn.ReadUpdateAutomationConfig(func(ac *om.AutomationConfig) error { if err := configureScramAgentUsers(ac, opts); err != nil { return err @@ -45,21 +39,21 @@ func (s AutomationConfigScramSha) EnableAgentAuthentication(conn om.Connection, }, log) } -func (s AutomationConfigScramSha) DisableAgentAuthentication(conn om.Connection, log *zap.SugaredLogger) error { +func (s *automationConfigScramSha) DisableAgentAuthentication(conn om.Connection, log *zap.SugaredLogger) error { return conn.ReadUpdateAutomationConfig(func(ac *om.AutomationConfig) error { ac.Auth.AutoAuthMechanisms = stringutil.Remove(ac.Auth.AutoAuthMechanisms, string(s.MechanismName)) return nil }, log) } -func (s AutomationConfigScramSha) DisableDeploymentAuthentication(conn om.Connection, log *zap.SugaredLogger) error { +func (s *automationConfigScramSha) DisableDeploymentAuthentication(conn om.Connection, log *zap.SugaredLogger) error { return conn.ReadUpdateAutomationConfig(func(ac *om.AutomationConfig) error { ac.Auth.DeploymentAuthMechanisms = stringutil.Remove(ac.Auth.DeploymentAuthMechanisms, string(s.MechanismName)) return nil }, log) } -func (s AutomationConfigScramSha) EnableDeploymentAuthentication(conn om.Connection, _ Options, log *zap.SugaredLogger) error { +func (s *automationConfigScramSha) EnableDeploymentAuthentication(conn om.Connection, _ Options, log *zap.SugaredLogger) error { return conn.ReadUpdateAutomationConfig(func(ac *om.AutomationConfig) error { if !stringutil.Contains(ac.Auth.DeploymentAuthMechanisms, string(s.MechanismName)) { ac.Auth.DeploymentAuthMechanisms = append(ac.Auth.DeploymentAuthMechanisms, string(s.MechanismName)) @@ -68,7 +62,7 @@ func (s AutomationConfigScramSha) EnableDeploymentAuthentication(conn om.Connect }, log) } -func (s AutomationConfigScramSha) IsAgentAuthenticationConfigured(ac *om.AutomationConfig, _ Options) bool { +func (s *automationConfigScramSha) IsAgentAuthenticationConfigured(ac *om.AutomationConfig, _ Options) bool { if ac.Auth.Disabled { return false } @@ -88,7 +82,7 @@ func (s AutomationConfigScramSha) IsAgentAuthenticationConfigured(ac *om.Automat return true } -func (s AutomationConfigScramSha) IsDeploymentAuthenticationConfigured(ac *om.AutomationConfig, _ Options) bool { +func (s *automationConfigScramSha) IsDeploymentAuthenticationConfigured(ac *om.AutomationConfig, _ Options) bool { return stringutil.Contains(ac.Auth.DeploymentAuthMechanisms, string(s.MechanismName)) } diff --git a/controllers/operator/authentication/scramsha_test.go b/controllers/operator/authentication/scramsha_test.go index 81bd81ce6..b4cc63cd1 100644 --- a/controllers/operator/authentication/scramsha_test.go +++ b/controllers/operator/authentication/scramsha_test.go @@ -11,6 +11,12 @@ import ( "github.com/mongodb/mongodb-kubernetes/pkg/util" ) +var ( + MongoDBCRMechanism = getMechanismByName(MongoDBCR) + ScramSha1Mechanism = getMechanismByName(ScramSha1) + ScramSha256Mechanism = getMechanismByName(ScramSha256) +) + func TestAgentsAuthentication(t *testing.T) { type TestConfig struct { mechanism Mechanism diff --git a/controllers/operator/authentication/x509.go b/controllers/operator/authentication/x509.go index f221fa58d..d8ce0124e 100644 --- a/controllers/operator/authentication/x509.go +++ b/controllers/operator/authentication/x509.go @@ -10,15 +10,13 @@ import ( "github.com/mongodb/mongodb-kubernetes/pkg/util/stringutil" ) -var MongoDBX509Mechanism Mechanism = ConnectionX509{} +type connectionX509 struct{} -type ConnectionX509 struct{} - -func (x ConnectionX509) GetName() MechanismName { +func (x *connectionX509) GetName() MechanismName { return MongoDBX509 } -func (x ConnectionX509) EnableAgentAuthentication(conn om.Connection, opts Options, log *zap.SugaredLogger) error { +func (x *connectionX509) EnableAgentAuthentication(conn om.Connection, opts Options, log *zap.SugaredLogger) error { log.Info("Configuring x509 authentication") err := conn.ReadUpdateAutomationConfig(func(ac *om.AutomationConfig) error { if err := ac.EnsureKeyFileContents(); err != nil { @@ -64,7 +62,7 @@ func (x ConnectionX509) EnableAgentAuthentication(conn om.Connection, opts Optio }, log) } -func (x ConnectionX509) DisableAgentAuthentication(conn om.Connection, log *zap.SugaredLogger) error { +func (x *connectionX509) DisableAgentAuthentication(conn om.Connection, log *zap.SugaredLogger) error { err := conn.ReadUpdateAutomationConfig(func(ac *om.AutomationConfig) error { ac.AgentSSL = &om.AgentSSL{ AutoPEMKeyFilePath: util.MergoDelete, @@ -93,7 +91,7 @@ func (x ConnectionX509) DisableAgentAuthentication(conn om.Connection, log *zap. }, log) } -func (x ConnectionX509) EnableDeploymentAuthentication(conn om.Connection, opts Options, log *zap.SugaredLogger) error { +func (x *connectionX509) EnableDeploymentAuthentication(conn om.Connection, opts Options, log *zap.SugaredLogger) error { return conn.ReadUpdateAutomationConfig(func(ac *om.AutomationConfig) error { if !stringutil.Contains(ac.Auth.DeploymentAuthMechanisms, util.AutomationConfigX509Option) { ac.Auth.DeploymentAuthMechanisms = append(ac.Auth.DeploymentAuthMechanisms, string(MongoDBX509)) @@ -105,14 +103,14 @@ func (x ConnectionX509) EnableDeploymentAuthentication(conn om.Connection, opts }, log) } -func (x ConnectionX509) DisableDeploymentAuthentication(conn om.Connection, log *zap.SugaredLogger) error { +func (x *connectionX509) DisableDeploymentAuthentication(conn om.Connection, log *zap.SugaredLogger) error { return conn.ReadUpdateAutomationConfig(func(ac *om.AutomationConfig) error { ac.Auth.DeploymentAuthMechanisms = stringutil.Remove(ac.Auth.DeploymentAuthMechanisms, string(MongoDBX509)) return nil }, log) } -func (x ConnectionX509) IsAgentAuthenticationConfigured(ac *om.AutomationConfig, _ Options) bool { +func (x *connectionX509) IsAgentAuthenticationConfigured(ac *om.AutomationConfig, _ Options) bool { if ac.Auth.Disabled { return false } @@ -132,7 +130,7 @@ func (x ConnectionX509) IsAgentAuthenticationConfigured(ac *om.AutomationConfig, return true } -func (x ConnectionX509) IsDeploymentAuthenticationConfigured(ac *om.AutomationConfig, _ Options) bool { +func (x *connectionX509) IsDeploymentAuthenticationConfigured(ac *om.AutomationConfig, _ Options) bool { return stringutil.Contains(ac.Auth.DeploymentAuthMechanisms, string(MongoDBX509)) } diff --git a/controllers/operator/authentication/x509_test.go b/controllers/operator/authentication/x509_test.go index 098204459..05016aee3 100644 --- a/controllers/operator/authentication/x509_test.go +++ b/controllers/operator/authentication/x509_test.go @@ -12,6 +12,8 @@ import ( "github.com/mongodb/mongodb-kubernetes/pkg/util" ) +var MongoDBX509Mechanism = getMechanismByName(MongoDBX509) + func TestX509EnableAgentAuthentication(t *testing.T) { conn := om.NewMockedOmConnection(om.NewDeployment()) From e53397691a1ba391f4af835763b1aaac076ebc6a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Maciej=20Kara=C5=9B?= Date: Mon, 5 May 2025 10:26:11 +0200 Subject: [PATCH 20/36] Review fixes --- api/v1/mdb/mongodb_validation.go | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/api/v1/mdb/mongodb_validation.go b/api/v1/mdb/mongodb_validation.go index 171974b79..5f9da2bab 100644 --- a/api/v1/mdb/mongodb_validation.go +++ b/api/v1/mdb/mongodb_validation.go @@ -2,7 +2,6 @@ package mdb import ( "errors" - "fmt" "strings" "k8s.io/apimachinery/pkg/runtime" @@ -178,9 +177,9 @@ func oidcProviderConfigsSingleWorkforceIdentityFederationValidation(configs []OI } if len(workforceIdentityFederationConfigs) > 1 { - msg := fmt.Sprintf("Only one OIDC provider config can be configured with Workforce Identity Federation. "+ - "The following configs are configured with Workforce Identity Federation: %s", strings.Join(workforceIdentityFederationConfigs, ", ")) - return v1.ValidationError("%s", msg) + configsSeparatedString := strings.Join(workforceIdentityFederationConfigs, ", ") + return v1.ValidationError("Only one OIDC provider config can be configured with Workforce Identity Federation. "+ + "The following configs are configured with Workforce Identity Federation: %s", configsSeparatedString) } return v1.ValidationSuccess() From a8306a7ea7844004f6bea037650b087b2d67fcff Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Maciej=20Kara=C5=9B?= Date: Mon, 5 May 2025 10:49:58 +0200 Subject: [PATCH 21/36] Add one more validation test --- api/v1/mdb/mongodb_validation_test.go | 26 ++++++++++++++++++++++++++ 1 file changed, 26 insertions(+) diff --git a/api/v1/mdb/mongodb_validation_test.go b/api/v1/mdb/mongodb_validation_test.go index 4529ef2c4..6f7fcc0ec 100644 --- a/api/v1/mdb/mongodb_validation_test.go +++ b/api/v1/mdb/mongodb_validation_test.go @@ -293,6 +293,32 @@ func TestOIDCAuthValidation(t *testing.T) { }, expectedErrorMessage: "Only one OIDC provider config can be configured with Workforce Identity Federation. The following configs are configured with Workforce Identity Federation: test-provider1, test-provider2", }, + { + name: "Multiple Workload Identity Federation configs", + auth: &Authentication{ + Enabled: true, + Agents: AgentAuthentication{Mode: util.SCRAMSHA256}, + Modes: []AuthMode{util.OIDC, util.SCRAMSHA256}, + OIDCProviderConfigs: []OIDCProviderConfig{ + { + ConfigurationName: "test-provider-workforce1", + IssuerURI: "https://example1.com", + AuthorizationMethod: OIDCAuthorizationMethodWorkforceIdentityFederation, + ClientId: "clientId1", + }, + { + ConfigurationName: "test-provider-workload2", + IssuerURI: "https://example2.com", + AuthorizationMethod: OIDCAuthorizationMethodWorkloadIdentityFederation, + }, + { + ConfigurationName: "test-provider-workload3", + IssuerURI: "https://example3.com", + AuthorizationMethod: OIDCAuthorizationMethodWorkloadIdentityFederation, + }, + }, + }, + }, { name: "Invalid issuer URI", auth: &Authentication{ From bcc1136fb759c599610180920f858fc56e73b8aa Mon Sep 17 00:00:00 2001 From: Lucian Tosa Date: Fri, 9 May 2025 13:36:06 +0200 Subject: [PATCH 22/36] Fix bug --- controllers/operator/authentication/authentication_mechanism.go | 2 ++ 1 file changed, 2 insertions(+) diff --git a/controllers/operator/authentication/authentication_mechanism.go b/controllers/operator/authentication/authentication_mechanism.go index 3a575473a..54aadbb70 100644 --- a/controllers/operator/authentication/authentication_mechanism.go +++ b/controllers/operator/authentication/authentication_mechanism.go @@ -132,6 +132,8 @@ func getMechanismByName(name MechanismName) Mechanism { return &connectionX509{} case LDAPPlain: return &ldapAuthMechanism{} + case MongoDBOIDC: + return &oidcAuthMechanism{} } panic(xerrors.Errorf("unknown mechanism name %s", name)) From 68750a4743c05fb16878d226ac69003a3777447a Mon Sep 17 00:00:00 2001 From: Lucian Tosa Date: Fri, 9 May 2025 14:07:46 +0200 Subject: [PATCH 23/36] Fix linter --- LICENSE-THIRD-PARTY | 2 +- go.mod | 2 ++ 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/LICENSE-THIRD-PARTY b/LICENSE-THIRD-PARTY index 132c0bbb8..04b948330 100644 --- a/LICENSE-THIRD-PARTY +++ b/LICENSE-THIRD-PARTY @@ -5,7 +5,7 @@ github.com/cenkalti/backoff/v4,v4.3.0,https://github.com/cenkalti/backoff/blob/v github.com/cespare/xxhash/v2,v2.3.0,https://github.com/cespare/xxhash/blob/v2.3.0/LICENSE.txt,MIT github.com/davecgh/go-spew/spew,v1.1.1,https://github.com/davecgh/go-spew/blob/v1.1.1/LICENSE,ISC github.com/emicklei/go-restful/v3,v3.11.0,https://github.com/emicklei/go-restful/blob/v3.11.0/LICENSE,MIT -github.com/evanphx/json-patch/v5,v5.9.0,https://github.com/evanphx/json-patch/blob/v5.9.0/v5/LICENSE,BSD-3-Clause +github.com/evanphx/json-patch/v5,v5.9.0,https://github.com/evanphx/json-patch/blob/v5.9.0/LICENSE,BSD-3-Clause github.com/fsnotify/fsnotify,v1.7.0,https://github.com/fsnotify/fsnotify/blob/v1.7.0/LICENSE,BSD-3-Clause github.com/ghodss/yaml,v1.0.0,https://github.com/ghodss/yaml/blob/v1.0.0/LICENSE,MIT github.com/go-jose/go-jose/v4,v4.0.5,https://github.com/go-jose/go-jose/blob/v4.0.5/LICENSE,Apache-2.0 diff --git a/go.mod b/go.mod index 46cf621ca..6b8ccd100 100644 --- a/go.mod +++ b/go.mod @@ -13,6 +13,8 @@ require ( github.com/hashicorp/go-retryablehttp v0.7.7 github.com/hashicorp/vault/api v1.16.0 github.com/imdario/mergo v0.3.15 + github.com/onsi/ginkgo/v2 v2.17.1 + github.com/onsi/gomega v1.32.0 github.com/pkg/errors v0.9.1 github.com/prometheus/client_golang v1.21.0 github.com/r3labs/diff/v3 v3.0.1 From f7ec0f1bad408aec8c0fb3230a6d9a943df7376f Mon Sep 17 00:00:00 2001 From: Anand Singh Date: Thu, 22 May 2025 14:19:29 +0200 Subject: [PATCH 24/36] implement the authentication_mechanism interface correctly for oidc --- controllers/operator/authentication/oidc.go | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/controllers/operator/authentication/oidc.go b/controllers/operator/authentication/oidc.go index ef3d8d721..bde78d4b5 100644 --- a/controllers/operator/authentication/oidc.go +++ b/controllers/operator/authentication/oidc.go @@ -56,7 +56,11 @@ func (o *oidcAuthMechanism) IsAgentAuthenticationConfigured(*om.AutomationConfig } func (o *oidcAuthMechanism) IsDeploymentAuthenticationConfigured(ac *om.AutomationConfig, opts Options) bool { - return stringutil.Contains(ac.Auth.DeploymentAuthMechanisms, string(MongoDBOIDC)) && oidcProviderConfigsEqual(ac.OIDCProviderConfigs, opts.OIDCProviderConfigs) + return o.IsDeploymentAuthenticationEnabled(ac) && oidcProviderConfigsEqual(ac.OIDCProviderConfigs, opts.OIDCProviderConfigs) +} + +func (o *oidcAuthMechanism) IsDeploymentAuthenticationEnabled(ac *om.AutomationConfig) bool { + return stringutil.Contains(ac.Auth.DeploymentAuthMechanisms, string(MongoDBOIDC)) } func oidcProviderConfigsEqual(lhs []oidc.ProviderConfig, rhs []oidc.ProviderConfig) bool { From c1f54a582682c3d18056d791de2729840744fbc8 Mon Sep 17 00:00:00 2001 From: Anand Singh Date: Thu, 22 May 2025 14:40:29 +0200 Subject: [PATCH 25/36] lint-fix --- controllers/operator/authentication/authentication.go | 2 +- controllers/operator/authentication/authentication_mechanism.go | 1 - 2 files changed, 1 insertion(+), 2 deletions(-) diff --git a/controllers/operator/authentication/authentication.go b/controllers/operator/authentication/authentication.go index 8e543fff6..1744317cd 100644 --- a/controllers/operator/authentication/authentication.go +++ b/controllers/operator/authentication/authentication.go @@ -322,7 +322,7 @@ func removeUnsupportedDeploymentMechanisms(conn om.Connection, opts Options, log automationConfigAuthMechanisms := convertToMechanismList(opts.Mechanisms, ac) unsupportedMechanisms := mechanismsToDisable(automationConfigAuthMechanisms) - + log.Infow("Removing unsupported deployment authentication mechanisms", "Mechanisms", unsupportedMechanisms) if err := ensureDeploymentMechanismsAreDisabled(conn, ac, unsupportedMechanisms, log); err != nil { return xerrors.Errorf("error ensuring deployment mechanisms are disabled: %w", err) diff --git a/controllers/operator/authentication/authentication_mechanism.go b/controllers/operator/authentication/authentication_mechanism.go index 324fc60de..90e6877e8 100644 --- a/controllers/operator/authentication/authentication_mechanism.go +++ b/controllers/operator/authentication/authentication_mechanism.go @@ -68,7 +68,6 @@ func (m MechanismList) Contains(mechanismName MechanismName) bool { // that can be configured by the Operator var supportedMechanisms = []MechanismName{ScramSha256, MongoDBCR, MongoDBX509, LDAPPlain, MongoDBOIDC} - // mechanismsToDisable returns mechanisms which need to be disabled // based on the currently supported authentication mechanisms and the desiredMechanisms func mechanismsToDisable(desiredMechanisms MechanismList) MechanismList { From e6ed367c67ad138e01a48778d360ccac6843d01f Mon Sep 17 00:00:00 2001 From: Anand Singh Date: Fri, 23 May 2025 00:55:27 +0200 Subject: [PATCH 26/36] update external auth validation --- controllers/operator/mongodbuser_controller.go | 6 +++++- pkg/util/constants.go | 1 + 2 files changed, 6 insertions(+), 1 deletion(-) diff --git a/controllers/operator/mongodbuser_controller.go b/controllers/operator/mongodbuser_controller.go index 54e9de068..ac2f5cb2f 100644 --- a/controllers/operator/mongodbuser_controller.go +++ b/controllers/operator/mongodbuser_controller.go @@ -481,7 +481,11 @@ func waitForReadyState(conn om.Connection, log *zap.SugaredLogger) error { } func externalAuthMechanismsAvailable(mechanisms []string) bool { - return stringutil.ContainsAny(mechanisms, util.AutomationConfigLDAPOption, util.AutomationConfigX509Option) + return stringutil.ContainsAny(mechanisms, + util.AutomationConfigLDAPOption, + util.AutomationConfigX509Option, + util.AutomationConfigOIDCOption, + ) } func getAnnotationsForUserResource(user *userv1.MongoDBUser) (map[string]string, error) { diff --git a/pkg/util/constants.go b/pkg/util/constants.go index d44b86223..b2082f6ee 100644 --- a/pkg/util/constants.go +++ b/pkg/util/constants.go @@ -140,6 +140,7 @@ const ( AutomationConfigLDAPOption = "PLAIN" AutomationConfigScramSha256Option = "SCRAM-SHA-256" AutomationConfigScramSha1Option = "MONGODB-CR" + AutomationConfigOIDCOption = "MONGODB-OIDC" AutomationAgentUserName = "mms-automation-agent" RequireClientCertificates = "REQUIRE" OptionalClientCertficates = "OPTIONAL" From 0d8fd6e850661c25ad7fc1c059a377a798cbb7d1 Mon Sep 17 00:00:00 2001 From: Lucian Tosa Date: Fri, 23 May 2025 15:19:04 +0200 Subject: [PATCH 27/36] Webhook validation tests --- .../e2e_mongodb_validation_webhook.py | 65 +++++++++++++++++++ .../invalid_oidc_duplicate_config_name.yaml | 41 ++++++++++++ .../invalid_oidc_invalid_clientid.yaml | 31 +++++++++ .../fixtures/invalid_oidc_invalid_uri.yaml | 32 +++++++++ .../invalid_oidc_missing_groupclaim.yaml | 31 +++++++++ .../invalid_oidc_mongodb_community.yaml | 32 +++++++++ .../invalid_oidc_multiple_workforce.yaml | 41 ++++++++++++ .../fixtures/invalid_oidc_no_providers.yaml | 22 +++++++ .../fixtures/invalid_oidc_single_auth.yaml | 29 +++++++++ 9 files changed, 324 insertions(+) create mode 100644 docker/mongodb-kubernetes-tests/tests/webhooks/fixtures/invalid_oidc_duplicate_config_name.yaml create mode 100644 docker/mongodb-kubernetes-tests/tests/webhooks/fixtures/invalid_oidc_invalid_clientid.yaml create mode 100644 docker/mongodb-kubernetes-tests/tests/webhooks/fixtures/invalid_oidc_invalid_uri.yaml create mode 100644 docker/mongodb-kubernetes-tests/tests/webhooks/fixtures/invalid_oidc_missing_groupclaim.yaml create mode 100644 docker/mongodb-kubernetes-tests/tests/webhooks/fixtures/invalid_oidc_mongodb_community.yaml create mode 100644 docker/mongodb-kubernetes-tests/tests/webhooks/fixtures/invalid_oidc_multiple_workforce.yaml create mode 100644 docker/mongodb-kubernetes-tests/tests/webhooks/fixtures/invalid_oidc_no_providers.yaml create mode 100644 docker/mongodb-kubernetes-tests/tests/webhooks/fixtures/invalid_oidc_single_auth.yaml diff --git a/docker/mongodb-kubernetes-tests/tests/webhooks/e2e_mongodb_validation_webhook.py b/docker/mongodb-kubernetes-tests/tests/webhooks/e2e_mongodb_validation_webhook.py index 716ebe854..2420f4fe3 100644 --- a/docker/mongodb-kubernetes-tests/tests/webhooks/e2e_mongodb_validation_webhook.py +++ b/docker/mongodb-kubernetes-tests/tests/webhooks/e2e_mongodb_validation_webhook.py @@ -79,6 +79,71 @@ def test_replicaset_members_is_specified(self): exception_reason="'spec.members' must be specified if type of MongoDB is ReplicaSet", ) + def test_oidc_auth_with_mongodb_community(self): + resource = yaml.safe_load(open(yaml_fixture("invalid_oidc_mongodb_community.yaml"))) + self.create_custom_resource_from_object( + self.get_namespace(), + resource, + exception_reason="Cannot enable OIDC authentication with MongoDB Community Builds", + ) + + def test_oidc_auth_with_single_method(self): + resource = yaml.safe_load(open(yaml_fixture("invalid_oidc_single_auth.yaml"))) + self.create_custom_resource_from_object( + self.get_namespace(), + resource, + exception_reason="OIDC authentication cannot be used as the only authentication mechanism", + ) + + def test_oidc_auth_with_duplicate_config_name(self): + resource = yaml.safe_load(open(yaml_fixture("invalid_oidc_duplicate_config_name.yaml"))) + self.create_custom_resource_from_object( + self.get_namespace(), + resource, + exception_reason="OIDC provider config name OIDC-test is not unique", + ) + + def test_oidc_auth_with_multiple_workforce(self): + resource = yaml.safe_load(open(yaml_fixture("invalid_oidc_multiple_workforce.yaml"))) + self.create_custom_resource_from_object( + self.get_namespace(), + resource, + exception_reason="Only one OIDC provider config can be configured with Workforce Identity Federation. " + + "The following configs are configured with Workforce Identity Federation: OIDC-test, OIDC-test-2", + ) + + def test_oidc_auth_with_invalid_uri(self): + resource = yaml.safe_load(open(yaml_fixture("invalid_oidc_invalid_uri.yaml"))) + self.create_custom_resource_from_object( + self.get_namespace(), + resource, + exception_reason='Invalid IssuerURI in OIDC provider config \\"OIDC-test\\": invalid URL scheme (http or https): ', + ) + + def test_oidc_auth_with_invalid_clientid(self): + resource = yaml.safe_load(open(yaml_fixture("invalid_oidc_invalid_clientid.yaml"))) + self.create_custom_resource_from_object( + self.get_namespace(), + resource, + exception_reason='ClientId has to be specified in OIDC provider config \\"OIDC-test\\" with Workforce Identity Federation', + ) + + def test_oidc_auth_with_missing_groupclaim(self): + resource = yaml.safe_load(open(yaml_fixture("invalid_oidc_missing_groupclaim.yaml"))) + self.create_custom_resource_from_object( + self.get_namespace(), + resource, + exception_reason='GroupsClaim has to be specified in OIDC provider config \\"OIDC-test\\" when using Group Membership authorization', + ) + + def test_oidc_auth_with_no_providers(self): + resource = yaml.safe_load(open(yaml_fixture("invalid_oidc_no_providers.yaml"))) + self.create_custom_resource_from_object( + self.get_namespace(), + resource, + exception_reason="At least one OIDC provider config needs to be specified when OIDC authentication is enabled", + ) + def test_replicaset_members_is_specified_without_webhook(self): self._assert_validates_without_webhook( "mdbpolicy.mongodb.com", diff --git a/docker/mongodb-kubernetes-tests/tests/webhooks/fixtures/invalid_oidc_duplicate_config_name.yaml b/docker/mongodb-kubernetes-tests/tests/webhooks/fixtures/invalid_oidc_duplicate_config_name.yaml new file mode 100644 index 000000000..1e4710c23 --- /dev/null +++ b/docker/mongodb-kubernetes-tests/tests/webhooks/fixtures/invalid_oidc_duplicate_config_name.yaml @@ -0,0 +1,41 @@ +apiVersion: mongodb.com/v1 +kind: MongoDB +metadata: + name: oidc-replica-set +spec: + type: ReplicaSet + members: 3 + version: 8.0.5-ent + + opsManager: + configMapRef: + name: my-project + credentials: my-credentials + + security: + authentication: + agents: + mode: SCRAM + enabled: true + modes: + - SCRAM + - OIDC + oidcProviderConfigs: + - audience: "example-audience" + clientId: "example-client-id" + issuerURI: "https://example-issuer.com" + requestedScopes: [ ] + userClaim: "sub" + groupsClaim: "cognito:groups" + authorizationMethod: "WorkloadIdentityFederation" + authorizationType: "GroupMembership" + configurationName: "OIDC-test" + - audience: "example-audience-2" + clientId: "example-client-id-2" + issuerURI: "https://example-issuer-2.com" + requestedScopes: [ ] + userClaim: "sub" + groupsClaim: "cognito:groups" + authorizationMethod: "WorkloadIdentityFederation" + authorizationType: "GroupMembership" + configurationName: "OIDC-test" diff --git a/docker/mongodb-kubernetes-tests/tests/webhooks/fixtures/invalid_oidc_invalid_clientid.yaml b/docker/mongodb-kubernetes-tests/tests/webhooks/fixtures/invalid_oidc_invalid_clientid.yaml new file mode 100644 index 000000000..70a9d0599 --- /dev/null +++ b/docker/mongodb-kubernetes-tests/tests/webhooks/fixtures/invalid_oidc_invalid_clientid.yaml @@ -0,0 +1,31 @@ +apiVersion: mongodb.com/v1 +kind: MongoDB +metadata: + name: oidc-replica-set +spec: + type: ReplicaSet + members: 3 + version: 8.0.5-ent + + opsManager: + configMapRef: + name: my-project + credentials: my-credentials + + security: + authentication: + agents: + mode: SCRAM + enabled: true + modes: + - SCRAM + - OIDC + oidcProviderConfigs: + - audience: "example-audience" + issuerURI: "https://example-issuer.com" + requestedScopes: [ ] + userClaim: "sub" + groupsClaim: "cognito:groups" + authorizationMethod: "WorkforceIdentityFederation" + authorizationType: "GroupMembership" + configurationName: "OIDC-test" diff --git a/docker/mongodb-kubernetes-tests/tests/webhooks/fixtures/invalid_oidc_invalid_uri.yaml b/docker/mongodb-kubernetes-tests/tests/webhooks/fixtures/invalid_oidc_invalid_uri.yaml new file mode 100644 index 000000000..e5509790e --- /dev/null +++ b/docker/mongodb-kubernetes-tests/tests/webhooks/fixtures/invalid_oidc_invalid_uri.yaml @@ -0,0 +1,32 @@ +apiVersion: mongodb.com/v1 +kind: MongoDB +metadata: + name: oidc-replica-set +spec: + type: ReplicaSet + members: 3 + version: 8.0.5-ent + + opsManager: + configMapRef: + name: my-project + credentials: my-credentials + + security: + authentication: + agents: + mode: SCRAM + enabled: true + modes: + - SCRAM + - OIDC + oidcProviderConfigs: + - audience: "example-audience" + clientId: "example-client-id" + issuerURI: "hps://example-issuer.com" + requestedScopes: [ ] + userClaim: "sub" + groupsClaim: "cognito:groups" + authorizationMethod: "WorkloadIdentityFederation" + authorizationType: "GroupMembership" + configurationName: "OIDC-test" diff --git a/docker/mongodb-kubernetes-tests/tests/webhooks/fixtures/invalid_oidc_missing_groupclaim.yaml b/docker/mongodb-kubernetes-tests/tests/webhooks/fixtures/invalid_oidc_missing_groupclaim.yaml new file mode 100644 index 000000000..3b883b0bc --- /dev/null +++ b/docker/mongodb-kubernetes-tests/tests/webhooks/fixtures/invalid_oidc_missing_groupclaim.yaml @@ -0,0 +1,31 @@ +apiVersion: mongodb.com/v1 +kind: MongoDB +metadata: + name: oidc-replica-set +spec: + type: ReplicaSet + members: 3 + version: 8.0.5-ent + + opsManager: + configMapRef: + name: my-project + credentials: my-credentials + + security: + authentication: + agents: + mode: SCRAM + enabled: true + modes: + - SCRAM + - OIDC + oidcProviderConfigs: + - audience: "example-audience" + clientId: "example-client-id" + issuerURI: "https://example-issuer.com" + requestedScopes: [ ] + userClaim: "sub" + authorizationMethod: "WorkloadIdentityFederation" + authorizationType: "GroupMembership" + configurationName: "OIDC-test" diff --git a/docker/mongodb-kubernetes-tests/tests/webhooks/fixtures/invalid_oidc_mongodb_community.yaml b/docker/mongodb-kubernetes-tests/tests/webhooks/fixtures/invalid_oidc_mongodb_community.yaml new file mode 100644 index 000000000..340c810da --- /dev/null +++ b/docker/mongodb-kubernetes-tests/tests/webhooks/fixtures/invalid_oidc_mongodb_community.yaml @@ -0,0 +1,32 @@ +apiVersion: mongodb.com/v1 +kind: MongoDB +metadata: + name: oidc-replica-set +spec: + type: ReplicaSet + members: 3 + version: 8.0.5 + + opsManager: + configMapRef: + name: my-project + credentials: my-credentials + + security: + authentication: + agents: + mode: SCRAM + enabled: true + modes: + - SCRAM + - OIDC + oidcProviderConfigs: + - audience: "example-audience" + clientId: "example-client-id" + issuerURI: "https://example-issuer.com" + requestedScopes: [ ] + userClaim: "sub" + groupsClaim: "cognito:groups" + authorizationMethod: "WorkloadIdentityFederation" + authorizationType: "GroupMembership" + configurationName: "OIDC-test" diff --git a/docker/mongodb-kubernetes-tests/tests/webhooks/fixtures/invalid_oidc_multiple_workforce.yaml b/docker/mongodb-kubernetes-tests/tests/webhooks/fixtures/invalid_oidc_multiple_workforce.yaml new file mode 100644 index 000000000..b0a81bd69 --- /dev/null +++ b/docker/mongodb-kubernetes-tests/tests/webhooks/fixtures/invalid_oidc_multiple_workforce.yaml @@ -0,0 +1,41 @@ +apiVersion: mongodb.com/v1 +kind: MongoDB +metadata: + name: oidc-replica-set +spec: + type: ReplicaSet + members: 3 + version: 8.0.5-ent + + opsManager: + configMapRef: + name: my-project + credentials: my-credentials + + security: + authentication: + agents: + mode: SCRAM + enabled: true + modes: + - SCRAM + - OIDC + oidcProviderConfigs: + - audience: "example-audience" + clientId: "example-client-id" + issuerURI: "https://example-issuer.com" + requestedScopes: [ ] + userClaim: "sub" + groupsClaim: "cognito:groups" + authorizationMethod: "WorkforceIdentityFederation" + authorizationType: "GroupMembership" + configurationName: "OIDC-test" + - audience: "example-audience-2" + clientId: "example-client-id-2" + issuerURI: "https://example-issuer-2.com" + requestedScopes: [ ] + userClaim: "sub" + groupsClaim: "cognito:groups" + authorizationMethod: "WorkforceIdentityFederation" + authorizationType: "GroupMembership" + configurationName: "OIDC-test-2" diff --git a/docker/mongodb-kubernetes-tests/tests/webhooks/fixtures/invalid_oidc_no_providers.yaml b/docker/mongodb-kubernetes-tests/tests/webhooks/fixtures/invalid_oidc_no_providers.yaml new file mode 100644 index 000000000..5824c170b --- /dev/null +++ b/docker/mongodb-kubernetes-tests/tests/webhooks/fixtures/invalid_oidc_no_providers.yaml @@ -0,0 +1,22 @@ +apiVersion: mongodb.com/v1 +kind: MongoDB +metadata: + name: oidc-replica-set +spec: + type: ReplicaSet + members: 3 + version: 8.0.5-ent + + opsManager: + configMapRef: + name: my-project + credentials: my-credentials + + security: + authentication: + agents: + mode: SCRAM + enabled: true + modes: + - SCRAM + - OIDC diff --git a/docker/mongodb-kubernetes-tests/tests/webhooks/fixtures/invalid_oidc_single_auth.yaml b/docker/mongodb-kubernetes-tests/tests/webhooks/fixtures/invalid_oidc_single_auth.yaml new file mode 100644 index 000000000..025144bd4 --- /dev/null +++ b/docker/mongodb-kubernetes-tests/tests/webhooks/fixtures/invalid_oidc_single_auth.yaml @@ -0,0 +1,29 @@ +apiVersion: mongodb.com/v1 +kind: MongoDB +metadata: + name: oidc-replica-set +spec: + type: ReplicaSet + members: 3 + version: 8.0.5-ent + + opsManager: + configMapRef: + name: my-project + credentials: my-credentials + + security: + authentication: + enabled: true + modes: + - OIDC + oidcProviderConfigs: + - audience: "example-audience" + clientId: "example-client-id" + issuerURI: "https://example-issuer.com" + requestedScopes: [ ] + userClaim: "sub" + groupsClaim: "cognito:groups" + authorizationMethod: "WorkloadIdentityFederation" + authorizationType: "GroupMembership" + configurationName: "OIDC-test" From 26e24a0cb1169cc22483469b591be45c58a0036c Mon Sep 17 00:00:00 2001 From: Anand Singh Date: Mon, 26 May 2025 16:53:42 +0200 Subject: [PATCH 28/36] remove default value for GroupsClaim --- api/v1/mdb/mongodb_types.go | 1 - 1 file changed, 1 deletion(-) diff --git a/api/v1/mdb/mongodb_types.go b/api/v1/mdb/mongodb_types.go index 705111f7c..068ef3306 100644 --- a/api/v1/mdb/mongodb_types.go +++ b/api/v1/mdb/mongodb_types.go @@ -1090,7 +1090,6 @@ type OIDCProviderConfig struct { // The identifier of the claim that includes the principal's IdP user group membership information. // Accept the default value unless your IdP uses a different claim, or you need a custom claim. // Required when selected GroupMembership as the authorization type, ignored otherwise - // +kubebuilder:default=groups // +kubebuilder:validation:Optional GroupsClaim string `json:"groupsClaim,omitempty"` From 7257b4ee379694ec5e2116cc6749fb13344e09c5 Mon Sep 17 00:00:00 2001 From: Anand Singh Date: Mon, 26 May 2025 17:22:44 +0200 Subject: [PATCH 29/36] remove default value for GroupsClaim --- config/crd/bases/mongodb.com_mongodb.yaml | 1 - config/crd/bases/mongodb.com_mongodbmulticluster.yaml | 1 - config/crd/bases/mongodb.com_opsmanagers.yaml | 1 - helm_chart/crds/mongodb.com_mongodb.yaml | 1 - helm_chart/crds/mongodb.com_mongodbmulticluster.yaml | 1 - helm_chart/crds/mongodb.com_opsmanagers.yaml | 1 - public/crds.yaml | 3 --- 7 files changed, 9 deletions(-) diff --git a/config/crd/bases/mongodb.com_mongodb.yaml b/config/crd/bases/mongodb.com_mongodb.yaml index 8b5ffc33e..bdf215522 100644 --- a/config/crd/bases/mongodb.com_mongodb.yaml +++ b/config/crd/bases/mongodb.com_mongodb.yaml @@ -1566,7 +1566,6 @@ spec: pattern: ^[a-zA-Z0-9-_]+$ type: string groupsClaim: - default: groups description: |- The identifier of the claim that includes the principal's IdP user group membership information. Accept the default value unless your IdP uses a different claim, or you need a custom claim. diff --git a/config/crd/bases/mongodb.com_mongodbmulticluster.yaml b/config/crd/bases/mongodb.com_mongodbmulticluster.yaml index 523238eb6..0eabff9fd 100644 --- a/config/crd/bases/mongodb.com_mongodbmulticluster.yaml +++ b/config/crd/bases/mongodb.com_mongodbmulticluster.yaml @@ -826,7 +826,6 @@ spec: pattern: ^[a-zA-Z0-9-_]+$ type: string groupsClaim: - default: groups description: |- The identifier of the claim that includes the principal's IdP user group membership information. Accept the default value unless your IdP uses a different claim, or you need a custom claim. diff --git a/config/crd/bases/mongodb.com_opsmanagers.yaml b/config/crd/bases/mongodb.com_opsmanagers.yaml index 2c33c0e49..42d1597d8 100644 --- a/config/crd/bases/mongodb.com_opsmanagers.yaml +++ b/config/crd/bases/mongodb.com_opsmanagers.yaml @@ -888,7 +888,6 @@ spec: pattern: ^[a-zA-Z0-9-_]+$ type: string groupsClaim: - default: groups description: |- The identifier of the claim that includes the principal's IdP user group membership information. Accept the default value unless your IdP uses a different claim, or you need a custom claim. diff --git a/helm_chart/crds/mongodb.com_mongodb.yaml b/helm_chart/crds/mongodb.com_mongodb.yaml index 8b5ffc33e..bdf215522 100644 --- a/helm_chart/crds/mongodb.com_mongodb.yaml +++ b/helm_chart/crds/mongodb.com_mongodb.yaml @@ -1566,7 +1566,6 @@ spec: pattern: ^[a-zA-Z0-9-_]+$ type: string groupsClaim: - default: groups description: |- The identifier of the claim that includes the principal's IdP user group membership information. Accept the default value unless your IdP uses a different claim, or you need a custom claim. diff --git a/helm_chart/crds/mongodb.com_mongodbmulticluster.yaml b/helm_chart/crds/mongodb.com_mongodbmulticluster.yaml index 523238eb6..0eabff9fd 100644 --- a/helm_chart/crds/mongodb.com_mongodbmulticluster.yaml +++ b/helm_chart/crds/mongodb.com_mongodbmulticluster.yaml @@ -826,7 +826,6 @@ spec: pattern: ^[a-zA-Z0-9-_]+$ type: string groupsClaim: - default: groups description: |- The identifier of the claim that includes the principal's IdP user group membership information. Accept the default value unless your IdP uses a different claim, or you need a custom claim. diff --git a/helm_chart/crds/mongodb.com_opsmanagers.yaml b/helm_chart/crds/mongodb.com_opsmanagers.yaml index 2c33c0e49..42d1597d8 100644 --- a/helm_chart/crds/mongodb.com_opsmanagers.yaml +++ b/helm_chart/crds/mongodb.com_opsmanagers.yaml @@ -888,7 +888,6 @@ spec: pattern: ^[a-zA-Z0-9-_]+$ type: string groupsClaim: - default: groups description: |- The identifier of the claim that includes the principal's IdP user group membership information. Accept the default value unless your IdP uses a different claim, or you need a custom claim. diff --git a/public/crds.yaml b/public/crds.yaml index 00adc4cbd..92e746730 100644 --- a/public/crds.yaml +++ b/public/crds.yaml @@ -1566,7 +1566,6 @@ spec: pattern: ^[a-zA-Z0-9-_]+$ type: string groupsClaim: - default: groups description: |- The identifier of the claim that includes the principal's IdP user group membership information. Accept the default value unless your IdP uses a different claim, or you need a custom claim. @@ -4202,7 +4201,6 @@ spec: pattern: ^[a-zA-Z0-9-_]+$ type: string groupsClaim: - default: groups description: |- The identifier of the claim that includes the principal's IdP user group membership information. Accept the default value unless your IdP uses a different claim, or you need a custom claim. @@ -5849,7 +5847,6 @@ spec: pattern: ^[a-zA-Z0-9-_]+$ type: string groupsClaim: - default: groups description: |- The identifier of the claim that includes the principal's IdP user group membership information. Accept the default value unless your IdP uses a different claim, or you need a custom claim. From aad26e629b71619dad34bab6cd2d45c18732dc24 Mon Sep 17 00:00:00 2001 From: Anand <13899132+anandsyncs@users.noreply.github.com> Date: Wed, 28 May 2025 14:17:40 +0200 Subject: [PATCH 30/36] Update controllers/operator/authentication/oidc.go --- controllers/operator/authentication/oidc.go | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/controllers/operator/authentication/oidc.go b/controllers/operator/authentication/oidc.go index bde78d4b5..69afec6fe 100644 --- a/controllers/operator/authentication/oidc.go +++ b/controllers/operator/authentication/oidc.go @@ -81,7 +81,16 @@ func sortOIDCPProviderConfigs(configs []oidc.ProviderConfig) []oidc.ProviderConf }) } -func oidcProviderConfigEqual(l oidc.ProviderConfig, r oidc.ProviderConfig) bool { +func oidcProviderConfigEqual(l, r oidc.ProviderConfig) bool { + return l.AuthNamePrefix == r.AuthNamePrefix && + l.Audience == r.Audience && + l.IssuerUri == r.IssuerUri && + slices.Equal(l.RequestedScopes, r.RequestedScopes) && + l.UserClaim == r.UserClaim && + l.GroupsClaim == r.GroupsClaim && + l.SupportsHumanFlows == r.SupportsHumanFlows && + l.UseAuthorizationClaim == r.UseAuthorizationClaim +} if l.AuthNamePrefix != r.AuthNamePrefix { return false } From 05903d599aae744801825d266845d08eec13b7c8 Mon Sep 17 00:00:00 2001 From: Anand Singh Date: Wed, 28 May 2025 14:52:25 +0200 Subject: [PATCH 31/36] fix typo --- controllers/operator/authentication/oidc.go | 34 --------------------- 1 file changed, 34 deletions(-) diff --git a/controllers/operator/authentication/oidc.go b/controllers/operator/authentication/oidc.go index 69afec6fe..553036498 100644 --- a/controllers/operator/authentication/oidc.go +++ b/controllers/operator/authentication/oidc.go @@ -91,40 +91,6 @@ func oidcProviderConfigEqual(l, r oidc.ProviderConfig) bool { l.SupportsHumanFlows == r.SupportsHumanFlows && l.UseAuthorizationClaim == r.UseAuthorizationClaim } - if l.AuthNamePrefix != r.AuthNamePrefix { - return false - } - - if l.Audience != r.Audience { - return false - } - - if l.IssuerUri != r.IssuerUri { - return false - } - - if !slices.Equal(l.RequestedScopes, r.RequestedScopes) { - return false - } - - if l.UserClaim != r.UserClaim { - return false - } - - if l.GroupsClaim != r.GroupsClaim { - return false - } - - if l.SupportsHumanFlows != r.SupportsHumanFlows { - return false - } - - if l.UseAuthorizationClaim != r.UseAuthorizationClaim { - return false - } - - return true -} func MapOIDCProviderConfigs(oidcProviderConfigs []mdbv1.OIDCProviderConfig) []oidc.ProviderConfig { if len(oidcProviderConfigs) == 0 { From 13e76a37e8a5fa8137fc6fb1becd7cf20bcddc86 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Maciej=20Kara=C5=9B?= Date: Thu, 29 May 2025 13:30:55 +0200 Subject: [PATCH 32/36] Moved `mongoDBOIDCMechanism` to oidc_test.go as local variable --- .../configure_authentication_test.go | 10 +++++----- .../operator/authentication/ldap_test.go | 10 +++++----- controllers/operator/authentication/oidc.go | 2 -- .../operator/authentication/oidc_test.go | 18 ++++++++++-------- .../operator/authentication/scramsha_test.go | 16 ++++++++-------- .../operator/authentication/x509_test.go | 8 ++++---- 6 files changed, 32 insertions(+), 32 deletions(-) diff --git a/controllers/operator/authentication/configure_authentication_test.go b/controllers/operator/authentication/configure_authentication_test.go index ae21a6611..8e8ae95df 100644 --- a/controllers/operator/authentication/configure_authentication_test.go +++ b/controllers/operator/authentication/configure_authentication_test.go @@ -153,12 +153,12 @@ func TestGetCorrectAuthMechanismFromVersion(t *testing.T) { mechanismList := convertToMechanismList([]string{"X509"}, ac) assert.Len(t, mechanismList, 1) - assert.Contains(t, mechanismList, MongoDBX509Mechanism) + assert.Contains(t, mechanismList, mongoDBX509Mechanism) mechanismList = convertToMechanismList([]string{"SCRAM", "X509"}, ac) - assert.Contains(t, mechanismList, ScramSha256Mechanism) - assert.Contains(t, mechanismList, MongoDBX509Mechanism) + assert.Contains(t, mechanismList, scramSha256Mechanism) + assert.Contains(t, mechanismList, mongoDBX509Mechanism) // enable MONGODB-CR ac.Auth.AutoAuthMechanism = "MONGODB-CR" @@ -166,8 +166,8 @@ func TestGetCorrectAuthMechanismFromVersion(t *testing.T) { mechanismList = convertToMechanismList([]string{"SCRAM", "X509"}, ac) - assert.Contains(t, mechanismList, MongoDBCRMechanism) - assert.Contains(t, mechanismList, MongoDBX509Mechanism) + assert.Contains(t, mechanismList, mongoDBCRMechanism) + assert.Contains(t, mechanismList, mongoDBX509Mechanism) } func assertAuthenticationEnabled(t *testing.T, auth *om.Auth) { diff --git a/controllers/operator/authentication/ldap_test.go b/controllers/operator/authentication/ldap_test.go index 225e1f39c..0b619e3df 100644 --- a/controllers/operator/authentication/ldap_test.go +++ b/controllers/operator/authentication/ldap_test.go @@ -11,7 +11,7 @@ import ( "github.com/mongodb/mongodb-kubernetes/controllers/operator/ldap" ) -var LDAPPlainMechanism = getMechanismByName(LDAPPlain) +var ldapPlainMechanism = getMechanismByName(LDAPPlain) func TestLdapDeploymentMechanism(t *testing.T) { conn := om.NewMockedOmConnection(om.NewDeployment()) @@ -24,7 +24,7 @@ func TestLdapDeploymentMechanism(t *testing.T) { }, } - err := LDAPPlainMechanism.EnableDeploymentAuthentication(conn, opts, zap.S()) + err := ldapPlainMechanism.EnableDeploymentAuthentication(conn, opts, zap.S()) require.NoError(t, err) ac, err := conn.ReadAutomationConfig() @@ -34,7 +34,7 @@ func TestLdapDeploymentMechanism(t *testing.T) { assert.Equal(t, "Servers", ac.Ldap.Servers) assert.Equal(t, "BindMethod", ac.Ldap.BindMethod) - err = LDAPPlainMechanism.DisableDeploymentAuthentication(conn, zap.S()) + err = ldapPlainMechanism.DisableDeploymentAuthentication(conn, zap.S()) require.NoError(t, err) ac, err = conn.ReadAutomationConfig() @@ -55,7 +55,7 @@ func TestLdapEnableAgentAuthentication(t *testing.T) { AutoPwd: "LDAPPassword.", } - err := LDAPPlainMechanism.EnableAgentAuthentication(conn, opts, zap.S()) + err := ldapPlainMechanism.EnableAgentAuthentication(conn, opts, zap.S()) require.NoError(t, err) ac, err := conn.ReadAutomationConfig() @@ -80,5 +80,5 @@ func TestLDAP_DisableAgentAuthentication(t *testing.T) { }, } - assertAgentAuthenticationDisabled(t, LDAPPlainMechanism, conn, opts) + assertAgentAuthenticationDisabled(t, ldapPlainMechanism, conn, opts) } diff --git a/controllers/operator/authentication/oidc.go b/controllers/operator/authentication/oidc.go index 553036498..4896cebb7 100644 --- a/controllers/operator/authentication/oidc.go +++ b/controllers/operator/authentication/oidc.go @@ -15,8 +15,6 @@ import ( "github.com/mongodb/mongodb-kubernetes/pkg/util/stringutil" ) -var MongoDBOIDCMechanism = &oidcAuthMechanism{} - type oidcAuthMechanism struct{} func (o *oidcAuthMechanism) GetName() MechanismName { diff --git a/controllers/operator/authentication/oidc_test.go b/controllers/operator/authentication/oidc_test.go index bb22e1d23..6664ba607 100644 --- a/controllers/operator/authentication/oidc_test.go +++ b/controllers/operator/authentication/oidc_test.go @@ -11,6 +11,8 @@ import ( "github.com/mongodb/mongodb-kubernetes/controllers/operator/oidc" ) +var mongoDBOIDCMechanism = getMechanismByName(MongoDBOIDC) + func TestOIDC_EnableDeploymentAuthentication(t *testing.T) { conn := om.NewMockedOmConnection(om.NewDeployment()) ac, err := conn.ReadAutomationConfig() @@ -46,10 +48,10 @@ func TestOIDC_EnableDeploymentAuthentication(t *testing.T) { OIDCProviderConfigs: providerConfigs, } - configured := MongoDBOIDCMechanism.IsDeploymentAuthenticationConfigured(ac, opts) + configured := mongoDBOIDCMechanism.IsDeploymentAuthenticationConfigured(ac, opts) assert.False(t, configured) - err = MongoDBOIDCMechanism.EnableDeploymentAuthentication(conn, opts, zap.S()) + err = mongoDBOIDCMechanism.EnableDeploymentAuthentication(conn, opts, zap.S()) require.NoError(t, err) ac, err = conn.ReadAutomationConfig() @@ -57,16 +59,16 @@ func TestOIDC_EnableDeploymentAuthentication(t *testing.T) { assert.Contains(t, ac.Auth.DeploymentAuthMechanisms, string(MongoDBOIDC)) assert.Equal(t, providerConfigs, ac.OIDCProviderConfigs) - configured = MongoDBOIDCMechanism.IsDeploymentAuthenticationConfigured(ac, opts) + configured = mongoDBOIDCMechanism.IsDeploymentAuthenticationConfigured(ac, opts) assert.True(t, configured) - err = MongoDBOIDCMechanism.DisableDeploymentAuthentication(conn, zap.S()) + err = mongoDBOIDCMechanism.DisableDeploymentAuthentication(conn, zap.S()) require.NoError(t, err) ac, err = conn.ReadAutomationConfig() require.NoError(t, err) - configured = MongoDBOIDCMechanism.IsDeploymentAuthenticationConfigured(ac, opts) + configured = mongoDBOIDCMechanism.IsDeploymentAuthenticationConfigured(ac, opts) assert.False(t, configured) assert.NotContains(t, ac.Auth.DeploymentAuthMechanisms, string(MongoDBOIDC)) @@ -82,12 +84,12 @@ func TestOIDC_EnableAgentAuthentication(t *testing.T) { ac, err := conn.ReadAutomationConfig() require.NoError(t, err) - configured := MongoDBOIDCMechanism.IsAgentAuthenticationConfigured(ac, opts) + configured := mongoDBOIDCMechanism.IsAgentAuthenticationConfigured(ac, opts) assert.False(t, configured) - err = MongoDBOIDCMechanism.EnableAgentAuthentication(conn, opts, zap.S()) + err = mongoDBOIDCMechanism.EnableAgentAuthentication(conn, opts, zap.S()) require.Error(t, err) - err = MongoDBOIDCMechanism.DisableAgentAuthentication(conn, zap.S()) + err = mongoDBOIDCMechanism.DisableAgentAuthentication(conn, zap.S()) require.Error(t, err) } diff --git a/controllers/operator/authentication/scramsha_test.go b/controllers/operator/authentication/scramsha_test.go index b4cc63cd1..1c97e9943 100644 --- a/controllers/operator/authentication/scramsha_test.go +++ b/controllers/operator/authentication/scramsha_test.go @@ -12,9 +12,9 @@ import ( ) var ( - MongoDBCRMechanism = getMechanismByName(MongoDBCR) - ScramSha1Mechanism = getMechanismByName(ScramSha1) - ScramSha256Mechanism = getMechanismByName(ScramSha256) + mongoDBCRMechanism = getMechanismByName(MongoDBCR) + scramSha1Mechanism = getMechanismByName(ScramSha1) + scramSha256Mechanism = getMechanismByName(ScramSha256) ) func TestAgentsAuthentication(t *testing.T) { @@ -23,13 +23,13 @@ func TestAgentsAuthentication(t *testing.T) { } tests := map[string]TestConfig{ "SCRAM-SHA-1": { - mechanism: ScramSha1Mechanism, + mechanism: scramSha1Mechanism, }, "SCRAM-SHA-256": { - mechanism: ScramSha256Mechanism, + mechanism: scramSha256Mechanism, }, "CR": { - mechanism: MongoDBCRMechanism, + mechanism: mongoDBCRMechanism, }, } for testName, testConfig := range tests { @@ -65,10 +65,10 @@ func TestAgentsAuthentication(t *testing.T) { func TestScramSha1_DisableAgentAuthentication(t *testing.T) { conn := om.NewMockedOmConnection(om.NewDeployment()) - assertAgentAuthenticationDisabled(t, ScramSha1Mechanism, conn, Options{}) + assertAgentAuthenticationDisabled(t, scramSha1Mechanism, conn, Options{}) } func TestScramSha256_DisableAgentAuthentication(t *testing.T) { conn := om.NewMockedOmConnection(om.NewDeployment()) - assertAgentAuthenticationDisabled(t, ScramSha256Mechanism, conn, Options{}) + assertAgentAuthenticationDisabled(t, scramSha256Mechanism, conn, Options{}) } diff --git a/controllers/operator/authentication/x509_test.go b/controllers/operator/authentication/x509_test.go index 05016aee3..f342b6702 100644 --- a/controllers/operator/authentication/x509_test.go +++ b/controllers/operator/authentication/x509_test.go @@ -12,7 +12,7 @@ import ( "github.com/mongodb/mongodb-kubernetes/pkg/util" ) -var MongoDBX509Mechanism = getMechanismByName(MongoDBX509) +var mongoDBX509Mechanism = getMechanismByName(MongoDBX509) func TestX509EnableAgentAuthentication(t *testing.T) { conn := om.NewMockedOmConnection(om.NewDeployment()) @@ -25,7 +25,7 @@ func TestX509EnableAgentAuthentication(t *testing.T) { }, AuthoritativeSet: true, } - if err := MongoDBX509Mechanism.EnableAgentAuthentication(conn, options, zap.S()); err != nil { + if err := mongoDBX509Mechanism.EnableAgentAuthentication(conn, options, zap.S()); err != nil { t.Fatal(err) } @@ -55,14 +55,14 @@ func TestX509_DisableAgentAuthentication(t *testing.T) { AutomationSubject: validSubject("automation"), }, } - assertAgentAuthenticationDisabled(t, MongoDBX509Mechanism, conn, opts) + assertAgentAuthenticationDisabled(t, mongoDBX509Mechanism, conn, opts) } func TestX509_DeploymentConfigured(t *testing.T) { conn := om.NewMockedOmConnection(om.NewDeployment()) opts := Options{AgentMechanism: "SCRAM", CAFilePath: util.CAFilePathInContainer} - assertDeploymentMechanismsConfigured(t, MongoDBX509Mechanism, conn, opts) + assertDeploymentMechanismsConfigured(t, mongoDBX509Mechanism, conn, opts) ac, err := conn.ReadAutomationConfig() require.NoError(t, err) From 646bec6295bec975f0ee46fa8cded62230e04b42 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Maciej=20Kara=C5=9B?= Date: Thu, 29 May 2025 13:57:47 +0200 Subject: [PATCH 33/36] Removed `util.MergoDelete` from oidc implementation --- api/v1/mdb/mongodb_types.go | 4 +- api/v1/mdb/mongodb_validation.go | 8 +- api/v1/mdb/mongodb_validation_test.go | 20 ++-- api/v1/mdb/zz_generated.deepcopy.go | 37 ++++++++ controllers/om/automation_config.go | 91 +++++++++---------- controllers/om/automation_config_test.go | 37 ++++---- controllers/operator/authentication/oidc.go | 15 +-- .../operator/authentication/oidc_test.go | 7 +- controllers/operator/oidc/oidc_types.go | 4 +- go.mod | 2 - 10 files changed, 125 insertions(+), 100 deletions(-) diff --git a/api/v1/mdb/mongodb_types.go b/api/v1/mdb/mongodb_types.go index 068ef3306..edf4d7af9 100644 --- a/api/v1/mdb/mongodb_types.go +++ b/api/v1/mdb/mongodb_types.go @@ -1091,7 +1091,7 @@ type OIDCProviderConfig struct { // Accept the default value unless your IdP uses a different claim, or you need a custom claim. // Required when selected GroupMembership as the authorization type, ignored otherwise // +kubebuilder:validation:Optional - GroupsClaim string `json:"groupsClaim,omitempty"` + GroupsClaim *string `json:"groupsClaim"` // Configure single-sign-on for human user access to Ops Manager deployments with Workforce Identity Federation. // For programmatic, application access to Ops Manager deployments use Workload Identity Federation. @@ -1103,7 +1103,7 @@ type OIDCProviderConfig struct { // registered with an external Identity Provider. // Required when selected Workforce Identity Federation authorization method // +kubebuilder:validation:Optional - ClientId string `json:"clientId,omitempty"` + ClientId *string `json:"clientId"` // Tokens that give users permission to request data from the authorization endpoint. // Only used for Workforce Identity Federation authorization method diff --git a/api/v1/mdb/mongodb_validation.go b/api/v1/mdb/mongodb_validation.go index 4670928e8..64c93c261 100644 --- a/api/v1/mdb/mongodb_validation.go +++ b/api/v1/mdb/mongodb_validation.go @@ -252,11 +252,11 @@ func oidcProviderConfigIssuerURIValidator(config OIDCProviderConfig) func(DbComm func oidcProviderConfigClientIdValidator(config OIDCProviderConfig) func(DbCommonSpec) v1.ValidationResult { return func(_ DbCommonSpec) v1.ValidationResult { if config.AuthorizationMethod == OIDCAuthorizationMethodWorkforceIdentityFederation { - if config.ClientId == "" { + if config.ClientId == nil || *config.ClientId == "" { return v1.ValidationError("ClientId has to be specified in OIDC provider config %q with Workforce Identity Federation", config.ConfigurationName) } } else if config.AuthorizationMethod == OIDCAuthorizationMethodWorkloadIdentityFederation { - if config.ClientId != "" { + if config.ClientId != nil { return v1.ValidationWarning("ClientId will be ignored in OIDC provider config %q with Workload Identity Federation", config.ConfigurationName) } } @@ -280,11 +280,11 @@ func oidcProviderConfigRequestedScopesValidator(config OIDCProviderConfig) func( func oidcProviderConfigAuthorizationTypeValidator(config OIDCProviderConfig) func(DbCommonSpec) v1.ValidationResult { return func(_ DbCommonSpec) v1.ValidationResult { if config.AuthorizationType == OIDCAuthorizationTypeGroupMembership { - if config.GroupsClaim == "" { + if config.GroupsClaim == nil || *config.GroupsClaim == "" { return v1.ValidationError("GroupsClaim has to be specified in OIDC provider config %q when using Group Membership authorization", config.ConfigurationName) } } else if config.AuthorizationType == OIDCAuthorizationTypeUserID { - if config.GroupsClaim != "" { + if config.GroupsClaim != nil { return v1.ValidationWarning("GroupsClaim will be ignored in OIDC provider config %q when using User ID authorization", config.ConfigurationName) } } diff --git a/api/v1/mdb/mongodb_validation_test.go b/api/v1/mdb/mongodb_validation_test.go index a2ba4e7b3..d7ac479ba 100644 --- a/api/v1/mdb/mongodb_validation_test.go +++ b/api/v1/mdb/mongodb_validation_test.go @@ -258,13 +258,13 @@ func TestOIDCAuthValidation(t *testing.T) { ConfigurationName: "provider", IssuerURI: "https://example1.com", AuthorizationMethod: OIDCAuthorizationMethodWorkforceIdentityFederation, - ClientId: "clientId1", + ClientId: ptr.To("clientId1"), }, { ConfigurationName: "provider", IssuerURI: "https://example2.com", AuthorizationMethod: OIDCAuthorizationMethodWorkforceIdentityFederation, - ClientId: "clientId2", + ClientId: ptr.To("clientId2"), }, }, }, @@ -281,13 +281,13 @@ func TestOIDCAuthValidation(t *testing.T) { ConfigurationName: "test-provider1", IssuerURI: "https://example1.com", AuthorizationMethod: OIDCAuthorizationMethodWorkforceIdentityFederation, - ClientId: "clientId1", + ClientId: ptr.To("clientId1"), }, { ConfigurationName: "test-provider2", IssuerURI: "https://example2.com", AuthorizationMethod: OIDCAuthorizationMethodWorkforceIdentityFederation, - ClientId: "clientId2", + ClientId: ptr.To("clientId2"), }, }, }, @@ -304,7 +304,7 @@ func TestOIDCAuthValidation(t *testing.T) { ConfigurationName: "test-provider-workforce1", IssuerURI: "https://example1.com", AuthorizationMethod: OIDCAuthorizationMethodWorkforceIdentityFederation, - ClientId: "clientId1", + ClientId: ptr.To("clientId1"), }, { ConfigurationName: "test-provider-workload2", @@ -376,7 +376,7 @@ func TestOIDCAuthValidation(t *testing.T) { ConfigurationName: "test-provider", IssuerURI: "https://example.com", AuthorizationMethod: OIDCAuthorizationMethodWorkloadIdentityFederation, - ClientId: "clientId", + ClientId: ptr.To("clientId"), }, }, }, @@ -410,7 +410,7 @@ func TestOIDCAuthValidation(t *testing.T) { ConfigurationName: "test-provider1", IssuerURI: "https://example.com", AuthorizationType: OIDCAuthorizationTypeGroupMembership, - GroupsClaim: "groups", + GroupsClaim: ptr.To("groups"), }, { ConfigurationName: "test-provider2", @@ -432,7 +432,7 @@ func TestOIDCAuthValidation(t *testing.T) { ConfigurationName: "test-provider1", IssuerURI: "https://example.com", AuthorizationType: OIDCAuthorizationTypeUserID, - GroupsClaim: "groups", + GroupsClaim: ptr.To("groups"), UserClaim: "sub", }, { @@ -456,13 +456,13 @@ func TestOIDCAuthValidation(t *testing.T) { ConfigurationName: "test-provider1", IssuerURI: "https://example.com", AuthorizationType: OIDCAuthorizationTypeGroupMembership, - GroupsClaim: "groups", + GroupsClaim: ptr.To("groups"), }, { ConfigurationName: "test-provider2", IssuerURI: "https://example.com", AuthorizationType: OIDCAuthorizationTypeGroupMembership, - GroupsClaim: "groups", + GroupsClaim: ptr.To("groups"), }, }, }, diff --git a/api/v1/mdb/zz_generated.deepcopy.go b/api/v1/mdb/zz_generated.deepcopy.go index e8c2e4813..ac09a290b 100644 --- a/api/v1/mdb/zz_generated.deepcopy.go +++ b/api/v1/mdb/zz_generated.deepcopy.go @@ -125,6 +125,13 @@ func (in *Authentication) DeepCopyInto(out *Authentication) { *out = new(Ldap) (*in).DeepCopyInto(*out) } + if in.OIDCProviderConfigs != nil { + in, out := &in.OIDCProviderConfigs, &out.OIDCProviderConfigs + *out = make([]OIDCProviderConfig, len(*in)) + for i := range *in { + (*in)[i].DeepCopyInto(&(*out)[i]) + } + } in.Agents.DeepCopyInto(&out.Agents) } @@ -948,6 +955,36 @@ func (in *MonitoringAgentConfig) DeepCopy() *MonitoringAgentConfig { return out } +// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. +func (in *OIDCProviderConfig) DeepCopyInto(out *OIDCProviderConfig) { + *out = *in + if in.GroupsClaim != nil { + in, out := &in.GroupsClaim, &out.GroupsClaim + *out = new(string) + **out = **in + } + if in.ClientId != nil { + in, out := &in.ClientId, &out.ClientId + *out = new(string) + **out = **in + } + if in.RequestedScopes != nil { + in, out := &in.RequestedScopes, &out.RequestedScopes + *out = make([]string, len(*in)) + copy(*out, *in) + } +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new OIDCProviderConfig. +func (in *OIDCProviderConfig) DeepCopy() *OIDCProviderConfig { + if in == nil { + return nil + } + out := new(OIDCProviderConfig) + in.DeepCopyInto(out) + return out +} + // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *PersistenceConfigBuilder) DeepCopyInto(out *PersistenceConfigBuilder) { *out = *in diff --git a/controllers/om/automation_config.go b/controllers/om/automation_config.go index 951382c0f..bfb61c369 100644 --- a/controllers/om/automation_config.go +++ b/controllers/om/automation_config.go @@ -62,47 +62,7 @@ func applyInto(a AutomationConfig, into *Deployment) error { } if len(a.OIDCProviderConfigs) > 0 { - deploymentConfigs := make([]map[string]any, 0) - if configs, ok := a.Deployment["oidcProviderConfigs"]; ok { - configsSlice := cast.ToSlice(configs) - for _, config := range configsSlice { - deploymentConfigs = append(deploymentConfigs, config.(map[string]any)) - } - } - - result := make([]map[string]any, 0) - for _, config := range a.OIDCProviderConfigs { - deploymentConfig := findOrCreateEmptyDeploymentConfig(deploymentConfigs, config.AuthNamePrefix) - - deploymentConfig["authNamePrefix"] = config.AuthNamePrefix - deploymentConfig["audience"] = config.Audience - deploymentConfig["issuerUri"] = config.IssuerUri - deploymentConfig["userClaim"] = config.UserClaim - deploymentConfig["supportsHumanFlows"] = config.SupportsHumanFlows - deploymentConfig["useAuthorizationClaim"] = config.UseAuthorizationClaim - - if config.ClientId == util.MergoDelete { - delete(deploymentConfig, "clientId") - } else { - deploymentConfig["clientId"] = config.ClientId - } - - if len(config.RequestedScopes) == 0 { - delete(deploymentConfig, "requestedScopes") - } else { - deploymentConfig["requestedScopes"] = config.RequestedScopes - } - - if config.GroupsClaim == util.MergoDelete { - delete(deploymentConfig, "groupsClaim") - } else { - deploymentConfig["groupsClaim"] = config.GroupsClaim - } - - result = append(result, deploymentConfig) - } - - (*into)["oidcProviderConfigs"] = result + updateOIDCProviderConfigs(a, into) } else { // Clear oidcProviderConfigs if no configs are provided delete(*into, "oidcProviderConfigs") @@ -111,14 +71,53 @@ func applyInto(a AutomationConfig, into *Deployment) error { return nil } -func findOrCreateEmptyDeploymentConfig(deploymentConfigs []map[string]any, configName string) map[string]any { - for _, deploymentConfig := range deploymentConfigs { - if configName == deploymentConfig["authNamePrefix"] { - return deploymentConfig +func updateOIDCProviderConfigs(a AutomationConfig, into *Deployment) { + deploymentConfigs := make(map[string]map[string]any) + if configs, ok := a.Deployment["oidcProviderConfigs"]; ok { + configsSliceAny := cast.ToSlice(configs) + for _, configAny := range configsSliceAny { + config := configAny.(map[string]any) + configName := config["authNamePrefix"].(string) + deploymentConfigs[configName] = config + } + } + + result := make([]map[string]any, 0) + for _, config := range a.OIDCProviderConfigs { + deploymentConfig, ok := deploymentConfigs[config.AuthNamePrefix] + if !ok { + deploymentConfig = make(map[string]any) } + + deploymentConfig["authNamePrefix"] = config.AuthNamePrefix + deploymentConfig["audience"] = config.Audience + deploymentConfig["issuerUri"] = config.IssuerUri + deploymentConfig["userClaim"] = config.UserClaim + deploymentConfig["supportsHumanFlows"] = config.SupportsHumanFlows + deploymentConfig["useAuthorizationClaim"] = config.UseAuthorizationClaim + + if config.ClientId == nil { + delete(deploymentConfig, "clientId") + } else { + deploymentConfig["clientId"] = config.ClientId + } + + if len(config.RequestedScopes) == 0 { + delete(deploymentConfig, "requestedScopes") + } else { + deploymentConfig["requestedScopes"] = config.RequestedScopes + } + + if config.GroupsClaim == nil { + delete(deploymentConfig, "groupsClaim") + } else { + deploymentConfig["groupsClaim"] = config.GroupsClaim + } + + result = append(result, deploymentConfig) } - return make(map[string]any) + (*into)["oidcProviderConfigs"] = result } // EqualsWithoutDeployment returns true if two AutomationConfig objects are meaningful equal by following the following conditions: diff --git a/controllers/om/automation_config_test.go b/controllers/om/automation_config_test.go index fb4875636..e554d5f5c 100644 --- a/controllers/om/automation_config_test.go +++ b/controllers/om/automation_config_test.go @@ -7,6 +7,7 @@ import ( "github.com/spf13/cast" "github.com/stretchr/testify/assert" "k8s.io/apimachinery/pkg/api/equality" + "k8s.io/utils/ptr" "github.com/mongodb/mongodb-kubernetes/controllers/operator/ldap" "github.com/mongodb/mongodb-kubernetes/controllers/operator/oidc" @@ -634,7 +635,7 @@ func TestAutomationConfigEquality(t *testing.T) { Audience: "aud", IssuerUri: "https://provider1.okta.com", UserClaim: "sub", - GroupsClaim: "groups", + GroupsClaim: ptr.To("groups"), SupportsHumanFlows: false, UseAuthorizationClaim: true, }, @@ -642,7 +643,7 @@ func TestAutomationConfigEquality(t *testing.T) { AuthNamePrefix: "provider-2", Audience: "aud", IssuerUri: "https://provider2.okta.com", - ClientId: "provider2-clientId", + ClientId: ptr.To("provider2-clientId"), RequestedScopes: []string{"openid", "profile"}, UserClaim: "sub", SupportsHumanFlows: true, @@ -655,9 +656,9 @@ func TestAutomationConfigEquality(t *testing.T) { AuthNamePrefix: "provider-1", Audience: "aud", IssuerUri: "https://provider1.okta.com", - ClientId: util.MergoDelete, + ClientId: nil, UserClaim: "sub", - GroupsClaim: "groups", + GroupsClaim: ptr.To("groups"), SupportsHumanFlows: false, UseAuthorizationClaim: true, }, @@ -665,10 +666,10 @@ func TestAutomationConfigEquality(t *testing.T) { AuthNamePrefix: "provider-2", Audience: "aud", IssuerUri: "https://provider2.okta.com", - ClientId: "provider2-clientId", + ClientId: ptr.To("provider2-clientId"), RequestedScopes: []string{"openid", "profile"}, UserClaim: "sub", - GroupsClaim: util.MergoDelete, + GroupsClaim: nil, SupportsHumanFlows: true, UseAuthorizationClaim: false, }, @@ -856,10 +857,10 @@ func TestOIDCProviderConfigsAreMerged(t *testing.T) { { AuthNamePrefix: "provider-new", Audience: "aud", - ClientId: util.MergoDelete, + ClientId: nil, IssuerUri: "https://provider-new.okta.com", UserClaim: "sub", - GroupsClaim: "groups", + GroupsClaim: ptr.To("groups"), SupportsHumanFlows: false, UseAuthorizationClaim: true, }, @@ -882,9 +883,9 @@ func TestOIDCProviderConfigsAreMerged(t *testing.T) { AuthNamePrefix: "OIDC_WORKFORCE_USERID", Audience: "aud", IssuerUri: "https://provider.okta.com", - ClientId: "oktaClientId", + ClientId: ptr.To("oktaClientId"), UserClaim: "sub", - GroupsClaim: util.MergoDelete, + GroupsClaim: nil, RequestedScopes: []string{"openid"}, SupportsHumanFlows: true, UseAuthorizationClaim: false, @@ -914,10 +915,10 @@ func TestOIDCProviderConfigsAreMerged(t *testing.T) { { AuthNamePrefix: "OIDC_WORKLOAD_USERID", Audience: "aud", - ClientId: util.MergoDelete, + ClientId: nil, IssuerUri: "https://provider1.okta.com", UserClaim: "sub", - GroupsClaim: "groups", + GroupsClaim: ptr.To("groups"), SupportsHumanFlows: false, UseAuthorizationClaim: true, }, @@ -943,10 +944,10 @@ func TestOIDCProviderConfigsAreMerged(t *testing.T) { { AuthNamePrefix: "OIDC_WORKFORCE_USERID", Audience: "aud", - ClientId: "oktaClientId", + ClientId: ptr.To("oktaClientId"), IssuerUri: "https://provider.okta.com", UserClaim: "sub", - GroupsClaim: util.MergoDelete, + GroupsClaim: nil, RequestedScopes: []string{"openid"}, SupportsHumanFlows: true, UseAuthorizationClaim: false, @@ -954,10 +955,10 @@ func TestOIDCProviderConfigsAreMerged(t *testing.T) { { AuthNamePrefix: "OIDC_WORKLOAD_USERID", Audience: "aud", - ClientId: util.MergoDelete, + ClientId: nil, IssuerUri: "https://provider.okta.com", UserClaim: "sub", - GroupsClaim: util.MergoDelete, + GroupsClaim: nil, RequestedScopes: []string{"openid"}, SupportsHumanFlows: false, UseAuthorizationClaim: false, @@ -965,10 +966,10 @@ func TestOIDCProviderConfigsAreMerged(t *testing.T) { { AuthNamePrefix: "OIDC_WORKLOAD_GROUP_MEMBERSHIP", Audience: "aud", - ClientId: util.MergoDelete, + ClientId: nil, IssuerUri: "https://provider.okta.com", UserClaim: "sub", - GroupsClaim: "groups", + GroupsClaim: ptr.To("groups"), SupportsHumanFlows: false, UseAuthorizationClaim: true, }, diff --git a/controllers/operator/authentication/oidc.go b/controllers/operator/authentication/oidc.go index 4896cebb7..7e9bf4c2c 100644 --- a/controllers/operator/authentication/oidc.go +++ b/controllers/operator/authentication/oidc.go @@ -11,7 +11,6 @@ import ( mdbv1 "github.com/mongodb/mongodb-kubernetes/api/v1/mdb" "github.com/mongodb/mongodb-kubernetes/controllers/om" "github.com/mongodb/mongodb-kubernetes/controllers/operator/oidc" - "github.com/mongodb/mongodb-kubernetes/pkg/util" "github.com/mongodb/mongodb-kubernetes/pkg/util/stringutil" ) @@ -97,24 +96,14 @@ func MapOIDCProviderConfigs(oidcProviderConfigs []mdbv1.OIDCProviderConfig) []oi result := make([]oidc.ProviderConfig, len(oidcProviderConfigs)) for i, providerConfig := range oidcProviderConfigs { - clientId := providerConfig.ClientId - if clientId == "" { - clientId = util.MergoDelete - } - - groupsClaim := providerConfig.GroupsClaim - if groupsClaim == "" { - groupsClaim = util.MergoDelete - } - result[i] = oidc.ProviderConfig{ AuthNamePrefix: providerConfig.ConfigurationName, Audience: providerConfig.Audience, IssuerUri: providerConfig.IssuerURI, - ClientId: clientId, + ClientId: providerConfig.ClientId, RequestedScopes: providerConfig.RequestedScopes, UserClaim: providerConfig.UserClaim, - GroupsClaim: groupsClaim, + GroupsClaim: providerConfig.GroupsClaim, SupportsHumanFlows: mapToSupportHumanFlows(providerConfig.AuthorizationMethod), UseAuthorizationClaim: mapToUseAuthorizationClaim(providerConfig.AuthorizationType), } diff --git a/controllers/operator/authentication/oidc_test.go b/controllers/operator/authentication/oidc_test.go index 6664ba607..6460db803 100644 --- a/controllers/operator/authentication/oidc_test.go +++ b/controllers/operator/authentication/oidc_test.go @@ -6,6 +6,7 @@ import ( "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" "go.uber.org/zap" + "k8s.io/utils/ptr" "github.com/mongodb/mongodb-kubernetes/controllers/om" "github.com/mongodb/mongodb-kubernetes/controllers/operator/oidc" @@ -25,7 +26,7 @@ func TestOIDC_EnableDeploymentAuthentication(t *testing.T) { AuthNamePrefix: "okta", Audience: "aud", IssuerUri: "https://okta.mongodb.com", - ClientId: "client1", + ClientId: ptr.To("client1"), RequestedScopes: []string{"openid", "profile"}, UserClaim: "sub", SupportsHumanFlows: true, @@ -35,9 +36,9 @@ func TestOIDC_EnableDeploymentAuthentication(t *testing.T) { AuthNamePrefix: "congito", Audience: "aud", IssuerUri: "https://congito.mongodb.com", - ClientId: "client2", + ClientId: ptr.To("client2"), UserClaim: "sub", - GroupsClaim: "groups", + GroupsClaim: ptr.To("groups"), SupportsHumanFlows: false, UseAuthorizationClaim: true, }, diff --git a/controllers/operator/oidc/oidc_types.go b/controllers/operator/oidc/oidc_types.go index c5c4ef80e..88cb871ec 100644 --- a/controllers/operator/oidc/oidc_types.go +++ b/controllers/operator/oidc/oidc_types.go @@ -4,10 +4,10 @@ type ProviderConfig struct { AuthNamePrefix string `json:"authNamePrefix"` Audience string `json:"audience"` IssuerUri string `json:"issuerUri"` - ClientId string `json:"clientId"` + ClientId *string `json:"clientId"` RequestedScopes []string `json:"requestedScopes"` UserClaim string `json:"userClaim"` - GroupsClaim string `json:"groupsClaim"` + GroupsClaim *string `json:"groupsClaim"` SupportsHumanFlows bool `json:"supportsHumanFlows"` UseAuthorizationClaim bool `json:"useAuthorizationClaim"` } diff --git a/go.mod b/go.mod index 7d2052c92..45c8cf406 100644 --- a/go.mod +++ b/go.mod @@ -13,8 +13,6 @@ require ( github.com/hashicorp/go-retryablehttp v0.7.7 github.com/hashicorp/vault/api v1.16.0 github.com/imdario/mergo v0.3.15 - github.com/onsi/ginkgo/v2 v2.17.1 - github.com/onsi/gomega v1.32.0 github.com/pkg/errors v0.9.1 github.com/prometheus/client_golang v1.22.0 github.com/r3labs/diff/v3 v3.0.1 From 3e2839e7626c4084ba7027978fcf1c0010a34b28 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Maciej=20Kara=C5=9B?= Date: Thu, 29 May 2025 13:57:47 +0200 Subject: [PATCH 34/36] unit test fixes --- controllers/om/automation_config_test.go | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/controllers/om/automation_config_test.go b/controllers/om/automation_config_test.go index e554d5f5c..69300956e 100644 --- a/controllers/om/automation_config_test.go +++ b/controllers/om/automation_config_test.go @@ -871,7 +871,7 @@ func TestOIDCProviderConfigsAreMerged(t *testing.T) { "audience": "aud", "issuerUri": "https://provider-new.okta.com", "userClaim": "sub", - "groupsClaim": "groups", + "groupsClaim": ptr.To("groups"), "supportsHumanFlows": false, "useAuthorizationClaim": true, }, @@ -896,7 +896,7 @@ func TestOIDCProviderConfigsAreMerged(t *testing.T) { "authNamePrefix": "OIDC_WORKFORCE_USERID", "audience": "aud", "issuerUri": "https://provider.okta.com", - "clientId": "oktaClientId", + "clientId": ptr.To("oktaClientId"), "requestedScopes": []string{ "openid", }, @@ -929,7 +929,7 @@ func TestOIDCProviderConfigsAreMerged(t *testing.T) { "audience": "aud", "issuerUri": "https://provider1.okta.com", "userClaim": "sub", - "groupsClaim": "groups", + "groupsClaim": ptr.To("groups"), "supportsHumanFlows": false, "useAuthorizationClaim": true, "JWKSPollSecs": float64(360), @@ -979,7 +979,7 @@ func TestOIDCProviderConfigsAreMerged(t *testing.T) { "authNamePrefix": "OIDC_WORKFORCE_USERID", "audience": "aud", "issuerUri": "https://provider.okta.com", - "clientId": "oktaClientId", + "clientId": ptr.To("oktaClientId"), "requestedScopes": []string{ "openid", }, @@ -1011,7 +1011,7 @@ func TestOIDCProviderConfigsAreMerged(t *testing.T) { "audience": "aud", "issuerUri": "https://provider.okta.com", "userClaim": "sub", - "groupsClaim": "groups", + "groupsClaim": ptr.To("groups"), "supportsHumanFlows": false, "useAuthorizationClaim": true, "JWKSPollSecs": float64(360), From 15a1cbc4aff121a9acbd1489b0a6513fc5f161d9 Mon Sep 17 00:00:00 2001 From: Anand Singh Date: Sun, 1 Jun 2025 18:28:32 +0200 Subject: [PATCH 35/36] replace community with non_enterprise to prevent confusion --- .../tests/webhooks/e2e_mongodb_validation_webhook.py | 6 +++--- ...munity.yaml => invalid_oidc_mongodb_non_enterprise.yaml} | 0 2 files changed, 3 insertions(+), 3 deletions(-) rename docker/mongodb-kubernetes-tests/tests/webhooks/fixtures/{invalid_oidc_mongodb_community.yaml => invalid_oidc_mongodb_non_enterprise.yaml} (100%) diff --git a/docker/mongodb-kubernetes-tests/tests/webhooks/e2e_mongodb_validation_webhook.py b/docker/mongodb-kubernetes-tests/tests/webhooks/e2e_mongodb_validation_webhook.py index 2420f4fe3..6a21432ef 100644 --- a/docker/mongodb-kubernetes-tests/tests/webhooks/e2e_mongodb_validation_webhook.py +++ b/docker/mongodb-kubernetes-tests/tests/webhooks/e2e_mongodb_validation_webhook.py @@ -79,12 +79,12 @@ def test_replicaset_members_is_specified(self): exception_reason="'spec.members' must be specified if type of MongoDB is ReplicaSet", ) - def test_oidc_auth_with_mongodb_community(self): - resource = yaml.safe_load(open(yaml_fixture("invalid_oidc_mongodb_community.yaml"))) + def test_oidc_auth_with_mongodb_non_enterprise(self): + resource = yaml.safe_load(open(yaml_fixture("invalid_oidc_mongodb_non_enterprise.yaml"))) self.create_custom_resource_from_object( self.get_namespace(), resource, - exception_reason="Cannot enable OIDC authentication with MongoDB Community Builds", + exception_reason="Cannot enable OIDC authentication with MongoDB non-enterprise builds", ) def test_oidc_auth_with_single_method(self): diff --git a/docker/mongodb-kubernetes-tests/tests/webhooks/fixtures/invalid_oidc_mongodb_community.yaml b/docker/mongodb-kubernetes-tests/tests/webhooks/fixtures/invalid_oidc_mongodb_non_enterprise.yaml similarity index 100% rename from docker/mongodb-kubernetes-tests/tests/webhooks/fixtures/invalid_oidc_mongodb_community.yaml rename to docker/mongodb-kubernetes-tests/tests/webhooks/fixtures/invalid_oidc_mongodb_non_enterprise.yaml From 489ac324365aabb668691e032d83d65f60a5833d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Maciej=20Kara=C5=9B?= Date: Mon, 2 Jun 2025 16:27:05 +0200 Subject: [PATCH 36/36] validation test fixes --- api/v1/mdb/mongodb_validation.go | 6 ++---- .../tests/webhooks/e2e_mongodb_validation_webhook.py | 2 +- 2 files changed, 3 insertions(+), 5 deletions(-) diff --git a/api/v1/mdb/mongodb_validation.go b/api/v1/mdb/mongodb_validation.go index 655dbbc9d..c8f6ea89e 100644 --- a/api/v1/mdb/mongodb_validation.go +++ b/api/v1/mdb/mongodb_validation.go @@ -298,16 +298,14 @@ func oidcProviderConfigAuthorizationTypeValidator(config OIDCProviderConfig) fun } func oidcAuthRequiresEnterprise(d DbCommonSpec) v1.ValidationResult { - authSpec := d.Security.Authentication - if authSpec != nil && authSpec.IsOIDCEnabled() && !strings.HasSuffix(d.Version, "-ent") { + if d.Security.Authentication.IsOIDCEnabled() && !strings.HasSuffix(d.Version, "-ent") { return v1.ValidationError("Cannot enable OIDC authentication with MongoDB Community Builds") } return v1.ValidationSuccess() } func ldapAuthRequiresEnterprise(d DbCommonSpec) v1.ValidationResult { - authSpec := d.Security.Authentication - if authSpec != nil && authSpec.IsLDAPEnabled() && !strings.HasSuffix(d.Version, "-ent") { + if d.Security.Authentication.IsLDAPEnabled() && !strings.HasSuffix(d.Version, "-ent") { return v1.ValidationError("Cannot enable LDAP authentication with MongoDB Community Builds") } return v1.ValidationSuccess() diff --git a/docker/mongodb-kubernetes-tests/tests/webhooks/e2e_mongodb_validation_webhook.py b/docker/mongodb-kubernetes-tests/tests/webhooks/e2e_mongodb_validation_webhook.py index 6a21432ef..0b60aa304 100644 --- a/docker/mongodb-kubernetes-tests/tests/webhooks/e2e_mongodb_validation_webhook.py +++ b/docker/mongodb-kubernetes-tests/tests/webhooks/e2e_mongodb_validation_webhook.py @@ -84,7 +84,7 @@ def test_oidc_auth_with_mongodb_non_enterprise(self): self.create_custom_resource_from_object( self.get_namespace(), resource, - exception_reason="Cannot enable OIDC authentication with MongoDB non-enterprise builds", + exception_reason="Cannot enable OIDC authentication with MongoDB Community Builds", ) def test_oidc_auth_with_single_method(self):