From 19f7305a6ae3e4ac6ca9752370f1734109cd4887 Mon Sep 17 00:00:00 2001 From: Mike Wilson Date: Mon, 6 Nov 2023 09:32:23 -0500 Subject: [PATCH 1/2] Store original roles with user login state. The user's original static roles are now stored along with the user login state. This is necessary to properly recalculate user access on access list membership change, as SSO users will expire after 24 hours, making it impossible to refer to the original user definition to determine the new role membership for the user. --- .../userloginstate/v1/userloginstate.pb.go | 41 +++++--- .../userloginstate/v1/userloginstate.proto | 3 + .../convert/v1/user_login_state.go | 14 +-- .../convert/v1/user_login_state_test.go | 3 +- api/types/userloginstate/user_login_state.go | 8 ++ lib/auth/userloginstate/generator.go | 9 +- lib/auth/userloginstate/generator_test.go | 93 ++++++------------- lib/auth/userloginstate/service_test.go | 16 ++-- 8 files changed, 91 insertions(+), 96 deletions(-) diff --git a/api/gen/proto/go/teleport/userloginstate/v1/userloginstate.pb.go b/api/gen/proto/go/teleport/userloginstate/v1/userloginstate.pb.go index 101d653814e46..af206daccdf73 100644 --- a/api/gen/proto/go/teleport/userloginstate/v1/userloginstate.pb.go +++ b/api/gen/proto/go/teleport/userloginstate/v1/userloginstate.pb.go @@ -106,6 +106,8 @@ type Spec struct { Traits []*v11.Trait `protobuf:"bytes,2,rep,name=traits,proto3" json:"traits,omitempty"` // user_type is the type of user this state represents. UserType string `protobuf:"bytes,3,opt,name=user_type,json=userType,proto3" json:"user_type,omitempty"` + // original_roles are the user roles that the user originally had prior to the user login state being generated. + OriginalRoles []string `protobuf:"bytes,4,rep,name=original_roles,json=originalRoles,proto3" json:"original_roles,omitempty"` } func (x *Spec) Reset() { @@ -161,6 +163,13 @@ func (x *Spec) GetUserType() string { return "" } +func (x *Spec) GetOriginalRoles() []string { + if x != nil { + return x.OriginalRoles + } + return nil +} + var File_teleport_userloginstate_v1_userloginstate_proto protoreflect.FileDescriptor var file_teleport_userloginstate_v1_userloginstate_proto_rawDesc = []byte{ @@ -181,21 +190,23 @@ var file_teleport_userloginstate_v1_userloginstate_proto_rawDesc = []byte{ 0x61, 0x64, 0x65, 0x72, 0x12, 0x34, 0x0a, 0x04, 0x73, 0x70, 0x65, 0x63, 0x18, 0x02, 0x20, 0x01, 0x28, 0x0b, 0x32, 0x20, 0x2e, 0x74, 0x65, 0x6c, 0x65, 0x70, 0x6f, 0x72, 0x74, 0x2e, 0x75, 0x73, 0x65, 0x72, 0x6c, 0x6f, 0x67, 0x69, 0x6e, 0x73, 0x74, 0x61, 0x74, 0x65, 0x2e, 0x76, 0x31, 0x2e, - 0x53, 0x70, 0x65, 0x63, 0x52, 0x04, 0x73, 0x70, 0x65, 0x63, 0x22, 0x6b, 0x0a, 0x04, 0x53, 0x70, - 0x65, 0x63, 0x12, 0x14, 0x0a, 0x05, 0x72, 0x6f, 0x6c, 0x65, 0x73, 0x18, 0x01, 0x20, 0x03, 0x28, - 0x09, 0x52, 0x05, 0x72, 0x6f, 0x6c, 0x65, 0x73, 0x12, 0x30, 0x0a, 0x06, 0x74, 0x72, 0x61, 0x69, - 0x74, 0x73, 0x18, 0x02, 0x20, 0x03, 0x28, 0x0b, 0x32, 0x18, 0x2e, 0x74, 0x65, 0x6c, 0x65, 0x70, - 0x6f, 0x72, 0x74, 0x2e, 0x74, 0x72, 0x61, 0x69, 0x74, 0x2e, 0x76, 0x31, 0x2e, 0x54, 0x72, 0x61, - 0x69, 0x74, 0x52, 0x06, 0x74, 0x72, 0x61, 0x69, 0x74, 0x73, 0x12, 0x1b, 0x0a, 0x09, 0x75, 0x73, - 0x65, 0x72, 0x5f, 0x74, 0x79, 0x70, 0x65, 0x18, 0x03, 0x20, 0x01, 0x28, 0x09, 0x52, 0x08, 0x75, - 0x73, 0x65, 0x72, 0x54, 0x79, 0x70, 0x65, 0x42, 0x60, 0x5a, 0x5e, 0x67, 0x69, 0x74, 0x68, 0x75, - 0x62, 0x2e, 0x63, 0x6f, 0x6d, 0x2f, 0x67, 0x72, 0x61, 0x76, 0x69, 0x74, 0x61, 0x74, 0x69, 0x6f, - 0x6e, 0x61, 0x6c, 0x2f, 0x74, 0x65, 0x6c, 0x65, 0x70, 0x6f, 0x72, 0x74, 0x2f, 0x61, 0x70, 0x69, - 0x2f, 0x67, 0x65, 0x6e, 0x2f, 0x70, 0x72, 0x6f, 0x74, 0x6f, 0x2f, 0x67, 0x6f, 0x2f, 0x74, 0x65, - 0x6c, 0x65, 0x70, 0x6f, 0x72, 0x74, 0x2f, 0x75, 0x73, 0x65, 0x72, 0x6c, 0x6f, 0x67, 0x69, 0x6e, - 0x73, 0x74, 0x61, 0x74, 0x65, 0x2f, 0x76, 0x31, 0x3b, 0x75, 0x73, 0x65, 0x72, 0x6c, 0x6f, 0x67, - 0x69, 0x6e, 0x73, 0x74, 0x61, 0x74, 0x65, 0x76, 0x31, 0x62, 0x06, 0x70, 0x72, 0x6f, 0x74, 0x6f, - 0x33, + 0x53, 0x70, 0x65, 0x63, 0x52, 0x04, 0x73, 0x70, 0x65, 0x63, 0x22, 0x92, 0x01, 0x0a, 0x04, 0x53, + 0x70, 0x65, 0x63, 0x12, 0x14, 0x0a, 0x05, 0x72, 0x6f, 0x6c, 0x65, 0x73, 0x18, 0x01, 0x20, 0x03, + 0x28, 0x09, 0x52, 0x05, 0x72, 0x6f, 0x6c, 0x65, 0x73, 0x12, 0x30, 0x0a, 0x06, 0x74, 0x72, 0x61, + 0x69, 0x74, 0x73, 0x18, 0x02, 0x20, 0x03, 0x28, 0x0b, 0x32, 0x18, 0x2e, 0x74, 0x65, 0x6c, 0x65, + 0x70, 0x6f, 0x72, 0x74, 0x2e, 0x74, 0x72, 0x61, 0x69, 0x74, 0x2e, 0x76, 0x31, 0x2e, 0x54, 0x72, + 0x61, 0x69, 0x74, 0x52, 0x06, 0x74, 0x72, 0x61, 0x69, 0x74, 0x73, 0x12, 0x1b, 0x0a, 0x09, 0x75, + 0x73, 0x65, 0x72, 0x5f, 0x74, 0x79, 0x70, 0x65, 0x18, 0x03, 0x20, 0x01, 0x28, 0x09, 0x52, 0x08, + 0x75, 0x73, 0x65, 0x72, 0x54, 0x79, 0x70, 0x65, 0x12, 0x25, 0x0a, 0x0e, 0x6f, 0x72, 0x69, 0x67, + 0x69, 0x6e, 0x61, 0x6c, 0x5f, 0x72, 0x6f, 0x6c, 0x65, 0x73, 0x18, 0x04, 0x20, 0x03, 0x28, 0x09, + 0x52, 0x0d, 0x6f, 0x72, 0x69, 0x67, 0x69, 0x6e, 0x61, 0x6c, 0x52, 0x6f, 0x6c, 0x65, 0x73, 0x42, + 0x60, 0x5a, 0x5e, 0x67, 0x69, 0x74, 0x68, 0x75, 0x62, 0x2e, 0x63, 0x6f, 0x6d, 0x2f, 0x67, 0x72, + 0x61, 0x76, 0x69, 0x74, 0x61, 0x74, 0x69, 0x6f, 0x6e, 0x61, 0x6c, 0x2f, 0x74, 0x65, 0x6c, 0x65, + 0x70, 0x6f, 0x72, 0x74, 0x2f, 0x61, 0x70, 0x69, 0x2f, 0x67, 0x65, 0x6e, 0x2f, 0x70, 0x72, 0x6f, + 0x74, 0x6f, 0x2f, 0x67, 0x6f, 0x2f, 0x74, 0x65, 0x6c, 0x65, 0x70, 0x6f, 0x72, 0x74, 0x2f, 0x75, + 0x73, 0x65, 0x72, 0x6c, 0x6f, 0x67, 0x69, 0x6e, 0x73, 0x74, 0x61, 0x74, 0x65, 0x2f, 0x76, 0x31, + 0x3b, 0x75, 0x73, 0x65, 0x72, 0x6c, 0x6f, 0x67, 0x69, 0x6e, 0x73, 0x74, 0x61, 0x74, 0x65, 0x76, + 0x31, 0x62, 0x06, 0x70, 0x72, 0x6f, 0x74, 0x6f, 0x33, } var ( diff --git a/api/proto/teleport/userloginstate/v1/userloginstate.proto b/api/proto/teleport/userloginstate/v1/userloginstate.proto index 56c7facbaa5c4..54c482dd4f0d0 100644 --- a/api/proto/teleport/userloginstate/v1/userloginstate.proto +++ b/api/proto/teleport/userloginstate/v1/userloginstate.proto @@ -40,4 +40,7 @@ message Spec { // user_type is the type of user this state represents. string user_type = 3; + + // original_roles are the user roles that the user originally had prior to the user login state being generated. + repeated string original_roles = 4; } diff --git a/api/types/userloginstate/convert/v1/user_login_state.go b/api/types/userloginstate/convert/v1/user_login_state.go index 92ebd1d708628..2b339b9a615ec 100644 --- a/api/types/userloginstate/convert/v1/user_login_state.go +++ b/api/types/userloginstate/convert/v1/user_login_state.go @@ -33,9 +33,10 @@ func FromProto(msg *userloginstatev1.UserLoginState) (*userloginstate.UserLoginS } uls, err := userloginstate.New(headerv1.FromMetadataProto(msg.Header.Metadata), userloginstate.Spec{ - Roles: msg.Spec.Roles, - Traits: traitv1.FromProto(msg.Spec.Traits), - UserType: types.UserType(msg.Spec.UserType), + OriginalRoles: msg.Spec.GetOriginalRoles(), + Roles: msg.Spec.Roles, + Traits: traitv1.FromProto(msg.Spec.Traits), + UserType: types.UserType(msg.Spec.UserType), }) return uls, trace.Wrap(err) @@ -46,9 +47,10 @@ func ToProto(uls *userloginstate.UserLoginState) *userloginstatev1.UserLoginStat return &userloginstatev1.UserLoginState{ Header: headerv1.ToResourceHeaderProto(uls.ResourceHeader), Spec: &userloginstatev1.Spec{ - Roles: uls.GetRoles(), - Traits: traitv1.ToProto(uls.GetTraits()), - UserType: string(uls.Spec.UserType), + OriginalRoles: uls.GetOriginalRoles(), + Roles: uls.GetRoles(), + Traits: traitv1.ToProto(uls.GetTraits()), + UserType: string(uls.Spec.UserType), }, } } diff --git a/api/types/userloginstate/convert/v1/user_login_state_test.go b/api/types/userloginstate/convert/v1/user_login_state_test.go index b5c4b0fc10995..9e7a2067fd183 100644 --- a/api/types/userloginstate/convert/v1/user_login_state_test.go +++ b/api/types/userloginstate/convert/v1/user_login_state_test.go @@ -77,7 +77,8 @@ func newUserLoginState(t *testing.T, name string) *userloginstate.UserLoginState Name: name, }, userloginstate.Spec{ - Roles: []string{"role1", "role2"}, + OriginalRoles: []string{"role1"}, + Roles: []string{"role1", "role2"}, Traits: trait.Traits{ "key1": []string{"value1"}, "key2": []string{"value2"}, diff --git a/api/types/userloginstate/user_login_state.go b/api/types/userloginstate/user_login_state.go index ad6b0f7611fa8..ea4de66d901b6 100644 --- a/api/types/userloginstate/user_login_state.go +++ b/api/types/userloginstate/user_login_state.go @@ -41,6 +41,9 @@ type UserLoginState struct { // Spec is the specification for the user login state. type Spec struct { + // OriginalRoles is the list of the original roles from the user login state. + OriginalRoles []string `json:"original_roles" yaml:"original_roles"` + // Roles is the list of roles attached to the user login state. Roles []string `json:"roles" yaml:"roles"` @@ -81,6 +84,11 @@ func (u *UserLoginState) CheckAndSetDefaults() error { return nil } +// GetOriginalRoles returns the original roles that the user login state was derived from. +func (u *UserLoginState) GetOriginalRoles() []string { + return u.Spec.OriginalRoles +} + // GetRoles returns the roles attached to the user login state. func (u *UserLoginState) GetRoles() []string { return u.Spec.Roles diff --git a/lib/auth/userloginstate/generator.go b/lib/auth/userloginstate/generator.go index c5229bbcc800b..0f69efcbd5568 100644 --- a/lib/auth/userloginstate/generator.go +++ b/lib/auth/userloginstate/generator.go @@ -119,14 +119,17 @@ func (g *Generator) Generate(ctx context.Context, user types.User) (*userloginst traits[k] = utils.CopyStrings(v) } } + // Create a new empty user login state. uls, err := userloginstate.New( header.Metadata{ Name: user.GetName(), }, userloginstate.Spec{ - Roles: utils.CopyStrings(user.GetRoles()), - Traits: traits, - UserType: user.GetUserType(), + // Original roles should be non-nil so that we can detect if it exists or not. + OriginalRoles: append([]string{}, user.GetRoles()...), + Roles: utils.CopyStrings(user.GetRoles()), + Traits: traits, + UserType: user.GetUserType(), }) if err != nil { return nil, trace.Wrap(err) diff --git a/lib/auth/userloginstate/generator_test.go b/lib/auth/userloginstate/generator_test.go index af077e7aabd66..89d7af5b5fad5 100644 --- a/lib/auth/userloginstate/generator_test.go +++ b/lib/auth/userloginstate/generator_test.go @@ -68,11 +68,10 @@ func TestAccessLists(t *testing.T) { user: user, cloud: true, roles: []string{"orole1"}, - expected: newUserLoginState(t, "user", []string{ - "orole1", - }, map[string][]string{ - "otrait1": {"value1", "value2"}, - }), + expected: newUserLoginState(t, "user", + []string{"orole1"}, + []string{"orole1"}, + trait.Traits{"otrait1": {"value1", "value2"}}), expectedRoleCount: 0, expectedTraitCount: 0, }, @@ -92,15 +91,9 @@ func TestAccessLists(t *testing.T) { members: append(newAccessListMembers(t, clock, "1", "user"), newAccessListMembers(t, clock, "2", "user")...), roles: []string{"orole1", "role1", "role2"}, expected: newUserLoginState(t, "user", - []string{ - "orole1", - "role1", - "role2", - }, trait.Traits{ - "otrait1": []string{"value1", "value2"}, - "trait1": []string{"value1", "value2"}, - "trait2": []string{"value3"}, - }), + []string{"orole1"}, + []string{"orole1", "role1", "role2"}, + trait.Traits{"otrait1": {"value1", "value2"}, "trait1": {"value1", "value2"}, "trait2": {"value3"}}), expectedRoleCount: 2, expectedTraitCount: 3, }, @@ -120,15 +113,9 @@ func TestAccessLists(t *testing.T) { members: append(newAccessListMembers(t, clock, "1", "user"), newAccessListMembers(t, clock, "2", "user")...), roles: []string{"orole1", "role1", "role2"}, expected: newUserLoginState(t, "user", - []string{ - "orole1", - "role1", - "role2", - }, trait.Traits{ - "otrait1": []string{"value1", "value2"}, - "trait1": []string{"value1", "value2"}, - "trait2": []string{"value3"}, - }), + []string{"orole1"}, + []string{"orole1", "role1", "role2"}, + trait.Traits{"otrait1": {"value1", "value2"}, "trait1": {"value1", "value2"}, "trait2": {"value3"}}), expectedRoleCount: 0, expectedTraitCount: 0, }, @@ -148,11 +135,9 @@ func TestAccessLists(t *testing.T) { members: append(newAccessListMembers(t, clock, "1", "user"), newAccessListMembers(t, clock, "2", "user")...), roles: []string{"orole1"}, expected: newUserLoginState(t, "user", - []string{"orole1"}, trait.Traits{ - "otrait1": []string{"value1", "value2"}, - "trait1": []string{"value1", "value2"}, - "trait2": []string{"value3"}, - }), + []string{"orole1"}, + []string{"orole1"}, + trait.Traits{"otrait1": {"value1", "value2"}, "trait1": {"value1", "value2"}, "trait2": {"value3"}}), expectedRoleCount: 0, expectedTraitCount: 3, }, @@ -172,13 +157,10 @@ func TestAccessLists(t *testing.T) { members: append(newAccessListMembers(t, clock, "1", "user"), newAccessListMembers(t, clock, "2", "not-user")...), roles: []string{"orole1", "role1", "role2"}, expected: newUserLoginState(t, "user", - []string{ - "orole1", - "role1", - }, trait.Traits{ - "otrait1": []string{"value1", "value2"}, - "trait1": []string{"value1"}, - }), + []string{"orole1"}, + []string{"orole1", "role1"}, + trait.Traits{ + "otrait1": {"value1", "value2"}, "trait1": {"value1"}}), expectedRoleCount: 1, expectedTraitCount: 1, }, @@ -193,14 +175,9 @@ func TestAccessLists(t *testing.T) { members: append(newAccessListMembers(t, clock, "1", "user"), newAccessListMembers(t, clock, "2", "user")...), roles: []string{"orole1", "role1", "role2", "role3"}, expected: newUserLoginState(t, "user", - []string{ - "orole1", - "role1", - "role2", - "role3", - }, trait.Traits{ - "otrait1": []string{"value1", "value2"}, - }), + []string{"orole1"}, + []string{"orole1", "role1", "role2", "role3"}, + trait.Traits{"otrait1": {"value1", "value2"}}), expectedRoleCount: 3, expectedTraitCount: 0, }, @@ -225,15 +202,9 @@ func TestAccessLists(t *testing.T) { members: append(newAccessListMembers(t, clock, "1", "user"), newAccessListMembers(t, clock, "2", "user")...), roles: []string{"orole1"}, expected: newUserLoginState(t, "user", - []string{ - "orole1", - }, - trait.Traits{ - "otrait1": []string{"value1", "value2"}, - "trait1": []string{"value1", "value2"}, - "trait2": []string{"value3", "value4", "value1"}, - "trait3": []string{"value5", "value6"}, - }), + []string{"orole1"}, + []string{"orole1"}, + trait.Traits{"otrait1": {"value1", "value2"}, "trait1": {"value1", "value2"}, "trait2": {"value3", "value4", "value1"}, "trait3": {"value5", "value6"}}), expectedRoleCount: 0, expectedTraitCount: 7, }, @@ -257,14 +228,9 @@ func TestAccessLists(t *testing.T) { members: append(newAccessListMembers(t, clock, "1", "user"), newAccessListMembers(t, clock, "2", "user")...), roles: []string{"role1"}, expected: newUserLoginState(t, "user", - []string{ - "role1", - }, - trait.Traits{ - "trait1": []string{"value1", "value2"}, - "trait2": []string{"value3", "value4"}, - "trait3": []string{"value5", "value6"}, - }), + []string{}, + []string{"role1"}, + trait.Traits{"trait1": {"value1", "value2"}, "trait2": {"value3", "value4"}, "trait3": {"value5", "value6"}}), expectedRoleCount: 1, expectedTraitCount: 6, }, @@ -411,14 +377,15 @@ func newAccessListMembers(t *testing.T, clock clockwork.Clock, accessList string return alMembers } -func newUserLoginState(t *testing.T, name string, roles []string, traits map[string][]string) *userloginstate.UserLoginState { +func newUserLoginState(t *testing.T, name string, originalRoles, roles []string, traits map[string][]string) *userloginstate.UserLoginState { t.Helper() uls, err := userloginstate.New(header.Metadata{ Name: name, }, userloginstate.Spec{ - Roles: roles, - Traits: traits, + OriginalRoles: originalRoles, + Roles: roles, + Traits: traits, }) require.NoError(t, err) diff --git a/lib/auth/userloginstate/service_test.go b/lib/auth/userloginstate/service_test.go index 208c7b4923c5c..27e1ce637a32d 100644 --- a/lib/auth/userloginstate/service_test.go +++ b/lib/auth/userloginstate/service_test.go @@ -68,8 +68,8 @@ func TestGetUserLoginStates(t *testing.T) { require.NoError(t, err) require.Empty(t, getResp.UserLoginStates) - uls1 := newUserLoginState(t, "1", stRoles, stTraits) - uls2 := newUserLoginState(t, "2", stRoles, stTraits) + uls1 := newUserLoginState(t, "1", stRoles, stRoles, stTraits) + uls2 := newUserLoginState(t, "2", stRoles, stRoles, stTraits) _, err = svc.UpsertUserLoginState(ctx, &userloginstatev1.UpsertUserLoginStateRequest{UserLoginState: conv.ToProto(uls1)}) require.NoError(t, err) @@ -94,8 +94,8 @@ func TestUpsertUserLoginStates(t *testing.T) { require.NoError(t, err) require.Empty(t, getResp.UserLoginStates) - uls1 := newUserLoginState(t, "1", stRoles, stTraits) - uls2 := newUserLoginState(t, "2", stRoles, stTraits) + uls1 := newUserLoginState(t, "1", stRoles, stRoles, stTraits) + uls2 := newUserLoginState(t, "2", stRoles, stRoles, stTraits) _, err = svc.UpsertUserLoginState(ctx, &userloginstatev1.UpsertUserLoginStateRequest{UserLoginState: conv.ToProto(uls1)}) require.NoError(t, err) @@ -117,7 +117,7 @@ func TestGetUserLoginState(t *testing.T) { require.NoError(t, err) require.Empty(t, getResp.UserLoginStates) - uls1 := newUserLoginState(t, "1", stRoles, stTraits) + uls1 := newUserLoginState(t, "1", stRoles, stRoles, stTraits) _, err = svc.UpsertUserLoginState(ctx, &userloginstatev1.UpsertUserLoginStateRequest{UserLoginState: conv.ToProto(uls1)}) require.NoError(t, err) @@ -143,7 +143,7 @@ func TestDeleteUserLoginState(t *testing.T) { require.NoError(t, err) require.Empty(t, getResp.UserLoginStates) - uls1 := newUserLoginState(t, "1", stRoles, stTraits) + uls1 := newUserLoginState(t, "1", stRoles, stRoles, stTraits) _, err = svc.UpsertUserLoginState(ctx, &userloginstatev1.UpsertUserLoginStateRequest{UserLoginState: conv.ToProto(uls1)}) require.NoError(t, err) @@ -170,8 +170,8 @@ func TestDeleteAllAccessLists(t *testing.T) { require.NoError(t, err) require.Empty(t, getResp.UserLoginStates) - uls1 := newUserLoginState(t, "1", stRoles, stTraits) - uls2 := newUserLoginState(t, "2", stRoles, stTraits) + uls1 := newUserLoginState(t, "1", stRoles, stRoles, stTraits) + uls2 := newUserLoginState(t, "2", stRoles, stRoles, stTraits) _, err = svc.UpsertUserLoginState(ctx, &userloginstatev1.UpsertUserLoginStateRequest{UserLoginState: conv.ToProto(uls1)}) require.NoError(t, err) From 44ba1e61d13f3e97b4d9352b48c5abc177e5d107 Mon Sep 17 00:00:00 2001 From: Mike Wilson Date: Wed, 8 Nov 2023 13:35:41 -0500 Subject: [PATCH 2/2] Tweak comments, code formatting, remove unnecessary nil vs. empty array distinction. --- .../go/teleport/userloginstate/v1/userloginstate.pb.go | 3 ++- api/proto/teleport/userloginstate/v1/userloginstate.proto | 3 ++- lib/auth/userloginstate/generator.go | 3 +-- lib/auth/userloginstate/generator_test.go | 7 +++++-- 4 files changed, 10 insertions(+), 6 deletions(-) diff --git a/api/gen/proto/go/teleport/userloginstate/v1/userloginstate.pb.go b/api/gen/proto/go/teleport/userloginstate/v1/userloginstate.pb.go index af206daccdf73..8f5b3e103146d 100644 --- a/api/gen/proto/go/teleport/userloginstate/v1/userloginstate.pb.go +++ b/api/gen/proto/go/teleport/userloginstate/v1/userloginstate.pb.go @@ -106,7 +106,8 @@ type Spec struct { Traits []*v11.Trait `protobuf:"bytes,2,rep,name=traits,proto3" json:"traits,omitempty"` // user_type is the type of user this state represents. UserType string `protobuf:"bytes,3,opt,name=user_type,json=userType,proto3" json:"user_type,omitempty"` - // original_roles are the user roles that the user originally had prior to the user login state being generated. + // original_roles are the user roles that are part of the user's static definition. These roles are + // not affected by access granted by access lists and are obtained prior to granting access list access. OriginalRoles []string `protobuf:"bytes,4,rep,name=original_roles,json=originalRoles,proto3" json:"original_roles,omitempty"` } diff --git a/api/proto/teleport/userloginstate/v1/userloginstate.proto b/api/proto/teleport/userloginstate/v1/userloginstate.proto index 54c482dd4f0d0..017da97f6ef0f 100644 --- a/api/proto/teleport/userloginstate/v1/userloginstate.proto +++ b/api/proto/teleport/userloginstate/v1/userloginstate.proto @@ -41,6 +41,7 @@ message Spec { // user_type is the type of user this state represents. string user_type = 3; - // original_roles are the user roles that the user originally had prior to the user login state being generated. + // original_roles are the user roles that are part of the user's static definition. These roles are + // not affected by access granted by access lists and are obtained prior to granting access list access. repeated string original_roles = 4; } diff --git a/lib/auth/userloginstate/generator.go b/lib/auth/userloginstate/generator.go index 0f69efcbd5568..af50a22bcc816 100644 --- a/lib/auth/userloginstate/generator.go +++ b/lib/auth/userloginstate/generator.go @@ -125,8 +125,7 @@ func (g *Generator) Generate(ctx context.Context, user types.User) (*userloginst header.Metadata{ Name: user.GetName(), }, userloginstate.Spec{ - // Original roles should be non-nil so that we can detect if it exists or not. - OriginalRoles: append([]string{}, user.GetRoles()...), + OriginalRoles: utils.CopyStrings(user.GetRoles()), Roles: utils.CopyStrings(user.GetRoles()), Traits: traits, UserType: user.GetUserType(), diff --git a/lib/auth/userloginstate/generator_test.go b/lib/auth/userloginstate/generator_test.go index 89d7af5b5fad5..104b2eeb4b620 100644 --- a/lib/auth/userloginstate/generator_test.go +++ b/lib/auth/userloginstate/generator_test.go @@ -228,9 +228,12 @@ func TestAccessLists(t *testing.T) { members: append(newAccessListMembers(t, clock, "1", "user"), newAccessListMembers(t, clock, "2", "user")...), roles: []string{"role1"}, expected: newUserLoginState(t, "user", - []string{}, + nil, []string{"role1"}, - trait.Traits{"trait1": {"value1", "value2"}, "trait2": {"value3", "value4"}, "trait3": {"value5", "value6"}}), + trait.Traits{ + "trait1": {"value1", "value2"}, + "trait2": {"value3", "value4"}, + "trait3": {"value5", "value6"}}), expectedRoleCount: 1, expectedTraitCount: 6, },