Refactor lib/utils/parse dependency in api package. (#5261)

api/types/oidc_test.go

@@ -21,7 +21,6 @@ import (
 	"github.com/gravitational/teleport"
 	"github.com/gravitational/teleport/lib/utils"
 
-	"github.com/coreos/go-oidc/jose"
 	"github.com/stretchr/testify/require"
 )
 
@@ -154,63 +153,3 @@ func TestOIDCUnmarshalInvalid(t *testing.T) {
 	_, err := GetOIDCConnectorMarshaler().UnmarshalOIDCConnector([]byte(input))
 	require.Error(t, err)
 }
-
-// Verify that an OIDC connector with no mappings produces no roles.
-func TestOIDCRoleMappingEmpty(t *testing.T) {
-	// create a connector
-	oidcConnector := NewOIDCConnector("example", OIDCConnectorSpecV2{
-		IssuerURL:    "https://www.exmaple.com",
-		ClientID:     "example-client-id",
-		ClientSecret: "example-client-secret",
-		RedirectURL:  "https://localhost:3080/v1/webapi/oidc/callback",
-		Display:      "sign in with example.com",
-		Scope:        []string{"foo", "bar"},
-	})
-
-	// create some claims
-	var claims = make(jose.Claims)
-	claims.Add("roles", "teleport-user")
-	claims.Add("email", "foo@example.com")
-	claims.Add("nickname", "foo")
-	claims.Add("full_name", "foo bar")
-
-	traits := OIDCClaimsToTraits(claims)
-	require.Len(t, traits, 4)
-
-	roles := oidcConnector.GetTraitMappings().TraitsToRoles(traits)
-	require.Len(t, roles, 0)
-}
-
-// TestOIDCRoleMapping verifies basic mapping from OIDC claims to roles.
-func TestOIDCRoleMapping(t *testing.T) {
-	// create a connector
-	oidcConnector := NewOIDCConnector("example", OIDCConnectorSpecV2{
-		IssuerURL:    "https://www.exmaple.com",
-		ClientID:     "example-client-id",
-		ClientSecret: "example-client-secret",
-		RedirectURL:  "https://localhost:3080/v1/webapi/oidc/callback",
-		Display:      "sign in with example.com",
-		Scope:        []string{"foo", "bar"},
-		ClaimsToRoles: []ClaimMapping{
-			{
-				Claim: "roles",
-				Value: "teleport-user",
-				Roles: []string{"user"},
-			},
-		},
-	})
-
-	// create some claims
-	var claims = make(jose.Claims)
-	claims.Add("roles", "teleport-user")
-	claims.Add("email", "foo@example.com")
-	claims.Add("nickname", "foo")
-	claims.Add("full_name", "foo bar")
-
-	traits := OIDCClaimsToTraits(claims)
-	require.Len(t, traits, 4)
-
-	roles := oidcConnector.GetTraitMappings().TraitsToRoles(traits)
-	require.Len(t, roles, 1)
-	require.Equal(t, "user", roles[0])
-}

api/types/role.go

@@ -19,14 +19,12 @@ package types
 import (
 	"encoding/json"
 	"fmt"
-	"strings"
 	"time"
 
 	"github.com/gravitational/teleport"
 	"github.com/gravitational/teleport/api/defaults"
 	"github.com/gravitational/teleport/api/types/wrappers"
 	"github.com/gravitational/teleport/lib/utils"
-	"github.com/gravitational/teleport/lib/utils/parse"
 
 	"github.com/gogo/protobuf/proto"
 	"github.com/gravitational/trace"
@@ -560,18 +558,6 @@ func (r *RoleV3) CheckAndSetDefaults() error {
 		return trace.BadParameter("found invalid option in session_recording: %v", opt)
 	}
 
-	// if we find {{ or }} but the syntax is invalid, the role is invalid
-	for _, condition := range []RoleConditionType{Allow, Deny} {
-		for _, login := range r.GetLogins(condition) {
-			if strings.Contains(login, "{{") || strings.Contains(login, "}}") {
-				_, err := parse.NewExpression(login)
-				if err != nil {
-					return trace.BadParameter("invalid login found: %v", login)
-				}
-			}
-		}
-	}
-
 	// check and correct the session ttl
 	if r.Spec.Options.MaxSessionTTL.Value() <= 0 {
 		r.Spec.Options.MaxSessionTTL = NewDuration(defaults.MaxCertDuration)
@@ -690,28 +676,6 @@ func (r *Rule) CheckAndSetDefaults() error {
 	if len(r.Verbs) == 0 {
 		return trace.BadParameter("missing verbs")
 	}
-	if len(r.Where) != 0 {
-		parser, err := GetWhereParserFn()(&Context{})
-		if err != nil {
-			return trace.Wrap(err)
-		}
-		_, err = parser.Parse(r.Where)
-		if err != nil {
-			return trace.BadParameter("could not parse 'where' rule: %q, error: %v", r.Where, err)
-		}
-	}
-	if len(r.Actions) != 0 {
-		parser, err := GetActionsParserFn()(&Context{})
-		if err != nil {
-			return trace.Wrap(err)
-		}
-		for i, action := range r.Actions {
-			_, err = parser.Parse(action)
-			if err != nil {
-				return trace.BadParameter("could not parse action %v %q, error: %v", i, action, err)
-			}
-		}
-	}
 	return nil
 }
 

api/types/traits.go

@@ -16,14 +16,6 @@ limitations under the License.
 
 package types
 
-import (
-	"github.com/gravitational/teleport/lib/utils"
-	"github.com/gravitational/teleport/lib/utils/parse"
-
-	"github.com/gravitational/trace"
-	log "github.com/sirupsen/logrus"
-)
-
 // TraitMapping is a mapping that maps a trait to one or
 // more teleport roles.
 type TraitMapping struct {
@@ -37,84 +29,3 @@ type TraitMapping struct {
 
 // TraitMappingSet is a set of trait mappings
 type TraitMappingSet []TraitMapping
-
-// TraitsToRoles maps the supplied traits to a list of teleport role names.
-func (ms TraitMappingSet) TraitsToRoles(traits map[string][]string) []string {
-	var roles []string
-	ms.traitsToRoles(traits, func(role string, expanded bool) {
-		roles = append(roles, role)
-	})
-	return utils.Deduplicate(roles)
-}
-
-// TraitsToRoleMatchers maps the supplied traits to a list of role matchers. Prefer calling
-// this function directly rather than calling TraitsToRoles and then building matchers from
-// the resulting list since this function forces any roles which include substitutions to
-// be literal matchers.
-func (ms TraitMappingSet) TraitsToRoleMatchers(traits map[string][]string) ([]parse.Matcher, error) {
-	var matchers []parse.Matcher
-	var firstErr error
-	ms.traitsToRoles(traits, func(role string, expanded bool) {
-		if expanded || utils.ContainsExpansion(role) {
-			// mapping process included variable expansion; we therefore
-			// "escape" normal matcher syntax and look only for exact matches.
-			// (note: this isn't about combatting maliciously constructed traits,
-			// traits are from trusted identity sources, this is just
-			// about avoiding unnecessary footguns).
-			matchers = append(matchers, literalMatcher{
-				value: role,
-			})
-			return
-		}
-		m, err := parse.NewMatcher(role)
-		if err != nil {
-			if firstErr == nil {
-				firstErr = err
-			}
-			return
-		}
-		matchers = append(matchers, m)
-	})
-	if firstErr != nil {
-		return nil, trace.Wrap(firstErr)
-	}
-	return matchers, nil
-}
-
-// TraitsToRoles maps the supplied traits to teleport role names and passes them to a collector.
-func (ms TraitMappingSet) traitsToRoles(traits map[string][]string, collect func(role string, expanded bool)) {
-	for _, mapping := range ms {
-		for traitName, traitValues := range traits {
-			if traitName != mapping.Trait {
-				continue
-			}
-		TraitLoop:
-			for _, traitValue := range traitValues {
-				for _, role := range mapping.Roles {
-					outRole, err := utils.ReplaceRegexp(mapping.Value, role, traitValue)
-					switch {
-					case err != nil:
-						if trace.IsNotFound(err) {
-							log.WithError(err).Debugf("Failed to match expression %v, replace with: %v input: %v", mapping.Value, role, traitValue)
-						}
-						// this trait value clearly did not match, move on to another
-						continue TraitLoop
-						// skip empty replacement or empty role
-					case outRole == "":
-					case outRole != "":
-						collect(outRole, outRole != role)
-					}
-				}
-			}
-		}
-	}
-}
-
-// literalMatcher is used to "escape" values which are not allowed to
-// take advantage of normal matcher syntax by limiting them to only
-// literal matches.
-type literalMatcher struct {
-	value string
-}
-
-func (m literalMatcher) Match(in string) bool { return m.value == in }

lib/auth/apiserver.go

@@ -2097,6 +2097,9 @@ func (s *APIServer) upsertRole(auth ClientI, w http.ResponseWriter, r *http.Requ
 	if err != nil {
 		return nil, trace.Wrap(err)
 	}
+	if err = services.ValidateRole(role); err != nil {
+		return nil, trace.Wrap(err)
+	}
 	err = auth.UpsertRole(r.Context(), role)
 	if err != nil {
 		return nil, trace.Wrap(err)

lib/auth/oidc.go

@@ -449,7 +449,7 @@ func (a *Server) calculateOIDCUser(connector services.OIDCConnector, claims jose
 
 	p.traits = services.OIDCClaimsToTraits(claims)
 
-	p.roles = connector.GetTraitMappings().TraitsToRoles(p.traits)
+	p.roles = services.TraitsToRoles(connector.GetTraitMappings(), p.traits)
 	if len(p.roles) == 0 {
 		return nil, trace.AccessDenied("unable to map claims to role for connector: %v", connector.GetName())
 	}

lib/auth/saml.go

@@ -143,7 +143,7 @@ func (a *Server) calculateSAMLUser(connector services.SAMLConnector, assertionIn
 
 	p.traits = services.SAMLAssertionsToTraits(assertionInfo)
 
-	p.roles = connector.GetTraitMappings().TraitsToRoles(p.traits)
+	p.roles = services.TraitsToRoles(connector.GetTraitMappings(), p.traits)
 	if len(p.roles) == 0 {
 		return nil, trace.AccessDenied("unable to map attributes to role for connector: %v", connector.GetName())
 	}

lib/services/access_request.go

@@ -158,7 +158,7 @@ func appendRoleMatchers(matchers []parse.Matcher, conditions AccessRequestCondit
 	}
 
 	// build matchers for all role mappings
-	ms, err := GetTraitMappings(conditions).TraitsToRoleMatchers(traits)
+	ms, err := TraitsToRoleMatchers(GetTraitMappings(conditions), traits)
 	if err != nil {
 		return nil, trace.Wrap(err)
 	}

lib/services/oidc_test.go

@@ -0,0 +1,84 @@
+/*
+Copyright 2021 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 services
+
+import (
+	"testing"
+
+	"github.com/coreos/go-oidc/jose"
+	"github.com/stretchr/testify/require"
+)
+
+// Verify that an OIDC connector with no mappings produces no roles.
+func TestOIDCRoleMappingEmpty(t *testing.T) {
+	// create a connector
+	oidcConnector := NewOIDCConnector("example", OIDCConnectorSpecV2{
+		IssuerURL:    "https://www.exmaple.com",
+		ClientID:     "example-client-id",
+		ClientSecret: "example-client-secret",
+		RedirectURL:  "https://localhost:3080/v1/webapi/oidc/callback",
+		Display:      "sign in with example.com",
+		Scope:        []string{"foo", "bar"},
+	})
+
+	// create some claims
+	var claims = make(jose.Claims)
+	claims.Add("roles", "teleport-user")
+	claims.Add("email", "foo@example.com")
+	claims.Add("nickname", "foo")
+	claims.Add("full_name", "foo bar")
+
+	traits := OIDCClaimsToTraits(claims)
+	require.Len(t, traits, 4)
+
+	roles := TraitsToRoles(oidcConnector.GetTraitMappings(), traits)
+	require.Len(t, roles, 0)
+}
+
+// TestOIDCRoleMapping verifies basic mapping from OIDC claims to roles.
+func TestOIDCRoleMapping(t *testing.T) {
+	// create a connector
+	oidcConnector := NewOIDCConnector("example", OIDCConnectorSpecV2{
+		IssuerURL:    "https://www.exmaple.com",
+		ClientID:     "example-client-id",
+		ClientSecret: "example-client-secret",
+		RedirectURL:  "https://localhost:3080/v1/webapi/oidc/callback",
+		Display:      "sign in with example.com",
+		Scope:        []string{"foo", "bar"},
+		ClaimsToRoles: []ClaimMapping{
+			{
+				Claim: "roles",
+				Value: "teleport-user",
+				Roles: []string{"user"},
+			},
+		},
+	})
+
+	// create some claims
+	var claims = make(jose.Claims)
+	claims.Add("roles", "teleport-user")
+	claims.Add("email", "foo@example.com")
+	claims.Add("nickname", "foo")
+	claims.Add("full_name", "foo bar")
+
+	traits := OIDCClaimsToTraits(claims)
+	require.Len(t, traits, 4)
+
+	roles := TraitsToRoles(oidcConnector.GetTraitMappings(), traits)
+	require.Len(t, roles, 1)
+	require.Equal(t, "user", roles[0])
+}

lib/services/parser.go

@@ -14,7 +14,7 @@ See the License for the specific language governing permissions and
 limitations under the License.
 */
 
-package types
+package services
 
 import (
 	"fmt"
@@ -301,42 +301,3 @@ func (r *EmptyResource) GetName() string {
 func (r *EmptyResource) GetMetadata() Metadata {
 	return r.Metadata
 }
-
-// NewParserFn returns function that creates parser of 'where' section
-// in access rules
-type NewParserFn func(ctx RuleContext) (predicate.Parser, error)
-
-var whereParser = NewWhereParser
-var actionsParser = NewActionsParser
-
-// GetWhereParserFn returns the global function that constructs WHERE predicate parsers
-// this function is used in external tools to override and extend 'where' in rules
-func GetWhereParserFn() NewParserFn {
-	marshalerMutex.RLock()
-	defer marshalerMutex.RUnlock()
-	return whereParser
-}
-
-// SetWhereParserFn sets the global function that creates WHERE predicate parsers
-// this function is used in external tools to override and extend 'where' in rules
-func SetWhereParserFn(fn NewParserFn) {
-	marshalerMutex.Lock()
-	defer marshalerMutex.Unlock()
-	whereParser = fn
-}
-
-// GetActionsParserFn returns global function that creates where parsers
-// this function is used in external tools to override and extend actions in rules
-func GetActionsParserFn() NewParserFn {
-	marshalerMutex.RLock()
-	defer marshalerMutex.RUnlock()
-	return actionsParser
-}
-
-// SetActionsParserFn sets global function that creates actions  parsers
-// this function is used in external tools to override and extend actions in rules
-func SetActionsParserFn(fn NewParserFn) {
-	marshalerMutex.Lock()
-	defer marshalerMutex.Unlock()
-	actionsParser = fn
-}

lib/services/role.go

@@ -259,6 +259,62 @@ const (
 	Deny RoleConditionType = false
 )
 
+// ValidateRole parses validates the role, and sets default values.
+func ValidateRole(r Role) error {
+	if err := r.CheckAndSetDefaults(); err != nil {
+		return err
+	}
+
+	// if we find {{ or }} but the syntax is invalid, the role is invalid
+	for _, condition := range []RoleConditionType{Allow, Deny} {
+		for _, login := range r.GetLogins(condition) {
+			if strings.Contains(login, "{{") || strings.Contains(login, "}}") {
+				_, err := parse.NewExpression(login)
+				if err != nil {
+					return trace.BadParameter("invalid login found: %v", login)
+				}
+			}
+		}
+	}
+
+	rules := append(r.GetRules(types.Allow), r.GetRules(types.Deny)...)
+	for _, rule := range rules {
+		if err := validateRule(rule); err != nil {
+			return trace.Wrap(err)
+		}
+	}
+
+	return nil
+}
+
+// validateRule parses the where and action fields to validate the rule.
+func validateRule(r Rule) error {
+	if len(r.Where) != 0 {
+		parser, err := NewWhereParser(&Context{})
+		if err != nil {
+			return trace.Wrap(err)
+		}
+		_, err = parser.Parse(r.Where)
+		if err != nil {
+			return trace.BadParameter("could not parse 'where' rule: %q, error: %v", r.Where, err)
+		}
+	}
+
+	if len(r.Actions) != 0 {
+		parser, err := NewActionsParser(&Context{})
+		if err != nil {
+			return trace.Wrap(err)
+		}
+		for i, action := range r.Actions {
+			_, err = parser.Parse(action)
+			if err != nil {
+				return trace.BadParameter("could not parse action %v %q, error: %v", i, action, err)
+			}
+		}
+	}
+	return nil
+}
+
 // ApplyTraits applies the passed in traits to any variables within the role
 // and returns itself.
 func ApplyTraits(r Role, traits map[string][]string) Role {
@@ -1499,11 +1555,11 @@ func (set RoleSet) String() string {
 // namespace to the specified resource and verb.
 // silent controls whether the access violations are logged.
 func (set RoleSet) CheckAccessToRule(ctx RuleContext, namespace string, resource string, verb string, silent bool) error {
-	whereParser, err := GetWhereParserFn()(ctx)
+	whereParser, err := NewWhereParser(ctx)
 	if err != nil {
 		return trace.Wrap(err)
 	}
-	actionsParser, err := GetActionsParserFn()(ctx)
+	actionsParser, err := NewActionsParser(ctx)
 	if err != nil {
 		return trace.Wrap(err)
 	}

lib/services/role_test.go

@@ -163,55 +163,6 @@ func TestRoleParse(t *testing.T) {
 			error:        trace.BadParameter(""),
 			matchMessage: "missing verbs",
 		},
-		{
-			name: "validation error, unsupported function in where",
-			in: `{
-							   		      "kind": "role",
-							   		      "version": "v3",
-							   		      "metadata": {"name": "name1"},
-							   		      "spec": {
-							                    "allow": {
-							                      "node_labels": {"a": "b"},
-							                      "namespaces": ["default"],
-							                      "rules": [
-							                        {
-							                          "resources": ["role"],
-							                          "verbs": ["read", "list"],
-							                          "where": "containz(user.spec.traits[\"groups\"], \"prod\")"
-							                        }
-							                      ]
-							                    }
-							   		      }
-							   		    }`,
-			error:        trace.BadParameter(""),
-			matchMessage: "unsupported function: containz",
-		},
-		{
-			name: "validation error, unsupported function in actions",
-			in: `{
-							   		      "kind": "role",
-							   		      "version": "v3",
-							   		      "metadata": {"name": "name1"},
-							   		      "spec": {
-							                    "allow": {
-							                      "node_labels": {"a": "b"},
-							                      "namespaces": ["default"],
-							                      "rules": [
-							                        {
-							                          "resources": ["role"],
-							                          "verbs": ["read", "list"],
-							                          "where": "contains(user.spec.traits[\"groups\"], \"prod\")",
-							                          "actions": [
-							                             "zzz(\"info\", \"log entry\")"
-							                          ]
-							                        }
-							                      ]
-							                    }
-							   		      }
-							   		    }`,
-			error:        trace.BadParameter(""),
-			matchMessage: "unsupported function: zzz",
-		},
 		{
 			name: "role with no spec still gets defaults",
 			in:   `{"kind": "role", "version": "v3", "metadata": {"name": "defrole"}, "spec": {}}`,
@@ -494,6 +445,9 @@ func TestRoleParse(t *testing.T) {
 				require.NoError(t, err)
 				require.Empty(t, cmp.Diff(*role, tc.role))
 
+				err := ValidateRole(role)
+				require.NoError(t, err)
+
 				out, err := json.Marshal(role)
 				require.NoError(t, err)
 
@@ -505,6 +459,87 @@ func TestRoleParse(t *testing.T) {
 	}
 }
 
+func TestValidateRole(t *testing.T) {
+	var tests = []struct {
+		name         string
+		spec         RoleSpecV3
+		err          error
+		matchMessage string
+	}{
+		{
+			name: "valid syntax",
+			spec: RoleSpecV3{
+				Allow: RoleConditions{
+					Logins: []string{`{{external["http://schemas.microsoft.com/ws/2008/06/identity/claims/windowsaccountname"]}}`},
+				},
+			},
+		},
+		{
+			name: "invalid role condition login syntax",
+			spec: RoleSpecV3{
+				Allow: RoleConditions{
+					Logins: []string{"{{foo"},
+				},
+			},
+			err:          trace.BadParameter(""),
+			matchMessage: "invalid login found",
+		},
+		{
+			name: "unsupported function in actions",
+			spec: RoleSpecV3{
+				Allow: RoleConditions{
+					Logins: []string{`{{external["http://schemas.microsoft.com/ws/2008/06/identity/claims/windowsaccountname"]}}`},
+					Rules: []Rule{
+						{
+							Resources: []string{"role"},
+							Verbs:     []string{"read", "list"},
+							Where:     "containz(user.spec.traits[\"groups\"], \"prod\")",
+						},
+					},
+				},
+			},
+			err:          trace.BadParameter(""),
+			matchMessage: "unsupported function: containz",
+		},
+		{
+			name: "unsupported function in where",
+			spec: RoleSpecV3{
+				Allow: RoleConditions{
+					Logins: []string{`{{external["http://schemas.microsoft.com/ws/2008/06/identity/claims/windowsaccountname"]}}`},
+					Rules: []Rule{
+						{
+							Resources: []string{"role"},
+							Verbs:     []string{"read", "list"},
+							Where:     "contains(user.spec.traits[\"groups\"], \"prod\")",
+							Actions:   []string{"zzz(\"info\", \"log entry\")"},
+						},
+					},
+				},
+			},
+			err:          trace.BadParameter(""),
+			matchMessage: "unsupported function: zzz",
+		},
+	}
+
+	for _, tc := range tests {
+		err := ValidateRole(&types.RoleV3{
+			Metadata: Metadata{
+				Name:      "name1",
+				Namespace: defaults.Namespace,
+			},
+			Spec: tc.spec,
+		})
+		if tc.err != nil {
+			require.Error(t, err, tc.name)
+			if tc.matchMessage != "" {
+				require.Contains(t, err.Error(), tc.matchMessage)
+			}
+		} else {
+			require.NoError(t, err, tc.name)
+		}
+	}
+}
+
 // TestLabelCompatibility makes sure that labels
 // are serialized in format understood by older servers with
 // scalar labels
@@ -1811,57 +1846,6 @@ func TestApplyTraits(t *testing.T) {
 	}
 }
 
-func TestCheckAndSetDefaults(t *testing.T) {
-	var tests = []struct {
-		inLogins []string
-		outError bool
-	}{
-		// 0 - invalid syntax
-		{
-			[]string{"{{foo"},
-			true,
-		},
-		// 1 - invalid syntax
-		{
-			[]string{"bar}}"},
-			true,
-		},
-		// 2 - valid syntax
-		{
-			[]string{"{{foo.bar}}"},
-			false,
-		},
-		// 3 - valid syntax
-		{
-			[]string{`{{external["http://schemas.microsoft.com/ws/2008/06/identity/claims/windowsaccountname"]}}`},
-			false,
-		},
-	}
-
-	for i, tt := range tests {
-		comment := fmt.Sprintf("Test %v", i)
-
-		role := &RoleV3{
-			Kind:    KindRole,
-			Version: V3,
-			Metadata: Metadata{
-				Name:      "name1",
-				Namespace: defaults.Namespace,
-			},
-			Spec: RoleSpecV3{
-				Allow: RoleConditions{
-					Logins: tt.inLogins,
-				},
-			},
-		}
-		if tt.outError {
-			require.Error(t, role.CheckAndSetDefaults(), comment)
-		} else {
-			require.NoError(t, role.CheckAndSetDefaults(), comment)
-		}
-	}
-}
-
 // TestExtractFrom makes sure roles and traits are extracted from SSH and TLS
 // certificates not services.User.
 func TestExtractFrom(t *testing.T) {

lib/services/traits.go

@@ -0,0 +1,106 @@
+/*
+Copyright 2021 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 services
+
+import (
+	"github.com/gravitational/teleport/lib/utils"
+	"github.com/gravitational/teleport/lib/utils/parse"
+
+	"github.com/gravitational/trace"
+	log "github.com/sirupsen/logrus"
+)
+
+// TraitsToRoles maps the supplied traits to a list of teleport role names.
+func TraitsToRoles(ms TraitMappingSet, traits map[string][]string) []string {
+	var roles []string
+	traitsToRoles(ms, traits, func(role string, expanded bool) {
+		roles = append(roles, role)
+	})
+	return utils.Deduplicate(roles)
+}
+
+// TraitsToRoleMatchers maps the supplied traits to a list of role matchers. Prefer calling
+// this function directly rather than calling TraitsToRoles and then building matchers from
+// the resulting list since this function forces any roles which include substitutions to
+// be literal matchers.
+func TraitsToRoleMatchers(ms TraitMappingSet, traits map[string][]string) ([]parse.Matcher, error) {
+	var matchers []parse.Matcher
+	var firstErr error
+	traitsToRoles(ms, traits, func(role string, expanded bool) {
+		if expanded || utils.ContainsExpansion(role) {
+			// mapping process included variable expansion; we therefore
+			// "escape" normal matcher syntax and look only for exact matches.
+			// (note: this isn't about combatting maliciously constructed traits,
+			// traits are from trusted identity sources, this is just
+			// about avoiding unnecessary footguns).
+			matchers = append(matchers, literalMatcher{
+				value: role,
+			})
+			return
+		}
+		m, err := parse.NewMatcher(role)
+		if err != nil {
+			if firstErr == nil {
+				firstErr = err
+			}
+			return
+		}
+		matchers = append(matchers, m)
+	})
+	if firstErr != nil {
+		return nil, trace.Wrap(firstErr)
+	}
+	return matchers, nil
+}
+
+// traitsToRoles maps the supplied traits to teleport role names and passes them to a collector.
+func traitsToRoles(ms TraitMappingSet, traits map[string][]string, collect func(role string, expanded bool)) {
+	for _, mapping := range ms {
+		for traitName, traitValues := range traits {
+			if traitName != mapping.Trait {
+				continue
+			}
+		TraitLoop:
+			for _, traitValue := range traitValues {
+				for _, role := range mapping.Roles {
+					outRole, err := utils.ReplaceRegexp(mapping.Value, role, traitValue)
+					switch {
+					case err != nil:
+						if trace.IsNotFound(err) {
+							log.WithError(err).Debugf("Failed to match expression %v, replace with: %v input: %v", mapping.Value, role, traitValue)
+						}
+						// this trait value clearly did not match, move on to another
+						continue TraitLoop
+					case outRole == "":
+					case outRole != "":
+						// skip empty replacement or empty role
+						collect(outRole, outRole != role)
+					}
+				}
+			}
+		}
+	}
+}
+
+// literalMatcher is used to "escape" values which are not allowed to
+// take advantage of normal matcher syntax by limiting them to only
+// literal matches.
+type literalMatcher struct {
+	value string
+}
+
+func (m literalMatcher) Match(in string) bool { return m.value == in }

lib/services/traits_test.go

@@ -14,7 +14,7 @@ See the License for the specific language governing permissions and
 limitations under the License.
 */
 
-package types
+package services
 
 import (
 	"testing"
@@ -117,7 +117,7 @@ func TestTraitsToRoleMatchers(t *testing.T) {
 	}
 
 	for _, tt := range tts {
-		matchers, err := TraitMappingSet([]TraitMapping{tt.tm}).TraitsToRoleMatchers(traits)
+		matchers, err := TraitsToRoleMatchers([]TraitMapping{tt.tm}, traits)
 		require.NoError(t, err, tt.desc)
 
 		// collect all roles which match at least on of the

lib/services/types.go

@@ -281,31 +281,6 @@ var (
 	OIDCClaimsToTraits = types.OIDCClaimsToTraits
 )
 
-// parser.go
-type (
-	RuleContext   = types.RuleContext
-	LogAction     = types.LogAction
-	Context       = types.Context
-	EmptyResource = types.EmptyResource
-)
-
-var (
-	ResourceNameExpr      = types.ResourceNameExpr
-	CertAuthorityTypeExpr = types.CertAuthorityTypeExpr
-
-	NewWhereParser     = types.NewWhereParser
-	GetStringMapValue  = types.GetStringMapValue
-	NewActionsParser   = types.NewActionsParser
-	NewLogActionFn     = types.NewLogActionFn
-	GetWhereParserFn   = types.GetWhereParserFn
-	SetWhereParserFn   = types.SetWhereParserFn
-	GetActionsParserFn = types.GetActionsParserFn
-	SetActionsParserFn = types.SetActionsParserFn
-
-	UserIdentifier     = types.UserIdentifier
-	ResourceIdentifier = types.ResourceIdentifier
-)
-
 // plugin_data.go
 type PluginData = types.PluginData
 

lib/services/user_test.go

@@ -14,17 +14,16 @@ See the License for the specific language governing permissions and
 limitations under the License.
 */
 
-package types
+package services
 
 import (
 	"encoding/json"
 	"fmt"
 	"testing"
 
+	"github.com/coreos/go-oidc/jose"
 	"github.com/gravitational/teleport/lib/defaults"
 	"github.com/gravitational/teleport/lib/utils"
-
-	"github.com/coreos/go-oidc/jose"
 	saml2 "github.com/russellhaering/gosaml2"
 	"github.com/russellhaering/gosaml2/types"
 	"gopkg.in/check.v1"
@@ -204,7 +203,7 @@ func (s *UserSuite) TestOIDCMapping(c *check.C) {
 		}
 		for _, input := range testCase.inputs {
 			comment := check.Commentf("OIDC Test case %v %q, input %q", i, testCase.comment, input.comment)
-			outRoles := conn.GetTraitMappings().TraitsToRoles(OIDCClaimsToTraits(input.claims))
+			outRoles := TraitsToRoles(conn.GetTraitMappings(), OIDCClaimsToTraits(input.claims))
 			c.Assert(outRoles, check.DeepEquals, input.roles, comment)
 		}
 
@@ -215,7 +214,7 @@ func (s *UserSuite) TestOIDCMapping(c *check.C) {
 		}
 		for _, input := range testCase.inputs {
 			comment := check.Commentf("SAML Test case %v %v, input %#v", i, testCase.comment, input)
-			outRoles := samlConn.GetTraitMappings().TraitsToRoles(SAMLAssertionsToTraits(claimsToAttributes(input.claims)))
+			outRoles := TraitsToRoles(samlConn.GetTraitMappings(), SAMLAssertionsToTraits(claimsToAttributes(input.claims)))
 			c.Assert(outRoles, check.DeepEquals, input.roles, comment)
 		}
 	}