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

Restrict AutoUpdateVersion to be created/updated for cloud #49008

Merged
merged 7 commits into from
Dec 12, 2024
Merged

Restrict AutoUpdateVersion to be created/updated for cloud #49008

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
a230d3a
Restrict AutoUpdateVersion to be created/updated for cloud
vapopov Nov 14, 2024
a0b0c02
Merge remote-tracking branch 'origin/master' into vapopov/autoupdate-…
vapopov Dec 6, 2024
2d4f8a7
Check builtin Admin role and Cloud feature
vapopov Dec 10, 2024
ac182a4
Merge remote-tracking branch 'origin/master' into vapopov/autoupdate-…
vapopov Dec 10, 2024
6078ad5
More informative error message
vapopov Dec 10, 2024
8891252
Remove KindAutoUpdateAgentRollout from editor role preset
vapopov Dec 10, 2024
5250710
Merge branch 'master' into vapopov/autoupdate-version-cloud-restriction
vapopov Dec 12, 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
26 changes: 26 additions & 0 deletions lib/auth/autoupdate/autoupdatev1/service.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ import (
apievents "github.com/gravitational/teleport/api/types/events"
"github.com/gravitational/teleport/lib/authz"
"github.com/gravitational/teleport/lib/events"
"github.com/gravitational/teleport/lib/modules"
"github.com/gravitational/teleport/lib/services"
)

Expand Down Expand Up @@ -292,6 +293,11 @@ func (s *Service) CreateAutoUpdateVersion(ctx context.Context, req *autoupdate.C
return nil, trace.Wrap(err)
}

if modules.GetModules().Features().Cloud && !isAdmin(authCtx) {
return nil, trace.AccessDenied("only role %q is allowed to modifying %q in cloud",
Copy link
Contributor

@hugoShaka hugoShaka Dec 10, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

This error is not very actionable for users. I think we should have something more user-oriented like:

This Teleport instance is running on Teleport Cloud and the "autoupdate_version" resource is managed by the Teleport Cloud team. You can use the "autoupdate_config" resource to opt-in, opt-out or configure update schedules.

types.RoleAdmin, types.KindAutoUpdateVersion)
}

if err := authCtx.CheckAccessToKind(types.KindAutoUpdateVersion, types.VerbCreate); err != nil {
return nil, trace.Wrap(err)
}
Expand Down Expand Up @@ -333,6 +339,11 @@ func (s *Service) UpdateAutoUpdateVersion(ctx context.Context, req *autoupdate.U
return nil, trace.Wrap(err)
}

if modules.GetModules().Features().Cloud && !isAdmin(authCtx) {
return nil, trace.AccessDenied("only role %q is allowed to modifying %q in cloud",
types.RoleAdmin, types.KindAutoUpdateVersion)
}

if err := authCtx.CheckAccessToKind(types.KindAutoUpdateVersion, types.VerbUpdate); err != nil {
return nil, trace.Wrap(err)
}
Expand Down Expand Up @@ -374,6 +385,11 @@ func (s *Service) UpsertAutoUpdateVersion(ctx context.Context, req *autoupdate.U
return nil, trace.Wrap(err)
}

if modules.GetModules().Features().Cloud && !isAdmin(authCtx) {
return nil, trace.AccessDenied("only role %q is allowed to modifying %q in cloud",
types.RoleAdmin, types.KindAutoUpdateVersion)
}

if err := authCtx.CheckAccessToKind(types.KindAutoUpdateVersion, types.VerbCreate, types.VerbUpdate); err != nil {
return nil, trace.Wrap(err)
}
Expand Down Expand Up @@ -415,6 +431,11 @@ func (s *Service) DeleteAutoUpdateVersion(ctx context.Context, req *autoupdate.D
return nil, trace.Wrap(err)
}

if modules.GetModules().Features().Cloud && !isAdmin(authCtx) {
return nil, trace.AccessDenied("only role %q is allowed to modifying %q in cloud",
types.RoleAdmin, types.KindAutoUpdateVersion)
}

if err := authCtx.CheckAccessToKind(types.KindAutoUpdateVersion, types.VerbDelete); err != nil {
return nil, trace.Wrap(err)
}
Expand Down Expand Up @@ -589,3 +610,8 @@ func (s *Service) emitEvent(ctx context.Context, e apievents.AuditEvent) {
)
}
}

