Add support for custom group claims

README.md

@@ -157,6 +157,15 @@ To allow a group, `ssh-users`, to ssh to your server as `root`, run:
 sudo opkssh add root oidc:groups:ssh-users google
 ```
 
+We can also enforce policy on custom claims.
+For instance to require that root access is only granted to users whose ID Token has a claim `https://acme.com/groups` with the value `ssh-users` run:
+
+```bash
+sudo opkssh add root oidc:\"https://acme.com/groups\":ssh-users google
+```
+
+which will add that line to your OPKSSH policy file.
+
 ## How it works
 
 We use two features of SSH to make this work.
@@ -234,7 +243,7 @@ Linux user accounts are typically referred to in SSH as *principals* and we cont
   - Subject ID - an unique ID for the user set by the OP. This is the `sub` claim in the ID Token.
   - Group - the name of the group that the user is part of. This uses the `groups` claim which is presumed to
     be an array. The group identifier uses a structured identifier. I.e. `oidc:groups:{groupId}`. Replace the `groupId`
-    with the id of your group.
+    with the id of your group. If your group contains a colon, escape it `oidc:"https://acme.com/groups":{groupId}`.
 - Column 3: Issuer URI
 
 ```bash
@@ -246,6 +255,7 @@ dev bob@microsoft.com https://login.microsoftonline.com/9188040d-6c67-4c5b-b112-
 
 # Group identifier
 dev oidc:groups:developer https://login.microsoftonline.com/9188040d-6c67-4c5b-b112-36a304b66dad/v2.0
+dev oidc:"https://acme.com/groups":developer https://login.microsoftonline.com/9188040d-6c67-4c5b-b112-36a304b66dad/v2.0
 ```
 
 To add new rule run:

docs/config.md

@@ -151,21 +151,30 @@ dev oidc-match-end:email:@example.com https://login.microsoftonline.com/9188040d
 
 These `auth_id` files can be edited by hand or you can use the add command to add new policies. The add command has the following syntax.
 
-`sudo opkssh add {USER} {EMAIL|SUB|GROUP} {ISSUER}`
+`sudo opkssh add {USER} {EMAIL|SUB|CLAIM} {ISSUER}`
 
 For convenience you can use the shorthand `google`, `azure`, `gitlab` rather than specifying the entire issuer.
 This is especially useful in the case of azure where the issuer contains a long and hard to remember random string.
 
 The following command will allow `alice@example.com` to ssh in as `root`.
 
-Groups must be prefixed with `oidc:group`. So to allow anyone with the group `admin` to ssh in as root you would run the command:
+Claims must be prefixed with `oidc:{CLAIM}` e.g. for the group claim `oidc:group`. To allow anyone with the group `admin` to ssh in as root you would run the command:
 
 ```bash
 sudo opkssh add root oidc:group:admin azure
 ```
 
 Note that currently Google does not put their groups in the ID Token, so groups based auth does not work if you OpenID Provider is Google.
 
