-
Notifications
You must be signed in to change notification settings - Fork 45
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
Add OIDC Federation Settings #479
base: main
Are you sure you want to change the base?
Add OIDC Federation Settings #479
Conversation
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: d34dh0r53 The full list of commands accepted by this bot can be found here.
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
api/v1beta1/keystoneapi_types.go
Outdated
// +kubebuilder:validation:Optional | ||
// +kubebuilder:default=false | ||
// Enablement of Federation configuration | ||
EnableFederation bool `json:"enableFederation,omitempty"` |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not needed then - just check for len(instance.spec.OIDCFederation) >0
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Right now I'm going to leave the toggle for testing. It also follows the pattern of Secure RBAC in that the deployer can explicitly enable or disable it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
OK - just realize of course that the reason upstream tests are failing is because you are not checking for len(instance.spec.OIDCFederation)>0. The reason this parameter is not like srbac is because the srbac setting does not have "omitEmpty"
This templates the OIDC federation settings needed to configure Keystone to perform federation authentication.
This addresses some missing brackets in the keystone.conf configuration as well as the incorrect handling of secrets. I've also added kubebuilder defaults and the missing OIDCMemCacheServers parameter.
Passwordselectors now return the error if the operation is unsuccessful.
Field was missing from the controller
Since this is a feature that requires enablement I think it makes more sense to do that direclty in the spec and then define the parameters for OIDC Federation. I've also moved the default setter for EnableFederation to the top to be consistent with the way other enablements work.
Makes the OIDC password selectors optional and provides defaults.
Then enableFederation boolean needs to be availalbe regardless.
This adds the KeystoneFederationIdentityProviderName parameter as we need that to compute the location match strings. This also removes the OIDCRedirectURI parameter as that is computed in the template.
Add configuration for specifying the number of fernet keys stored in the keystone secret. More than 2 keys are needed, since rotating 2 keys would expire sessions on every rotation. After configuration change, keys need to be added/removed and rotated in the proper order, to ensure that the sessions don't expire prematurely. Fernet key rotation is triggered in the reconcile loop. The "rotated at" timestamp is set in the secret annotation. Co-Authored-By: Grzegorz Grasza <[email protected]>
16455c7
to
0fdb6c4
Compare
This templates the OIDC federation settings needed to configure Keystone to perform federation authentication.