Skip to content

Commit 47d713b

Browse files
authored
fix: Add OIDC issuer alias. Fixes argoproj#6759 (argoproj#6831)
Signed-off-by: shea_sullivan <[email protected]>
1 parent 55d3bc0 commit 47d713b

File tree

6 files changed

+49
-10
lines changed

6 files changed

+49
-10
lines changed

docs/workflow-controller-configmap.yaml

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -303,6 +303,8 @@ data:
303303
sso: |
304304
# This is the root URL of the OIDC provider (required).
305305
issuer: https://issuer.root.url/
306+
# Some OIDC providers have alternate root URLs that can be included. These should be reviewed carefully. (optional)
307+
issuerAlias: https://altissuer.root.url
306308
# This defines how long your login is valid for (in hours). (optional)
307309
# If omitted, defaults to 10h. Example below is 10 days.
308310
sessionExpiry: 240h

go.mod

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,7 @@ require (
1515
github.com/baiyubin/aliyun-sts-go-sdk v0.0.0-20180326062324-cfa1a18b161f // indirect
1616
github.com/blushft/go-diagrams v0.0.0-20201006005127-c78c821223d9
1717
github.com/colinmarc/hdfs v1.1.4-0.20180805212432-9746310a4d31
18-
github.com/coreos/go-oidc v2.2.1+incompatible
18+
github.com/coreos/go-oidc/v3 v3.1.0
1919
github.com/doublerebel/bellows v0.0.0-20160303004610-f177d92a03d3
2020
github.com/emicklei/go-restful v2.15.0+incompatible // indirect
2121
github.com/evanphx/json-patch v4.9.0+incompatible

go.sum

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -248,6 +248,8 @@ github.com/coreos/go-etcd v2.0.0+incompatible/go.mod h1:Jez6KQU2B/sWsbdaef3ED8Nz
248248
github.com/coreos/go-oidc v2.1.0+incompatible/go.mod h1:CgnwVTmzoESiwO9qyAFEMiHoZ1nMCKZlZ9V6mm3/LKc=
249249
github.com/coreos/go-oidc v2.2.1+incompatible h1:mh48q/BqXqgjVHpy2ZY7WnWAbenxRjsz9N1i1YxjHAk=
250250
github.com/coreos/go-oidc v2.2.1+incompatible/go.mod h1:CgnwVTmzoESiwO9qyAFEMiHoZ1nMCKZlZ9V6mm3/LKc=
251+
github.com/coreos/go-oidc/v3 v3.1.0 h1:6avEvcdvTa1qYsOZ6I5PRkSYHzpTNWgKYmaJfaYbrRw=
252+
github.com/coreos/go-oidc/v3 v3.1.0/go.mod h1:rEJ/idjfUyfkBit1eI1fvyr+64/g9dcKpAm8MJMesvo=
251253
github.com/coreos/go-semver v0.2.0/go.mod h1:nnelYz7RCh+5ahJtPPxZlU+153eP4D4r3EedlOD2RNk=
252254
github.com/coreos/go-semver v0.3.0/go.mod h1:nnelYz7RCh+5ahJtPPxZlU+153eP4D4r3EedlOD2RNk=
253255
github.com/coreos/go-systemd v0.0.0-20180511133405-39ca1b05acc7/go.mod h1:F5haX7vjVVG0kc13fIWeqUViNPyEJxv/OmvnBo0Yme4=
@@ -945,7 +947,6 @@ github.com/pmezard/go-difflib v1.0.0 h1:4DBwDE0NGyQoBHbLQYPwSUPoCMWR5BEzIk/f1lZb
945947
github.com/pmezard/go-difflib v1.0.0/go.mod h1:iKH77koFhYxTK1pcRnkKkqfTogsbg7gZNVY4sRDYZ/4=
946948
github.com/posener/complete v1.1.1/go.mod h1:em0nMJCgc9GFtwrmVmEMR/ZL6WyhyjMBndrE9hABlRI=
947949
github.com/pquerna/cachecontrol v0.0.0-20171018203845-0dec1b30a021/go.mod h1:prYjPmNq4d1NPVmpShWobRqXY3q7Vp+80DqgxxUrUIA=
948-
github.com/pquerna/cachecontrol v0.0.0-20180517163645-1555304b9b35 h1:J9b7z+QKAmPf4YLrFg6oQUotqHQeUNWwkvo7jZp1GLU=
949950
github.com/pquerna/cachecontrol v0.0.0-20180517163645-1555304b9b35/go.mod h1:prYjPmNq4d1NPVmpShWobRqXY3q7Vp+80DqgxxUrUIA=
950951
github.com/prometheus/client_golang v0.9.1/go.mod h1:7SWBe2y4D6OKWSNQJUaRYU/AaXPKyh/dDVn+NZz0KFw=
951952
github.com/prometheus/client_golang v0.9.2/go.mod h1:OsXs2jCmiKlQ1lTBmv21f2mNfw4xf/QclQDMrYNZzcM=
@@ -1312,6 +1313,7 @@ golang.org/x/net v0.0.0-20200226121028-0de0cce0169b/go.mod h1:z5CRVTTTmAJ677TzLL
13121313
golang.org/x/net v0.0.0-20200301022130-244492dfa37a/go.mod h1:z5CRVTTTmAJ677TzLLGU+0bjPO0LkuOLi4/5GtJWs/s=
13131314
golang.org/x/net v0.0.0-20200324143707-d3edc9973b7e/go.mod h1:qpuaurCH72eLCgpAm/N6yyVIVM9cpaDIP3A8BGJEC5A=
13141315
golang.org/x/net v0.0.0-20200501053045-e0ff5e5a1de5/go.mod h1:qpuaurCH72eLCgpAm/N6yyVIVM9cpaDIP3A8BGJEC5A=
1316+
golang.org/x/net v0.0.0-20200505041828-1ed23360d12c/go.mod h1:qpuaurCH72eLCgpAm/N6yyVIVM9cpaDIP3A8BGJEC5A=
13151317
golang.org/x/net v0.0.0-20200506145744-7e3656a0809f/go.mod h1:qpuaurCH72eLCgpAm/N6yyVIVM9cpaDIP3A8BGJEC5A=
13161318
golang.org/x/net v0.0.0-20200513185701-a91f0712d120/go.mod h1:qpuaurCH72eLCgpAm/N6yyVIVM9cpaDIP3A8BGJEC5A=
13171319
golang.org/x/net v0.0.0-20200520004742-59133d7f0dd7/go.mod h1:qpuaurCH72eLCgpAm/N6yyVIVM9cpaDIP3A8BGJEC5A=

manifests/quick-start/sso/overlays/workflow-controller-configmap.yaml

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@ apiVersion: v1
22
data:
33
sso: |
44
issuer: http://dex:5556/dex
5+
issuerAlias: http://mydex:5556/dex
56
clientId:
67
name: argo-server-sso
78
key: clientID

server/auth/sso/sso.go

Lines changed: 19 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,7 @@ import (
1212
"time"
1313

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

4747
type sso struct {
4848
config *oauth2.Config
49+
issuer string
4950
idTokenVerifier *oidc.IDTokenVerifier
5051
baseHRef string
5152
secure bool
@@ -63,6 +64,7 @@ func (s *sso) IsRBACEnabled() bool {
6364

6465
type Config struct {
6566
Issuer string `json:"issuer"`
67+
IssuerAlias string `json:"issuerAlias,omitempty"`
6668
ClientID apiv1.SecretKeySelector `json:"clientId"`
6769
ClientSecret apiv1.SecretKeySelector `json:"clientSecret"`
6870
RedirectURL string `json:"redirectUrl"`
@@ -122,7 +124,14 @@ func newSso(
122124
if err != nil {
123125
return nil, err
124126
}
125-
provider, err := factory(context.Background(), c.Issuer)
127+
// Some offspec providers like Azure, Oracle IDCS have oidc discovery url different from issuer url which causes issuerValidation to fail
128+
// This providerCtx will allow the Verifier to succeed if the alternate/alias URL is in the config
129+
var providerCtx context.Context = context.Background()
130+
if c.IssuerAlias != "" {
131+
providerCtx = oidc.InsecureIssuerURLContext(ctx, c.IssuerAlias)
132+
}
133+
134+
provider, err := factory(providerCtx, c.Issuer)
126135
if err != nil {
127136
return nil, err
128137
}
@@ -179,7 +188,12 @@ func newSso(
179188
if err != nil {
180189
return nil, fmt.Errorf("failed to create JWT encrpytor: %w", err)
181190
}
182-
log.WithFields(log.Fields{"redirectUrl": config.RedirectURL, "issuer": c.Issuer, "clientId": c.ClientID, "scopes": config.Scopes}).Info("SSO configuration")
191+
lf := log.Fields{"redirectUrl": config.RedirectURL, "issuer": c.Issuer, "issuerAlias": "DISABLED", "clientId": c.ClientID, "scopes": config.Scopes}
192+
if c.IssuerAlias != "" {
193+
lf["issuerAlias"] = c.IssuerAlias
194+
}
195+
log.WithFields(lf).Info("SSO configuration")
196+
183197
return &sso{
184198
config: config,
185199
idTokenVerifier: idTokenVerifier,
@@ -191,6 +205,7 @@ func newSso(
191205
expiry: c.GetSessionExpiry(),
192206
customClaimName: c.CustomGroupClaimName,
193207
userInfoPath: c.UserInfoPath,
208+
issuer: c.Issuer,
194209
}, nil
195210
}
196211

@@ -266,7 +281,7 @@ func (s *sso) HandleCallback(w http.ResponseWriter, r *http.Request) {
266281
// Some SSO implementations (Okta) require a call to
267282
// the OIDC user info path to get attributes like groups
268283
if s.userInfoPath != "" {
269-
groups, err = c.GetUserInfoGroups(oauth2Token.AccessToken, c.Issuer, s.userInfoPath)
284+
groups, err = c.GetUserInfoGroups(oauth2Token.AccessToken, s.issuer, s.userInfoPath)
270285
if err != nil {
271286
w.WriteHeader(401)
272287
_, _ = w.Write([]byte(fmt.Sprintf("failed to get groups claim: %v", err)))

server/auth/sso/sso_test.go

Lines changed: 23 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@ import (
55
"testing"
66
"time"
77

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

1616
const testNamespace = "argo"
1717

18-
type fakeOidcProvider struct{}
18+
type fakeOidcProvider struct {
19+
Ctx context.Context
20+
Issuer string
21+
}
1922

2023
func (fakeOidcProvider) Endpoint() oauth2.Endpoint {
2124
return oauth2.Endpoint{}
@@ -26,7 +29,7 @@ func (fakeOidcProvider) Verifier(config *oidc.Config) *oidc.IDTokenVerifier {
2629
}
2730

2831
func fakeOidcFactory(ctx context.Context, issuer string) (providerInterface, error) {
29-
return fakeOidcProvider{}, nil
32+
return fakeOidcProvider{ctx, issuer}, nil
3033
}
3134

3235
func getSecretKeySelector(secret, key string) apiv1.SecretKeySelector {
@@ -54,6 +57,7 @@ func TestLoadSsoClientIdFromSecret(t *testing.T) {
5457
fakeClient := fake.NewSimpleClientset(ssoConfigSecret).CoreV1().Secrets(testNamespace)
5558
config := Config{
5659
Issuer: "https://test-issuer",
60+
IssuerAlias: "",
5761
ClientID: getSecretKeySelector("argo-sso-secret", "client-id"),
5862
ClientSecret: getSecretKeySelector("argo-sso-secret", "client-secret"),
5963
RedirectURL: "https://dummy",
@@ -65,10 +69,25 @@ func TestLoadSsoClientIdFromSecret(t *testing.T) {
6569
assert.Equal(t, "sso-client-id-value", ssoObject.config.ClientID)
6670
assert.Equal(t, "sso-client-secret-value", ssoObject.config.ClientSecret)
6771
assert.Equal(t, "argo_groups", ssoObject.customClaimName)
68-
72+
assert.Equal(t, "", config.IssuerAlias)
6973
assert.Equal(t, 10*time.Hour, ssoObject.expiry)
7074
}
7175

76+
func TestNewSsoWithIssuerAlias(t *testing.T) {
77+
// if there's an issuer alias present, the oidc provider will allow validation from either of the issuer or the issuerAlias.
78+
fakeClient := fake.NewSimpleClientset(ssoConfigSecret).CoreV1().Secrets(testNamespace)
79+
config := Config{
80+
Issuer: "https://test-issuer",
81+
IssuerAlias: "https://test-issuer-alias",
82+
ClientID: getSecretKeySelector("argo-sso-secret", "client-id"),
83+
ClientSecret: getSecretKeySelector("argo-sso-secret", "client-secret"),
84+
RedirectURL: "https://dummy",
85+
CustomGroupClaimName: "argo_groups",
86+
}
87+
_, err := newSso(fakeOidcFactory, config, fakeClient, "/", false)
88+
assert.NoError(t, err)
89+
90+
}
7291
func TestLoadSsoClientIdFromDifferentSecret(t *testing.T) {
7392
clientIDSecret := &apiv1.Secret{
7493
ObjectMeta: metav1.ObjectMeta{

0 commit comments

Comments
 (0)