oidc: support array-of-strings user property

etuttle commented

2020-01-20 02:16:46 +01:00

(Migrated from github.com)

I noticed that the "user group" support in the standard Jenkins OIDC plugin relies on the userdata having an array property. So I set out to create support for a custom array-of-strings property in glewlwyd.

babelouest commented

2020-01-20 05:05:00 +01:00

(Migrated from github.com)

Thanks for this, I'll take a closer look at this patch later, there are some fixes to make on the first look, but I'm wondering if this is worth the trouble.

The bottom line is Jenkins is requesting an array property, can you give me an example of such data?

Thanks for this, I'll take a closer look at this patch later, there are some fixes to make on the first look, but I'm wondering if this is worth the trouble. The bottom line is Jenkins is requesting an array property, can you give me an example of such data?

babelouest commented

2020-01-20 17:33:44 +01:00

(Migrated from github.com)

I reviewed your PR and I'm not gonna accept it as-is because there are flaws in it , especially concerning what to do if the claim is supposed to be converted in boolean or number. But I will make the change so you can have array of values in the /userinfo result or id_token payload.

Also, you don't have to check that every value in the array are consistent, because by design, all user properties are string values, except for system values like enabled.

I'm closing this PR but I'll make the change soon.

I reviewed your PR and I'm not gonna accept it as-is because there are flaws in it , especially concerning what to do if the claim is supposed to be converted in boolean or number. But I will make the change so you can have array of values in the /userinfo result or id_token payload. Also, you don't have to check that every value in the array are consistent, because by design, all user properties are string values, except for system values like `enabled`. I'm closing this PR but I'll make the change soon.

👍 1

etuttle commented

2020-01-21 00:09:58 +01:00

(Migrated from github.com)

Glad to hear you have a plan to support this! I should have opened an issue instead of throwing shoddy PRs at you :)

Here's an answer to your earlier question about what Jenkins is expecting. (This came up after I started playing with custom user properties in Glewlwyd, very cool BTW):

I saw in the UI for the Jenkins OIDC plugin that it supports a "groups" field with a configurable name. I read the code to learn the value of that field is expected to deserialize to List<String>. (There might be some deserialization magic that turns delimited strings into Lists? That I didn't check for). This getField call returns the value typed as an Object (and then cast):

https://github.com/jenkinsci/oic-auth-plugin/blob/oic-auth-1.7/src/main/java/org/jenkinsci/plugins/oic/OicSecurityRealm.java#L562

Unit test:

https://github.com/jenkinsci/oic-auth-plugin/blob/oic-auth-1.7/src/test/java/org/jenkinsci/plugins/oic/PluginTest.java#L277

Knowing just the basics about the spec, I looked into what inspired this implementation. It goes back to this PR which describes how to set it up with Keycloak:

https://github.com/jenkinsci/oic-auth-plugin/pull/12

I also came across a related PR with a sample token from Dex, which also apparently has a groups property which is also an array of strings:

https://github.com/jenkinsci/oic-auth-plugin/issues/34#issuecomment-391372610

Glad to hear you have a plan to support this! I should have opened an issue instead of throwing shoddy PRs at you :) Here's an answer to your earlier question about what Jenkins is expecting. (This came up after I started playing with custom user properties in Glewlwyd, very cool BTW): I saw in the UI for the Jenkins OIDC plugin that it supports a "groups" field with a configurable name. I read the code to learn the value of that field is expected to deserialize to `List<String>`. (There might be some deserialization magic that turns delimited strings into Lists? That I didn't check for). This `getField` call returns the value typed as an Object (and then cast): https://github.com/jenkinsci/oic-auth-plugin/blob/oic-auth-1.7/src/main/java/org/jenkinsci/plugins/oic/OicSecurityRealm.java#L562 Unit test: https://github.com/jenkinsci/oic-auth-plugin/blob/oic-auth-1.7/src/test/java/org/jenkinsci/plugins/oic/PluginTest.java#L277 Knowing just the basics about the spec, I looked into what inspired this implementation. It goes back to this PR which describes how to set it up with Keycloak: https://github.com/jenkinsci/oic-auth-plugin/pull/12 I also came across a related PR with a sample token from Dex, which also apparently has a groups property which is also an array of strings: https://github.com/jenkinsci/oic-auth-plugin/issues/34#issuecomment-391372610

babelouest commented

2020-01-21 05:52:53 +01:00

(Migrated from github.com)

Hello @etuttle ,

Can you try the last commit? I've made changes in the oidc plugin so a list f values can be added to the /userinfo result or the id_token payload.
You need to specify the user property in the Additional claims in the ID Token or the /userinfo endpoint and that should do it.

Hello @etuttle , Can you try the [last commit](https://github.com/babelouest/glewlwyd/commit/cf2075ebd1f654bdaae724d5357b75df009bc357)? I've made changes in the oidc plugin so a list f values can be added to the /userinfo result or the id_token payload. You need to specify the user property in the [Additional claims in the ID Token or the /userinfo endpoint](https://github.com/babelouest/glewlwyd/blob/master/docs/OIDC.md#additional-claims-in-the-id-token-or-the-userinfo-endpoint) and that should do it.

etuttle commented

2020-01-22 02:43:14 +01:00

(Migrated from github.com)

@babelouest I built cf2075ebd1 and tried it out with my existing config and DB. Works like a charm. I was able to add and remove groups from a user in Glewlwyd and and confirm the updates appeared in Jenkins. (after a logout / login anyway).

@babelouest I built cf2075ebd1f654bdaae724d5357b75df009bc357 and tried it out with my existing config and DB. Works like a charm. I was able to add and remove groups from a user in Glewlwyd and and confirm the updates appeared in Jenkins. (after a logout / login anyway).

👍 1

Pull request closed

This pull request cannot be reopened because the branch was deleted.

Rows
Columns

oidc: support array-of-strings user property #96

Pull request closed