Skip to content

Commit

Permalink
OCPBUGS-45371: Fix issue where TokenReview was not working as expected
Browse files Browse the repository at this point in the history
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
  • Loading branch information
TheRealJon committed Jan 16, 2025
1 parent e72cc4c commit bc0b3e8
Show file tree
Hide file tree
Showing 7 changed files with 51 additions and 99 deletions.
14 changes: 5 additions & 9 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -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):
Expand Down
5 changes: 5 additions & 0 deletions cmd/bridge/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -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")

Expand Down Expand Up @@ -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"},
Expand Down
3 changes: 3 additions & 0 deletions examples/.gitignore
Original file line number Diff line number Diff line change
@@ -1,3 +1,6 @@
ca.crt
console-client-secret
config.local.yaml
service-ca.crt
namespace
token
1 change: 1 addition & 0 deletions examples/run-bridge.sh
Original file line number Diff line number Diff line change
Expand Up @@ -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}')" \
$@
26 changes: 0 additions & 26 deletions examples/sa-secrets.yaml

This file was deleted.

8 changes: 8 additions & 0 deletions examples/secret.yaml
Original file line number Diff line number Diff line change
@@ -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
93 changes: 29 additions & 64 deletions pkg/auth/oauth2/auth_openshift.go
Original file line number Diff line number Diff line change
@@ -1,13 +1,11 @@
package oauth2

import (
"bytes"
"context"
"crypto/sha256"
"encoding/base64"
"encoding/json"
"fmt"
"io/ioutil"
"net/http"
"net/url"
"strings"
Expand All @@ -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"

Expand All @@ -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]
}
Expand All @@ -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)
Expand All @@ -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
}
Expand Down Expand Up @@ -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,
}
Expand All @@ -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",
Expand All @@ -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
}
Expand All @@ -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
}

Expand Down

0 comments on commit bc0b3e8

Please sign in to comment.