Skip to content

Commit

Permalink
fix: binding check subjects and auth resource delete
Browse files Browse the repository at this point in the history
  • Loading branch information
yadzhang authored and choujimmy committed Dec 25, 2019
1 parent 2c4b711 commit b85e454
Show file tree
Hide file tree
Showing 16 changed files with 209 additions and 127 deletions.
2 changes: 1 addition & 1 deletion api/auth/v1/conversion.go
Original file line number Diff line number Diff line change
Expand Up @@ -212,7 +212,7 @@ func AddFieldLabelConversionsForUser(scheme *runtime.Scheme) error {
// field selectors of IdentityProvider from the given version to internal version
// representation.
func AddFieldLabelConversionsForGroup(scheme *runtime.Scheme) error {
return scheme.AddFieldLabelConversionFunc(SchemeGroupVersion.WithKind("IdentityProvider"),
return scheme.AddFieldLabelConversionFunc(SchemeGroupVersion.WithKind("Group"),
func(label, value string) (string, string, error) {
switch label {
case "keyword":
Expand Down
8 changes: 6 additions & 2 deletions pkg/auth/controller/config/config_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -225,8 +225,11 @@ func (c *Controller) syncItem(key string) error {

func (c *Controller) processConfig(idp *v1.IdentityProvider, key string) error {
var errs []error
if err := c.loadPolicy(idp.Name); err != nil {
errs = append(errs, err)

if c.policyFile != "" {
if err := c.loadPolicy(idp.Name); err != nil {
errs = append(errs, err)
}
}
if err := c.createAdmin(idp.Name); err != nil {
errs = append(errs, err)
Expand Down Expand Up @@ -333,6 +336,7 @@ func (c *Controller) loadCategory() error {
func (c *Controller) loadPolicy(tenantID string) error {
log.Info("Handle default policy for tenant", log.String("tenantID", tenantID))
var policyList []*v1.Policy

bytes, err := ioutil.ReadFile(c.policyFile)
if err != nil {
return err
Expand Down
59 changes: 58 additions & 1 deletion pkg/auth/controller/group/deletion/grouped_resources_deleter.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,8 @@ package deletion

import (
"fmt"
"strings"
"tkestack.io/tke/pkg/auth/util"

"github.com/casbin/casbin/v2"
"k8s.io/apimachinery/pkg/api/errors"
Expand Down Expand Up @@ -246,6 +248,7 @@ func (d *groupedResourcesDeleter) finalizeGroup(group *v1.LocalGroup) (*v1.Local
type deleteResourceFunc func(deleter *groupedResourcesDeleter, group *v1.LocalGroup) error

var deleteResourceFuncs = []deleteResourceFunc{
deleteRelatedRoles,
deleteRelatedRules,
}

Expand All @@ -272,8 +275,62 @@ func (d *groupedResourcesDeleter) deleteAllContent(group *v1.LocalGroup) error {
return nil
}

func deleteRelatedRoles(deleter *groupedResourcesDeleter, group *v1.LocalGroup) error {
log.Debug("LocalGroup controller - deleteRelatedRoles", log.String("group", group.ObjectMeta.Name))

subj := util.GroupKey(group.Spec.TenantID, group.Name)
roles, err := deleter.enforcer.GetRolesForUser(subj)
if err != nil {
return err
}

binding := v1.Binding{}
binding.Groups = append(binding.Groups, v1.Subject{ID: group.Name, Name: group.Spec.DisplayName})

log.Info("Try removing policy for group", log.String("group", group.Name), log.Strings("policies", roles))
var errs []error
for _, role := range roles {
switch {
case strings.HasPrefix(role, "pol-"):
pol := &v1.Policy{}
err = deleter.authClient.RESTClient().Post().
Resource("policies").
Name(role).
SubResource("unbinding").
Body(&binding).
Do().Into(pol)
if err != nil {
log.Error("Unbind policy for group failed", log.String("group", group.Name),
log.String("policy", role), log.Err(err))
errs = append(errs, err)
}
case strings.HasPrefix(role, "rol-"):
rol := &v1.Role{}
err = deleter.authClient.RESTClient().Post().
Resource("roles").
Name(role).
SubResource("unbinding").
Body(&binding).
Do().Into(rol)
if err != nil {
log.Error("Unbind role for group failed", log.String("group", group.Name),
log.String("policy", role), log.Err(err))
errs = append(errs, err)
}
default:
log.Error("Unknown role name for group, remove it", log.String("group", group.Name), log.String("role", role))
_, err = deleter.enforcer.DeleteRoleForUser(subj, role)
if err != nil {
errs = append(errs, err)
}
}
}

return utilerrors.NewAggregate(errs)
}

func deleteRelatedRules(deleter *groupedResourcesDeleter, group *v1.LocalGroup) error {
log.Info("LocalGroup controller - deleteRelatedRules", log.String("groupName", group.ObjectMeta.Name))
_, err := deleter.enforcer.DeleteRole(group.Name)
_, err := deleter.enforcer.DeleteRole(util.GroupKey(group.Spec.TenantID, group.Name))
return err
}
Original file line number Diff line number Diff line change
Expand Up @@ -119,21 +119,17 @@ func (d *loalIdentitiedResourcesDeleter) Delete(localIdentityName string) error
return nil
}

log.Info("1")
// Delete the localIdentity if it is already finalized.
log.Infof("localIdentity: %+v", localIdentity)
if d.deleteLocalIdentityWhenDone && finalized(localIdentity) {
return d.deleteLocalIdentity(localIdentity)
}

log.Info("2")
// there may still be content for us to remove
err = d.deleteAllContent(localIdentity)
if err != nil {
return err
}

log.Info("3")
// we have removed content, so mark it finalized by us
localIdentity, err = d.retryOnConflictError(localIdentity, d.finalizeLocalIdentity)
if err != nil {
Expand Down Expand Up @@ -251,7 +247,7 @@ func (d *loalIdentitiedResourcesDeleter) finalizeLocalIdentity(localIdentity *v1
type deleteResourceFunc func(deleter *loalIdentitiedResourcesDeleter, localIdentity *v1.LocalIdentity) error

var deleteResourceFuncs = []deleteResourceFunc{
deleteRelatedRules,
deleteRelatedRoles,
deleteApikeys,
}

Expand All @@ -278,8 +274,8 @@ func (d *loalIdentitiedResourcesDeleter) deleteAllContent(localIdentity *v1.Loca
return nil
}

func deleteRelatedRules(deleter *loalIdentitiedResourcesDeleter, localIdentity *v1.LocalIdentity) error {
log.Debug("LocalIdentity controller - deleteRelatedRules", log.String("localIdentityName", localIdentity.ObjectMeta.Name))
func deleteRelatedRoles(deleter *loalIdentitiedResourcesDeleter, localIdentity *v1.LocalIdentity) error {
log.Debug("LocalIdentity controller - deleteRelatedRoles", log.String("localIdentityName", localIdentity.ObjectMeta.Name))

subj := util.UserKey(localIdentity.Spec.TenantID, localIdentity.Spec.Username)
roles, err := deleter.enforcer.GetRolesForUser(subj)
Expand All @@ -288,54 +284,48 @@ func deleteRelatedRules(deleter *loalIdentitiedResourcesDeleter, localIdentity *
}

binding := v1.Binding{}
binding.Users = append(binding.Users, v1.Subject{Name: localIdentity.Spec.Username})
binding.Users = append(binding.Users, v1.Subject{ID: localIdentity.Name, Name: localIdentity.Spec.Username})

log.Info("Try removing policy for user", log.String("user", localIdentity.Spec.Username), log.Strings("policies", roles))
var errs []error
pol := &v1.Policy{}
for _, role := range roles {
switch {
case strings.HasPrefix(role, "pol-"):
pol := &v1.Policy{}
err = deleter.authClient.RESTClient().Post().
Resource("policies").
Name(role).
SubResource("unbinding").
Body(&binding).
Do().Into(pol)
if err != nil {
if errors.IsNotFound(err) {
continue
}
log.Error("Unbind policy for user failed", log.String("user", localIdentity.Spec.Username),
log.String("policy", role), log.Err(err))
errs = append(errs, err)
}
case strings.HasPrefix(role, util.GroupPrefix(localIdentity.Spec.TenantID)):
grp := &v1.LocalGroup{}
err = deleter.authClient.RESTClient().Post().
Resource("groups").
Name(role).
Resource("localgroups").
Name(strings.TrimPrefix(role, util.GroupPrefix(localIdentity.Spec.TenantID))).
SubResource("unbinding").
Body(&binding).
Do().Into(pol)
Do().Into(grp)
log.Info("errr", log.Err(err))
if err != nil {
if errors.IsNotFound(err) {
continue
}
log.Error("Unbind group for user failed", log.String("user", localIdentity.Spec.Username),
log.String("group", role), log.Err(err))
errs = append(errs, err)
}
case strings.HasPrefix(role, "rol-"):
rol := &v1.Role{}
err = deleter.authClient.RESTClient().Post().
Resource("roles").
Name(role).
SubResource("unbinding").
Body(&binding).
Do().Into(pol)
Do().Into(rol)
if err != nil {
if errors.IsNotFound(err) {
continue
}
log.Error("Unbind group for user failed", log.String("user", localIdentity.Spec.Username),
log.String("group", role), log.Err(err))
errs = append(errs, err)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -279,7 +279,7 @@ func deleteRelatedRules(deleter *policiedResourcesDeleter, policy *v1.Policy) er
}

func detachRelatedRoles(deleter *policiedResourcesDeleter, policy *v1.Policy) error {
log.Info("Policy controller - deleteRelatedRules", log.String("policyName", policy.ObjectMeta.Name))
log.Info("Policy controller - detachRelatedRoles", log.String("policyName", policy.ObjectMeta.Name))

roles, err := deleter.enforcer.GetRolesForUser(policy.ObjectMeta.Name)
if err != nil {
Expand All @@ -288,17 +288,17 @@ func detachRelatedRoles(deleter *policiedResourcesDeleter, policy *v1.Policy) er

var errs []error

unbinding := []string{policy.ObjectMeta.Name}
pol := &v1.Role{}
unbinding := v1.PolicyBinding{Policies: []string{policy.ObjectMeta.Name}}
for _, role := range roles {
switch {
case strings.HasPrefix(role, "rol-"):
rol := &v1.Role{}
err = deleter.authClient.RESTClient().Post().
Resource("roles").
Name(role).
SubResource("policyunbinding").
Body(&unbinding).
Do().Into(pol)
Do().Into(rol)
if err != nil {
if errors.IsNotFound(err) {
continue
Expand Down
6 changes: 3 additions & 3 deletions pkg/auth/registry/localgroup/storage/binding.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,9 +21,6 @@ package storage
import (
"context"

"tkestack.io/tke/pkg/auth/util"
"tkestack.io/tke/pkg/util/log"

"k8s.io/apimachinery/pkg/api/errors"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/runtime"
Expand All @@ -33,6 +30,8 @@ import (

"tkestack.io/tke/api/auth"
authinternalclient "tkestack.io/tke/api/client/clientset/internalversion/typed/auth/internalversion"
"tkestack.io/tke/pkg/auth/util"
"tkestack.io/tke/pkg/util/log"
)

// BindingREST implements the REST endpoint.
Expand Down Expand Up @@ -65,6 +64,7 @@ func (r *BindingREST) Create(ctx context.Context, obj runtime.Object, createVali

for _, sub := range bind.Users {
if !util.InSubjects(sub, group.Status.Users) {
sub.Name = ""
group.Status.Users = append(group.Status.Users, sub)
}
}
Expand Down
20 changes: 14 additions & 6 deletions pkg/auth/registry/localgroup/strategy.go
Original file line number Diff line number Diff line change
Expand Up @@ -64,14 +64,18 @@ func (Strategy) DefaultGarbageCollectionGroup(ctx context.Context) rest.GarbageC
// object.
func (Strategy) PrepareForUpdate(ctx context.Context, obj, old runtime.Object) {
_, tenantID := authentication.GetUsernameAndTenantID(ctx)
oldGroup := old.(*auth.LocalGroup)
group, _ := obj.(*auth.LocalGroup)

if len(tenantID) != 0 {
oldGroup := old.(*auth.LocalGroup)
group, _ := obj.(*auth.LocalGroup)
if oldGroup.Spec.TenantID != tenantID {
log.Panic("Unauthorized update group information", log.String("oldTenantID", oldGroup.Spec.TenantID), log.String("newTenantID", group.Spec.TenantID), log.String("userTenantID", tenantID))
}
group.Spec.TenantID = tenantID
}

// Update bind users, use binding api
group.Status.Users = oldGroup.Status.Users
}

// NamespaceScoped is false for policies.
Expand Down Expand Up @@ -103,12 +107,16 @@ func (Strategy) PrepareForCreate(ctx context.Context, obj runtime.Object) {
auth.LocalGroupFinalize,
}

for i := range group.Status.Users {
group.Status.Users[i].Name = ""
}

group.Status.Phase = auth.GroupActive
}

// Validate validates a new group.
func (s *Strategy) Validate(ctx context.Context, obj runtime.Object) field.ErrorList {
return ValidateGroup(obj.(*auth.LocalGroup), s.authClient)
return ValidateLocalGroup(obj.(*auth.LocalGroup), s.authClient)
}

// AllowCreateOnUpdate is false for policies.
Expand All @@ -129,7 +137,7 @@ func (Strategy) Canonicalize(obj runtime.Object) {

// ValidateUpdate is the default update validation for an end group.
func (s *Strategy) ValidateUpdate(ctx context.Context, obj, old runtime.Object) field.ErrorList {
return ValidateGroupUpdate(obj.(*auth.LocalGroup), old.(*auth.LocalGroup), s.authClient)
return ValidateLocalGroupUpdate(obj.(*auth.LocalGroup), old.(*auth.LocalGroup), s.authClient)
}

// GetAttrs returns labels and fields of a given object for filtering purposes.
Expand Down Expand Up @@ -192,7 +200,7 @@ func (StatusStrategy) PrepareForUpdate(ctx context.Context, obj, old runtime.Obj
// filled in before the object is persisted. This method should not mutate
// the object.
func (s *StatusStrategy) ValidateUpdate(ctx context.Context, obj, old runtime.Object) field.ErrorList {
return ValidateGroupUpdate(obj.(*auth.LocalGroup), old.(*auth.LocalGroup), s.authClient)
return ValidateLocalGroupUpdate(obj.(*auth.LocalGroup), old.(*auth.LocalGroup), s.authClient)
}

// FinalizeStrategy implements finalizer logic for Machine.
Expand Down Expand Up @@ -221,5 +229,5 @@ func (FinalizeStrategy) PrepareForUpdate(ctx context.Context, obj, old runtime.O
// filled in before the object is persisted. This method should not mutate
// the object.
func (s *FinalizeStrategy) ValidateUpdate(ctx context.Context, obj, old runtime.Object) field.ErrorList {
return ValidateGroupUpdate(obj.(*auth.LocalGroup), old.(*auth.LocalGroup), s.authClient)
return ValidateLocalGroupUpdate(obj.(*auth.LocalGroup), old.(*auth.LocalGroup), s.authClient)
}
Loading

0 comments on commit b85e454

Please sign in to comment.