Skip to content
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

Update to pass http Header as well as context for authz. #6151

Merged
merged 1 commit into from
Mar 29, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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