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

Convert lib/auth/bot to use slog #50520

Merged
merged 1 commit into from
Dec 21, 2024
Merged

Convert lib/auth/bot to use slog #50520

Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
55 changes: 31 additions & 24 deletions lib/auth/bot.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,6 @@ import (

"github.com/google/uuid"
"github.com/gravitational/trace"
"github.com/sirupsen/logrus"
"google.golang.org/protobuf/types/known/timestamppb"

"github.com/gravitational/teleport/api/client/proto"
Expand All @@ -42,6 +41,7 @@ import (
"github.com/gravitational/teleport/lib/events"
"github.com/gravitational/teleport/lib/services"
"github.com/gravitational/teleport/lib/sshutils"
logutils "github.com/gravitational/teleport/lib/utils/log"
)

// legacyValidateGenerationLabel validates and updates a generation label.
Expand Down Expand Up @@ -121,7 +121,10 @@ func (a *Server) legacyValidateGenerationLabel(ctx context.Context, username str
// The current generations must match to continue:
if currentIdentityGeneration != currentUserGeneration {
if err := a.tryLockBotDueToGenerationMismatch(ctx, user.GetName()); err != nil {
log.WithError(err).Warnf("Failed to lock bot %q when a generation mismatch was detected", user.GetName())
a.logger.WarnContext(ctx, "Failed to lock bot when a generation mismatch was detected",
"error", err,
"bot", user.GetName(),
)
}

return trace.AccessDenied(
Expand Down Expand Up @@ -248,7 +251,7 @@ func (a *Server) tryLockBotDueToGenerationMismatch(ctx context.Context, username
},
UserMetadata: userMetadata,
}); err != nil {
log.WithError(err).Warn("Failed to emit renewable cert generation mismatch event")
a.logger.WarnContext(ctx, "Failed to emit renewable cert generation mismatch event", "error", err)
}

return nil
Expand Down Expand Up @@ -348,11 +351,11 @@ func (a *Server) updateBotInstance(
authRecord.Generation = 1
}

log.WithFields(logrus.Fields{
"bot_name": botName,
"invalid_instance_id": botInstanceID,
"new_instance_id": instanceID.String(),
}).Info("bot has no valid instance ID, a new instance will be generated")
a.logger.InfoContext(ctx, "bot has no valid instance ID, a new instance will be generated",
"bot_name", botName,
"invalid_instance_id", botInstanceID,
"new_instance_id", logutils.StringerAttr(instanceID),
)

expires := a.GetClock().Now().Add(req.ttl + machineidv1.ExpiryMargin)

Expand All @@ -371,22 +374,22 @@ func (a *Server) updateBotInstance(
return nil
}

l := log.WithFields(logrus.Fields{
"bot_name": botName,
"bot_instance_id": botInstanceID,
})
log := a.logger.With(
"bot_name", botName,
"bot_instance_id", botInstanceID,
)

if currentIdentityGeneration == 0 {
// Nothing to do.
l.Warn("bot attempted to fetch certificates without providing a current identity generation, this is not allowed")
log.WarnContext(ctx, "bot attempted to fetch certificates without providing a current identity generation, this is not allowed")

return trace.AccessDenied("a current identity generation must be provided")
} else if currentIdentityGeneration > 0 && currentIdentityGeneration != instanceGeneration {
// For now, continue to only enforce generation counter checks on
// renewable (i.e. token) identities.
if req.renewable {
if err := a.tryLockBotDueToGenerationMismatch(ctx, username); err != nil {
l.WithError(err).Warn("Failed to lock bot when a generation mismatch was detected")
log.WarnContext(ctx, "Failed to lock bot when a generation mismatch was detected", "error", err)
}

return trace.AccessDenied(
Expand All @@ -398,12 +401,13 @@ func (a *Server) updateBotInstance(
// We'll still log the check failure, but won't deny access. This
// log data will help make an informed decision about reliability of
// the generation counter for all join methods in the future.
l.WithFields(logrus.Fields{
"bot_instance_generation": instanceGeneration,
"bot_identity_generation": currentIdentityGeneration,
"bot_join_method": authRecord.JoinMethod,
}).Warn("Bot generation counter mismatch detected. This check is not enforced for this join method, " +
"but may indicate multiple uses of a bot identity and possibly a compromised certificate.")
const msg = "Bot generation counter mismatch detected. This check is not enforced for this join method, " +
"but may indicate multiple uses of a bot identity and possibly a compromised certificate."
log.WarnContext(ctx, msg,
"bot_instance_generation", instanceGeneration,
"bot_identity_generation", currentIdentityGeneration,
"bot_join_method", authRecord.JoinMethod,
)
}
}

Expand All @@ -419,7 +423,7 @@ func (a *Server) updateBotInstance(
// setting this for other methods will break compatibility.
if req.renewable {
if err := a.commitLegacyGenerationCounterToBotUser(ctx, username, uint64(newGeneration)); err != nil {
l.WithError(err).Warn("unable to commit legacy generation counter to bot user")
log.WarnContext(ctx, "unable to commit legacy generation counter to bot user", "error", err)
}
}

Expand All @@ -442,7 +446,7 @@ func (a *Server) updateBotInstance(
// An initial auth record should have been added during initial join,
// but if not, add it now.
if bi.Status.InitialAuthentication == nil {
l.Warn("bot instance is missing its initial authentication record, a new one will be added")
log.WarnContext(ctx, "bot instance is missing its initial authentication record, a new one will be added")
bi.Status.InitialAuthentication = authRecord
}

Expand Down Expand Up @@ -500,13 +504,16 @@ func (a *Server) generateInitialBotCerts(
// permissions to read user data.
userState, err := a.GetUserOrLoginState(ctx, username)
if err != nil {
log.WithError(err).Debugf("Could not impersonate user %v. The user could not be fetched from local store.", username)
a.logger.DebugContext(ctx, "Could not impersonate user - the user could not be fetched from local store",
"error", err,
"user", username,
)
return nil, "", trace.AccessDenied("access denied")
}

// Do not allow SSO users to be impersonated.
if userState.GetUserType() == types.UserTypeSSO {
log.Warningf("Tried to issue a renewable cert for externally managed user %v, this is not supported.", username)
a.logger.WarnContext(ctx, "Tried to issue a renewable cert for externally managed user, this is not supported", "user", username)
return nil, "", trace.AccessDenied("access denied")
}

Expand Down
Loading