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

Temporarily remove app label checker for saml_idp_service_provider resources #48027

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
+261 −311
Open

Temporarily remove app label checker for saml_idp_service_provider resources #48027

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
0f63d6a
temporarily remove app label checker for saml_idp_service_provider re…
flyinghermit Oct 28, 2024
45473e3
add todo comment
flyinghermit Oct 28, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
79 changes: 7 additions & 72 deletions lib/auth/auth_with_roles.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1263,6 +1263,11 @@ func (c *resourceAccess) checkAccess(resource types.ResourceWithLabels, filter s
return false, nil
}

// KindSAMLIdPServiceProvider does not support label matcher
Copy link
Collaborator

Choose a reason for hiding this comment

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

Let's leave a todo item here to re-enable this when we're ready to properly introduce role v8? And maybe link this PR as a reference.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

45473e3

if resourceKind == types.KindSAMLIdPServiceProvider {
return true, nil
}

// check access normally if base checker doesnt exist
if c.baseAuthChecker == nil {
if err := c.accessChecker.CanAccess(resource); err != nil {
Expand Down Expand Up @@ -6773,38 +6778,13 @@ func (a *ServerWithRoles) ListReleases(ctx context.Context) ([]*types.Release, e
return a.authServer.releaseService.ListReleases(ctx)
}

// TODO(sshah): set MFARequired for SAML IdP admin actions?
func (a *ServerWithRoles) checkAccessToSAMLIdPServiceProvider(sp types.SAMLIdPServiceProvider) error {
return a.context.Checker.CheckAccess(
sp,
// MFA is not required for operations on SAML resources but
// will be enforced at the connection time.
services.AccessState{})
}

// ListSAMLIdPServiceProviders returns a paginated list of SAML IdP service provider resources.
func (a *ServerWithRoles) ListSAMLIdPServiceProviders(ctx context.Context, pageSize int, nextToken string) ([]types.SAMLIdPServiceProvider, string, error) {
if err := a.action(apidefaults.Namespace, types.KindSAMLIdPServiceProvider, types.VerbList); err != nil {
return nil, "", trace.Wrap(err)
}

sps, nextKey, err := a.authServer.ListSAMLIdPServiceProviders(ctx, pageSize, nextToken)
if err != nil {
return nil, "", trace.Wrap(err)
}

// Filter out service providers the caller doesn't have access to.
var filtered []types.SAMLIdPServiceProvider
for _, sp := range sps {
err := a.checkAccessToSAMLIdPServiceProvider(sp)
if err != nil && !trace.IsAccessDenied(err) {
return nil, "", trace.Wrap(err)
} else if err == nil {
filtered = append(filtered, sp)
}

}
return filtered, nextKey, nil
return a.authServer.ListSAMLIdPServiceProviders(ctx, pageSize, nextToken)
}

// GetSAMLIdPServiceProvider returns the specified SAML IdP service provider resources.
Expand All @@ -6813,16 +6793,7 @@ func (a *ServerWithRoles) GetSAMLIdPServiceProvider(ctx context.Context, name st
return nil, trace.Wrap(err)
}

sp, err := a.authServer.GetSAMLIdPServiceProvider(ctx, name)
if err != nil {
return nil, trace.Wrap(err)
}

if err = a.checkAccessToSAMLIdPServiceProvider(sp); err != nil {
return nil, trace.Wrap(err)
}

return sp, nil
return a.authServer.GetSAMLIdPServiceProvider(ctx, name)
}

// CreateSAMLIdPServiceProvider creates a new SAML IdP service provider resource.
Expand Down Expand Up @@ -6860,10 +6831,6 @@ func (a *ServerWithRoles) CreateSAMLIdPServiceProvider(ctx context.Context, sp t
return trace.Wrap(err)
}

if err = a.checkAccessToSAMLIdPServiceProvider(sp); err != nil {
return trace.Wrap(err)
}

if err := services.ValidateSAMLIdPACSURLAndRelayStateInputs(sp); err != nil {
return trace.Wrap(err)
}
Expand Down Expand Up @@ -6913,17 +6880,6 @@ func (a *ServerWithRoles) UpdateSAMLIdPServiceProvider(ctx context.Context, sp t
return trace.Wrap(err)
}

existingSP, err := a.authServer.GetSAMLIdPServiceProvider(ctx, sp.GetName())
if err != nil {
return trace.Wrap(err)
}
if err = a.checkAccessToSAMLIdPServiceProvider(existingSP); err != nil {
return trace.Wrap(err)
}
if err = a.checkAccessToSAMLIdPServiceProvider(sp); err != nil {
return trace.Wrap(err)
}

if err := services.ValidateSAMLIdPACSURLAndRelayStateInputs(sp); err != nil {
return trace.Wrap(err)
}
Expand Down Expand Up @@ -6984,10 +6940,6 @@ func (a *ServerWithRoles) DeleteSAMLIdPServiceProvider(ctx context.Context, name
return trace.Wrap(err)
}

if err = a.checkAccessToSAMLIdPServiceProvider(sp); err != nil {
return trace.Wrap(err)
}

name = sp.GetName()
entityID = sp.GetEntityID()

Expand Down Expand Up @@ -7026,23 +6978,6 @@ func (a *ServerWithRoles) DeleteAllSAMLIdPServiceProviders(ctx context.Context)
return trace.Wrap(err)
}

var startKey string
for {
sps, nextKey, err := a.authServer.ListSAMLIdPServiceProviders(ctx, apidefaults.DefaultChunkSize, startKey)
if err != nil {
return trace.Wrap(err)
}
for _, sp := range sps {
if err := a.checkAccessToSAMLIdPServiceProvider(sp); err != nil {
return trace.Wrap(err)
}
}
if nextKey == "" {
break
}
startKey = nextKey
}

err = a.authServer.DeleteAllSAMLIdPServiceProviders(ctx)
return trace.Wrap(err)
}
Expand Down
Loading
Loading