+We support policy on claims that are also URIs as this is a common pattern for groups in some systems. 
+To require that root access is only granted to users whose ID Token has a claim `https://acme.com/groups` with the value `ssh-users` run:
+
+```bash
+sudo opkssh add root oidc:\"https://acme.com/groups\":ssh-users google
+```
+
+which will add that line to your OPKSSH policy file.
+
 The system authorized identity file requires the following permissions:
 
 ```bash

policy/enforcer.go

@@ -30,7 +30,7 @@ import (
 )
 
 const (
-	OIDC_GROUPS         = "oidc:groups:"
+	OIDC_CLAIMS         = "oidc:"
 	OIDC_WILDCARD_EMAIL = "oidc-match-end:email:"
 )
 
@@ -48,26 +48,98 @@ type Enforcer struct {
 
 // type for Identity Token checkedClaims
 type checkedClaims struct {
-	Email  string   `json:"email"`
-	Sub    string   `json:"sub"`
-	Groups []string `json:"groups"`
+	Email       string              `json:"email"`
+	Sub         string              `json:"sub"`
+	ExtraClaims map[string][]string `json:"-"`
+}
+
+func (s *checkedClaims) UnmarshalJSON(data []byte) error {
+
+	// Avoid infinite recursion
+	type checkedClaimsAlias checkedClaims
+	var a checkedClaimsAlias
+
+	// Unmarshal the required claims
+	if err := json.Unmarshal(data, &a); err != nil {
+		return err
+	}
+	*s = checkedClaims(a)
+
+	// Unmarshal everything else
+	var schema map[string]interface{}
+	err := json.Unmarshal([]byte(data), &schema)
+	if err != nil {
+		return err
+	}
+
+	var raw map[string]any
+	if err := json.Unmarshal(data, &raw); err != nil {
+		return err
+	}
+
+	s.ExtraClaims = make(map[string][]string, len(raw))
+
+	for k, v := range raw {
+		switch t := v.(type) {
+		case string:
+			s.ExtraClaims[k] = []string{t}
+		case []any:
+			// Turn all elements in a list into a string
+			out := make([]string, 0, len(t))
+			for _, e := range t {
+				if s, ok := e.(string); ok {
+					out = append(out, s)
+				} else {
+					out = append(out, fmt.Sprint(e))
+				}
+			}
+			s.ExtraClaims[k] = out
+		default:
+			// Turn numbers/bools etc into strings
+			s.ExtraClaims[k] = []string{fmt.Sprint(t)}
+		}
+	}
+
+	return nil
 }
 
 // The default location for policy plugins
 const pluginPolicyDir = "/etc/opk/policy.d"
 
+// EscapedSplit splits a string by a separator while ignoring the separator in quoted sections.
+// This is useful for strings that may contain the separator character as part of the string
+// and not as a delimiter.
+func EscapedSplit(s string, sep rune) []string {
+	quoted := false
+	a := strings.FieldsFunc(s, func(r rune) bool {
+		if r == '"' {
+			quoted = !quoted
+		}
+		return !quoted && r == sep
+	})
+	return a
+}
+
 // Validates that the server defined identity attribute matches the
 // respective claim from the identity token
 func validateClaim(claims *checkedClaims, user *User) bool {
+	// Should we match on the email claim?
 	if strings.HasPrefix(claims.Email, OIDC_WILDCARD_EMAIL) {
 		return false
 	}
 
-	if strings.HasPrefix(user.IdentityAttribute, OIDC_GROUPS) {
-		oidcGroupSections := strings.Split(user.IdentityAttribute, ":")
+	// Should we match on an oidc claim?
+	if strings.HasPrefix(user.IdentityAttribute, OIDC_CLAIMS) {
+		oidcGroupSections := EscapedSplit(user.IdentityAttribute, ':')
+		oidcGroupsName := strings.Trim(oidcGroupSections[1], "\"")
 
-		return slices.Contains(claims.Groups, oidcGroupSections[len(oidcGroupSections)-1])
+		return slices.Contains(
+			claims.ExtraClaims[oidcGroupsName],
+			oidcGroupSections[len(oidcGroupSections)-1],
+		)
 	}
+
+	// Should we match on the email wildcard claim?
 	wildCardEmailMatch := false
 	if strings.HasPrefix(user.IdentityAttribute, OIDC_WILDCARD_EMAIL) {
 		if strings.HasSuffix(strings.ToLower(claims.Email), strings.ToLower(user.IdentityAttribute[len(OIDC_WILDCARD_EMAIL):len(user.IdentityAttribute)])) {

policy/enforcer_test.go

@@ -44,11 +44,11 @@ func NewMockOpenIdSubProvider(t *testing.T, sub string) providers.OpenIdProvider
 	return op
 }
 
-func NewMockOpenIdProviderGroups(t *testing.T, groups []string) providers.OpenIdProvider {
+func NewMockOpenIdProviderGroups(t *testing.T, claimName string, groups []string) providers.OpenIdProvider {
 	providerOpts := providers.DefaultMockProviderOpts()
 	op, _, idTokenTemplate, err := providers.NewMockProvider(providerOpts)
 	require.NoError(t, err)
-	idTokenTemplate.ExtraClaims = map[string]any{"email": "arthur.aardvark@example.com", "groups": groups}
+	idTokenTemplate.ExtraClaims = map[string]any{"email": "arthur.aardvark@example.com", claimName: groups}
 	return op
 }
 
@@ -143,6 +143,11 @@ var policyWithOidcGroup = &policy.Policy{
 			Principals:        []string{"test"},
 			Issuer:            "https://accounts.example.com",
 		},
+		{
+			IdentityAttribute: "oidc:\"https://acme.com/groups\":e",
+			Principals:        []string{"test"},
+			Issuer:            "https://accounts.example.com",
+		},
 		{
 			IdentityAttribute: "oidc-match-end:email:@example2.com",
 			Principals:        []string{"test"},
@@ -324,7 +329,25 @@ func TestPolicyDeniedWrongIssuer(t *testing.T) {
 func TestPolicyApprovedOidcGroups(t *testing.T) {
 	t.Parallel()
 
-	op := NewMockOpenIdProviderGroups(t, []string{"a", "b", "c"})
+	op := NewMockOpenIdProviderGroups(t, "groups", []string{"a", "b", "c"})
+
+	opkClient, err := client.New(op)
+	require.NoError(t, err)
+	pkt, err := opkClient.Auth(context.Background())
+	require.NoError(t, err)
+
+	policyEnforcer := &policy.Enforcer{
+		PolicyLoader: &MockPolicyLoader{Policy: policyWithOidcGroup},
+	}
+
+	err = policyEnforcer.CheckPolicy("test", pkt, "", "example-base64Cert", "ssh-rsa", policy.DenyList{})
+	require.NoError(t, err)
+}
+
+func TestPolicyApprovedOidcGroupsUrlClaim(t *testing.T) {
+	t.Parallel()
+
+	op := NewMockOpenIdProviderGroups(t, "https://acme.com/groups", []string{"e"})
 
 	opkClient, err := client.New(op)
 	require.NoError(t, err)
@@ -342,7 +365,7 @@ func TestPolicyApprovedOidcGroups(t *testing.T) {
 func TestPolicyApprovedOidcGroupWithAtSign(t *testing.T) {
 	t.Parallel()
 
-	op := NewMockOpenIdProviderGroups(t, []string{"it.infra@my_domain.com"})
+	op := NewMockOpenIdProviderGroups(t, "groups", []string{"it.infra@my_domain.com"})
 
 	policyLine := &policy.Policy{
 		Users: []policy.User{
@@ -370,7 +393,7 @@ func TestPolicyApprovedOidcGroupWithAtSign(t *testing.T) {
 func TestPolicyDeniedOidcGroups(t *testing.T) {
 	t.Parallel()
 
-	op := NewMockOpenIdProviderGroups(t, []string{"z"})
+	op := NewMockOpenIdProviderGroups(t, "groups", []string{"z"})
 
 	opkClient, err := client.New(op)
 	require.NoError(t, err)
@@ -426,7 +449,7 @@ func TestEnforcerTableTest(t *testing.T) {
 	}{
 		{
 			name:         "Happy path (No userinfo supplied but ID Token has groups claim)",
-			op:           NewMockOpenIdProviderGroups(t, []string{"group1", "group2"}),
+			op:           NewMockOpenIdProviderGroups(t, "groups", []string{"group1", "group2"}),
 			policyLoader: &MockPolicyLoader{Policy: policyWithOidcGroup},
 		},
 		{
@@ -605,3 +628,20 @@ func TestLocalEmail(t *testing.T) {
 	err = policyEnforcer.CheckPolicy("test", pkt, "", "example-base64Cert", "ssh-rsa", policy.DenyList{})
 	require.Error(t, err, "user should not have access")
 }
+
+func TestEscapedSplit(t *testing.T) {
+	t.Parallel()
+
+	escaped := policy.EscapedSplit("abc:def:ghi", ':')
+	require.Equal(t, []string{"abc", "def", "ghi"}, escaped)
+
+	escaped = policy.EscapedSplit(`abc:"xxx:yyy"`, ':')
+	require.Equal(t, []string{"abc", `"xxx:yyy"`}, escaped)
+
+	escaped = policy.EscapedSplit(`aaa:"bbb:c" zzz:"qqq:www"`, ':')
+	require.Equal(t, []string{"aaa", "\"bbb:c\" zzz", "\"qqq:www\""}, escaped)
+
+	// Escaped strings which prevent the separator from being recognized
+	escaped = policy.EscapedSplit(`abc:\"def:ghi\"`, ':')
+	require.Equal(t, []string{"abc", "\\\"def:ghi\\\""}, escaped)
+}