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.

Sign up for GitHub

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

Jump to bottom

Allow non admin users to manage BSL #6589

Merged
merged 5 commits into from Mar 14, 2024
Merged

Allow non admin users to manage BSL #6589

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
a6f9859
allow other than admins to manage cbsl
ahmadhamzh Mar 7, 2024
b7ead75
add impersonated client
ahmadhamzh Mar 12, 2024
0cc2373
refactor code
ahmadhamzh Mar 13, 2024
543798f
improve errors
moelsayed Mar 14, 2024
b2fc8fd
review comments
moelsayed Mar 14, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
24 changes: 12 additions & 12 deletions modules/api/cmd/kubermatic-api/swagger.json
View file
Open in desktop
Expand Up @@ -11729,6 +11729,13 @@
],
"operationId": "listAWSSizesNoCredentialsV2",
"parameters": [
{
"type": "string",
"x-go-name": "Architecture",
"description": "architecture query parameter. Supports: arm64 and x64 types.",
"name": "architecture",
"in": "query"
},
{
"type": "string",
"x-go-name": "ProjectID",
Expand All @@ -11742,13 +11749,6 @@
"name": "cluster_id",
"in": "path",
"required": true
},
{
"type": "string",
"x-go-name": "Architecture",
"description": "architecture query parameter. Supports: arm64 and x64 types.",
"name": "architecture",
"in": "query"
}
],
"responses": {
Expand Down Expand Up @@ -12263,6 +12263,11 @@
],
"operationId": "listKubeVirtInstancetypesNoCredentials",
"parameters": [
{
"type": "string",
"name": "DatacenterName",
"in": "header"
},
{
"type": "string",
"x-go-name": "ProjectID",
Expand All @@ -12276,11 +12281,6 @@
"name": "cluster_id",
"in": "path",
"required": true
},
{
"type": "string",
"name": "DatacenterName",
"in": "header"
}
],
"responses": {
Expand Down
57 changes: 51 additions & 6 deletions modules/api/pkg/ee/clusterbackup/storage-location/handler.go
View file
Open in desktop
Expand Up @@ -96,11 +96,18 @@ const (
displayNameLabelKey = "csbl-display-name"
)

func ListCBSL(ctx context.Context, request interface{}, provider provider.BackupStorageProvider, projectProvider provider.ProjectProvider) ([]*apiv2.ClusterBackupStorageLocation, error) {
func ListCBSL(ctx context.Context, request interface{}, userInfoGetter provider.UserInfoGetter, provider provider.BackupStorageProvider, projectProvider provider.ProjectProvider) ([]*apiv2.ClusterBackupStorageLocation, error) {
req, ok := request.(listCbslReq)
if !ok {
return nil, utilerrors.NewBadRequest("invalid request")
}

_, err := userInfoGetter(ctx, req.ProjectID)

if err != nil {
return nil, err
}

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this essentially only queries the user roles and so on, but since you are not using them to check for anything (like in CreateCSBL), I believe it can be removed

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

same for the GetCSBL func

labelSet := map[string]string{
kubermaticv1.ProjectIDLabelKey: req.ProjectID,
}
Expand All @@ -122,11 +129,18 @@ func ListCBSL(ctx context.Context, request interface{}, provider provider.Backup
return resp, nil
}

func GetCSBL(ctx context.Context, request interface{}, provider provider.BackupStorageProvider, projectProvider provider.ProjectProvider) (*apiv2.ClusterBackupStorageLocation, error) {
func GetCSBL(ctx context.Context, request interface{}, userInfoGetter provider.UserInfoGetter, provider provider.BackupStorageProvider, projectProvider provider.ProjectProvider) (*apiv2.ClusterBackupStorageLocation, error) {
req, ok := request.(getCbslReq)
if !ok {
return nil, utilerrors.NewBadRequest("invalid request")
}

_, err := userInfoGetter(ctx, req.ProjectID)

if err != nil {
return nil, err
}

labelSet := map[string]string{
kubermaticv1.ProjectIDLabelKey: req.ProjectID,
}
Expand All @@ -144,11 +158,22 @@ func GetCSBL(ctx context.Context, request interface{}, provider provider.BackupS
}, nil
}

func CreateCBSL(ctx context.Context, request interface{}, provider provider.BackupStorageProvider, projectProvider provider.ProjectProvider) (*apiv2.ClusterBackupStorageLocation, error) {
func CreateCBSL(ctx context.Context, request interface{}, userInfoGetter provider.UserInfoGetter, provider provider.BackupStorageProvider, projectProvider provider.ProjectProvider) (*apiv2.ClusterBackupStorageLocation, error) {
req, ok := request.(createCbslReq)
if !ok {
return nil, utilerrors.NewBadRequest("invalid request")
}

user, err := userInfoGetter(ctx, req.ProjectID)

if err != nil {
return nil, err
}

if user.Roles.Has("viewers") {
return nil, fmt.Errorf("user with a viewer role is not permitted to create storage locations.")
}

cbslName := req.Body.Name
cbslSpec := req.Body.CBSLSpec.DeepCopy()
creds := req.Body.Credentials
Expand All @@ -168,21 +193,31 @@ func CreateCBSL(ctx context.Context, request interface{}, provider provider.Back
}, nil
}

func DeleteCBSL(ctx context.Context, request interface{}, provider provider.BackupStorageProvider, projectProvider provider.ProjectProvider) error {
func DeleteCBSL(ctx context.Context, request interface{}, userInfoGetter provider.UserInfoGetter, provider provider.BackupStorageProvider, projectProvider provider.ProjectProvider) error {
req, ok := request.(deleteCbslReq)
if !ok {
return utilerrors.NewBadRequest("invalid request")
}

err := provider.DeleteUnsecured(ctx, req.ClusterBackupStorageLocationName)
user, err := userInfoGetter(ctx, req.ProjectID)

if err != nil {
return err
}

if user.Roles.Has("viewers") {
return fmt.Errorf("user with a viewer role is not permitted to delete storage locations.")
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

my understanding from other code is that it is possible for a user to have both "viewer" as well as another role. As a result we also need to check if viewers is the only role the user has.

For example like this is done here

dashboard/modules/api/pkg/provider/kubernetes/cluster.go

Line 428 in 140d3a4

if userInfo.Roles.Has("viewers") && userInfo.Roles.Len() == 1 {

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same for Create and Patch as well


err = provider.DeleteUnsecured(ctx, req.ClusterBackupStorageLocationName)
if err != nil {
return err
}

return nil
}

func PatchCBSL(ctx context.Context, request interface{}, provider provider.BackupStorageProvider, projectProvider provider.ProjectProvider) (*apiv2.ClusterBackupStorageLocation, error) {
func PatchCBSL(ctx context.Context, request interface{}, userInfoGetter provider.UserInfoGetter, provider provider.BackupStorageProvider, projectProvider provider.ProjectProvider) (*apiv2.ClusterBackupStorageLocation, error) {
req, ok := request.(patchCbslReq)
if !ok {
return nil, utilerrors.NewBadRequest("invalid request")
Expand All @@ -197,6 +232,16 @@ func PatchCBSL(ctx context.Context, request interface{}, provider provider.Backu
return nil, err
}

user, err := userInfoGetter(ctx, req.ProjectID)

if err != nil {
return nil, err
}

if user.Roles.Has("viewers") {
return nil, fmt.Errorf("user with a viewer role is not permitted to edit storage locations.")
}

return &apiv2.ClusterBackupStorageLocation{
Name: patched.Name,
DisplayName: patched.Labels[displayNameLabelKey],
Expand Down
48 changes: 5 additions & 43 deletions modules/api/pkg/handler/v2/clusterbackup/storage-location/cbsl.go
View file
Open in desktop
Expand Up @@ -18,76 +18,38 @@ package storagelocation

import (
"context"
"fmt"
"net/http"

"github.com/go-kit/kit/endpoint"

"k8c.io/dashboard/v2/pkg/provider"
utilerrors "k8c.io/kubermatic/v2/pkg/util/errors"
)

func ListCBSLEndpoint(userInfoGetter provider.UserInfoGetter, provider provider.BackupStorageProvider, projectProvider provider.ProjectProvider) endpoint.Endpoint {
return func(ctx context.Context, req interface{}) (interface{}, error) {
userInfo, err := userInfoGetter(ctx, "")
if err != nil {
return nil, err
}
if !userInfo.IsAdmin {
return nil, utilerrors.New(http.StatusForbidden, fmt.Sprintf("forbidden: \"%s\" doesn't have admin rights", userInfo.Email))
}
return listCBSL(ctx, req, provider, projectProvider)
return listCBSL(ctx, req, userInfoGetter, provider, projectProvider)
}
}

func GetCBSLEndpoint(userInfoGetter provider.UserInfoGetter, provider provider.BackupStorageProvider, projectProvider provider.ProjectProvider) endpoint.Endpoint {
return func(ctx context.Context, req interface{}) (interface{}, error) {
userInfo, err := userInfoGetter(ctx, "")
if err != nil {
return nil, err
}
if !userInfo.IsAdmin {
return nil, utilerrors.New(http.StatusForbidden, fmt.Sprintf("forbidden: \"%s\" doesn't have admin rights", userInfo.Email))
}
return getCBSL(ctx, req, provider, projectProvider)
return getCBSL(ctx, req, userInfoGetter, provider, projectProvider)
}
}

func CreateCBSLEndpoint(userInfoGetter provider.UserInfoGetter, provider provider.BackupStorageProvider, projectProvider provider.ProjectProvider) endpoint.Endpoint {
return func(ctx context.Context, req interface{}) (interface{}, error) {
userInfo, err := userInfoGetter(ctx, "")
if err != nil {
return nil, err
}
if !userInfo.IsAdmin {
return nil, utilerrors.New(http.StatusForbidden, fmt.Sprintf("forbidden: \"%s\" doesn't have admin rights", userInfo.Email))
}
return createCBSL(ctx, req, provider, projectProvider)
return createCBSL(ctx, req, userInfoGetter, provider, projectProvider)
}
}

func DeleteCBSLEndpoint(userInfoGetter provider.UserInfoGetter, provider provider.BackupStorageProvider, projectProvider provider.ProjectProvider) endpoint.Endpoint {
return func(ctx context.Context, req interface{}) (interface{}, error) {
userInfo, err := userInfoGetter(ctx, "")
if err != nil {
return nil, err
}
if !userInfo.IsAdmin {
return nil, utilerrors.New(http.StatusForbidden, fmt.Sprintf("forbidden: \"%s\" doesn't have admin rights", userInfo.Email))
}
return nil, deleteCBSL(ctx, req, provider, projectProvider)
return nil, deleteCBSL(ctx, req, userInfoGetter, provider, projectProvider)
}
}

func PatchCBSLEndpoint(userInfoGetter provider.UserInfoGetter, provider provider.BackupStorageProvider, projectProvider provider.ProjectProvider) endpoint.Endpoint {
return func(ctx context.Context, req interface{}) (interface{}, error) {
userInfo, err := userInfoGetter(ctx, "")
if err != nil {
return nil, err
}
if !userInfo.IsAdmin {
return nil, utilerrors.New(http.StatusForbidden, fmt.Sprintf("forbidden: \"%s\" doesn't have admin rights", userInfo.Email))
}
return patchCBSL(ctx, req, provider, projectProvider)
return patchCBSL(ctx, req, userInfoGetter, provider, projectProvider)
}
}
5 changes: 5 additions & 0 deletions modules/api/pkg/handler/v2/clusterbackup/storage-location/wrappers_ce.go
View file
Open in desktop
Expand Up @@ -29,6 +29,7 @@ import (
func listCBSL(
_ context.Context,
_ interface{},
_ provider.UserInfoGetter,
_ provider.BackupStorageProvider,
_ provider.ProjectProvider,
) ([]*apiv2.ClusterBackupStorageLocation, error) {
Expand All @@ -38,6 +39,7 @@ func listCBSL(
func getCBSL(
_ context.Context,
_ interface{},
_ provider.UserInfoGetter,
_ provider.BackupStorageProvider,
_ provider.ProjectProvider,
) (*apiv2.ClusterBackupStorageLocation, error) {
Expand All @@ -47,6 +49,7 @@ func getCBSL(
func createCBSL(
_ context.Context,
_ interface{},
_ provider.UserInfoGetter,
_ provider.BackupStorageProvider,
_ provider.ProjectProvider,
) (*apiv2.ClusterBackupStorageLocation, error) {
Expand All @@ -56,6 +59,7 @@ func createCBSL(
func deleteCBSL(
_ context.Context,
_ interface{},
_ provider.UserInfoGetter,
_ provider.BackupStorageProvider,
_ provider.ProjectProvider,
) error {
Expand All @@ -65,6 +69,7 @@ func deleteCBSL(
func patchCBSL(
_ context.Context,
_ interface{},
_ provider.UserInfoGetter,
_ provider.BackupStorageProvider,
_ provider.ProjectProvider,
) (*apiv2.ClusterBackupStorageLocation, error) {
Expand Down
20 changes: 10 additions & 10 deletions modules/api/pkg/handler/v2/clusterbackup/storage-location/wrappers_ee.go
View file
Open in desktop
Expand Up @@ -27,24 +27,24 @@ import (
"k8c.io/dashboard/v2/pkg/provider"
)

func listCBSL(ctx context.Context, request interface{}, provider provider.BackupStorageProvider, projectProvider provider.ProjectProvider) ([]*apiv2.ClusterBackupStorageLocation, error) {
return storagelocation.ListCBSL(ctx, request, provider, projectProvider)
func listCBSL(ctx context.Context, request interface{}, userInfoGetter provider.UserInfoGetter, provider provider.BackupStorageProvider, projectProvider provider.ProjectProvider) ([]*apiv2.ClusterBackupStorageLocation, error) {
return storagelocation.ListCBSL(ctx, request, userInfoGetter, provider, projectProvider)
}

func getCBSL(ctx context.Context, request interface{}, provider provider.BackupStorageProvider, projectProvider provider.ProjectProvider) (*apiv2.ClusterBackupStorageLocation, error) {
return storagelocation.GetCSBL(ctx, request, provider, projectProvider)
func getCBSL(ctx context.Context, request interface{}, userInfoGetter provider.UserInfoGetter, provider provider.BackupStorageProvider, projectProvider provider.ProjectProvider) (*apiv2.ClusterBackupStorageLocation, error) {
return storagelocation.GetCSBL(ctx, request, userInfoGetter, provider, projectProvider)
}

func createCBSL(ctx context.Context, request interface{}, provider provider.BackupStorageProvider, projectProvider provider.ProjectProvider) (*apiv2.ClusterBackupStorageLocation, error) {
return storagelocation.CreateCBSL(ctx, request, provider, projectProvider)
func createCBSL(ctx context.Context, request interface{}, userInfoGetter provider.UserInfoGetter, provider provider.BackupStorageProvider, projectProvider provider.ProjectProvider) (*apiv2.ClusterBackupStorageLocation, error) {
return storagelocation.CreateCBSL(ctx, request, userInfoGetter, provider, projectProvider)
}

func deleteCBSL(ctx context.Context, request interface{}, provider provider.BackupStorageProvider, projectProvider provider.ProjectProvider) error {
return storagelocation.DeleteCBSL(ctx, request, provider, projectProvider)
func deleteCBSL(ctx context.Context, request interface{}, userInfoGetter provider.UserInfoGetter, provider provider.BackupStorageProvider, projectProvider provider.ProjectProvider) error {
return storagelocation.DeleteCBSL(ctx, request, userInfoGetter, provider, projectProvider)
}

func patchCBSL(ctx context.Context, request interface{}, provider provider.BackupStorageProvider, projectProvider provider.ProjectProvider) (*apiv2.ClusterBackupStorageLocation, error) {
return storagelocation.PatchCBSL(ctx, request, provider, projectProvider)
func patchCBSL(ctx context.Context, request interface{}, userInfoGetter provider.UserInfoGetter, provider provider.BackupStorageProvider, projectProvider provider.ProjectProvider) (*apiv2.ClusterBackupStorageLocation, error) {
return storagelocation.PatchCBSL(ctx, request, userInfoGetter, provider, projectProvider)
}

func DecodeListProjectCBSLReq(ctx context.Context, r *http.Request) (interface{}, error) {
Expand Down