Add support for custom group claims

[mvanderlee]

This adds the ability set policy based on the value of any claim in the ID Token.

For instance:

dev oidc:someClaim:developer https://login.microsoftonline.com/9188040d-6c67-4c5b-b112-36a304b66dad/v2.0

Would allow anyone with an ID Token whose claim "someClaim" had the value "developer" to login as the linux user dev (assuming the ID Token is valid for the configured OP.

Fixes #123

I don't have much Go experience, but I have tested this on EC2 with Auth0 as IDP using a URL as group name.

[EthanHeilman]

Thanks for putting this together. I fixed up some the linter errors. It is on my list of PRs to review, but I might not get to it for a few days.

[EthanHeilman]

@mvanderlee Lets wait on this until we have quotes and escaped spaces in #120

Then implementing this using CEL

[EthanHeilman]

@mvanderlee The prereq for this PR #158 which added escaping has been merged. No need to wait on escaping anymore

[mvanderlee]

@EthanHeilman What is needed for this PR to be merged?
I don't really understand what #158 has added.

[EthanHeilman]

@mvanderlee

I don't really understand what #158 has added.

Imagine you have a custom group claim like IT admins.

operator oidc:opkssh_groups:IT admins https://accounts.google.com

This would break policy since the policy file is space delimited. It would see four rows

operator 
oidc:opkssh_groups:IT
admins 
https://accounts.google.com

Now we can use quotes to include groups with spaces.

operator "oidc:opkssh_groups:IT admins" https://accounts.google.com

It will see three rows

operator 
"idc:opkssh_groups:IT admins"
https://accounts.google.com

What is needed for this PR to be merged?

1. Unit tests

2. Documentation

Wouldn't this allow using any ID Token as a policy rule? If so I see that a good thing and we should frame it that way in the documentation and code. Rather than potentialGroupClaims, it could be potentialClaims.

[EthanHeilman]

Why the switch from : to |?

[mvanderlee]

The switch was to allow URLs as group identifiers.
i.e.: ec2-user oidc|https://acme.com/groups|PROD_SSH https://acme.com/

[EthanHeilman]

Can we escape quote here instead? We are already stuck with : and putting pipes (|) would require quotes anyways to shell escape them

ec2-user oidc:"https://acme.com/groups":"PROD_SSH" https://acme.com/

[mvanderlee]

Sorry I haven't looked at go since April. The readme change to : is incorrect and should be reverted.
The currently supported oidc:groups prefix continues to work with :.
If we don't find a match, but it does start with oidc then we check for a piped split with custom group attribute.

Example, all of these work in the current code.

dev oidc:groups:developer https://login.microsoftonline.com/9188040d-6c67-4c5b-b112-36a304b66dad/v2.0
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

But I'm checking on quoting now.

[mvanderlee]

@EthanHeilman ready for review. I've added escaping, documentation, and testing.

Example

dev oidc:"https://acme.com/groups":developer https://login.microsoftonline.com/9188040d-6c67-4c5b-b112-36a304b66dad/v2.0

[EthanHeilman]

@mvanderlee Thanks for all your work on this. I would have time to take a look at it until Thursday as I really want to test it out and play around with the code. Policy changes are always tricky in unexpected ways

[EthanHeilman]

LGTM, I made some minor changes

[EthanHeilman]

Thanks for putting this PR together. Sorry it took me so long to review and merge. Normally I am much faster about reviews

[EthanHeilman]

Tested this by letting anyone with the name Ethan log into my vm. =)

cat /etc/opk/auth_id
ethan oidc:given_name:Ethan https://accounts.google.com

[mvanderlee]

Nice! I've been using it as well, and it work well! We do need to escape the quotes.

ec2-user oidc:\\\"https://acme.com/groups\\\":prod https://adme.com/

[EthanHeilman]

Do you escape the quotes in opkssh add or are you editing the file directly?

[mvanderlee]

We edit the file directly through our IAC via Terraform + Ansible.

[EthanHeilman]

Does a single escape not work?

ec2-useroidc:\"https://acme.com/groups\":prod https://adme.com

Is escaping the escapes in example needed?

ec2-user oidc:\\\"https://acme.com/groups\\\":prod https://adme.com

If so, we have a bug because opkssh add creates ec2-useroidc:"https://acme.com/groups\":prod https://adme.com` not ec2-user oidc:\\\"https://acme.com/groups\\\":prod https://adme.com

[mvanderlee]

Yes because of the shellquote added in PR #158

[EthanHeilman]

@mvanderlee

I wrote the following test to double check

var pathToAuthFile = "path/to/auth_id2.test"

func TestPolicyUrlClaim(t *testing.T) {
	loader := &policy.PolicyLoader{
		FileLoader: files.FileLoader{
			Fs:           afero.NewOsFs(),
			RequiredPerm: fs.FileMode(0666),
		},
	}

	p, err := loader.LoadPolicyAtPath(pathToAuthFile)
	require.NoError(t, err)
	require.Len(t, p.Users, 1)

	providerOpts := providers.DefaultMockProviderOpts()
	op, _, idTokenTemplate, err := providers.NewMockProvider(providerOpts)
	require.NoError(t, err)
	idTokenTemplate.ExtraClaims = map[string]any{"email": "arthur.aardvark@example.com",
		"https://acme.com/groups": []string{"prod"}}

	opkClient, err := client.New(op)
	require.NoError(t, err)
	pkt, err := opkClient.Auth(context.Background())
	require.NoError(t, err)

	payloadB64 := bytes.Split(pkt.OpToken, []byte{'.'})[1]
	require.NoError(t, err)

	pktJson, err := util.Base64DecodeForJWT(payloadB64)
	require.NoError(t, err)

	var claims map[string]any
	err = json.Unmarshal(pktJson, &claims)
	require.NoError(t, err)

	policyEnforcer := &policy.Enforcer{
		PolicyLoader: &MockPolicyLoader{Policy: p},
	}

	err = policyEnforcer.CheckPolicy("test_user", pkt, "", "example-base64Cert", "ssh-rsa", policy.DenyList{})
	require.NoError(t, err)
}

The test passes for:

test_user oidc:\"https://acme.com/groups\":prod https://accounts.example.com

the test fails for:

test_user oidc:\\\"https://acme.com/groups\\\:prod https://accounts.example.com

So I don't think there is a bug. Are you using test_user oidc:\\\"https://acme.com/groups\\\:prod https://accounts.example.com in your actual auth_id file and it is working for you?