Skip to content

Commit

Permalink
feat(bff): user info endpoint (#630)
Browse files Browse the repository at this point in the history
* feat(bff): user info endpoint

Signed-off-by: Eder Ignatowicz <ignatowicz@gmail.com>

* Update clients/ui/bff/README.md

Co-authored-by: Griffin Sullivan <48397354+Griffin-Sullivan@users.noreply.github.com>
Signed-off-by: Eder Ignatowicz <ignatowicz@gmail.com>

---------

Signed-off-by: Eder Ignatowicz <ignatowicz@gmail.com>
Co-authored-by: Griffin Sullivan <48397354+Griffin-Sullivan@users.noreply.github.com>
  • Loading branch information
ederign and Griffin-Sullivan authored Dec 11, 2024
1 parent 20c04c7 commit 1449fb3
Show file tree
Hide file tree
Showing 11 changed files with 333 additions and 21 deletions.
5 changes: 5 additions & 0 deletions clients/ui/bff/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,7 @@ make docker-build
| URL Pattern | Handler | Action |
|----------------------------------------------------------------------------------------------|----------------------------------------------|-------------------------------------------------------------|
| GET /v1/healthcheck | HealthcheckHandler | Show application information. |
| GET /v1/user | UserHandler | Show "kubeflow-user-id" from header information. |
| GET /v1/model_registry | ModelRegistryHandler | Get all model registries, |
| GET /v1/model_registry/{model_registry_id}/registered_models | GetAllRegisteredModelsHandler | Gets a list of all RegisteredModel entities. |
| POST /v1/model_registry/{model_registry_id}/registered_models | CreateRegisteredModelHandler | Create a RegisteredModel entity. |
Expand All @@ -78,6 +79,10 @@ You will need to inject your requests with a kubeflow-userid header for authoriz
curl -i -H "kubeflow-userid: user@example.com" localhost:4000/api/v1/healthcheck
```
```
# GET /v1/user
curl -i -H "kubeflow-userid: user@example.com" localhost:4000/api/v1/user
```
```
# GET /v1/model_registry
curl -i -H "kubeflow-userid: user@example.com" localhost:4000/api/v1/model_registry
```
Expand Down
2 changes: 2 additions & 0 deletions clients/ui/bff/internal/api/app.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ const (
ModelVersionId = "model_version_id"
ModelArtifactId = "model_artifact_id"
HealthCheckPath = PathPrefix + "/healthcheck"
UserPath = PathPrefix + "/user"
ModelRegistryListPath = PathPrefix + "/model_registry"
ModelRegistryPath = ModelRegistryListPath + "/:" + ModelRegistryId
RegisteredModelListPath = ModelRegistryPath + "/registered_models"
Expand Down Expand Up @@ -103,6 +104,7 @@ func (app *App) Routes() http.Handler {
router.POST(ModelVersionArtifactListPath, app.AttachRESTClient(app.CreateModelArtifactByModelVersionHandler))

// Kubernetes client routes
router.GET(UserPath, app.UserHandler)
router.GET(ModelRegistryListPath, app.ModelRegistryHandler)
router.PATCH(ModelRegistryPath, app.AttachRESTClient(app.UpdateModelVersionHandler))

Expand Down
6 changes: 6 additions & 0 deletions clients/ui/bff/internal/api/middleware.go
Original file line number Diff line number Diff line change
Expand Up @@ -83,6 +83,12 @@ func (app *App) RequireAccessControl(next http.Handler) http.Handler {
return
}

// Skip SAR for user info
if r.URL.Path == UserPath {
next.ServeHTTP(w, r)
return
}

user := r.Header.Get(kubeflowUserId)
if user == "" {
app.forbiddenResponse(w, r, "missing kubeflow-userid header")
Expand Down
36 changes: 36 additions & 0 deletions clients/ui/bff/internal/api/user_handler.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,36 @@
package api

import (
"errors"
"github.com/julienschmidt/httprouter"
"github.com/kubeflow/model-registry/ui/bff/internal/models"
"net/http"
)

type UserEnvelope Envelope[*models.User, None]

func (app *App) UserHandler(w http.ResponseWriter, r *http.Request, _ httprouter.Params) {

userHeader := r.Header.Get(kubeflowUserId)
if userHeader == "" {
app.serverErrorResponse(w, r, errors.New("kubeflow-userid not present on header"))
return
}

user, err := app.repositories.User.GetUser(app.kubernetesClient, userHeader)
if err != nil {
app.serverErrorResponse(w, r, err)
return
}

userRes := UserEnvelope{
Data: user,
}

err = app.WriteJSON(w, http.StatusOK, userRes, nil)

if err != nil {
app.serverErrorResponse(w, r, err)
}

}
119 changes: 119 additions & 0 deletions clients/ui/bff/internal/api/user_handler_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,119 @@
package api

import (
"encoding/json"
"io"
"net/http"
"net/http/httptest"

"github.com/kubeflow/model-registry/ui/bff/internal/repositories"
. "github.com/onsi/ginkgo/v2"
. "github.com/onsi/gomega"
)

const (
KubeflowUserIDHeaderValue = "user@example.com"
DoraNonAdminUser = "doraNonAdmin@example.com"
)

var _ = Describe("TestUserHandler", func() {
Context("fetching user details", Ordered, func() {
var testApp App

BeforeAll(func() {
By("creating the test app")
testApp = App{
kubernetesClient: k8sClient,
repositories: repositories.NewRepositories(mockMRClient),
logger: logger,
}
})

It("should show that KubeflowUserIDHeaderValue (user@example.com) is a cluster-admin", func() {
By("creating the http request")
req, err := http.NewRequest(http.MethodGet, UserPath, nil)
Expect(err).NotTo(HaveOccurred())

req.Header.Set(kubeflowUserId, KubeflowUserIDHeaderValue)

By("creating the http test infrastructure")
rr := httptest.NewRecorder()

By("invoking the UserHandler")
testApp.UserHandler(rr, req, nil)
rs := rr.Result()
defer rs.Body.Close()
body, err := io.ReadAll(rs.Body)
Expect(err).NotTo(HaveOccurred())

By("unmarshalling the user response")
var actual UserEnvelope
err = json.Unmarshal(body, &actual)
Expect(err).NotTo(HaveOccurred())
Expect(rr.Code).To(Equal(http.StatusOK))

By("checking that the user is cluster-admin")
Expect(actual.Data.UserID).To(Equal(KubeflowUserIDHeaderValue))
Expect(actual.Data.ClusterAdmin).To(BeTrue(), "Expected this user to be cluster-admin")
})

It("should show that DoraNonAdminUser (doraNonAdmin@example.com) is not a cluster-admin", func() {
By("creating the http request")
req, err := http.NewRequest(http.MethodGet, UserPath, nil)
Expect(err).NotTo(HaveOccurred())

req.Header.Set(kubeflowUserId, DoraNonAdminUser)

By("creating the http test infrastructure")
rr := httptest.NewRecorder()

By("invoking the UserHandler")
testApp.UserHandler(rr, req, nil)
rs := rr.Result()
defer rs.Body.Close()
body, err := io.ReadAll(rs.Body)
Expect(err).NotTo(HaveOccurred())

By("unmarshalling the user response")
var actual UserEnvelope
err = json.Unmarshal(body, &actual)
Expect(err).NotTo(HaveOccurred())
Expect(rr.Code).To(Equal(http.StatusOK))

By("checking that the user is not cluster-admin")
Expect(actual.Data.UserID).To(Equal(DoraNonAdminUser))
Expect(actual.Data.ClusterAdmin).To(BeFalse(), "Expected this user to not be cluster-admin")
})

It("should show that a random non-existent user is not a cluster-admin", func() {
randomUser := "bellaUser@example.com"

By("creating the http request")
req, err := http.NewRequest(http.MethodGet, UserPath, nil)
Expect(err).NotTo(HaveOccurred())

req.Header.Set(kubeflowUserId, randomUser)

By("creating the http test infrastructure")
rr := httptest.NewRecorder()

By("invoking the UserHandler")
testApp.UserHandler(rr, req, nil)
rs := rr.Result()
defer rs.Body.Close()
body, err := io.ReadAll(rs.Body)
Expect(err).NotTo(HaveOccurred())

By("unmarshalling the user response")
var actual UserEnvelope
err = json.Unmarshal(body, &actual)
Expect(err).NotTo(HaveOccurred())
Expect(rr.Code).To(Equal(http.StatusOK))

By("checking that the user is not cluster-admin")
Expect(actual.Data.UserID).To(Equal(randomUser))
Expect(actual.Data.ClusterAdmin).To(BeFalse(), "Expected this user to not be cluster-admin")
})
})

})
28 changes: 28 additions & 0 deletions clients/ui/bff/internal/integrations/k8s.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import (
helper "github.com/kubeflow/model-registry/ui/bff/internal/helpers"
authv1 "k8s.io/api/authorization/v1"
corev1 "k8s.io/api/core/v1"
rbacv1 "k8s.io/api/rbac/v1"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/client-go/kubernetes"
"k8s.io/client-go/rest"
Expand All @@ -27,6 +28,7 @@ type KubernetesClientInterface interface {
Shutdown(ctx context.Context, logger *slog.Logger) error
IsInCluster() bool
PerformSAR(user string) (bool, error)
IsClusterAdmin(user string) (bool, error)
}

type ServiceDetails struct {
Expand Down Expand Up @@ -286,3 +288,29 @@ func (kc *KubernetesClient) PerformSAR(user string) (bool, error) {

return true, nil
}

func (kc *KubernetesClient) IsClusterAdmin(user string) (bool, error) {
//using a context here, because checking ClusterRoleBindings could be expensive in large clusters
ctx, cancel := context.WithTimeout(context.Background(), 10*time.Second)
defer cancel()

clusterRoleBindings := &rbacv1.ClusterRoleBindingList{}
err := kc.ControllerRuntimeClient.List(ctx, clusterRoleBindings)
if err != nil {
return false, fmt.Errorf("failed to list ClusterRoleBindings: %w", err)
}

for _, crb := range clusterRoleBindings.Items {
if crb.RoleRef.Name != "cluster-admin" {
continue
}
for _, subject := range crb.Subjects {

if subject.Kind == "User" && subject.Name == user {
return true, nil
}
}
}

return false, nil
}
Loading

0 comments on commit 1449fb3

Please sign in to comment.