Skip to content

Commit

Permalink
feat(server): Support OIDC custom claims for AuthN. Closes #5953 (#6444)
Browse files Browse the repository at this point in the history
* (docs) Added custom claim config for OIDC groups.

Co-authored-by: Thomas Cocozzello<thomas.cocozzello@gmail.com>
Signed-off-by: M Faizan Ali <mdfaizanali82@gmail.com>

* (feature) WIP: Migrated claim from struct to map for dynamic config.

Co-authored-by: Thomas Cocozzello<thomas.cocozzello@gmail.com>
Signed-off-by: M Faizan Ali <mdfaizanali82@gmail.com>

* Add partial support for bring your own custom claim name

Currently with the sso implementation the custom claim has to be named "groups"
which might not work with some Open ID systems. This change allows
users to write the service account policies to match anything inside the jwt
token instead of restricting it to an hardened interface.

partial-fix: #5953
Co-authored-by: M Faizan Ali <mdfaizanali82@gmail.com>
Signed-off-by: Thomas Cocozzello <thomas.cocozzello@gmail.com>

* Add partial support for bring your own custom claim name

Currently with the sso implementation the custom claim has to be named "groups"
which might not work with some Open ID systems. This change allows
users to write the service account policies to match anything inside the jwt
token instead of restricting it to an hardened interface.

partial-fix: #5953
Co-authored-by: M Faizan Ali <mdfaizanali82@gmail.com>
Signed-off-by: Thomas Cocozzello <thomas.cocozzello@gmail.com>

* (docs) added example for custom claim.

Co-authored-by: Thomas Cocozzello <thomas.cocozzello@gmail.com>
Signed-off-by: M Faizan Ali <mdfaizanali82@gmail.com>

* Added alphabetical ordering for users.md

Co-authored-by: Thomas Cocozzello <thomas.cocozzello@gmail.com>
Signed-off-by: M Faizan Ali <mdfaizanali82@gmail.com>

* Revert our changes to incorporate PR feedback

Co-authored-by: Thomas Cocozzello <thomas.cocozzello@gmail.com>
Signed-off-by: M Faizan Ali <mdfaizanali82@gmail.com>

* PR updated to incorporate feedback. Minimal changes for custom group claim

Co-authored-by: Thomas Cocozzello <thomas.cocozzello@gmail.com>
Signed-off-by: M Faizan Ali <mdfaizanali82@gmail.com>

* Added missing return statement

Co-authored-by: Thomas Cocozzello <thomas.cocozzello@gmail.com>
Signed-off-by: M Faizan Ali <mdfaizanali82@gmail.com>

Co-authored-by: Thomas Cocozzello <thomas.cocozzello@gmail.com>
  • Loading branch information