// isAdmin returns true if the given context has the builtin admin role.
func isAdmin(authCtx *authz.Context) bool {
return authz.HasBuiltinRole(*authCtx, string(types.RoleAdmin))
}
10 changes: 5 additions & 5 deletions lib/modules/modules.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,6 @@ import (

"github.com/gravitational/teleport"
"github.com/gravitational/teleport/api/client/proto"
"github.com/gravitational/teleport/api/gen/proto/go/teleport/autoupdate/v1"
"github.com/gravitational/teleport/api/types"
"github.com/gravitational/teleport/api/types/accesslist"
"github.com/gravitational/teleport/api/utils/keys"
Expand Down Expand Up @@ -333,8 +332,11 @@ func GetModules() Modules {
var ErrCannotDisableSecondFactor = errors.New("cannot disable multi-factor authentication")

// ValidateResource performs additional resource checks.
func ValidateResource(res any) error {
if GetModules().Features().Cloud || !IsInsecureTestMode() {
func ValidateResource(res types.Resource) error {
// todo(tross): DELETE WHEN ABLE TO [remove env var, leave insecure test mode]
if GetModules().Features().Cloud ||
(os.Getenv(teleport.EnvVarAllowNoSecondFactor) != "yes" && !IsInsecureTestMode()) {

switch r := res.(type) {
case types.AuthPreference:
if !r.IsSecondFactorEnforced() {
Expand All @@ -357,8 +359,6 @@ func ValidateResource(res any) error {
if !r.GetProxyChecksHostKeys() {
return trace.BadParameter("cannot disable strict host key checking on Cloud")
}
case autoupdate.AutoUpdateVersion:
return trace.BadParameter("cannot modify auto update version on Cloud")
}
return nil
}
Expand Down
10 changes: 0 additions & 10 deletions lib/services/local/autoupdate.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,6 @@ import (
"github.com/gravitational/teleport/api/types"
update "github.com/gravitational/teleport/api/types/autoupdate"
"github.com/gravitational/teleport/lib/backend"
"github.com/gravitational/teleport/lib/modules"
"github.com/gravitational/teleport/lib/services"
"github.com/gravitational/teleport/lib/services/local/generic"
)
Expand Down Expand Up @@ -143,9 +142,6 @@ func (s *AutoUpdateService) CreateAutoUpdateVersion(
ctx context.Context,
v *autoupdate.AutoUpdateVersion,
) (*autoupdate.AutoUpdateVersion, error) {
if err := modules.ValidateResource(v); err != nil {
return nil, trace.Wrap(err)
}
version, err := s.version.CreateResource(ctx, v)
return version, trace.Wrap(err)
}
Expand All @@ -155,9 +151,6 @@ func (s *AutoUpdateService) UpdateAutoUpdateVersion(
ctx context.Context,
v *autoupdate.AutoUpdateVersion,
) (*autoupdate.AutoUpdateVersion, error) {
if err := modules.ValidateResource(v); err != nil {
return nil, trace.Wrap(err)
}
version, err := s.version.ConditionalUpdateResource(ctx, v)
return version, trace.Wrap(err)
}
Expand All @@ -167,9 +160,6 @@ func (s *AutoUpdateService) UpsertAutoUpdateVersion(
ctx context.Context,
v *autoupdate.AutoUpdateVersion,
) (*autoupdate.AutoUpdateVersion, error) {
if err := modules.ValidateResource(v); err != nil {
return nil, trace.Wrap(err)
}
version, err := s.version.UpsertResource(ctx, v)
return version, trace.Wrap(err)
}
Expand Down
3 changes: 3 additions & 0 deletions lib/services/presets.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -184,6 +184,9 @@ func NewPresetEditorRole() types.Role {
types.NewRule(types.KindUserTask, RW()),
types.NewRule(types.KindIdentityCenter, RW()),
types.NewRule(types.KindContact, RW()),
types.NewRule(types.KindAutoUpdateVersion, RW()),
types.NewRule(types.KindAutoUpdateConfig, RW()),
types.NewRule(types.KindAutoUpdateAgentRollout, RW()),
vapopov marked this conversation as resolved.
Show resolved Hide resolved
},
},
},
Expand Down
Loading