Add the internal access list review resource. (#32563)

* Add the internal access list review resource. The internal access list review resource has been added. * Parse date and duration strings. * Run GCI.
gravitational · Oct 2, 2023 · 0ec08a2 · 0ec08a2
1 parent a4b3248
commit 0ec08a2
Show file tree

Hide file tree

Showing 6 changed files with 604 additions and 1 deletion.
diff --git a/api/types/accesslist/convert/v1/member.go b/api/types/accesslist/convert/v1/member.go
@@ -30,7 +30,7 @@ type MemberOption func(*accesslist.AccessListMember)
 // FromMemberProto converts a v1 access list member into an internal access list member object.
 func FromMemberProto(msg *accesslistv1.Member, opts ...MemberOption) (*accesslist.AccessListMember, error) {
 	if msg == nil {
-		return nil, trace.BadParameter("access list message is nil")
+		return nil, trace.BadParameter("access list member message is nil")
 	}
 
 	if msg.Spec == nil {

diff --git a/api/types/accesslist/convert/v1/review.go b/api/types/accesslist/convert/v1/review.go
@@ -0,0 +1,113 @@
+/*
+Copyright 2023 Gravitational, Inc.
+
+Licensed under the Apache License, Version 2.0 (the "License");
+you may not use this file except in compliance with the License.
+You may obtain a copy of the License at
+
+    http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+package v1
+
+import (
+	"time"
+
+	"github.com/gravitational/trace"
+	"google.golang.org/protobuf/types/known/durationpb"
+	"google.golang.org/protobuf/types/known/timestamppb"
+
+	accesslistv1 "github.com/gravitational/teleport/api/gen/proto/go/teleport/accesslist/v1"
+	"github.com/gravitational/teleport/api/types/accesslist"
+	headerv1 "github.com/gravitational/teleport/api/types/header/convert/v1"
+	traitv1 "github.com/gravitational/teleport/api/types/trait/convert/v1"
+)
+
+// FromReviewProto converts a v1 access list review into an internal access list review object.
+func FromReviewProto(msg *accesslistv1.Review) (*accesslist.Review, error) {
+	if msg == nil {
+		return nil, trace.BadParameter("access list review message is nil")
+	}
+
+	if msg.Spec == nil {
+		return nil, trace.BadParameter("spec is missing")
+	}
+
+	// Manually check for the presence of the time so that we can be sure that the review date is
+	// zero if the proto message's review date is nil.
+	var reviewDate time.Time
+	if msg.Spec.ReviewDate != nil {
+		reviewDate = msg.Spec.ReviewDate.AsTime()
+	}
+
+	var reviewChanges accesslist.ReviewChanges
+	if msg.Spec.Changes != nil {
+		if msg.Spec.Changes.FrequencyChanged != nil {
+			reviewChanges.FrequencyChanged = msg.Spec.Changes.FrequencyChanged.AsDuration()
+		}
+		if msg.Spec.Changes.MembershipRequirementsChanged != nil {
+			reviewChanges.MembershipRequirementsChanged = &accesslist.Requires{
+				Roles:  msg.Spec.Changes.MembershipRequirementsChanged.Roles,
+				Traits: traitv1.FromProto(msg.Spec.Changes.MembershipRequirementsChanged.Traits),
+			}
+		}
+		reviewChanges.RemovedMembers = msg.Spec.Changes.RemovedMembers
+	}
+
+	member, err := accesslist.NewReview(headerv1.FromMetadataProto(msg.Header.Metadata), accesslist.ReviewSpec{
+		AccessList: msg.Spec.AccessList,
+		Reviewers:  msg.Spec.Reviewers,
+		ReviewDate: reviewDate,
+		Notes:      msg.Spec.Notes,
+		Changes:    reviewChanges,
+	})
+	if err != nil {
+		return nil, trace.Wrap(err)
+	}
+
+	return member, nil
+}
+
+// ToReviewProto converts an internal access list review into a v1 access list review object.
+func ToReviewProto(review *accesslist.Review) *accesslistv1.Review {
+	var reviewChanges *accesslistv1.ReviewChanges
+	if review.Spec.Changes.FrequencyChanged > 0 {
+		reviewChanges = &accesslistv1.ReviewChanges{
+			FrequencyChanged: durationpb.New(review.Spec.Changes.FrequencyChanged),
+		}
+	}
+	if review.Spec.Changes.MembershipRequirementsChanged != nil {
+		if reviewChanges == nil {
+			reviewChanges = &accesslistv1.ReviewChanges{}
+		}
+
+		reviewChanges.MembershipRequirementsChanged = &accesslistv1.AccessListRequires{
+			Roles:  review.Spec.Changes.MembershipRequirementsChanged.Roles,
+			Traits: traitv1.ToProto(review.Spec.Changes.MembershipRequirementsChanged.Traits),
+		}
+	}
+	if len(review.Spec.Changes.RemovedMembers) > 0 {
+		if reviewChanges == nil {
+			reviewChanges = &accesslistv1.ReviewChanges{}
+		}
+
+		reviewChanges.RemovedMembers = review.Spec.Changes.RemovedMembers
+	}
+
+	return &accesslistv1.Review{
+		Header: headerv1.ToResourceHeaderProto(review.ResourceHeader),
+		Spec: &accesslistv1.ReviewSpec{
+			AccessList: review.Spec.AccessList,
+			Reviewers:  review.Spec.Reviewers,
+			ReviewDate: timestamppb.New(review.Spec.ReviewDate),
+			Notes:      review.Spec.Notes,
+			Changes:    reviewChanges,
+		},
+	}
+}
diff --git a/api/types/accesslist/convert/v1/review_test.go b/api/types/accesslist/convert/v1/review_test.go
@@ -0,0 +1,193 @@
+/*
+Copyright 2023 Gravitational, Inc.
+
+Licensed under the Apache License, Version 2.0 (the "License");
+you may not use this file except in compliance with the License.
+You may obtain a copy of the License at
+
+    http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+package v1
+
+import (
+	"testing"
+	"time"
+
+	"github.com/google/go-cmp/cmp"
+	"github.com/stretchr/testify/require"
+
+	"github.com/gravitational/teleport/api/types/accesslist"
+	"github.com/gravitational/teleport/api/types/header"
+	"github.com/gravitational/teleport/api/types/trait"
+	traitv1 "github.com/gravitational/teleport/api/types/trait/convert/v1"
+)
+
+func TestReviewRoundtrip(t *testing.T) {
+	t.Parallel()
+
+	review := newAccessListReview(t, "access-list-review")
+
+	converted, err := FromReviewProto(ToReviewProto(review))
+	require.NoError(t, err)
+
+	require.Empty(t, cmp.Diff(review, converted))
+}
+
+// Make sure that we don't panic if any of the message fields are missing.
+func TestReviewFromProtoNils(t *testing.T) {
+	t.Parallel()
+
+	// Message is nil
+	_, err := FromReviewProto(nil)
+	require.Error(t, err)
+
+	// Spec is nil
+	review := ToReviewProto(newAccessListReview(t, "access-list-review"))
+	review.Spec = nil
+
+	_, err = FromReviewProto(review)
+	require.Error(t, err)
+
+	// AccessList is empty
+	review = ToReviewProto(newAccessListReview(t, "access-list-review"))
+	review.Spec.AccessList = ""
+
+	_, err = FromReviewProto(review)
+	require.Error(t, err)
+
+	// Reviewers is empty
+	review = ToReviewProto(newAccessListReview(t, "access-list-review"))
+	review.Spec.Reviewers = nil
+
+	_, err = FromReviewProto(review)
+	require.Error(t, err)
+
+	// ReviewDate is nil
+	review = ToReviewProto(newAccessListReview(t, "access-list-review"))
+	review.Spec.ReviewDate = nil
+
+	_, err = FromReviewProto(review)
+	require.Error(t, err)
+
+	// Notes is empty
+	review = ToReviewProto(newAccessListReview(t, "access-list-review"))
+	review.Spec.Notes = ""
+
+	_, err = FromReviewProto(review)
+	require.NoError(t, err)
+
+	// Changes is nil
+	review = ToReviewProto(newAccessListReview(t, "access-list-review"))
+	review.Spec.Changes = nil
+
+	_, err = FromReviewProto(review)
+	require.NoError(t, err)
+
+	// FrequencyChanged is nil
+	review = ToReviewProto(newAccessListReview(t, "access-list-review"))
+	review.Spec.Changes.FrequencyChanged = nil
+
+	_, err = FromReviewProto(review)
+	require.NoError(t, err)
+
+	// MembershipRequirementsChanged is nil
+	review = ToReviewProto(newAccessListReview(t, "access-list-review"))
+	review.Spec.Changes.MembershipRequirementsChanged = nil
+
+	_, err = FromReviewProto(review)
+	require.NoError(t, err)
+
+	// RemovedMembers is nil
+	review = ToReviewProto(newAccessListReview(t, "access-list-review"))
+	review.Spec.Changes.RemovedMembers = nil
+
+	_, err = FromReviewProto(review)
+	require.NoError(t, err)
+}
+
+func TestReviewToProtoChanges(t *testing.T) {
+	t.Parallel()
+
+	// No changes.
+	review := newAccessListReview(t, "access-list-review")
+	review.Spec.Changes.FrequencyChanged = 0
+	review.Spec.Changes.MembershipRequirementsChanged = nil
+	review.Spec.Changes.RemovedMembers = nil
+
+	msg := ToReviewProto(review)
+	require.Nil(t, msg.Spec.Changes)
+
+	// Only frequency changes.
+	review = newAccessListReview(t, "access-list-review")
+	review.Spec.Changes.MembershipRequirementsChanged = nil
+	review.Spec.Changes.RemovedMembers = nil
+
+	msg = ToReviewProto(review)
+	require.Equal(t, review.Spec.Changes.FrequencyChanged, msg.Spec.Changes.FrequencyChanged.AsDuration())
+	require.Nil(t, msg.Spec.Changes.MembershipRequirementsChanged)
+	require.Nil(t, msg.Spec.Changes.RemovedMembers)
+
+	// Only membership requires changes.
+	review = newAccessListReview(t, "access-list-review")
+	review.Spec.Changes.FrequencyChanged = 0
+	review.Spec.Changes.RemovedMembers = nil
+
+	msg = ToReviewProto(review)
+	require.Equal(t, time.Duration(0), review.Spec.Changes.FrequencyChanged)
+	require.Equal(t, review.Spec.Changes.MembershipRequirementsChanged.Roles, msg.Spec.Changes.MembershipRequirementsChanged.Roles)
+	require.Equal(t, review.Spec.Changes.MembershipRequirementsChanged.Traits, traitv1.FromProto(msg.Spec.Changes.MembershipRequirementsChanged.Traits))
+	require.Nil(t, msg.Spec.Changes.RemovedMembers)
+
+	// Only removed members changes.
+	review = newAccessListReview(t, "access-list-review")
+	review.Spec.Changes.FrequencyChanged = 0
+	review.Spec.Changes.MembershipRequirementsChanged = nil
+
+	msg = ToReviewProto(review)
+	require.Equal(t, time.Duration(0), review.Spec.Changes.FrequencyChanged)
+	require.Nil(t, msg.Spec.Changes.MembershipRequirementsChanged)
+	require.Equal(t, review.Spec.Changes.RemovedMembers, msg.Spec.Changes.RemovedMembers)
+}
+
+func newAccessListReview(t *testing.T, name string) *accesslist.Review {
+	t.Helper()
+
+	accessList, err := accesslist.NewReview(
+		header.Metadata{
+			Name: name,
+		},
+		accesslist.ReviewSpec{
+			AccessList: "access-list",
+			Reviewers: []string{
+				"reviewer1",
+				"reviewer2",
+			},
+			ReviewDate: time.Date(2023, 01, 01, 0, 0, 0, 0, time.UTC),
+			Notes:      "some notes",
+			Changes: accesslist.ReviewChanges{
+				FrequencyChanged: 20 * time.Hour,
+				MembershipRequirementsChanged: &accesslist.Requires{
+					Roles: []string{"role1", "role2"},
+					Traits: trait.Traits{
+						"trait1": []string{"value1"},
+						"trait2": []string{"value2"},
+					},
+				},
+				RemovedMembers: []string{
+					"removed1",
+					"removed2",
+					"removed3",
+				},
+			},
+		},
+	)
+	require.NoError(t, err)
+	return accessList
+}