Skip to content

Add max duration to msteams plugin cards #54110

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

Merged
EdwardDowling merged 13 commits into from
Apr 29, 2025
Merged

Add max duration to msteams plugin cards #54110

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
13 commits
Select commit Hold shift + click to select a range
341a4ef
Add max duration to msteams plugin cards
EdwardDowling Apr 17, 2025
09545a1
Merge branch 'master' into edwarddowling/msteams-request-duration
EdwardDowling Apr 23, 2025
c58e919
Add test coverage to max_duration addition to plugindata
EdwardDowling Apr 23, 2025
7128fd8
Merge branch 'master' into edwarddowling/msteams-request-duration
EdwardDowling Apr 23, 2025
0fff468
Merge branch 'master' into edwarddowling/msteams-request-duration
EdwardDowling Apr 25, 2025
922f198
Fix incorrect usage of maxDuration in msteams plugin
EdwardDowling Apr 25, 2025
9a04b4e
Update integrations/lib/plugindata/access_request.go
EdwardDowling Apr 28, 2025
9ed88f4
Fix error return from access request plugin data
EdwardDowling Apr 28, 2025
21d02b9
Fix formating in plugindata tests
EdwardDowling Apr 28, 2025
1ab23ea
Merge branch 'master' into edwarddowling/msteams-request-duration
EdwardDowling Apr 28, 2025
3da8308
Update msteams plugin tests
EdwardDowling Apr 28, 2025
27a0659
Merge branch 'master' into edwarddowling/msteams-request-duration
EdwardDowling Apr 29, 2025
94621e2
Merge branch 'master' into edwarddowling/msteams-request-duration
EdwardDowling Apr 29, 2025
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
43 changes: 26 additions & 17 deletions integrations/access/accessrequest/plugindata_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,32 +20,39 @@ package accessrequest

import (
"testing"
"time"

"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"

"github.com/gravitational/teleport/integrations/lib/plugindata"
)

