Skip to content

Commit

Permalink
Convert lib/auth/bot to use slog (#50520)
Browse files Browse the repository at this point in the history
  • Loading branch information
rosstimothy authored Dec 21, 2024
1 parent 9900ee1 commit db73df3
Showing 1 changed file with 31 additions and 24 deletions.
55 changes: 31 additions & 24 deletions lib/auth/bot.go
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

0 comments on commit db73df3

Please sign in to comment.