Skip to content

Commit

Permalink
feat(backend): Refactor authz to perform SubjectAccessReview. Fixes k…
Browse files Browse the repository at this point in the history
…ubeflow#3513 (kubeflow#4723)

* [Backend] Return proper error codes for failures during auth

* [Backend] Implement helpers to initialize a SubjectAccessReview client

In preparation of SubjectAccessReview, we implement some helpers to
create a new Kubernetes Authorization clientset and return the
SubjectAccessReview client.
We also define some fake clients to be used by future tests.

* [Backend] Introduce RBAC-related constants

In preparation of SubjectAccessReview, introduce RBAC groups, resources,
and verbs.

* [Backend] Extend managers with a SubjectAccessReviewClient

* [Backend] Refactor the authorization mechanism for requests

Authorization should be based on performing some action on a resource
living in a namespace. This commit refactors the authorization utilities
to reflect this and perform SubjectAccessReview.

This commit also deletes some tests based on old authn/authz mechanism.
A following commit will fix/extend the tests for the new mechanism

* [Backend] Adjust endpoints to pass resource attributes for authz

With KFAM authorization, we passed only the namespace attribute for
authorization. With SubjectAccessReview, we need a richer list of
attributes. Thus, we adjust endpoints to pass request details (resource
attributes) necessary for authorizing the request. We only change the
already authorized endpoints, not introducing any new checks.

* [Backend] Adjust apiserver/server tests to SubjectAccessReview

* [Backend] Purge KFAM

Since we no longer use KFAM, we may as well purge it

* [Backend] Update BUILD files

Signed-off-by: Ilias Katsakioris <[email protected]>

* [Manifests] Extend manifests for SubjectAccessReview

* API Server: Allow creating SubjectAccessReviews
* Add view/edit roles in a multi-user kustomization
  • Loading branch information
