Skip to content

Commit

Permalink
fix: record applied login rules in github login event (#27607)
Browse files Browse the repository at this point in the history
  • Loading branch information
nklaassen committed Jun 8, 2023
1 parent 89d7b1e commit 980b459
Show file tree
Hide file tree
Showing 2 changed files with 17 additions and 5 deletions.
6 changes: 4 additions & 2 deletions lib/auth/github.go
Original file line number Diff line number Diff line change
Expand Up @@ -371,6 +371,7 @@ func validateGithubAuthCallbackHelper(ctx context.Context, m githubManager, diag

auth, err := m.validateGithubAuthCallback(ctx, diagCtx, q)
diagCtx.Info.Error = trace.UserMessage(err)
event.AppliedLoginRules = diagCtx.Info.AppliedLoginRules

diagCtx.WriteToBackend(ctx)

Expand Down Expand Up @@ -597,7 +598,7 @@ func (a *Server) validateGithubAuthCallback(ctx context.Context, diagCtx *SSODia

// Calculate (figure out name, roles, traits, session TTL) of user and
// create the user in the backend.
params, err := a.calculateGithubUser(ctx, connector, claims, req)
params, err := a.calculateGithubUser(ctx, diagCtx, connector, claims, req)
if err != nil {
return nil, trace.Wrap(err, "Failed to calculate user attributes.")
}
Expand Down Expand Up @@ -723,7 +724,7 @@ type CreateUserParams struct {
SessionTTL time.Duration
}

func (a *Server) calculateGithubUser(ctx context.Context, connector types.GithubConnector, claims *types.GithubClaims, request *types.GithubAuthRequest) (*CreateUserParams, error) {
func (a *Server) calculateGithubUser(ctx context.Context, diagCtx *SSODiagContext, connector types.GithubConnector, claims *types.GithubClaims, request *types.GithubAuthRequest) (*CreateUserParams, error) {
p := CreateUserParams{
ConnectorName: connector.GetName(),
Username: claims.Username,
Expand All @@ -749,6 +750,7 @@ func (a *Server) calculateGithubUser(ctx context.Context, connector types.Github
return nil, trace.Wrap(err)
}
p.Traits = evaluationOutput.Traits
diagCtx.Info.AppliedLoginRules = evaluationOutput.AppliedRules

// Kube groups and users are ultimately only set in the traits, not any
// other property of the User. In case the login rules changed the relevant
Expand Down
16 changes: 13 additions & 3 deletions lib/auth/github_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@ import (
"github.com/gravitational/teleport/api/constants"
"github.com/gravitational/teleport/api/defaults"
"github.com/gravitational/teleport/api/types"
apievents "github.com/gravitational/teleport/api/types/events"
"github.com/gravitational/teleport/lib/auth/keystore"
authority "github.com/gravitational/teleport/lib/auth/testauthority"
"github.com/gravitational/teleport/lib/backend"
Expand Down Expand Up @@ -212,11 +213,13 @@ func TestValidateGithubAuthCallbackEventsEmitted(t *testing.T) {
diagCtx := ssoDiagContextFixture(false /* testFlow */)
m.mockValidateGithubAuthCallback = func(ctx context.Context, diagCtx *SSODiagContext, q url.Values) (*GithubAuthResponse, error) {
diagCtx.Info.GithubClaims = claims
diagCtx.Info.AppliedLoginRules = []string{"login-rule"}
return auth, nil
}
_, _ = validateGithubAuthCallbackHelper(context.Background(), m, diagCtx, nil, tt.a.emitter)
require.Equal(t, tt.mockEmitter.LastEvent().GetType(), events.UserLoginEvent)
require.Equal(t, tt.mockEmitter.LastEvent().GetCode(), events.UserSSOLoginCode)
require.Equal(t, tt.mockEmitter.LastEvent().(*apievents.UserLogin).AppliedLoginRules, []string{"login-rule"})
require.Equal(t, ssoDiagInfoCalls, 0)
tt.mockEmitter.Reset()

Expand Down Expand Up @@ -279,7 +282,9 @@ func TestCalculateGithubUserNoTeams(t *testing.T) {
})
require.NoError(t, err)

_, err = a.calculateGithubUser(ctx, connector, &types.GithubClaims{
diagCtx := &SSODiagContext{}

_, err = a.calculateGithubUser(ctx, diagCtx, connector, &types.GithubClaims{
Username: "octocat",
OrganizationToTeams: map[string][]string{
"org1": {"team1", "team2"},
Expand Down Expand Up @@ -324,6 +329,7 @@ func TestCalculateGithubUserWithLoginRules(t *testing.T) {
}
mockEvaluator := &mockLoginRuleEvaluator{
outputTraits: evaluatedTraits,
ruleNames: []string{"mock"},
}
a.SetLoginRuleEvaluator(mockEvaluator)

Expand All @@ -339,7 +345,9 @@ func TestCalculateGithubUserWithLoginRules(t *testing.T) {
})
require.NoError(t, err)

userParams, err := a.calculateGithubUser(ctx, connector, &types.GithubClaims{
diagCtx := &SSODiagContext{}

userParams, err := a.calculateGithubUser(ctx, diagCtx, connector, &types.GithubClaims{
Username: "octocat",
OrganizationToTeams: map[string][]string{
"org1": {"team1"},
Expand All @@ -358,6 +366,7 @@ func TestCalculateGithubUserWithLoginRules(t *testing.T) {
SessionTTL: defaults.MaxCertDuration,
}, userParams, "user params does not match expected")
require.Equal(t, 1, mockEvaluator.evaluatedCount, "login rules were not evaluated exactly once")
require.Equal(t, mockEvaluator.ruleNames, diagCtx.Info.AppliedLoginRules)
}

type mockRoleCache struct {
Expand All @@ -372,13 +381,14 @@ func (m *mockRoleCache) GetRole(_ context.Context, name string) (types.Role, err
type mockLoginRuleEvaluator struct {
outputTraits map[string][]string
evaluatedCount int
ruleNames []string
}

func (m *mockLoginRuleEvaluator) Evaluate(context.Context, *loginrule.EvaluationInput) (*loginrule.EvaluationOutput, error) {
m.evaluatedCount++
return &loginrule.EvaluationOutput{
Traits: m.outputTraits,
AppliedRules: []string{"mock"},
AppliedRules: m.ruleNames,
}, nil
}

Expand Down

0 comments on commit 980b459

Please sign in to comment.