var samplePluginData = PluginData{
AccessRequestData: plugindata.AccessRequestData{
User: "user-foo",
Roles: []string{"role-foo", "role-bar"},
Resources: []string{"cluster-a/node/foo", "cluster-a/node/bar"},
RequestReason: "foo reason",
ReviewsCount: 3,
ResolutionTag: plugindata.ResolvedApproved,
ResolutionReason: "foo ok",
},
SentMessages: SentMessages{
{ChannelID: "CHANNEL1", MessageID: "0000001"},
{ChannelID: "CHANNEL2", MessageID: "0000002"},
},
func getSamplePluginData(t *testing.T) PluginData {
maxDuration, err := time.Parse(time.RFC3339, "2006-01-02T15:04:05Z")
require.NoError(t, err)
return PluginData{
AccessRequestData: plugindata.AccessRequestData{
User: "user-foo",
Roles: []string{"role-foo", "role-bar"},
Resources: []string{"cluster-a/node/foo", "cluster-a/node/bar"},
RequestReason: "foo reason",
ReviewsCount: 3,
ResolutionTag: plugindata.ResolvedApproved,
ResolutionReason: "foo ok",
MaxDuration: &maxDuration,
},
SentMessages: SentMessages{
{ChannelID: "CHANNEL1", MessageID: "0000001"},
{ChannelID: "CHANNEL2", MessageID: "0000002"},
},
}
}

func TestEncodePluginData(t *testing.T) {
dataMap, err := EncodePluginData(samplePluginData)
dataMap, err := EncodePluginData(getSamplePluginData(t))
assert.NoError(t, err)
assert.Len(t, dataMap, 8)
assert.Len(t, dataMap, 9)
assert.Equal(t, "user-foo", dataMap["user"])
assert.Equal(t, "role-foo,role-bar", dataMap["roles"])
assert.Equal(t, `["cluster-a/node/foo","cluster-a/node/bar"]`, dataMap["resources"])
Expand All @@ -54,6 +61,7 @@ func TestEncodePluginData(t *testing.T) {
assert.Equal(t, "APPROVED", dataMap["resolution"])
assert.Equal(t, "foo ok", dataMap["resolve_reason"])
assert.Equal(t, "CHANNEL1/0000001,CHANNEL2/0000002", dataMap["messages"])
assert.Equal(t, "2006-01-02T15:04:05Z", dataMap["max_duration"])
}

func TestDecodePluginData(t *testing.T) {
Expand All @@ -66,9 +74,10 @@ func TestDecodePluginData(t *testing.T) {
"resolution": "APPROVED",
"resolve_reason": "foo ok",
"messages": "CHANNEL1/0000001,CHANNEL2/0000002",
"max_duration": "2006-01-02T15:04:05Z",
})
assert.NoError(t, err)
assert.Equal(t, samplePluginData, pluginData)
assert.Equal(t, getSamplePluginData(t), pluginData)
}

func TestEncodeEmptyPluginData(t *testing.T) {
Expand Down
2 changes: 2 additions & 0 deletions integrations/access/msteams/app.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -337,10 +337,12 @@ func (a *App) onWatcherEvent(ctx context.Context, event types.Event) error {
// onPendingRequest is called when there's a new request or a review
func (a *App) onPendingRequest(ctx context.Context, req types.AccessRequest) error {
id := req.GetName()
maxDuration := req.GetMaxDuration()
data := pd.AccessRequestData{
User: req.GetUser(),
Roles: req.GetRoles(),
RequestReason: req.GetRequestReason(),
MaxDuration: &maxDuration,
}

log := a.log.With("request_id", id)
Expand Down
1 change: 1 addition & 0 deletions integrations/access/msteams/card.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,7 @@ func BuildCard(id string, webProxyURL *url.URL, clusterName string, data plugind
{Title: "Cluster", Value: clusterName},
{Title: "User", Value: data.User},
{Title: "Role(s)", Value: strings.Join(data.Roles, ", ")},
{Title: "Max Duration", Value: data.MaxDuration.String()},
}

if data.RequestReason != "" {
Expand Down
4 changes: 2 additions & 2 deletions integrations/access/msteams/testlib/message_parser.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@ func (msg testTeamsMessage) checkStatusApproved(t *testing.T, reason string) {
require.GreaterOrEqual(t, len(body), 3)
require.Equal(t, "✅", body[1].Columns[0].Items[0].Text)
require.Equal(t, "APPROVED", body[1].Columns[1].Items[0].Text)
require.Equal(t, reason, body[2].Facts[4].Value)
require.Equal(t, reason, body[2].Facts[5].Value)
}

func (msg testTeamsMessage) checkStatusDenied(t *testing.T, reason string) {
Expand All @@ -61,7 +61,7 @@ func (msg testTeamsMessage) checkStatusDenied(t *testing.T, reason string) {
require.GreaterOrEqual(t, len(body), 3)
require.Equal(t, "❌", body[1].Columns[0].Items[0].Text)
require.Equal(t, "DENIED", body[1].Columns[1].Items[0].Text)
require.Equal(t, reason, body[2].Facts[4].Value)
require.Equal(t, reason, body[2].Facts[5].Value)
}

func (msg testTeamsMessage) checkStatusExpired(t *testing.T) {
Expand Down
14 changes: 14 additions & 0 deletions integrations/lib/plugindata/access_request.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ import (
"encoding/json"
"fmt"
"strings"
"time"

"github.com/gravitational/trace"
)
Expand Down Expand Up @@ -49,6 +50,7 @@ type AccessRequestData struct {
Resources []string
SuggestedReviewers []string
LoginsByRole map[string][]string
MaxDuration *time.Time
Copy link
Copy Markdown
Contributor

@hugoShaka hugoShaka Apr 23, 2025

Choose a reason for hiding this comment

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

I'm confused by the type here. The name says "duration", but this is a "time" object. Should it be the access request end date, or a time.Duration?

Copy link
Copy Markdown
Contributor Author

@EdwardDowling EdwardDowling Apr 24, 2025

Choose a reason for hiding this comment

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

It is the the // GetMaxDuration gets the maximum time at which the access should be approved for. I think we just call it maxDuration since we take it in as a duration usually and convert it to the target time on request creation to avoid having to store the duration and the time the duration is from.

Copy link
Copy Markdown
Contributor

@hugoShaka hugoShaka Apr 25, 2025

Choose a reason for hiding this comment

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

My bad, I never realized that we made this mistake 2 years ago when introducing max duration 🤦 .
MaxDuration on a role is indeed a duration, but because naming is hard we also named it MaxDuration on the access request even though it's a timestamp.

Copy link
Copy Markdown
Contributor

@tigrato tigrato Apr 25, 2025

Choose a reason for hiding this comment

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

Although we did this error, can we at least rename this field to be LatestApprovalTime?

Copy link
Copy Markdown
Contributor Author

@EdwardDowling EdwardDowling Apr 28, 2025

Choose a reason for hiding this comment

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

Would changing the name only here make things clearer or would that just add to the existing confusion?

}

// DecodeAccessRequestData deserializes a string map to PluginData struct.
Expand All @@ -63,6 +65,15 @@ func DecodeAccessRequestData(dataMap map[string]string) (data AccessRequestData,
}
data.ResolutionTag = ResolutionTag(dataMap["resolution"])
data.ResolutionReason = dataMap["resolve_reason"]
if str := dataMap["max_duration"]; str != "" {
var maxDuration time.Time
maxDuration, err = time.Parse(time.RFC3339, str)
if err != nil {
err = trace.Wrap(err)
return
Comment thread
EdwardDowling marked this conversation as resolved.
}
data.MaxDuration = &maxDuration
}

if str, ok := dataMap["resources"]; ok {
err = json.Unmarshal([]byte(str), &data.Resources)
Expand Down Expand Up @@ -131,6 +142,9 @@ func EncodeAccessRequestData(data AccessRequestData) (map[string]string, error)
result["reviews_count"] = reviewsCountStr
result["resolution"] = string(data.ResolutionTag)
result["resolve_reason"] = data.ResolutionReason
if data.MaxDuration != nil {
result["max_duration"] = data.MaxDuration.Format(time.RFC3339)
}

if len(data.SystemAnnotations) != 0 {
annotaions, err := json.Marshal(data.SystemAnnotations)
Expand Down
Loading