elikatsis authored and Jeffwan committed Dec 9, 2020
1 parent cc26378 commit 1567bee
Show file tree
Hide file tree
Showing 31 changed files with 890 additions and 564 deletions.
2 changes: 2 additions & 0 deletions backend/src/apiserver/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ go_library(
visibility = ["//visibility:private"],
deps = [
"//backend/api:go_default_library",
"//backend/src/apiserver/archive:go_default_library",
"//backend/src/apiserver/client:go_default_library",
"//backend/src/apiserver/common:go_default_library",
"//backend/src/apiserver/model:go_default_library",
Expand All @@ -21,6 +22,7 @@ go_library(
"@com_github_cenkalti_backoff//:go_default_library",
"@com_github_fsnotify_fsnotify//:go_default_library",
"@com_github_golang_glog//:go_default_library",
"@com_github_gorilla_mux//:go_default_library",
"@com_github_grpc_ecosystem_grpc_gateway//runtime:go_default_library",
"@com_github_jinzhu_gorm//:go_default_library",
"@com_github_jinzhu_gorm//dialects/sqlite:go_default_library",
Expand Down
15 changes: 5 additions & 10 deletions backend/src/apiserver/client/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -5,14 +5,14 @@ go_library(
srcs = [
"argo.go",
"argo_fake.go",
"kfam.go",
"kfam_fake.go",
"kubernetes_core.go",
"kubernetes_core_fake.go",
"minio.go",
"pod_fake.go",
"scheduled_workflow_fake.go",
"sql.go",
"subject_access_review.go",
"subject_access_review_fake.go",
"swf.go",
"swf_fake.go",
"workflow_fake.go",
Expand All @@ -33,6 +33,7 @@ go_library(
"@com_github_minio_minio_go//:go_default_library",
"@com_github_minio_minio_go//pkg/credentials:go_default_library",
"@com_github_pkg_errors//:go_default_library",
"@io_k8s_api//authorization/v1:go_default_library",
"@io_k8s_api//core/v1:go_default_library",
"@io_k8s_api//policy/v1beta1:go_default_library",
"@io_k8s_apimachinery//pkg/api/errors:go_default_library",
Expand All @@ -48,14 +49,8 @@ go_library(

go_test(
name = "go_default_test",
srcs = [
"kfam_test.go",
"sql_test.go",
],
srcs = ["sql_test.go"],
data = glob(["test/**/*"]), # keep
embed = [":go_default_library"],
deps = [
"@com_github_go_sql_driver_mysql//:go_default_library",
"@com_github_stretchr_testify//assert:go_default_library",
],
deps = ["@com_github_go_sql_driver_mysql//:go_default_library"],
)
100 changes: 0 additions & 100 deletions backend/src/apiserver/client/kfam.go

This file was deleted.

37 changes: 0 additions & 37 deletions backend/src/apiserver/client/kfam_fake.go

This file was deleted.

46 changes: 0 additions & 46 deletions backend/src/apiserver/client/kfam_test.go

This file was deleted.

5 changes: 1 addition & 4 deletions backend/src/apiserver/client/kubernetes_core.go
Original file line number Diff line number Diff line change
Expand Up @@ -41,10 +41,7 @@ func CreateKubernetesCoreOrFatal(initConnectionTimeout time.Duration) Kubernetes
var err error
var operation = func() error {
client, err = createKubernetesCore()
if err != nil {
return err
}
return nil
return err
}
b := backoff.NewExponentialBackOff()
b.MaxElapsedTime = initConnectionTimeout
Expand Down
61 changes: 61 additions & 0 deletions backend/src/apiserver/client/subject_access_review.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,61 @@
// Copyright 2020 Arrikto Inc.
//
// Licensed under the Apache License, Version 2.0 (the "License");
// you may not use this file except in compliance with the License.
// You may obtain a copy of the License at
//
// https://www.apache.org/licenses/LICENSE-2.0
//
// Unless required by applicable law or agreed to in writing, software
// distributed under the License is distributed on an "AS IS" BASIS,
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
// See the License for the specific language governing permissions and
// limitations under the License.

package client

import (
"time"

"github.com/cenkalti/backoff"
"github.com/golang/glog"
"github.com/pkg/errors"
authzv1 "k8s.io/api/authorization/v1"
"k8s.io/client-go/kubernetes"
"k8s.io/client-go/rest"
)

type SubjectAccessReviewInterface interface {
Create(sar *authzv1.SubjectAccessReview) (result *authzv1.SubjectAccessReview, err error)
}

func createSubjectAccessReviewClient() (SubjectAccessReviewInterface, error) {
restConfig, err := rest.InClusterConfig()
if err != nil {
return nil, errors.Wrap(err, "Failed to initialize kubernetes client.")
}

clientSet, err := kubernetes.NewForConfig(restConfig)
if err != nil {
return nil, errors.Wrap(err, "Failed to initialize kubernetes client set.")
}
return clientSet.AuthorizationV1().SubjectAccessReviews(), nil
}

// CreateSubjectAccessReviewClientOrFatal creates a new SubjectAccessReview client.
func CreateSubjectAccessReviewClientOrFatal(initConnectionTimeout time.Duration) SubjectAccessReviewInterface {
var client SubjectAccessReviewInterface
var err error
var operation = func() error {
client, err = createSubjectAccessReviewClient()
return err
}
b := backoff.NewExponentialBackOff()
b.MaxElapsedTime = initConnectionTimeout
err = backoff.Retry(operation, b)

if err != nil {
glog.Fatalf("Failed to create SubjectAccessReview client. Error: %v", err)
}
return client
}
63 changes: 63 additions & 0 deletions backend/src/apiserver/client/subject_access_review_fake.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,63 @@
// Copyright 2020 Arrikto Inc.
//
// Licensed under the Apache License, Version 2.0 (the "License");
// you may not use this file except in compliance with the License.
// You may obtain a copy of the License at
//
// https://www.apache.org/licenses/LICENSE-2.0
//
// Unless required by applicable law or agreed to in writing, software
// distributed under the License is distributed on an "AS IS" BASIS,
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
// See the License for the specific language governing permissions and
// limitations under the License.

package client

import (
"github.com/pkg/errors"
authzv1 "k8s.io/api/authorization/v1"
)

type FakeSubjectAccessReviewClient struct {
}

func (FakeSubjectAccessReviewClient) Create(*authzv1.SubjectAccessReview) (*authzv1.SubjectAccessReview, error) {
return &authzv1.SubjectAccessReview{Status: authzv1.SubjectAccessReviewStatus{
Allowed: true,
Denied: false,
Reason: "",
EvaluationError: "",
}}, nil
}

func NewFakeSubjectAccessReviewClient() FakeSubjectAccessReviewClient {
return FakeSubjectAccessReviewClient{}
}

type FakeSubjectAccessReviewClientUnauthorized struct {
}

func (FakeSubjectAccessReviewClientUnauthorized) Create(*authzv1.SubjectAccessReview) (*authzv1.SubjectAccessReview, error) {
return &authzv1.SubjectAccessReview{Status: authzv1.SubjectAccessReviewStatus{
Allowed: false,
Denied: false,
Reason: "this is not allowed",
EvaluationError: "",
}}, nil
}

func NewFakeSubjectAccessReviewClientUnauthorized() FakeSubjectAccessReviewClientUnauthorized {
return FakeSubjectAccessReviewClientUnauthorized{}
}

type FakeSubjectAccessReviewClientError struct {
}

func (FakeSubjectAccessReviewClientError) Create(*authzv1.SubjectAccessReview) (*authzv1.SubjectAccessReview, error) {
return nil, errors.New("failed to create subject access review")
}

func NewFakeSubjectAccessReviewClientError() FakeSubjectAccessReviewClientError {
return FakeSubjectAccessReviewClientError{}
}
Loading

0 comments on commit 1567bee

Please sign in to comment.