Skip to content

Commit

Permalink
Ensure that the correct cluster is queried by authgate for catalog. (#…
Browse files Browse the repository at this point in the history
…2038)

* Ensure that the correct cluster is queried by authgate for catalog.

* Use the kube helper to create the cluster-aware config.

* Lint
  • Loading branch information
absoludity authored Sep 16, 2020
1 parent 40bfd00 commit e49b5ee
Show file tree
Hide file tree
Showing 7 changed files with 39 additions and 19 deletions.
2 changes: 1 addition & 1 deletion chart/kubeapps/values.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ enableIPv6: false
## the name you are assigning to the cluster on which Kubeapps is itself installed. Specifying more than
## one cluster without an apiServiceURL will cause the chart display an error.
## The base64-encoded certificateAuthorityData can be obtained from the additional cluster's kube config
## file, for example:
## file, for example, to get the ca data for the 0th cluster in your config (adjust the index 0 as necessary):
## kubectl --kubeconfig ~/.kube/kind-config-kubeapps-additional config view --raw -o jsonpath='{.clusters[0].cluster.certificate-authority-data}'
#
# clusters:
Expand Down
2 changes: 1 addition & 1 deletion cmd/kubeops/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -129,7 +129,7 @@ func main() {
// TODO(mnelson) remove this reverse proxy once the haproxy frontend
// proxies requests directly to the assetsvc. Move the authz to the
// assetsvc itself.
authGate := auth.AuthGate(kubeappsNamespace)
authGate := auth.AuthGate(clustersConfig, kubeappsNamespace)
parsedAssetsvcURL, err := url.Parse(assetsvcURL)
if err != nil {
log.Fatalf("Unable to parse the assetsvc URL: %v", err)
Expand Down
4 changes: 3 additions & 1 deletion cmd/tiller-proxy/internal/handler/handler.go
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ import (
"github.com/kubeapps/kubeapps/pkg/auth"
chartUtils "github.com/kubeapps/kubeapps/pkg/chart"
"github.com/kubeapps/kubeapps/pkg/handlerutil"
"github.com/kubeapps/kubeapps/pkg/kube"
proxy "github.com/kubeapps/kubeapps/pkg/proxy"
log "github.com/sirupsen/logrus"
)
Expand All @@ -47,6 +48,7 @@ type TillerProxy struct {
ListLimit int
ChartClient chartUtils.Resolver
ProxyClient proxy.TillerClient
ClustersConfig kube.ClustersConfig
}

func (h *TillerProxy) logStatus(name string) {
Expand Down Expand Up @@ -272,7 +274,7 @@ func (h *TillerProxy) forbiddenActionsForRelease(req *http.Request, namespace, v
}

func (h *TillerProxy) forbiddenActionsForManifest(req *http.Request, namespace, verb, manifest string) ([]auth.Action, error) {
userAuth, err := h.CheckerForRequest(req)
userAuth, err := h.CheckerForRequest(h.ClustersConfig, req)
if err != nil {
return nil, err
}
Expand Down
3 changes: 2 additions & 1 deletion cmd/tiller-proxy/internal/handler/handler_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ import (
"github.com/kubeapps/kubeapps/pkg/auth"
authFake "github.com/kubeapps/kubeapps/pkg/auth/fake"
chartFake "github.com/kubeapps/kubeapps/pkg/chart/fake"
"github.com/kubeapps/kubeapps/pkg/kube"
proxyFake "github.com/kubeapps/kubeapps/pkg/proxy/fake"
)

Expand Down Expand Up @@ -412,7 +413,7 @@ func TestActions(t *testing.T) {
ProxyClient: proxy,
}
req := httptest.NewRequest("GET", fmt.Sprintf("http://foo.bar%s", test.RequestQuery), strings.NewReader(test.RequestBody))
handler.CheckerForRequest = func(req *http.Request) (auth.Checker, error) {
handler.CheckerForRequest = func(clustersConfig kube.ClustersConfig, req *http.Request) (auth.Checker, error) {
return &authFake.FakeAuth{
ForbiddenActions: test.ForbiddenActions,
}, nil
Expand Down
17 changes: 14 additions & 3 deletions cmd/tiller-proxy/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -136,7 +136,16 @@ func main() {
log.Fatalf("POD_NAMESPACE should be defined")
}

kubeHandler, err := kube.NewHandler(kubeappsNamespace, kube.ClustersConfig{KubeappsClusterName: kubeappsClusterName})
// Tiller proxy does not support multi-cluster.
clustersConfig := kube.ClustersConfig{
KubeappsClusterName: kubeappsClusterName,
Clusters: map[string]kube.ClusterConfig{
kubeappsClusterName: {
Name: kubeappsClusterName,
},
},
}
kubeHandler, err := kube.NewHandler(kubeappsNamespace, clustersConfig)
if err != nil {
log.Fatalf("Failed to create handler: %v", err)
}
Expand All @@ -156,6 +165,7 @@ func main() {
ListLimit: listLimit,
ChartClient: chartClient,
ProxyClient: proxy,
ClustersConfig: clustersConfig,
}

// Routes
Expand All @@ -175,7 +185,7 @@ func main() {
apiv1.Methods("DELETE").Path("/clusters/{cluster}/namespaces/{namespace}/releases/{releaseName}").Handler(handlerutil.WithParams(h.DeleteRelease))

// Backend routes unrelated to tiller-proxy functionality.
err = backendHandlers.SetupDefaultRoutes(r.PathPrefix("/backend/v1").Subrouter(), kube.ClustersConfig{KubeappsClusterName: kubeappsClusterName})
err = backendHandlers.SetupDefaultRoutes(r.PathPrefix("/backend/v1").Subrouter(), clustersConfig)
if err != nil {
log.Fatalf("Unable to setup backend routes: %+v", err)
}
Expand All @@ -194,7 +204,8 @@ func main() {
// Logos don't require authentication so bypass that step. Nor are they cluster-aware as they're
// embedded as links in the stored chart data.
assetsvcRouter.Methods("GET").Path("/v1/ns/{ns}/assets/{repo}/{id}/logo").Handler(http.StripPrefix(assetsvcPrefix, assetsvcProxy))
authGate := auth.AuthGate(kubeappsNamespace)

authGate := auth.AuthGate(clustersConfig, kubeappsNamespace)
assetsvcRouter.PathPrefix("/v1/clusters/{cluster}/namespaces/{namespace}/").Handler(negroni.New(
authGate,
negroni.Wrap(http.StripPrefix(assetsvcPrefix, assetsvcProxy)),
Expand Down
10 changes: 6 additions & 4 deletions pkg/auth/auth.go
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ import (
"regexp"
"strings"

"github.com/kubeapps/kubeapps/pkg/kube"
yamlUtils "github.com/kubeapps/kubeapps/pkg/yaml"
authorizationapi "k8s.io/api/authorization/v1"
k8sErrors "k8s.io/apimachinery/pkg/api/errors"
Expand Down Expand Up @@ -91,14 +92,15 @@ type Checker interface {
}

// NewAuth creates an auth agent
func NewAuth(token string) (*UserAuth, error) {
func NewAuth(token, clusterName string, clustersConfig kube.ClustersConfig) (*UserAuth, error) {
config, err := rest.InClusterConfig()
if err != nil {
return nil, err
}
// Overwrite default token
config.BearerToken = token
config.BearerTokenFile = "" // /~https://github.com/kubeapps/kubeapps/pull/1359#issuecomment-564077326
config, err = kube.NewClusterConfig(config, token, clusterName, clustersConfig)
if err != nil {
return nil, err
}
kubeClient, err := kubernetes.NewForConfig(config)
if err != nil {
return nil, err
Expand Down
20 changes: 12 additions & 8 deletions pkg/auth/authgate.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import (
"github.com/gorilla/mux"
"github.com/kubeapps/common/response"
"github.com/kubeapps/kubeapps/pkg/dbutils"
"github.com/kubeapps/kubeapps/pkg/kube"
"github.com/urfave/negroni"
)

Expand All @@ -16,26 +17,27 @@ const tokenPrefix = "Bearer "

// CheckerForRequest defines a function type so we can also inject a fake for tests
// rather than setting a context value.
type CheckerForRequest func(req *http.Request) (Checker, error)
type CheckerForRequest func(clustersConfig kube.ClustersConfig, req *http.Request) (Checker, error)

func AuthCheckerForRequest(req *http.Request) (Checker, error) {
func AuthCheckerForRequest(clustersConfig kube.ClustersConfig, req *http.Request) (Checker, error) {
token := ExtractToken(req.Header.Get("Authorization"))
if token == "" {
return nil, fmt.Errorf("Authorization token missing")
}
return NewAuth(token)
clusterName := mux.Vars(req)["cluster"]
return NewAuth(token, clusterName, clustersConfig)
}

// AuthGate implements middleware to check if the user has access to read from
// AuthGate implements middleware to check if the user has access to charts from
// the specific namespace before continuing.
// * If the path being handled by the
// AuthGate middleware does not include the 'namespace' mux var, or the value
// is _all, then the check is for cluster-wide access.
// * If the namespace is the global chart namespace (ie. kubeappsNamespace) then
// we allow read access regardless.
func AuthGate(kubeappsNamespace string) negroni.HandlerFunc {
func AuthGate(clustersConfig kube.ClustersConfig, kubeappsNamespace string) negroni.HandlerFunc {
return func(w http.ResponseWriter, req *http.Request, next http.HandlerFunc) {
userAuth, err := AuthCheckerForRequest(req)
userAuth, err := AuthCheckerForRequest(clustersConfig, req)
if err != nil {
response.NewErrorResponse(http.StatusUnauthorized, err.Error()).Write(w)
return
Expand All @@ -45,9 +47,11 @@ func AuthGate(kubeappsNamespace string) negroni.HandlerFunc {
namespace = ""
}

// If the request is for the global public charts (ie. kubeappsNamespace)
// we do not check authz.
// The auth-gate is used only for access to the asset-svc and the functionality should be
// moved to the assetsvc itself if and when the assetsvc is updated to be cluster aware.
authz := false
// TODO(absoludity): Update to allow access to assets from the global kubeapps namespace
// on the kubeapps cluster only. See #2037.
if namespace == kubeappsNamespace {
authz = true
} else {
Expand Down

0 comments on commit e49b5ee

Please sign in to comment.