Skip to content

Commit

Permalink
fix: Add OIDC issuer alias. Fixes #6759 (#6831)
Browse files Browse the repository at this point in the history
Signed-off-by: shea_sullivan <shea.sullivan@oracle.com>
  • Loading branch information
stsully authored Oct 4, 2021
1 parent 55d3bc0 commit 47d713b
Show file tree
Hide file tree
Showing 6 changed files with 49 additions and 10 deletions.
2 changes: 2 additions & 0 deletions docs/workflow-controller-configmap.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -303,6 +303,8 @@ data:
sso: |
# This is the root URL of the OIDC provider (required).
issuer: https://issuer.root.url/
# Some OIDC providers have alternate root URLs that can be included. These should be reviewed carefully. (optional)
issuerAlias: https://altissuer.root.url
# This defines how long your login is valid for (in hours). (optional)
# If omitted, defaults to 10h. Example below is 10 days.
sessionExpiry: 240h
Expand Down
2 changes: 1 addition & 1 deletion go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ require (
github.com/baiyubin/aliyun-sts-go-sdk v0.0.0-20180326062324-cfa1a18b161f // indirect
github.com/blushft/go-diagrams v0.0.0-20201006005127-c78c821223d9
github.com/colinmarc/hdfs v1.1.4-0.20180805212432-9746310a4d31
github.com/coreos/go-oidc v2.2.1+incompatible
github.com/coreos/go-oidc/v3 v3.1.0
github.com/doublerebel/bellows v0.0.0-20160303004610-f177d92a03d3
github.com/emicklei/go-restful v2.15.0+incompatible // indirect
github.com/evanphx/json-patch v4.9.0+incompatible
Expand Down
4 changes: 3 additions & 1 deletion go.sum
Original file line number Diff line number Diff line change
Expand Up @@ -248,6 +248,8 @@ github.com/coreos/go-etcd v2.0.0+incompatible/go.mod h1:Jez6KQU2B/sWsbdaef3ED8Nz
github.com/coreos/go-oidc v2.1.0+incompatible/go.mod h1:CgnwVTmzoESiwO9qyAFEMiHoZ1nMCKZlZ9V6mm3/LKc=
github.com/coreos/go-oidc v2.2.1+incompatible h1:mh48q/BqXqgjVHpy2ZY7WnWAbenxRjsz9N1i1YxjHAk=
github.com/coreos/go-oidc v2.2.1+incompatible/go.mod h1:CgnwVTmzoESiwO9qyAFEMiHoZ1nMCKZlZ9V6mm3/LKc=
github.com/coreos/go-oidc/v3 v3.1.0 h1:6avEvcdvTa1qYsOZ6I5PRkSYHzpTNWgKYmaJfaYbrRw=
github.com/coreos/go-oidc/v3 v3.1.0/go.mod h1:rEJ/idjfUyfkBit1eI1fvyr+64/g9dcKpAm8MJMesvo=
github.com/coreos/go-semver v0.2.0/go.mod h1:nnelYz7RCh+5ahJtPPxZlU+153eP4D4r3EedlOD2RNk=
github.com/coreos/go-semver v0.3.0/go.mod h1:nnelYz7RCh+5ahJtPPxZlU+153eP4D4r3EedlOD2RNk=
github.com/coreos/go-systemd v0.0.0-20180511133405-39ca1b05acc7/go.mod h1:F5haX7vjVVG0kc13fIWeqUViNPyEJxv/OmvnBo0Yme4=
Expand Down Expand Up @@ -945,7 +947,6 @@ github.com/pmezard/go-difflib v1.0.0 h1:4DBwDE0NGyQoBHbLQYPwSUPoCMWR5BEzIk/f1lZb
github.com/pmezard/go-difflib v1.0.0/go.mod h1:iKH77koFhYxTK1pcRnkKkqfTogsbg7gZNVY4sRDYZ/4=
github.com/posener/complete v1.1.1/go.mod h1:em0nMJCgc9GFtwrmVmEMR/ZL6WyhyjMBndrE9hABlRI=
github.com/pquerna/cachecontrol v0.0.0-20171018203845-0dec1b30a021/go.mod h1:prYjPmNq4d1NPVmpShWobRqXY3q7Vp+80DqgxxUrUIA=
github.com/pquerna/cachecontrol v0.0.0-20180517163645-1555304b9b35 h1:J9b7z+QKAmPf4YLrFg6oQUotqHQeUNWwkvo7jZp1GLU=
github.com/pquerna/cachecontrol v0.0.0-20180517163645-1555304b9b35/go.mod h1:prYjPmNq4d1NPVmpShWobRqXY3q7Vp+80DqgxxUrUIA=
github.com/prometheus/client_golang v0.9.1/go.mod h1:7SWBe2y4D6OKWSNQJUaRYU/AaXPKyh/dDVn+NZz0KFw=
github.com/prometheus/client_golang v0.9.2/go.mod h1:OsXs2jCmiKlQ1lTBmv21f2mNfw4xf/QclQDMrYNZzcM=
Expand Down Expand Up @@ -1312,6 +1313,7 @@ golang.org/x/net v0.0.0-20200226121028-0de0cce0169b/go.mod h1:z5CRVTTTmAJ677TzLL
golang.org/x/net v0.0.0-20200301022130-244492dfa37a/go.mod h1:z5CRVTTTmAJ677TzLLGU+0bjPO0LkuOLi4/5GtJWs/s=
golang.org/x/net v0.0.0-20200324143707-d3edc9973b7e/go.mod h1:qpuaurCH72eLCgpAm/N6yyVIVM9cpaDIP3A8BGJEC5A=
golang.org/x/net v0.0.0-20200501053045-e0ff5e5a1de5/go.mod h1:qpuaurCH72eLCgpAm/N6yyVIVM9cpaDIP3A8BGJEC5A=
golang.org/x/net v0.0.0-20200505041828-1ed23360d12c/go.mod h1:qpuaurCH72eLCgpAm/N6yyVIVM9cpaDIP3A8BGJEC5A=
golang.org/x/net v0.0.0-20200506145744-7e3656a0809f/go.mod h1:qpuaurCH72eLCgpAm/N6yyVIVM9cpaDIP3A8BGJEC5A=
golang.org/x/net v0.0.0-20200513185701-a91f0712d120/go.mod h1:qpuaurCH72eLCgpAm/N6yyVIVM9cpaDIP3A8BGJEC5A=
golang.org/x/net v0.0.0-20200520004742-59133d7f0dd7/go.mod h1:qpuaurCH72eLCgpAm/N6yyVIVM9cpaDIP3A8BGJEC5A=
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ apiVersion: v1
data:
sso: |
issuer: http://dex:5556/dex
issuerAlias: http://mydex:5556/dex
clientId:
name: argo-server-sso
key: clientID
Expand Down
23 changes: 19 additions & 4 deletions server/auth/sso/sso.go
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ import (
"time"

pkgrand "github.com/argoproj/pkg/rand"
"github.com/coreos/go-oidc"
"github.com/coreos/go-oidc/v3/oidc"
log "github.com/sirupsen/logrus"
"golang.org/x/oauth2"
"gopkg.in/square/go-jose.v2"
Expand Down Expand Up @@ -46,6 +46,7 @@ var _ Interface = &sso{}

type sso struct {
config *oauth2.Config
issuer string
idTokenVerifier *oidc.IDTokenVerifier
baseHRef string
secure bool
Expand All @@ -63,6 +64,7 @@ func (s *sso) IsRBACEnabled() bool {

type Config struct {
Issuer string `json:"issuer"`
IssuerAlias string `json:"issuerAlias,omitempty"`
ClientID apiv1.SecretKeySelector `json:"clientId"`
ClientSecret apiv1.SecretKeySelector `json:"clientSecret"`
RedirectURL string `json:"redirectUrl"`
Expand Down Expand Up @@ -122,7 +124,14 @@ func newSso(
if err != nil {
return nil, err
}
provider, err := factory(context.Background(), c.Issuer)
// Some offspec providers like Azure, Oracle IDCS have oidc discovery url different from issuer url which causes issuerValidation to fail
// This providerCtx will allow the Verifier to succeed if the alternate/alias URL is in the config
var providerCtx context.Context = context.Background()
if c.IssuerAlias != "" {
providerCtx = oidc.InsecureIssuerURLContext(ctx, c.IssuerAlias)
}

provider, err := factory(providerCtx, c.Issuer)
if err != nil {
return nil, err
}
Expand Down Expand Up @@ -179,7 +188,12 @@ func newSso(
if err != nil {
return nil, fmt.Errorf("failed to create JWT encrpytor: %w", err)
}
log.WithFields(log.Fields{"redirectUrl": config.RedirectURL, "issuer": c.Issuer, "clientId": c.ClientID, "scopes": config.Scopes}).Info("SSO configuration")
lf := log.Fields{"redirectUrl": config.RedirectURL, "issuer": c.Issuer, "issuerAlias": "DISABLED", "clientId": c.ClientID, "scopes": config.Scopes}
if c.IssuerAlias != "" {
lf["issuerAlias"] = c.IssuerAlias
}
log.WithFields(lf).Info("SSO configuration")

return &sso{
config: config,
idTokenVerifier: idTokenVerifier,
Expand All @@ -191,6 +205,7 @@ func newSso(
expiry: c.GetSessionExpiry(),
customClaimName: c.CustomGroupClaimName,
userInfoPath: c.UserInfoPath,
issuer: c.Issuer,
}, nil
}

Expand Down Expand Up @@ -266,7 +281,7 @@ func (s *sso) HandleCallback(w http.ResponseWriter, r *http.Request) {
// Some SSO implementations (Okta) require a call to
// the OIDC user info path to get attributes like groups
if s.userInfoPath != "" {
groups, err = c.GetUserInfoGroups(oauth2Token.AccessToken, c.Issuer, s.userInfoPath)
groups, err = c.GetUserInfoGroups(oauth2Token.AccessToken, s.issuer, s.userInfoPath)
if err != nil {
w.WriteHeader(401)
_, _ = w.Write([]byte(fmt.Sprintf("failed to get groups claim: %v", err)))
Expand Down
27 changes: 23 additions & 4 deletions server/auth/sso/sso_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ import (
"testing"
"time"

"github.com/coreos/go-oidc"
"github.com/coreos/go-oidc/v3/oidc"
"github.com/stretchr/testify/assert"
"golang.org/x/oauth2"
apiv1 "k8s.io/api/core/v1"
Expand All @@ -15,7 +15,10 @@ import (

const testNamespace = "argo"

type fakeOidcProvider struct{}
type fakeOidcProvider struct {
Ctx context.Context
Issuer string
}

func (fakeOidcProvider) Endpoint() oauth2.Endpoint {
return oauth2.Endpoint{}
Expand All @@ -26,7 +29,7 @@ func (fakeOidcProvider) Verifier(config *oidc.Config) *oidc.IDTokenVerifier {
}

func fakeOidcFactory(ctx context.Context, issuer string) (providerInterface, error) {
return fakeOidcProvider{}, nil
return fakeOidcProvider{ctx, issuer}, nil
}

func getSecretKeySelector(secret, key string) apiv1.SecretKeySelector {
Expand Down Expand Up @@ -54,6 +57,7 @@ func TestLoadSsoClientIdFromSecret(t *testing.T) {
fakeClient := fake.NewSimpleClientset(ssoConfigSecret).CoreV1().Secrets(testNamespace)
config := Config{
Issuer: "https://test-issuer",
IssuerAlias: "",
ClientID: getSecretKeySelector("argo-sso-secret", "client-id"),
ClientSecret: getSecretKeySelector("argo-sso-secret", "client-secret"),
RedirectURL: "https://dummy",
Expand All @@ -65,10 +69,25 @@ func TestLoadSsoClientIdFromSecret(t *testing.T) {
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, "", config.IssuerAlias)
assert.Equal(t, 10*time.Hour, ssoObject.expiry)
}

func TestNewSsoWithIssuerAlias(t *testing.T) {
// if there's an issuer alias present, the oidc provider will allow validation from either of the issuer or the issuerAlias.
fakeClient := fake.NewSimpleClientset(ssoConfigSecret).CoreV1().Secrets(testNamespace)
config := Config{
Issuer: "https://test-issuer",
IssuerAlias: "https://test-issuer-alias",
ClientID: getSecretKeySelector("argo-sso-secret", "client-id"),
ClientSecret: getSecretKeySelector("argo-sso-secret", "client-secret"),
RedirectURL: "https://dummy",
CustomGroupClaimName: "argo_groups",
}
_, err := newSso(fakeOidcFactory, config, fakeClient, "/", false)
assert.NoError(t, err)

}
func TestLoadSsoClientIdFromDifferentSecret(t *testing.T) {
clientIDSecret := &apiv1.Secret{
ObjectMeta: metav1.ObjectMeta{
Expand Down

0 comments on commit 47d713b

Please sign in to comment.