Skip to content

Commit

Permalink
Fix Okta OIDC (#11718)
Browse files Browse the repository at this point in the history
Using the OIDC connector with Okta would fail due to an issue in our
fork of go-oidc. Update this dependency to get the fix.

Additionally, clean up the logic for syncing the connector
configuration, which was using a context.Context in order to implement
a timeout. This can be expressed in a simpler way with time.After()
  • Loading branch information
zmb3 committed Apr 14, 2022
1 parent 3c54d85 commit 4c963a3
Show file tree
Hide file tree
Showing 8 changed files with 38 additions and 38 deletions.
5 changes: 5 additions & 0 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,8 @@ build
_obj
_test
tmp
target/
test-logs/

# Architecture specific extensions/prefixes
[568vq].out
Expand Down Expand Up @@ -63,3 +65,6 @@ ssh.config

# Code signing certificate for Windows binaries
/windows-signing-cert.pfx

# build tooling
build.assets/tooling/bin/**
2 changes: 1 addition & 1 deletion Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -795,7 +795,7 @@ update-vendor:
# delete the vendored api package. In its place
# create a symlink to the the original api package
rm -r vendor/github.com/gravitational/teleport/api
ln -s -r $(shell readlink -f api) vendor/github.com/gravitational/teleport
cd vendor/github.com/gravitational/teleport && ln -s ../../../../api api

# update-webassets updates the minified code in the webassets repo using the latest webapps
# repo and creates a PR in the teleport repo to update webassets submodule.
Expand Down
2 changes: 2 additions & 0 deletions constants.go
Original file line number Diff line number Diff line change
Expand Up @@ -438,6 +438,8 @@ const (
// Ping is the common backend for all Ping Identity-branded identity
// providers (including PingOne, PingFederate, etc).
Ping = "ping"
// Okta should be used for Okta OIDC providers.
Okta = "okta"
)

const (
Expand Down
2 changes: 1 addition & 1 deletion go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -131,7 +131,7 @@ require (
)

replace (
github.com/coreos/go-oidc => github.com/gravitational/go-oidc v0.0.5
github.com/coreos/go-oidc => github.com/gravitational/go-oidc v0.0.6
github.com/gogo/protobuf => github.com/gravitational/protobuf v1.3.2-0.20201123192827-2b9fcfaffcbf
github.com/gravitational/teleport/api => ./api
github.com/siddontang/go-mysql v1.1.0 => github.com/gravitational/go-mysql v1.1.1-0.20210212011549-886316308a77
Expand Down
4 changes: 2 additions & 2 deletions go.sum
Original file line number Diff line number Diff line change
Expand Up @@ -357,8 +357,8 @@ github.com/gravitational/form v0.0.0-20151109031454-c4048f792f70 h1:To76nCJtM3DI
github.com/gravitational/form v0.0.0-20151109031454-c4048f792f70/go.mod h1:88hFR45MpUd23d2vNWE/dYtesU50jKsbz0I9kH7UaBY=
github.com/gravitational/go-mysql v1.1.1-0.20210212011549-886316308a77 h1:ivambM2XeST8qfxeSm+0Y8CP/DlNbS3o/9tSF2KtGFk=
github.com/gravitational/go-mysql v1.1.1-0.20210212011549-886316308a77/go.mod h1:re0JQZ1Cy5dVlIDGq0YksfDIla/GRZlxqOoC0XPSSGE=
github.com/gravitational/go-oidc v0.0.5 h1:kxsCknoOZ+KqIAoYLLdHuQcvcc+SrQlnT7xxIM8oo6o=
github.com/gravitational/go-oidc v0.0.5/go.mod h1:SevmOUNdOB0aD9BAIgjptZ6oHkKxMZZgA70nwPfgU/w=
github.com/gravitational/go-oidc v0.0.6 h1:DCllahGYxDAvxWsq8UILgO+/i1EheQRxcNzS+D+wP5I=
github.com/gravitational/go-oidc v0.0.6/go.mod h1:SevmOUNdOB0aD9BAIgjptZ6oHkKxMZZgA70nwPfgU/w=
github.com/gravitational/kingpin v2.1.11-0.20190130013101-742f2714c145+incompatible h1:CfyZl3nyo9K5lLqOmqvl9/IElY1UCnOWKZiQxJ8HKdA=
github.com/gravitational/kingpin v2.1.11-0.20190130013101-742f2714c145+incompatible/go.mod h1:LWxG30M3FcrjhOn3T4zz7JmBoQJ45MWZmOXgy9Ganoc=
github.com/gravitational/license v0.0.0-20210218173955-6d8fb49b117a h1:PN5vAN1ZA0zqdpM6wNdx6+bkdlQ5fImd75oaIHSbOhY=
Expand Down
41 changes: 14 additions & 27 deletions lib/auth/oidc.go
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ import (
"io/ioutil"
"net/http"
"net/url"
"time"

"github.com/gravitational/teleport"
"github.com/gravitational/teleport/api/constants"
Expand Down Expand Up @@ -80,36 +81,22 @@ func (a *Server) createOIDCClient(conn types.OIDCConnector) (*oidc.Client, error
return nil, trace.Wrap(err)
}

ctx, cancel := context.WithTimeout(context.Background(), defaults.WebHeadersTimeout)
defer cancel()

doneSyncing := make(chan struct{})
go func() {
defer cancel()
defer close(doneSyncing)
client.SyncProviderConfig(conn.GetIssuerURL())
}()

select {
case <-ctx.Done():
case <-doneSyncing:
case <-time.After(defaults.WebHeadersTimeout):
return nil, trace.ConnectionProblem(nil,
"timed out syncing oidc connector %v, ensure URL %q is valid and accessible and check configuration",
conn.GetName(), conn.GetIssuerURL())
case <-a.closeCtx.Done():
return nil, trace.ConnectionProblem(nil, "auth server is shutting down")
}

// Canceled is expected in case if sync provider config finishes faster
// than the deadline
if ctx.Err() != nil && ctx.Err() != context.Canceled {
var err error
if ctx.Err() == context.DeadlineExceeded {
err = trace.ConnectionProblem(err,
"failed to reach out to oidc connector %v, most likely URL %q is not valid or not accessible, check configuration and try to re-create the connector",
conn.GetName(), conn.GetIssuerURL())
} else {
err = trace.ConnectionProblem(err,
"unknown problem with connector %v, most likely URL %q is not valid or not accessible, check configuration and try to re-create the connector",
conn.GetName(), conn.GetIssuerURL())
}
return nil, err
}

a.lock.Lock()
defer a.lock.Unlock()

Expand Down Expand Up @@ -914,20 +901,20 @@ func (a *Server) getClaims(oidcClient *oidc.Client, connector types.OIDCConnecto

// getOAuthClient returns a Oauth2 client from the oidc.Client. If the connector is set as a Ping provider sets the Client Secret Post auth method
func (a *Server) getOAuthClient(oidcClient *oidc.Client, connector types.OIDCConnector) (*oauth2.Client, error) {

oac, err := oidcClient.OAuthClient()

if err != nil {
return nil, trace.Wrap(err)
}

//If the default client secret basic is used the Ping OIDC
// will throw an error of multiple client credentials. Even if you set in Ping
// to use Client Secret Post it will return to use client secret basic.
// Issue https://github.com/gravitational/teleport/issues/8374
if connector.GetProvider() == teleport.Ping {
// For OIDC, Ping and Okta will throw an error when the
// default client secret basic method is used.
// See: https://github.com/gravitational/teleport/issues/8374
switch connector.GetProvider() {
case teleport.Ping, teleport.Okta:
oac.SetAuthMethod(oauth2.AuthMethodClientSecretPost)
}

return oac, err
}

Expand Down
16 changes: 11 additions & 5 deletions vendor/github.com/coreos/go-oidc/oidc/provider.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

4 changes: 2 additions & 2 deletions vendor/modules.txt
Original file line number Diff line number Diff line change
Expand Up @@ -208,7 +208,7 @@ github.com/boombuler/barcode/qr
github.com/boombuler/barcode/utils
# github.com/cespare/xxhash/v2 v2.1.1
github.com/cespare/xxhash/v2
# github.com/coreos/go-oidc v0.0.4 => github.com/gravitational/go-oidc v0.0.5
# github.com/coreos/go-oidc v0.0.4 => github.com/gravitational/go-oidc v0.0.6
## explicit
github.com/coreos/go-oidc/http
github.com/coreos/go-oidc/jose
Expand Down Expand Up @@ -1175,6 +1175,6 @@ k8s.io/utils/integer
sigs.k8s.io/structured-merge-diff/v4/value
# sigs.k8s.io/yaml v1.2.0
sigs.k8s.io/yaml
# github.com/coreos/go-oidc => github.com/gravitational/go-oidc v0.0.5
# github.com/coreos/go-oidc => github.com/gravitational/go-oidc v0.0.6
# github.com/gogo/protobuf => github.com/gravitational/protobuf v1.3.2-0.20201123192827-2b9fcfaffcbf
# github.com/gravitational/teleport/api => ./api

0 comments on commit 4c963a3

Please sign in to comment.