Sorry, something went wrong.
|
Thanks a lot for this 🙌 We’re currently using a bit of a temporary hack by overriding oidc_token_parser, so really looking forward to this landing in a release and finally getting rid of it 🙂 Really appreciate the work here! 💜 |
Sorry, something went wrong.
|
Thank you for this work. Could you add an override for the "current_user" parameter? Our OIDC server uses the UPN instead of preferred_username and I'm currently working on a monkey patch solution for this. But it would be much more convenient to have this in the official release. if "upn" in data:
current_user = data["upn"]
elif "preferred_username" in data:
current_user = data["preferred_username"]
else:
raise AuthenticationError(
"Missing upn or preferred_username field in access token."
)
|
Sorry, something went wrong.
| value: quay.io/feastdev/feature-server:0.61.0 | ||
| - name: RELATED_IMAGE_CRON_JOB | ||
| value: quay.io/openshift/origin-cli:4.17 | ||
| - name: OIDC_ISSUER_URL |
There was a problem hiding this comment.
🔴 OIDC_ISSUER_URL env var missing value: field in bundle CSV makes OLM deployment spec invalid
In the operator's OLM bundle CSV at line 273, the OIDC_ISSUER_URL env entry is missing a value: key. Compare the correct form in config/manager/manager.yaml:82-83 which has value: "". Without value:, the YAML parser treats the next line (image: quay.io/...) as a key within the env var map entry rather than a sibling container-level field. This makes the container spec structurally invalid: the container loses its image field entirely. OLM deployments using this CSV will fail to create the operator pod.
Expected vs actual YAML structureExpected (from config/manager/manager.yaml):
Actual (bundle CSV):
Was this helpful? React with 👍 or 👎 to provide feedback.
Sorry, something went wrong.
There was a problem hiding this comment.
operator-sdk generate bundle strips value: "" from env vars by design. This is a known operator-sdk behavior as it considers value: "" equivalent to absent and removes it during YAML serialization. The source manager.yaml has value: "", kustomize build (install.yaml) preserves it, but operator-sdk (CSV) strips it.
Sorry, something went wrong.
| try: | ||
| await self._validate_token(access_token) | ||
| logger.debug("Token successfully validated.") | ||
| except Exception as e: | ||
| if self._is_ssl_error(e): | ||
| logger.error( | ||
| "OIDC provider SSL certificate verification failed. " | ||
| "If using a self-signed certificate, set verify_ssl: false " | ||
| "or provide a CA certificate via ca_cert_path." | ||
| ) | ||
| logger.error(f"Token validation failed: {e}") | ||
| raise AuthenticationError(f"Invalid token: {e}") |
There was a problem hiding this comment.
🔴 OidcTokenParser._decode_token called redundantly even though unverified decode already happened
In OidcTokenParser.user_details_from_access_token (oidc_token_parser.py:186), _decode_token calls PyJWKClient and jwt.decode to fully decode the token. However, the method already did an unverified jwt.decode at line 152 to get unverified. The unverified payload is discarded and the token is decoded again via _decode_token, but the _validate_token call at line 174 already checked it with OAuth2AuthorizationCodeBearer. When _decode_token is called, PyJWKClient makes an HTTP call to the JWKS endpoint using an ssl_context derived from config — but _validate_token at line 54-60 does not use the same ssl_context. So _validate_token may fail with an SSL error on self-signed certs (using system trust), while _decode_token would succeed (with custom CA). The SSL context configuration is only applied to the JWKS fetch in _decode_token, not to the OAuth2AuthorizationCodeBearer validation which internally hits the OIDC discovery endpoints without the custom ssl_context/verify_ssl settings.
Prompt for agentsWas this helpful? React with 👍 or 👎 to provide feedback.
Sorry, something went wrong.
There was a problem hiding this comment.
The unverified decode is intentional for routing (SA token detection, intra-comm check)
OAuth2AuthorizationCodeBearer.call() does not make network calls, so there is no SSL mismatch
The _validate_token being a lightweight header check is pre-existing behavior, not introduced by this PR, and is tracked as a future improvement
Sorry, something went wrong.
| apimeta.RemoveStatusCondition(&authz.Handler.FeatureStore.Status.Conditions, feastKubernetesAuthConditions[metav1.ConditionTrue].Type) | ||
|
|
||
| if authz.isOidcAuth() { | ||
| if err := authz.createFeastClusterRole(); err != nil { |
There was a problem hiding this comment.
createFeastClusterRoleis using setFeastClusterRole, which gives OIDC-only deployments unnecessary exctra permissions, but since those are only read-only, considering it as non-blocker, can be handled as followup
Sorry, something went wrong.
There was a problem hiding this comment.
this also leave the ClusterRole and ClusterRoleBinding are orphaned when a user switches from OIDC auth to no auth (or to Kubernetes auth)
Sorry, something went wrong.
There was a problem hiding this comment.
Will address this in follow up PR. Thanks for review.
Sorry, something went wrong.
There was a problem hiding this comment.
Overall changes looking good, big PR and functionality
Sorry, something went wrong.
|
@aniketpalu Please make sure to have followup changes as required for non blocking issues |
Sorry, something went wrong.
What this PR does / why we need it:
Extends OIDC authentication in Feast to support GroupBasedPolicy and NamespaceBasedPolicy for OIDC users, adds flexible client token injection, and wires up operator support for OIDC deployments.
Previously, the OIDC token parser only extracted preferred_username and resource_access roles from the JWT. GroupBasedPolicy, NamespaceBasedPolicy, and CombinedGroupNamespacePolicy could never grant access for OIDC users because the User object was always created with empty groups and namespaces.
Server Side Changes
Files: oidc_token_parser.py, oidc_service.py, utils.py
When auth.type: oidc, the Feast server now handles two types of incoming tokens:
Additional server improvements:
Server Configuration (feature_store.yaml)
Keycloak Setup Required
The OIDC client in Keycloak must have a Groups claim mapper configured:
Client > Mappers > Add "Group Membership" mapper > claim name: groups
Without this mapper, JWTs will not contain the groups claim and GroupBasedPolicy will deny all access.
Client Side Changes
Files: oidc_authentication_client_manager.py, auth_model.py, repo_config.py
OidcAuthClientManager.get_token() now supports multiple token sources with a clear priority:
token, token_env_var, and client_secret are mutually exclusive (enforced by a Pydantic validator at config load time).
Client Configuration Examples
Zero config (workbench pods or kube-authkit users):
Picks up FEAST_OIDC_TOKEN env var if set, otherwise reads the mounted SA token.
Named environment variable (supports external token refresh):
Static token (testing / CI):
Client credentials flow (service to service):
Operator Changes
Files: featurestore_types.go, repo_config.go, tls.go, services_types.go, authz.go
The operator now generates separate server and client OIDC configs:
New CRD Fields on OidcAuthz
Discovery URL Resolution Priority
CA Certificate Resolution Priority
RBAC
When authz: oidc is configured, the operator provisions a ClusterRole and ClusterRoleBinding granting the Feast server SA tokenreviews/create permission. This enables the SA token delegation path for workbench pods.
Operator CR Example
Upstream (with issuerUrl):
ODH (OpenDataHub) zero config (platform injects OIDC_ISSUER_URL):
Permission Examples
Which issue(s) this PR fixes:
#6088
Misc