From bc0b3e83a504b90dac470b00c398aac692def897 Mon Sep 17 00:00:00 2001 From: Jon Jackson Date: Wed, 15 Jan 2025 12:52:42 -0500 Subject: [PATCH] OCPBUGS-45371: Fix issue where TokenReview was not working as expected We expected the console service account bearer token to be present on the internal k8s REST configuration, but it was not. This caused all TokenReview requests to be skipped since no bearer token was available to make the requests. - Update OpenShift authenticator to use a k8s client-go Clientset to make TokenReview requests. This Clientset is configured using the same REST config that the main server uses to proxy console service account delegated requests, meaning whatever console service account bearer token is configured on the main server is the same one used for TokenReview requests. - Update main.go to accept an off-cluster bearer token, which is used to make console service account delegated requests in an off-cluster, auth-enabled environment. - Update the README instructions for auth-enabled dev environments to include setting up a console service account API token and consume it through the new bridge arg. - Update `examples/run-bridge.sh` to consume an off-cluster service acccount token file --- README.md | 14 ++--- cmd/bridge/main.go | 5 ++ examples/.gitignore | 3 + examples/run-bridge.sh | 1 + examples/sa-secrets.yaml | 26 --------- examples/secret.yaml | 8 +++ pkg/auth/oauth2/auth_openshift.go | 93 ++++++++++--------------------- 7 files changed, 51 insertions(+), 99 deletions(-) delete mode 100644 examples/sa-secrets.yaml create mode 100644 examples/secret.yaml diff --git a/README.md b/README.md index 15e76b43e58..d825a26e37e 100644 --- a/README.md +++ b/README.md @@ -77,17 +77,13 @@ oc process -f examples/console-oauth-client.yaml | oc apply -f - oc get oauthclient console-oauth-client -o jsonpath='{.secret}' > examples/console-client-secret ``` -If the CA bundle of the OpenShift API server is unavailable, fetch the CA -certificates from a service account secret. Due to [upstream changes](https://kubernetes.io/docs/tasks/configure-pod-container/configure-service-account/#manually-create-an-api-token-for-a-serviceaccount), -these service account secrets need to be created manually. -Otherwise copy the CA bundle to -`examples/ca.crt`: +Create a long-lived API token Secret for the console ServiceAccount, and extract it to the +`examples` folder. This creates the `token` and `ca.crt` files, which are necessary for `bridge` to +proxy API server requests: ``` -oc apply -f examples/sa-secrets.yaml -oc get secrets -n default --field-selector type=kubernetes.io/service-account-token -o json | \ - jq '.items[0].data."ca.crt"' -r | python -m base64 -d > examples/ca.crt -# Note: use "openssl base64" because the "base64" tool is different between mac and linux +oc apply -f examples/secret.yaml +oc extract secret/off-cluster-token -n openshift-console --to ./examples --confirm ``` Finally run the console and visit [localhost:9000](http://localhost:9000): diff --git a/cmd/bridge/main.go b/cmd/bridge/main.go index 0ad656eb4cd..3cd5ccd3297 100644 --- a/cmd/bridge/main.go +++ b/cmd/bridge/main.go @@ -90,6 +90,7 @@ func main() { fK8sModeOffClusterThanos := fs.String("k8s-mode-off-cluster-thanos", "", "DEV ONLY. URL of the cluster's Thanos server.") fK8sModeOffClusterAlertmanager := fs.String("k8s-mode-off-cluster-alertmanager", "", "DEV ONLY. URL of the cluster's AlertManager server.") fK8sModeOffClusterCatalogd := fs.String("k8s-mode-off-cluster-catalogd", "", "DEV ONLY. URL of the cluster's catalogd server.") + fK8sModeOffClusterServiceAccountBearerTokenFile := fs.String("k8s-mode-off-cluster-service-account-bearer-token-file", "", "DEV ONLY. bearer token file for the service account used for internal K8s API server calls.") fK8sAuth := fs.String("k8s-auth", "", "this option is deprecated, setting it has no effect") @@ -439,6 +440,10 @@ func main() { Transport: &http.Transport{TLSClientConfig: serviceProxyTLSConfig}, } + if *fK8sModeOffClusterServiceAccountBearerTokenFile != "" { + srv.InternalProxiedK8SClientConfig.BearerTokenFile = *fK8sModeOffClusterServiceAccountBearerTokenFile + } + srv.K8sProxyConfig = &proxy.Config{ TLSClientConfig: serviceProxyTLSConfig, HeaderBlacklist: []string{"Cookie", "X-CSRFToken"}, diff --git a/examples/.gitignore b/examples/.gitignore index e00475a5e07..8ce7cd424ab 100644 --- a/examples/.gitignore +++ b/examples/.gitignore @@ -1,3 +1,6 @@ ca.crt console-client-secret config.local.yaml +service-ca.crt +namespace +token diff --git a/examples/run-bridge.sh b/examples/run-bridge.sh index c3769fd0f86..5b5fc1501b6 100755 --- a/examples/run-bridge.sh +++ b/examples/run-bridge.sh @@ -14,6 +14,7 @@ set -exuo pipefail --user-auth-oidc-client-id=console-oauth-client \ --user-auth-oidc-client-secret-file=examples/console-client-secret \ --user-auth-oidc-ca-file=examples/ca.crt \ + --k8s-mode-off-cluster-service-account-bearer-token-file=examples/token \ --k8s-mode-off-cluster-alertmanager="$(oc -n openshift-config-managed get configmap monitoring-shared-config -o jsonpath='{.data.alertmanagerPublicURL}')" \ --k8s-mode-off-cluster-thanos="$(oc -n openshift-config-managed get configmap monitoring-shared-config -o jsonpath='{.data.thanosPublicURL}')" \ $@ diff --git a/examples/sa-secrets.yaml b/examples/sa-secrets.yaml deleted file mode 100644 index 4538744c514..00000000000 --- a/examples/sa-secrets.yaml +++ /dev/null @@ -1,26 +0,0 @@ -apiVersion: v1 -kind: Secret -metadata: - name: builder-token - namespace: default - annotations: - kubernetes.io/service-account.name: builder -type: kubernetes.io/service-account-token ---- -apiVersion: v1 -kind: Secret -metadata: - name: default-token - namespace: default - annotations: - kubernetes.io/service-account.name: default -type: kubernetes.io/service-account-token ---- -apiVersion: v1 -kind: Secret -metadata: - name: deployer-token - namespace: default - annotations: - kubernetes.io/service-account.name: deployer -type: kubernetes.io/service-account-token diff --git a/examples/secret.yaml b/examples/secret.yaml new file mode 100644 index 00000000000..2b2a4ef99f1 --- /dev/null +++ b/examples/secret.yaml @@ -0,0 +1,8 @@ +apiVersion: v1 +kind: Secret +metadata: + name: off-cluster-token + namespace: openshift-console + annotations: + kubernetes.io/service-account.name: console +type: kubernetes.io/service-account-token diff --git a/pkg/auth/oauth2/auth_openshift.go b/pkg/auth/oauth2/auth_openshift.go index 88a64a920cc..cc1bcbaa4fd 100644 --- a/pkg/auth/oauth2/auth_openshift.go +++ b/pkg/auth/oauth2/auth_openshift.go @@ -1,13 +1,11 @@ package oauth2 import ( - "bytes" "context" "crypto/sha256" "encoding/base64" "encoding/json" "fmt" - "io/ioutil" "net/http" "net/url" "strings" @@ -17,6 +15,7 @@ import ( authv1 "k8s.io/api/authentication/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/client-go/kubernetes" "k8s.io/client-go/rest" "k8s.io/klog/v2" @@ -28,14 +27,13 @@ import ( "github.com/openshift/console/pkg/serverutils/asynccache" ) -const tokenReviewPath = "/apis/authentication.k8s.io/v1/tokenreviews" - // openShiftAuth implements OpenShift Authentication as defined in: // https://access.redhat.com/documentation/en-us/openshift_container_platform/4.9/html/authentication_and_authorization/understanding-authentication type openShiftAuth struct { *oidcConfig - k8sClient *http.Client + k8sHTTPClient *http.Client + internalK8sClientset *kubernetes.Clientset oauthEndpointCache *asynccache.AsyncCache[*oidcDiscovery] } @@ -59,14 +57,18 @@ func validateAbsURL(value string) error { return nil } -func newOpenShiftAuth(ctx context.Context, k8sClient *http.Client, c *oidcConfig) (loginMethod, error) { +func newOpenShiftAuth(ctx context.Context, k8sHTTPClient *http.Client, c *oidcConfig) (loginMethod, error) { + internalK8sClientset, err := kubernetes.NewForConfig(c.internalK8sConfig) + if err != nil { + return nil, fmt.Errorf("failed to create internal K8s Clientset: %v", err) + } o := &openShiftAuth{ - oidcConfig: c, - k8sClient: k8sClient, + oidcConfig: c, + k8sHTTPClient: k8sHTTPClient, + internalK8sClientset: internalK8sClientset, } // TODO: repeat the discovery several times as in the auth.go logic - var err error o.oauthEndpointCache, err = asynccache.NewAsyncCache[*oidcDiscovery](ctx, 5*time.Minute, o.getOIDCDiscoveryInternal) if err != nil { return nil, fmt.Errorf("failed to construct OAuth endpoint cache: %w", err) @@ -90,7 +92,7 @@ func (o *openShiftAuth) getOIDCDiscoveryInternal(ctx context.Context) (*oidcDisc return nil, err } - resp, err := o.k8sClient.Do(req.WithContext(ctx)) + resp, err := o.k8sHTTPClient.Do(req.WithContext(ctx)) if err != nil { return nil, err } @@ -201,7 +203,7 @@ func (o *openShiftAuth) logout(w http.ResponseWriter, r *http.Request) { configWithBearerToken := &rest.Config{ Host: "https://" + k8sURL.Host, - Transport: o.k8sClient.Transport, + Transport: o.k8sHTTPClient.Transport, BearerToken: cookie.Value, Timeout: 30 * time.Second, } @@ -226,13 +228,7 @@ func (o *openShiftAuth) LogoutRedirectURL() string { return o.logoutRedirectOverride } -func (o *openShiftAuth) reviewToken(token string) (*authv1.TokenReview, error) { - tokenReviewURL, err := url.Parse(o.issuerURL) - if err != nil { - return nil, err - } - tokenReviewURL.Path = tokenReviewPath - +func (o *openShiftAuth) reviewToken(ctx context.Context, token string) (*authv1.TokenReview, error) { tokenReview := &authv1.TokenReview{ TypeMeta: metav1.TypeMeta{ APIVersion: "authentication.k8s.io/v1", @@ -243,45 +239,20 @@ func (o *openShiftAuth) reviewToken(token string) (*authv1.TokenReview, error) { }, } - tokenReviewJSON, err := json.Marshal(tokenReview) - if err != nil { - return nil, err - } + tokenReview, err := o.internalK8sClientset. + AuthenticationV1(). + TokenReviews(). + Create(ctx, tokenReview, metav1.CreateOptions{}) - req, err := http.NewRequest(http.MethodPost, tokenReviewURL.String(), bytes.NewBuffer(tokenReviewJSON)) if err != nil { - return nil, err - } - - req.Header.Set("Authorization", fmt.Sprintf("Bearer %s", o.internalK8sConfig.BearerToken)) - - res, err := o.k8sClient.Do(req) - if err != nil { - return nil, err - } - defer res.Body.Close() - - if res.StatusCode != http.StatusCreated { - return nil, fmt.Errorf("unable to validate user token: %v", res.Status) - } - - body, err := ioutil.ReadAll(res.Body) - if err != nil { - return nil, err - } - - // Unmarshal the response into a TokenReview object - var responseTokenReview authv1.TokenReview - err = json.Unmarshal(body, &responseTokenReview) - if err != nil { - return nil, err + return nil, fmt.Errorf("failed to create TokenReview: %v", err) } // Check if the token is authenticated - if !responseTokenReview.Status.Authenticated { + if !tokenReview.Status.Authenticated { err := fmt.Errorf("invalid token: %v", token) - if responseTokenReview.Status.Error != "" { - err = fmt.Errorf("invalid token: %s", responseTokenReview.Status.Error) + if tokenReview.Status.Error != "" { + err = fmt.Errorf("invalid token: %s", tokenReview.Status.Error) } return nil, err } @@ -299,22 +270,16 @@ func (o *openShiftAuth) Authenticate(_ http.ResponseWriter, r *http.Request) (*a return nil, fmt.Errorf("unauthenticated, no value for cookie %s", sessions.OpenshiftAccessTokenCookieName) } - if o.internalK8sConfig.BearerToken != "" { - tokenReviewResponse, err := o.reviewToken(cookie.Value) - if err != nil { - klog.Errorf("failed to authenticate user token: %v", err) - return nil, err - } - return &auth.User{ - Token: cookie.Value, - Username: tokenReviewResponse.Status.User.Username, - ID: tokenReviewResponse.Status.User.UID, - }, nil + tokenReviewResponse, err := o.reviewToken(r.Context(), cookie.Value) + if err != nil { + klog.Errorf("failed to authenticate user token: %v", err) + return nil, err } - klog.V(4).Info("TokenReview skipped, no bearer token is set on internal K8s rest config") return &auth.User{ - Token: cookie.Value, + Token: cookie.Value, + Username: tokenReviewResponse.Status.User.Username, + ID: tokenReviewResponse.Status.User.UID, }, nil }