Skip to content

Commit 9938399

Browse files
bernardjkimmmcallister
authored andcommitted
access_monitoring_rules: Implement schedules conditions (#59044)
* Implement schedules conditions * Fix lint * Remove schedules predicate expression Consider all configured schedules by default * Support schedules within notification rules * Use timezone during evaluation * Add debug logs
1 parent 2871614 commit 9938399

File tree

7 files changed

+593
-17
lines changed

7 files changed

+593
-17
lines changed

integrations/access/accessmonitoring/access_monitoring_rules.go

Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -154,6 +154,22 @@ func (amrh *RuleHandler) RecipientsFromAccessMonitoringRules(ctx context.Context
154154
}
155155

156156
for _, rule := range amrh.getAccessMonitoringRules() {
157+
// Check if creation time is within rule schedules.
158+
isInSchedules, err := accessmonitoring.InSchedules(rule.GetSpec().GetSchedules(), env.CreationTime)
159+
if err != nil {
160+
log.WarnContext(ctx, "Failed to evaluate access monitoring rule",
161+
"error", err,
162+
"rule", rule.GetMetadata().GetName(),
163+
)
164+
continue
165+
}
166+
if len(rule.GetSpec().GetSchedules()) != 0 && !isInSchedules {
167+
log.DebugContext(ctx, "Access request does not satisfy schedule condition",
168+
"rule", rule.GetMetadata().GetName())
169+
continue
170+
}
171+
172+
// Check if environment matches rule conditions.
157173
match, err := accessmonitoring.EvaluateCondition(rule.Spec.Condition, env)
158174
if err != nil {
159175
log.WarnContext(ctx, "Failed to parse access monitoring notification rule",
@@ -188,6 +204,22 @@ func (amrh *RuleHandler) RawRecipientsFromAccessMonitoringRules(ctx context.Cont
188204
}
189205

190206
for _, rule := range amrh.getAccessMonitoringRules() {
207+
// Check if creation time is within rule schedules.
208+
isInSchedules, err := accessmonitoring.InSchedules(rule.GetSpec().GetSchedules(), env.CreationTime)
209+
if err != nil {
210+
log.WarnContext(ctx, "Failed to evaluate access monitoring rule",
211+
"error", err,
212+
"rule", rule.GetMetadata().GetName(),
213+
)
214+
continue
215+
}
216+
if len(rule.GetSpec().GetSchedules()) != 0 && !isInSchedules {
217+
log.DebugContext(ctx, "Access request does not satisfy schedule condition",
218+
"rule", rule.GetMetadata().GetName())
219+
continue
220+
}
221+
222+
// Check if environment matches rule conditions.
191223
match, err := accessmonitoring.EvaluateCondition(rule.Spec.Condition, env)
192224
if err != nil {
193225
log.WarnContext(ctx, "Failed to parse access monitoring notification rule",

integrations/access/accessmonitoring/access_monitoring_rules_test.go

Lines changed: 82 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@ package accessmonitoring
2121
import (
2222
"context"
2323
"testing"
24+
"time"
2425

2526
"github.com/stretchr/testify/mock"
2627
"github.com/stretchr/testify/require"
@@ -52,7 +53,7 @@ func mockFetchRecipient(ctx context.Context, recipient string) (*common.Recipien
5253
return nil, nil
5354
}
5455

55-
func TestRecipeints(t *testing.T) {
56+
func TestRecipients(t *testing.T) {
5657
const (
5758
pluginName = "fakePluginName"
5859
pluginType = "fakePluginType"
@@ -133,7 +134,7 @@ func TestRecipeints(t *testing.T) {
133134
require.ElementsMatch(t, []string{}, rawRecipients)
134135
}
135136

136-
func TestRecipeintsWithResources(t *testing.T) {
137+
func TestRecipientsWithResources(t *testing.T) {
137138
const (
138139
pluginName = "fakePluginName"
139140
pluginType = "fakePluginType"
@@ -205,6 +206,85 @@ func TestRecipeintsWithResources(t *testing.T) {
205206
require.ElementsMatch(t, []string{recipient}, rawRecipients)
206207
}
207208

209+
func TestRecipientsWithSchedules(t *testing.T) {
210+
const (
211+
pluginName = "fakePluginName"
212+
pluginType = "fakePluginType"
213+
recipient = "[email protected]"
214+
)
215+
216+
teleportClient := &mockTeleportClient{}
217+
teleportClient.
218+
On("GetUser", mock.Anything, mock.Anything, mock.Anything).
219+
Return(&types.UserV2{}, nil)
220+
221+
amrh := NewRuleHandler(RuleHandlerConfig{
222+
Client: teleportClient,
223+
PluginType: pluginType,
224+
PluginName: pluginName,
225+
FetchRecipientCallback: func(ctx context.Context, recipient string) (*common.Recipient, error) {
226+
return emailRecipient(recipient), nil
227+
},
228+
})
229+
230+
rule1, err := services.NewAccessMonitoringRuleWithLabels("rule1", nil, &pb.AccessMonitoringRuleSpec{
231+
Subjects: []string{types.KindAccessRequest},
232+
Schedules: map[string]*pb.Schedule{
233+
"default": {
234+
Time: &pb.TimeSchedule{
235+
Shifts: []*pb.TimeSchedule_Shift{
236+
{
237+
Weekday: time.Monday.String(),
238+
Start: "14:00",
239+
End: "15:00",
240+
},
241+
},
242+
},
243+
},
244+
},
245+
Condition: `true`,
246+
Notification: &pb.Notification{
247+
Name: pluginName,
248+
Recipients: []string{recipient},
249+
},
250+
})
251+
require.NoError(t, err)
252+
err = amrh.HandleAccessMonitoringRule(context.Background(), types.Event{
253+
Type: types.OpPut,
254+
Resource: types.Resource153ToLegacy(rule1),
255+
})
256+
require.NoError(t, err)
257+
require.Len(t, amrh.getAccessMonitoringRules(), 1)
258+
259+
ctx := context.Background()
260+
261+
// Expect recipient from matching rule.
262+
req := &types.AccessRequestV3{
263+
Spec: types.AccessRequestSpecV3{
264+
Created: time.Date(2025, time.August, 11, 14, 30, 0, 0, time.UTC),
265+
},
266+
}
267+
268+
recipients := amrh.RecipientsFromAccessMonitoringRules(ctx, req)
269+
require.ElementsMatch(t, []common.Recipient{*emailRecipient(recipient)}, recipients.ToSlice())
270+
271+
rawRecipients := amrh.RawRecipientsFromAccessMonitoringRules(ctx, req)
272+
require.ElementsMatch(t, []string{recipient}, rawRecipients)
273+
274+
// Expect no recipient when not in schedule.
275+
req = &types.AccessRequestV3{
276+
Spec: types.AccessRequestSpecV3{
277+
Created: time.Date(2025, time.August, 11, 15, 30, 0, 0, time.UTC),
278+
},
279+
}
280+
281+
recipients = amrh.RecipientsFromAccessMonitoringRules(ctx, req)
282+
require.ElementsMatch(t, []common.Recipient{}, recipients.ToSlice())
283+
284+
rawRecipients = amrh.RawRecipientsFromAccessMonitoringRules(ctx, req)
285+
require.ElementsMatch(t, []string{}, rawRecipients)
286+
}
287+
208288
func emailRecipient(recipient string) *common.Recipient {
209289
return &common.Recipient{
210290
Name: recipient,

lib/accessmonitoring/review/review.go

Lines changed: 17 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -241,15 +241,31 @@ func (handler *Handler) getMatchingRule(
241241
var reviewRule *accessmonitoringrulesv1.AccessMonitoringRule
242242

243243
for _, rule := range handler.rules.Get() {
244-
conditionMatch, err := accessmonitoring.EvaluateCondition(rule.GetSpec().GetCondition(), env)
244+
245+
// Check if creation time is within rule schedules.
246+
isInSchedules, err := accessmonitoring.InSchedules(rule.GetSpec().GetSchedules(), env.CreationTime)
245247
if err != nil {
246248
handler.Logger.WarnContext(ctx, "Failed to evaluate access monitoring rule",
247249
"error", err,
248250
"rule", rule.GetMetadata().GetName(),
249251
)
250252
continue
251253
}
254+
if len(rule.GetSpec().GetSchedules()) != 0 && !isInSchedules {
255+
handler.Logger.DebugContext(ctx, "Access request does not satisfy schedule condition",
256+
"rule", rule.GetMetadata().GetName())
257+
continue
258+
}
252259

260+
// Check if environment matches rule conditions.
261+
conditionMatch, err := accessmonitoring.EvaluateCondition(rule.GetSpec().GetCondition(), env)
262+
if err != nil {
263+
handler.Logger.WarnContext(ctx, "Failed to evaluate access monitoring rule",
264+
"error", err,
265+
"rule", rule.GetMetadata().GetName(),
266+
)
267+
continue
268+
}
253269
if !conditionMatch {
254270
continue
255271
}

lib/accessmonitoring/review/review_test.go

Lines changed: 107 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -198,6 +198,113 @@ func TestConflictingRules(t *testing.T) {
198198
require.NoError(t, handler.HandleAccessRequest(ctx, event))
199199
}
200200

201+
func TestScheduleRequest(t *testing.T) {
202+
ctx, cancel := context.WithTimeout(context.Background(), time.Second*30)
203+
t.Cleanup(cancel)
204+
205+
testReqID := uuid.New().String()
206+
testRuleName := "test-rule"
207+
withSecretsFalse := false
208+
requesterUserName := "requester"
209+
210+
requester, err := types.NewUser(requesterUserName)
211+
require.NoError(t, err)
212+
213+
testRule := newApprovedRule(
214+
testRuleName,
215+
`true`)
216+
217+
testRule.Spec.Schedules = map[string]*accessmonitoringrulesv1.Schedule{
218+
"test-schedule": {
219+
Time: &accessmonitoringrulesv1.TimeSchedule{
220+
Shifts: []*accessmonitoringrulesv1.TimeSchedule_Shift{
221+
{
222+
Weekday: time.Monday.String(),
223+
Start: "14:00",
224+
End: "15:00",
225+
},
226+
},
227+
},
228+
},
229+
}
230+
231+
cache := accessmonitoring.NewCache()
232+
cache.Put([]*accessmonitoringrulesv1.AccessMonitoringRule{testRule})
233+
234+
tests := []struct {
235+
description string
236+
setupMock func(m *mockClient)
237+
creationTime time.Time
238+
assertErr require.ErrorAssertionFunc
239+
}{
240+
{
241+
description: "test within schedule",
242+
setupMock: func(m *mockClient) {
243+
m.On("GetUser", mock.Anything, requesterUserName, withSecretsFalse).
244+
Return(requester, nil)
245+
246+
review, err := newAccessReview(
247+
requesterUserName,
248+
testRuleName,
249+
types.RequestState_APPROVED.String(),
250+
time.Time{},
251+
)
252+
require.NoError(t, err)
253+
254+
m.On("SubmitAccessReview", mock.Anything, types.AccessReviewSubmission{
255+
RequestID: testReqID,
256+
Review: review,
257+
}).Return(mock.Anything, nil)
258+
},
259+
creationTime: time.Date(2025, time.August, 11, 14, 30, 0, 0, time.UTC),
260+
assertErr: require.NoError,
261+
},
262+
{
263+
description: "test outside schedule",
264+
setupMock: func(m *mockClient) {
265+
m.On("GetUser", mock.Anything, requesterUserName, withSecretsFalse).
266+
Return(requester, nil)
267+
268+
m.AssertNotCalled(t, "SubmitAccessReview")
269+
},
270+
creationTime: time.Date(2025, time.August, 11, 15, 30, 0, 0, time.UTC),
271+
assertErr: require.NoError,
272+
},
273+
}
274+
275+
for _, test := range tests {
276+
t.Run(test.description, func(t *testing.T) {
277+
t.Parallel()
278+
279+
client := &mockClient{}
280+
if test.setupMock != nil {
281+
test.setupMock(client)
282+
}
283+
284+
handler, err := NewHandler(Config{
285+
HandlerName: handlerName,
286+
Client: client,
287+
Cache: cache,
288+
})
289+
require.NoError(t, err)
290+
291+
req, err := types.NewAccessRequest(
292+
testReqID,
293+
requesterUserName,
294+
"role",
295+
)
296+
require.NoError(t, err)
297+
req.SetCreationTime(test.creationTime)
298+
299+
test.assertErr(t, handler.HandleAccessRequest(ctx, types.Event{
300+
Type: types.OpPut,
301+
Resource: req,
302+
}))
303+
client.AssertExpectations(t)
304+
})
305+
}
306+
}
307+
201308
func TestResourceRequest(t *testing.T) {
202309
ctx, cancel := context.WithTimeout(context.Background(), time.Second*30)
203310
t.Cleanup(cancel)

lib/accessmonitoring/schedule.go

Lines changed: 93 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,93 @@
1+
/*
2+
Copyright 2025 Gravitational, Inc.
3+
4+
Licensed under the Apache License, Version 2.0 (the "License");
5+
you may not use this file except in compliance with the License.
6+
You may obtain a copy of the License at
7+
8+
http://www.apache.org/licenses/LICENSE-2.0
9+
10+
Unless required by applicable law or agreed to in writing, software
11+
distributed under the License is distributed on an "AS IS" BASIS,
12+
WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
13+
See the License for the specific language governing permissions and
14+
limitations under the License.
15+
*/
16+
17+
package accessmonitoring
18+
19+
import (
20+
"time"
21+
22+
"github.com/gravitational/trace"
23+
24+
accessmonitoringrulesv1 "github.com/gravitational/teleport/api/gen/proto/go/teleport/accessmonitoringrules/v1"
25+
)
26+
27+
// ClockTime returns a new time value overriding the hour and minute.
28+
func ClockTime(timestamp time.Time, hourMinute string) (time.Time, error) {
29+
const hourMinuteFormat = "15:04" // 24-hour HH:MM format
30+
31+
parsed, err := time.ParseInLocation(hourMinuteFormat, hourMinute, timestamp.Location())
32+
if err != nil {
33+
return time.Time{}, trace.Wrap(err)
34+
}
35+
36+
return time.Date(timestamp.Year(), timestamp.Month(), timestamp.Day(),
37+
parsed.Hour(), parsed.Minute(), 0, 0, timestamp.Location()), nil
38+
}
39+
40+
// inSchedule returns true if the timestamp is within the schedule.
41+
func inSchedule(schedule *accessmonitoringrulesv1.Schedule, timestamp time.Time) (bool, error) {
42+
if schedule.GetTime() == nil {
43+
return false, nil
44+
}
45+
46+
if len(schedule.GetTime().GetShifts()) == 0 {
47+
return false, nil
48+
}
49+
50+
loc, err := time.LoadLocation(schedule.GetTime().GetTimezone())
51+
if err != nil {
52+
return false, trace.Wrap(err)
53+
}
54+
55+
timestamp = timestamp.In(loc)
56+
weekday := timestamp.Weekday().String()
57+
58+
for _, shift := range schedule.GetTime().GetShifts() {
59+
if weekday != shift.Weekday {
60+
continue
61+
}
62+
63+
startTime, err := ClockTime(timestamp, shift.Start)
64+
if err != nil {
65+
return false, trace.Wrap(err, "invalid start time: %q", shift.Start)
66+
}
67+
68+
endTime, err := ClockTime(timestamp, shift.End)
69+
if err != nil {
70+
return false, trace.Wrap(err, "invalid end time: %q", shift.End)
71+
}
72+
73+
if !timestamp.Before(startTime) && !timestamp.After(endTime) {
74+
return true, nil
75+
}
76+
}
77+
return false, nil
78+
}
79+
80+
// InSchedules returns true if the provided timestamp is within an of the provided
81+
// schedules. Returns false if schedules is empty.
82+
func InSchedules(schedules map[string]*accessmonitoringrulesv1.Schedule, timestamp time.Time) (bool, error) {
83+
for _, schedule := range schedules {
84+
isInSchedule, err := inSchedule(schedule, timestamp)
85+
if err != nil {
86+
return false, trace.Wrap(err)
87+
}
88+
if isInSchedule {
89+
return true, nil
90+
}
91+
}
92+
return false, nil
93+
}

0 commit comments

Comments
 (0)