Skip to content

Commit

Permalink
fix deprecated autocluster AppInst org check (#1373)
Browse files Browse the repository at this point in the history
  • Loading branch information
gainsley authored Mar 16, 2021
1 parent e7ded52 commit 1cd5458
Show file tree
Hide file tree
Showing 2 changed files with 27 additions and 14 deletions.
21 changes: 10 additions & 11 deletions mc/orm/authz_cloudlet.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@ import (
"context"
fmt "fmt"
"net/http"
"strings"

"github.com/labstack/echo"
"github.com/mobiledgex/edge-cloud-infra/mc/ormapi"
Expand Down Expand Up @@ -208,17 +207,17 @@ func authzCreateAppInst(ctx context.Context, region, username string, obj *edgep
if authzOk, _ := authzCloudlet.Ok(&cloudlet); !authzOk {
return echo.ErrForbidden
}
// Developers can't create AppInsts on other developer's ClusterInsts,
// except for autoclusters where ClusterInst org is MobiledgeX.
autocluster := false
if strings.HasPrefix(obj.Key.ClusterInstKey.ClusterKey.Name, cloudcommon.AutoClusterPrefix) {
if obj.Key.ClusterInstKey.Organization != cloudcommon.OrganizationMobiledgeX {
return echo.NewHTTPError(http.StatusBadRequest, fmt.Errorf("Autocluster AppInst's ClusterInst organization must be %s", cloudcommon.OrganizationMobiledgeX))
// The autocluster organization checks are now dependent on the CRM version,
// so these checks are left to the Controller. The MC is only
// concerned about RBAC permissions, so only ensures that different
// organizations are not encroaching on each other.
if obj.Key.AppKey.Organization != obj.Key.ClusterInstKey.Organization && obj.Key.ClusterInstKey.Organization != "" {
// Sidecar apps may have MobiledgeX organization, or
// target ClusterInst may be MobiledgeX reservable/multitenant.
// So one of the orgs must be MobiledgeX to pass RBAC.
if obj.Key.AppKey.Organization != cloudcommon.OrganizationMobiledgeX && obj.Key.ClusterInstKey.Organization != cloudcommon.OrganizationMobiledgeX {
return echo.ErrForbidden
}
autocluster = true
}
if !authzCloudlet.admin && !autocluster && obj.Key.ClusterInstKey.Organization != "" && obj.Key.ClusterInstKey.Organization != obj.Key.AppKey.Organization {
return echo.NewHTTPError(http.StatusBadRequest, fmt.Errorf("AppInst organization must match ClusterInst organization"))
}
return nil
}
Expand Down
20 changes: 17 additions & 3 deletions mc/orm/ctrl_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -385,11 +385,25 @@ func TestController(t *testing.T) {
// developers can't create AppInsts on other developemar's ClusterInsts
appinst := edgeproto.AppInst{}
appinst.Key.AppKey.Organization = org1
appinst.Key.ClusterInstKey.Organization = cloudcommon.OrganizationMobiledgeX
appinst.Key.ClusterInstKey.Organization = org2
_, status, err := ormtestutil.TestCreateAppInst(mcClient, uri, tokenDev, ctrl.Region, &appinst)
require.NotNil(t, err)
require.Contains(t, err.Error(), "AppInst organization must match ClusterInst organization")
// but admin can
require.Contains(t, err.Error(), "Forbidden")
// developers can create against MobiledgeX ClusterInsts
// (reservable or multitenant).
appinst.Key.AppKey.Organization = org1
appinst.Key.ClusterInstKey.Organization = cloudcommon.OrganizationMobiledgeX
_, status, err = ormtestutil.TestCreateAppInst(mcClient, uri, tokenDev, ctrl.Region, &appinst)
require.Nil(t, err)
require.Equal(t, http.StatusOK, status)
_, status, err = ormtestutil.TestDeleteAppInst(mcClient, uri, tokenDev, ctrl.Region, &appinst)
require.Nil(t, err)
require.Equal(t, http.StatusOK, status)
// Only admin can create MobiledgeX sidecar apps, since other
// developers won't have App rbac perms for org MobiledgeX.
testCreateOrg(t, mcClient, uri, tokenAd, OrgTypeDeveloper, cloudcommon.OrganizationMobiledgeX)
appinst.Key.AppKey.Organization = cloudcommon.OrganizationMobiledgeX
appinst.Key.ClusterInstKey.Organization = org2
_, status, err = ormtestutil.TestCreateAppInst(mcClient, uri, tokenAd, ctrl.Region, &appinst)
require.Nil(t, err)
require.Equal(t, http.StatusOK, status)
Expand Down

0 comments on commit 1cd5458

Please sign in to comment.