From 31885c56a8e2bec972887b6f372ac5e13c5c77b6 Mon Sep 17 00:00:00 2001 From: Tim Ross Date: Thu, 7 Nov 2024 12:39:04 -0500 Subject: [PATCH] Remove OIDCClaimsToTraits helper function The function was relocated to the only place it was being called in https://github.com/gravitational/teleport.e/pull/5374 and is no longer needed in lib/services. This also has the added benefit of removing go-oidc as a direct dependency of lib/services. --- lib/services/oidc.go | 28 ---------------------------- lib/services/oidc_test.go | 30 ------------------------------ lib/services/user_test.go | 21 ++++++++++++++++++++- 3 files changed, 20 insertions(+), 59 deletions(-) diff --git a/lib/services/oidc.go b/lib/services/oidc.go index 27e7e9e2260b6..721230fa7afa4 100644 --- a/lib/services/oidc.go +++ b/lib/services/oidc.go @@ -21,40 +21,12 @@ package services import ( "net/url" - "github.com/coreos/go-oidc/jose" "github.com/gravitational/trace" "github.com/gravitational/teleport/api/types" "github.com/gravitational/teleport/lib/utils" ) -// GetClaimNames returns a list of claim names from the claim values -func GetClaimNames(claims jose.Claims) []string { - var out []string - for claim := range claims { - out = append(out, claim) - } - return out -} - -// OIDCClaimsToTraits converts OIDC-style claims into teleport-specific trait format -func OIDCClaimsToTraits(claims jose.Claims) map[string][]string { - traits := make(map[string][]string) - - for claimName := range claims { - claimValue, ok, _ := claims.StringClaim(claimName) - if ok { - traits[claimName] = []string{claimValue} - } - claimValues, ok, _ := claims.StringsClaim(claimName) - if ok { - traits[claimName] = claimValues - } - } - - return traits -} - // GetRedirectURL gets a redirect URL for the given connector. If the connector // has a redirect URL which matches the host of the given Proxy address, then // that one will be returned. Otherwise, the first URL in the list will be returned. diff --git a/lib/services/oidc_test.go b/lib/services/oidc_test.go index b53760561d3da..86be24382cfcf 100644 --- a/lib/services/oidc_test.go +++ b/lib/services/oidc_test.go @@ -21,7 +21,6 @@ package services import ( "testing" - "github.com/coreos/go-oidc/jose" "github.com/gravitational/trace" "github.com/stretchr/testify/require" @@ -29,35 +28,6 @@ import ( "github.com/gravitational/teleport/api/types" ) -// TestOIDCRoleMapping verifies basic mapping from OIDC claims to roles. -func TestOIDCRoleMapping(t *testing.T) { - // create a connector - oidcConnector, err := types.NewOIDCConnector("example", types.OIDCConnectorSpecV3{ - IssuerURL: "https://www.exmaple.com", - ClientID: "example-client-id", - ClientSecret: "example-client-secret", - Display: "sign in with example.com", - Scope: []string{"foo", "bar"}, - ClaimsToRoles: []types.ClaimMapping{{Claim: "roles", Value: "teleport-user", Roles: []string{"user"}}}, - RedirectURLs: []string{"https://localhost:3080/v1/webapi/oidc/callback"}, - }) - require.NoError(t, err) - - // 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]) -} - // TestOIDCUnmarshal tests UnmarshalOIDCConnector func TestOIDCUnmarshal(t *testing.T) { for _, tc := range []struct { diff --git a/lib/services/user_test.go b/lib/services/user_test.go index 7e64cc25ec965..4f47f15c72aa3 100644 --- a/lib/services/user_test.go +++ b/lib/services/user_test.go @@ -273,7 +273,7 @@ func TestOIDCMapping(t *testing.T) { } for _, input := range testCase.inputs { comment := fmt.Sprintf("OIDC Test case %v %q, input %q", i, testCase.comment, input.comment) - _, outRoles := TraitsToRoles(conn.GetTraitMappings(), OIDCClaimsToTraits(input.claims)) + _, outRoles := TraitsToRoles(conn.GetTraitMappings(), oidcClaimsToTraits(input.claims)) require.Empty(t, cmp.Diff(outRoles, input.expectedRoles), comment) } @@ -325,6 +325,25 @@ func claimMappingsToAttributeMappings(in []types.ClaimMapping) []types.Attribute return out } +// oidcClaimsToTraits converts OIDC-style claims into teleport-specific trait format +func oidcClaimsToTraits(claims jose.Claims) map[string][]string { + traits := make(map[string][]string) + + for claimName := range claims { + claimValue, ok, _ := claims.StringClaim(claimName) + if ok { + traits[claimName] = []string{claimValue} + continue + } + claimValues, ok, _ := claims.StringsClaim(claimName) + if ok { + traits[claimName] = claimValues + } + } + + return traits +} + // claimsToAttributes maps jose.Claims type to attributes for testing func claimsToAttributes(claims jose.Claims) saml2.AssertionInfo { info := saml2.AssertionInfo{