liafizan and tommydroptables committed Jul 30, 2021
1 parent 3e9d837 commit 9034152
Show file tree
Hide file tree
Showing 7 changed files with 334 additions and 25 deletions.
1 change: 1 addition & 0 deletions USERS.md
Original file line number Diff line number Diff line change
Expand Up @@ -95,6 +95,7 @@ Currently, the following organizations are **officially** using Argo Workflows:
1. [nrd.io](https://nrd.io/)
1. [NVIDIA](https://www.nvidia.com/)
1. [Onepanel](https://docs.onepanel.ai)
1. [Oracle](https://www.oracle.com/)
1. [OVH](https://www.ovh.com/)
1. [Peak AI](https://www.peak.ai/)
1. [PDOK](https://www.pdok.nl/)
Expand Down
42 changes: 31 additions & 11 deletions docs/argo-server-sso.md
Original file line number Diff line number Diff line change
Expand Up @@ -31,16 +31,16 @@ argo server --auth-mode sso --auth-mode ...
As of v2.12 we issue a JWE token for users rather than give them the ID token from your OAuth2 provider. This token is opaque and has a longer expiry time (10h by default).

The token encryption key is automatically generated by the Argo Server and stored in a Kubernetes secret name "sso".
The token encryption key is automatically generated by the Argo Server and stored in a Kubernetes secret name "sso".

You can revoke all tokens by deleting the encryption key and restarting the Argo Server (so it generates a new key).
You can revoke all tokens by deleting the encryption key and restarting the Argo Server (so it generates a new key).

```
kubectl delete secret sso
```

!!! Warning
The old key will be in the memory the any running Argo Server, and they will therefore accept and user with token encrypted using the old key. Every Argo Server MUST be restarted.
The old key will be in the memory the any running Argo Server, and they will therefore accept and user with token encrypted using the old key. Every Argo Server MUST be restarted.

All users will need to log in again. Sorry.

Expand Down Expand Up @@ -77,21 +77,21 @@ kind: ServiceAccount
metadata:
name: admin-user
annotations:
# The rule is an expression used to determine if this service account
# should be used.
# The rule is an expression used to determine if this service account
# should be used.
# * `groups` - an array of the OIDC groups
# * `iss` - the issuer ("argo-server")
# * `sub` - the subject (typically the username)
# Must evaluate to a boolean.
# Must evaluate to a boolean.
# If you want an account to be the default to use, this rule can be "true".
# Details of the expression language are available in
# https://github.com/antonmedv/expr/blob/master/docs/Language-Definition.md.
workflows.argoproj.io/rbac-rule: "'admin' in groups"
# The precedence is used to determine which service account to use whe
# Precedence is an integer. It may be negative. If omitted, it defaults to "0".
# Numerically higher values have higher precedence (not lower, which maybe
# Numerically higher values have higher precedence (not lower, which maybe
# counter-intuitive to you).
# If two rules match and have the same precedence, then which one used will
# If two rules match and have the same precedence, then which one used will
# be arbitrary.
workflows.argoproj.io/rbac-rule-precedence: "1"
```
Expand All @@ -101,16 +101,16 @@ If no rule matches, we deny the user access.
!!! Tip
You'll probably want to configure a default account to use if no other rule matches, e.g. a read-only account, you can do this as follows:
```yaml
metadata:
name: read-only
annotations:
workflows.argoproj.io/rbac-rule: "true"
workflows.argoproj.io/rbac-rule-precedence: "0"
```
The precedence must be the lowest of all your service accounts.
The precedence must be the lowest of all your service accounts.
## SSO Login Time
Expand All @@ -123,3 +123,23 @@ sso:
# Expiry defines how long your login is valid for in hours. (optional)
sessionExpiry: 240h
```
## Custom claims
> v3.1.4 and after
If your OIDC provider provides groups information with a claim name other than `groups`, you could confiure config-map to specify custom claim name for groups. Argo now arbitary custom claims and any claim can be used for `expr eval`. However, since group information is displayed in UI, it still needs to be an array of strings with group names as elements.

customClaim in this case will be mapped to `groups` key and we can use the same key `groups` for evaluating our expressions

```yaml
sso:
# Specify custom claim name for OIDC groups.
customGroupClaimName: argo_groups
```

#### Example expr

```shell
# assuming customClaimGroupName: argo_groups
workflows.argoproj.io/rbac-rule: "'argo_admins' in groups"
```
17 changes: 14 additions & 3 deletions docs/running-locally.md
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@
We recommend using [K3D](https://k3d.io/) to set up the local Kubernetes cluster since this will allow you to test RBAC set-up and is fast. You can set-up K3D to be part of your default kube config as follows:

k3d cluster start --wait

Alternatively, you can use [Minikube](https://github.com/kubernetes/minikube) to set up the local Kubernetes cluster. Once a local Kubernetes cluster has started via `minikube start`, your kube config will use Minikube's context automatically.


Expand All @@ -32,7 +32,7 @@ Add to /etc/hosts:

To install into the “argo” namespace of your cluster: Argo and MinIO (for saving artifacts and logs):

make start
make start

### 4. (Optional) Set up a DB for the Workflow archive

Expand Down Expand Up @@ -62,9 +62,20 @@ Before submitting/running workflows, build all Argo images, so they're available

make build

### 7. SSO with Dex
For testing SSO integration, you can start a Argo with sso profile which will deploy
a pre-configured dex instance in argo namespace

```sh
make start PROFILE=SSO
```

## Troubleshooting Notes

If you get a similar error when running one of the make pre-commit tests `make: *** [pkg/apiclient/clusterworkflowtemplate/cluster-workflow-template.swagger.json] Error 1`, ensure you are working within your $GOPATH (YOUR-GOPATH/src/github.com/argoproj/argo-workflows).
* If you get a similar error when running one of the make pre-commit tests `make: *** [pkg/apiclient/clusterworkflowtemplate/cluster-workflow-template.swagger.json] Error 1`, ensure you are working within your $GOPATH (YOUR-GOPATH/src/github.com/argoproj/argo-workflows).

* If you encounter out of heap issues when building UI through Docker, please validate resources allocated to Docker. Compilation may fail if allocated RAM is less than 4Gi


## Clean

Expand Down
27 changes: 25 additions & 2 deletions server/auth/sso/sso.go
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,7 @@ type sso struct {
encrypter jose.Encrypter
rbacConfig *rbac.Config
expiry time.Duration
customClaimName string
}

func (s *sso) IsRBACEnabled() bool {
Expand All @@ -68,6 +69,8 @@ type Config struct {
// additional scopes (on top of "openid")
Scopes []string `json:"scopes,omitempty"`
SessionExpiry metav1.Duration `json:"sessionExpiry,omitempty"`
// customGroupClaimName will override the groups claim name
CustomGroupClaimName string `json:"customGroupClaimName,omitempty"`
}

func (c Config) GetSessionExpiry() time.Duration {
Expand Down Expand Up @@ -184,6 +187,7 @@ func newSso(
encrypter: encrypter,
rbacConfig: c.RBAC,
expiry: c.GetSessionExpiry(),
customClaimName: c.CustomGroupClaimName,
}, nil
}

Expand Down Expand Up @@ -238,20 +242,37 @@ func (s *sso) HandleCallback(w http.ResponseWriter, r *http.Request) {
_, _ = w.Write([]byte(fmt.Sprintf("failed to get claims: %v", err)))
return
}

// Default to groups claim but if customClaimName is set
// extract groups based on that claim key
groups := c.Groups
if s.customClaimName != "" {
groups, err = c.GetCustomGroup(s.customClaimName)
if err != nil {
w.WriteHeader(401)
_, _ = w.Write([]byte(fmt.Sprintf("failed to get custom claim: %v", err)))
return
}
}

argoClaims := &types.Claims{
Claims: jwt.Claims{
Issuer: issuer,
Subject: c.Subject,
Expiry: jwt.NewNumericDate(time.Now().Add(s.expiry)),
},
Groups: c.Groups,
Groups: groups,
RawClaim: c.RawClaim,
Email: c.Email,
EmailVerified: c.EmailVerified,
ServiceAccountName: c.ServiceAccountName,
}

raw, err := jwt.Encrypted(s.encrypter).Claims(argoClaims).CompactSerialize()
if err != nil {
panic(err)
w.WriteHeader(401)
_, _ = w.Write([]byte(fmt.Sprintf("failed to encode claims: %v", err)))
return
}
value := Prefix + raw
log.Debugf("handing oauth2 callback %v", value)
Expand Down Expand Up @@ -287,9 +308,11 @@ func (s *sso) Authorize(authorization string) (*types.Claims, error) {
if err := tok.Claims(s.privateKey, c); err != nil {
return nil, fmt.Errorf("failed to parse claims: %v", err)
}

if err := c.Validate(jwt.Expected{Issuer: issuer}); err != nil {
return nil, fmt.Errorf("failed to validate claims: %v", err)
}

return c, nil
}

Expand Down
11 changes: 7 additions & 4 deletions server/auth/sso/sso_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -53,16 +53,19 @@ var ssoConfigSecret = &apiv1.Secret{
func TestLoadSsoClientIdFromSecret(t *testing.T) {
fakeClient := fake.NewSimpleClientset(ssoConfigSecret).CoreV1().Secrets(testNamespace)
config := Config{
Issuer: "https://test-issuer",
ClientID: getSecretKeySelector("argo-sso-secret", "client-id"),
ClientSecret: getSecretKeySelector("argo-sso-secret", "client-secret"),
RedirectURL: "https://dummy",
Issuer: "https://test-issuer",
ClientID: getSecretKeySelector("argo-sso-secret", "client-id"),
ClientSecret: getSecretKeySelector("argo-sso-secret", "client-secret"),
RedirectURL: "https://dummy",
CustomGroupClaimName: "argo_groups",
}
ssoInterface, err := newSso(fakeOidcFactory, config, fakeClient, "/", false)
assert.NoError(t, err)
ssoObject := ssoInterface.(*sso)
assert.Equal(t, "sso-client-id-value", ssoObject.config.ClientID)
assert.Equal(t, "sso-client-secret-value", ssoObject.config.ClientSecret)
assert.Equal(t, "argo_groups", ssoObject.customClaimName)

assert.Equal(t, 10*time.Hour, ssoObject.expiry)
}

Expand Down
63 changes: 58 additions & 5 deletions server/auth/types/claims.go
Original file line number Diff line number Diff line change
@@ -1,11 +1,64 @@
package types

import "gopkg.in/square/go-jose.v2/jwt"
import (
"encoding/json"
"fmt"

"gopkg.in/square/go-jose.v2/jwt"
)

type Claims struct {
jwt.Claims
Groups []string `json:"groups,omitempty"`
Email string `json:"email,omitempty"`
EmailVerified bool `json:"email_verified,omitempty"`
ServiceAccountName string `json:"service_account_name,omitempty"`
Groups []string `json:"groups,omitempty"`
Email string `json:"email,omitempty"`
EmailVerified bool `json:"email_verified,omitempty"`
ServiceAccountName string `json:"service_account_name,omitempty"`
RawClaim map[string]interface{} `json:"-"`
}

// UnmarshalJSON is a custom Unmarshal that overwrites
// json.Unmarshal to mash every claim into a custom map
func (c *Claims) UnmarshalJSON(data []byte) error {
type claimAlias Claims
var localClaim claimAlias = claimAlias(*c)

// Populate the claims struct as much as possible
err := json.Unmarshal(data, &localClaim)
if err != nil {
return err
}

// Populate the raw data struct
err = json.Unmarshal(data, &localClaim.RawClaim)
if err != nil {
return err
}

*c = Claims(localClaim)
return nil
}

// GetCustomGroup is responsible for extracting groups based on the
// provided custom claim key
func (c *Claims) GetCustomGroup(customKeyName string) ([]string, error) {
groups, ok := c.RawClaim[customKeyName]
if !ok {
return nil, fmt.Errorf("No claim found for key: %v", customKeyName)
}

sliceInterface, ok := groups.([]interface{})
if !ok {
return nil, fmt.Errorf("Expected an array, got %v", groups)
}

newSlice := []string{}
for _, a := range sliceInterface {
val, ok := a.(string)
if !ok {
return nil, fmt.Errorf("Group name %v was not a string", a)
}
newSlice = append(newSlice, val)
}

return newSlice, nil
}
Loading

0 comments on commit 9034152

Please sign in to comment.