Skip to content

CLOUDP-314901 OIDC CRD changes + validation #50

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 13 commits into
base: master
Choose a base branch
from

Conversation

MaciejKaras
Copy link
Collaborator

@MaciejKaras MaciejKaras commented Apr 27, 2025

Summary

Adding new OIDCProviderConfig struct to Authentication struct and new AuthMode OIDC.

type OIDCProviderConfig struct {
	// 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: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.
	// +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
	// +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
	// +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
	// +kubebuilder:validation:Optional
	RequestedScopes []string `json:"requestedScopes,omitempty"`
}

// +kubebuilder:validation:Enum=GroupMembership;UserID
type OIDCAuthorizationType string

// +kubebuilder:validation:Enum=WorkforceIdentityFederation;WorkloadIdentityFederation
type OIDCAuthorizationMethod string

⚠️ Because Security.Authentication struct is reused also in AppDBSpec it will be available there as well. It will be as usual overridden in

om.Spec.AppDB.Security = ensureSecurityWithSCRAM(om.Spec.AppDB.Security)

I believe it is worth noting this behaviour, but not change it as part of this project.

Proof of Work

New Unit test that verify validation and another set of webhook tests are under way.

Next steps

  • Add release notes
  • Add validation tests based on webhooks
  • Start discussion with docs team

Checklist

  • Have you linked a jira ticket and/or is the ticket in the title?
  • Have you checked whether your jira ticket required DOCSP changes?
  • Have you checked for release_note changes?

Reminder (Please remove this when merging)

  • Please try to Approve or Reject Changes the PR, keep PRs in review as short as possible
  • Our Short Guide for PRs: Link
  • Remember the following Communication Standards - use comment prefixes for clarity:
    • blocking: Must be addressed before approval.
    • follow-up: Can be addressed in a later PR or ticket.
    • q: Clarifying question.
    • nit: Non-blocking suggestions.
    • note: Side-note, non-actionable. Example: Praise
    • --> no prefix is considered a question

@MaciejKaras MaciejKaras changed the title feature/mk-oidc-crd-validations CLOUDP-314901 OIDC CRD changes + validation Apr 27, 2025
@MaciejKaras MaciejKaras force-pushed the feature/mk-oidc-crd-validations branch from 713d6d2 to 7c23143 Compare April 27, 2025 14:35
@MaciejKaras MaciejKaras marked this pull request as ready for review April 27, 2025 14:50
@MaciejKaras MaciejKaras requested review from dan-mckean, vinilage and a team as code owners April 27, 2025 14:50
Copy link
Member

@SimonBaeumer SimonBaeumer left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, one small comment to the URL parse func.

@@ -801,6 +807,22 @@ func (s *Security) IsTLSEnabled() bool {
return s.CertificatesSecretsPrefix != ""
}

func (s *Security) IsOIDCEnabled() bool {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit, also just an opinion:

Suggested change
func (s *Security) IsOIDCEnabled() bool {
func (s *Security) IsOIDCEnabled() bool {
if s == nil || s.Authentication == nil || !s.Authentication.Enabled {
return false
}
return s.Authentication.IsOIDCEnabled()
}

return v1.ValidationSuccess()
}
}

func ldapAuthRequiresEnterprise(d DbCommonSpec) v1.ValidationResult {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we need this check for OIDC as well?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants