Skip to content

Commit

Permalink
Update to pass http Header as well as context for authz. (#6151)
Browse files Browse the repository at this point in the history
<!--
Before you open the request please review the following guidelines and
tips to help it be more easily integrated:

 - Describe the scope of your change - i.e. what the change does.
 - Describe any known limitations with your change.
- Please run any tests or examples that can exercise your modified code.

 Thank you for contributing!
 -->

### Description of the change

<!-- Describe the scope of your change - i.e. what the change does. -->
This PR does some background prep, updating the config getter signature
to include the http headers (which the connect gRPC uses).

### Benefits

<!-- What benefits will be realized by the code change? -->
Following PRs just have changes for actual plugins.


### Possible drawbacks

<!-- Describe any known limitations with your change -->

### Applicable issues

<!-- Enter any applicable Issues here (You can reference an issue using
#) -->

- ref #6013 

### Additional information

<!-- If there's anything else that's important and relevant to your pull
request, mention that information here.-->

Signed-off-by: Michael Nelson <minelson@vmware.com>
  • Loading branch information
absoludity authored Mar 29, 2023
1 parent 28c036b commit 567fc59
Show file tree
Hide file tree
Showing 7 changed files with 56 additions and 22 deletions.
39 changes: 25 additions & 14 deletions cmd/kubeapps-apis/core/plugins/v1alpha1/plugins.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import (
"context"
"fmt"
"io/fs"
"net/http"
"os"
"path"
"path/filepath"
Expand Down Expand Up @@ -315,10 +316,10 @@ func createConfigGetter(serveOpts core.ServeOptions, clustersConfig kube.Cluster
func createConfigGetterWithParams(inClusterConfig *rest.Config, serveOpts core.ServeOptions, clustersConfig kube.ClustersConfig) (core.KubernetesConfigGetter, error) {
// return the closure function that takes the context, but preserving the required scope,
// 'inClusterConfig' and 'config'
return func(ctx context.Context, cluster string) (*rest.Config, error) {
return func(ctx context.Context, headers http.Header, cluster string) (*rest.Config, error) {
log.V(4).Infof("+clientGetter.GetClient")
var err error
token, err := extractToken(ctx)
token, err := extractToken(ctx, headers)
if err != nil {
return nil, status.Errorf(codes.Unauthenticated, "invalid authorization metadata: %v", err)
}
Expand Down Expand Up @@ -348,22 +349,32 @@ func createConfigGetterWithParams(inClusterConfig *rest.Config, serveOpts core.S
}, nil
}

// extractToken returns the token passed through the gRPC request in the "authorization" metadata in the context
// extractToken returns the token passed through the gRPC request in the
// "authorization" metadata in the context (improbable-eng grpc) or headers
// (connect gRPC)
// It is equivalent to the "Authorization" usual HTTP 1 header
// For instance: authorization="Bearer abc" will return "abc"
func extractToken(ctx context.Context) (string, error) {
// per https://github.com/vmware-tanzu/kubeapps/issues/3560
// extractToken() to raise an error if there is no metadata with the context.
// note, the caller will wrap this as a codes.Unauthenticated status
md, ok := metadata.FromIncomingContext(ctx)
if !ok {
return "", fmt.Errorf("missing authorization metadata")
func extractToken(ctx context.Context, headers http.Header) (string, error) {
bearerToken := headers.Get("Authorization")

if bearerToken == "" {
// per https://github.com/vmware-tanzu/kubeapps/issues/3560
// extractToken() to raise an error if there is no metadata with the context.
// note, the caller will wrap this as a codes.Unauthenticated status
md, ok := metadata.FromIncomingContext(ctx)
if !ok {
return "", fmt.Errorf("missing authorization metadata")
}

// metadata is always lowercased
if len(md["authorization"]) > 0 {
bearerToken = md["authorization"][0]
}
}

// metadata is always lowercased
if len(md["authorization"]) > 0 {
if strings.HasPrefix(md["authorization"][0], "Bearer ") {
return strings.TrimPrefix(md["authorization"][0], "Bearer "), nil
if len(bearerToken) > 0 {
if strings.HasPrefix(bearerToken, "Bearer ") {
return strings.TrimPrefix(bearerToken, "Bearer "), nil
} else {
return "", fmt.Errorf("malformed authorization metadata")
}
Expand Down
21 changes: 19 additions & 2 deletions cmd/kubeapps-apis/core/plugins/v1alpha1/plugins_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import (
"context"
"fmt"
"io/fs"
"net/http"
"path/filepath"
"runtime"
"testing"
Expand Down Expand Up @@ -288,6 +289,7 @@ func TestExtractToken(t *testing.T) {
name string
contextKey string
contextValue string
headers http.Header
expectedToken string
expectedErr error
}{
Expand All @@ -298,6 +300,14 @@ func TestExtractToken(t *testing.T) {
expectedToken: "abc",
expectedErr: nil,
},
{
name: "it returns the expected token without error for a valid 'Authorization' header",
contextKey: "",
contextValue: "",
headers: http.Header{"Authorization": []string{"Bearer abc"}},
expectedToken: "abc",
expectedErr: nil,
},
{
name: "it returns no token with an error if the 'authorization' metadata value is invalid",
contextKey: "authorization",
Expand All @@ -321,7 +331,7 @@ func TestExtractToken(t *testing.T) {
tc.contextKey: tc.contextValue,
}))

token, err := extractToken(context)
token, err := extractToken(context, tc.headers)

if tc.expectedErr != nil && err != nil {
if got, want := err.Error(), tc.expectedErr.Error(); !cmp.Equal(want, got) {
Expand Down Expand Up @@ -366,6 +376,7 @@ func TestCreateConfigGetterWithParams(t *testing.T) {
cluster string
contextKey string
contextValue string
headers http.Header
expectedAPIHost string
expectedErrMsg error
}{
Expand Down Expand Up @@ -397,6 +408,12 @@ func TestCreateConfigGetterWithParams(t *testing.T) {
expectedAPIHost: OtherK8sAPI,
expectedErrMsg: status.Errorf(codes.Unauthenticated, "invalid authorization metadata: missing authorization metadata"),
},
{
name: "it creates the config for the default cluster when passing a valid value for the authorization headers",
headers: http.Header{"Authorization": []string{"Bearer token-value"}},
expectedAPIHost: DefaultK8sAPI,
expectedErrMsg: nil,
},
}

for _, tc := range testCases {
Expand All @@ -414,7 +431,7 @@ func TestCreateConfigGetterWithParams(t *testing.T) {
t.Fatalf("in %s: fail creating the configGetter: %+v", tc.name, err)
}

restConfig, err := configGetter(ctx, tc.cluster)
restConfig, err := configGetter(ctx, tc.headers, tc.cluster)
if tc.expectedErrMsg != nil && err != nil {
if got, want := err.Error(), tc.expectedErrMsg.Error(); !cmp.Equal(want, got) {
t.Errorf("in %s: mismatch (-want +got):\n%s", tc.name, cmp.Diff(want, got))
Expand Down
3 changes: 2 additions & 1 deletion cmd/kubeapps-apis/core/serveopts.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ package core

import (
"context"
"net/http"

"github.com/grpc-ecosystem/grpc-gateway/v2/runtime"
"google.golang.org/grpc"
Expand Down Expand Up @@ -37,4 +38,4 @@ type GatewayHandlerArgs struct {
// KubernetesConfigGetter is a function type used throughout the apis server so
// that call-sites don't need to know how to obtain an authenticated client, but
// rather can just pass the request context and the cluster to get one.
type KubernetesConfigGetter func(ctx context.Context, cluster string) (*rest.Config, error)
type KubernetesConfigGetter func(ctx context.Context, headers http.Header, cluster string) (*rest.Config, error)
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import (
"context"
"encoding/json"
"fmt"
"net/http"
"os"

"github.com/cppforlife/go-cli-ui/ui"
Expand Down Expand Up @@ -126,7 +127,7 @@ func NewServer(configGetter core.KubernetesConfigGetter, clientQPS float32, clie
return ctlapp.Apps{}, ctlres.IdentifiedResources{}, nil, ctlres.ResourceFilter{}, status.Errorf(codes.Internal, "configGetter arg required")
}
// Retrieve the k8s REST client from the configGetter
config, err := configGetter(ctx, cluster)
config, err := configGetter(ctx, http.Header{}, cluster)
if err != nil {
return ctlapp.Apps{}, ctlres.IdentifiedResources{}, nil, ctlres.ResourceFilter{}, status.Errorf(codes.FailedPrecondition, "unable to get config due to: %v", err)
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ package clientgetter

import (
"context"
"net/http"

"github.com/vmware-tanzu/kubeapps/cmd/kubeapps-apis/core"
"google.golang.org/grpc/codes"
Expand Down Expand Up @@ -187,7 +188,7 @@ func buildClientsProviderFunction(configGetter core.KubernetesConfigGetter, opti
if configGetter == nil {
return nil, status.Errorf(codes.Internal, "configGetter arg required")
}
config, err := configGetter(ctx, cluster)
config, err := configGetter(ctx, http.Header{}, cluster)
if err != nil {
code := codes.FailedPrecondition
if status.Code(err) == codes.Unauthenticated {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,8 @@ package helm

import (
"context"
"net/http"

"github.com/vmware-tanzu/kubeapps/cmd/kubeapps-apis/core"
"github.com/vmware-tanzu/kubeapps/cmd/kubeapps-apis/plugins/pkg/helm/agent"
"google.golang.org/grpc/codes"
Expand All @@ -22,7 +24,7 @@ func NewHelmActionConfigGetter(configGetter core.KubernetesConfigGetter, cluster
if configGetter == nil {
return nil, status.Errorf(codes.Internal, "configGetter arg required")
}
config, err := configGetter(ctx, cluster)
config, err := configGetter(ctx, http.Header{}, cluster)
if err != nil {
return nil, status.Errorf(codes.FailedPrecondition, "unable to get config due to: %v", err)
}
Expand Down
5 changes: 3 additions & 2 deletions cmd/kubeapps-apis/plugins/resources/v1alpha1/server.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import (
"context"
"encoding/json"
"fmt"
"net/http"
"os"
"sync"

Expand Down Expand Up @@ -165,7 +166,7 @@ func NewServer(configGetter core.KubernetesConfigGetter, clientQPS float32, clie

func newClientGetter(configGetter core.KubernetesConfigGetter, useServiceAccount bool, clustersConfig kube.ClustersConfig) (clientgetter.ClientProviderInterface, error) {

customConfigGetter := func(ctx context.Context, cluster string) (*rest.Config, error) {
customConfigGetter := func(ctx context.Context, headers http.Header, cluster string) (*rest.Config, error) {
if useServiceAccount {
// If a service account client getter has been requested, the service account
// to use depends on which cluster is targeted.
Expand All @@ -182,7 +183,7 @@ func newClientGetter(configGetter core.KubernetesConfigGetter, useServiceAccount
// Rest config for a *user* created here - must already have token? So
// it is at this point where we should *not* pass the user credential /
// token if it is not needed?
restConfig, err := configGetter(ctx, cluster)
restConfig, err := configGetter(ctx, http.Header{}, cluster)
if err != nil {
return nil, status.Errorf(codes.FailedPrecondition, "unable to get config : %v", err.Error())
}
Expand Down

0 comments on commit 567fc59

Please sign in to comment.