Skip to content

Commit

Permalink
Fix Okta OIDC
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 13, 2022
1 parent a820257 commit eeacf60
Show file tree
Hide file tree
Showing 4 changed files with 20 additions and 31 deletions.
2 changes: 2 additions & 0 deletions constants.go
Original file line number Diff line number Diff line change
Expand Up @@ -453,6 +453,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
4 changes: 2 additions & 2 deletions go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -89,6 +89,7 @@ require (
github.com/sirupsen/logrus v1.8.1
github.com/stretchr/testify v1.7.0
github.com/tstranex/u2f v0.0.0-20160508205855-eb799ce68da4
github.com/ucarion/urlpath v0.0.0-20200424170820-7ccc79b76bbb
github.com/vulcand/predicate v1.2.0
go.etcd.io/etcd/api/v3 v3.5.1
go.etcd.io/etcd/client/v3 v3.5.1
Expand Down Expand Up @@ -231,7 +232,6 @@ require (
github.com/spf13/cobra v1.2.1 // indirect
github.com/spf13/pflag v1.0.5 // indirect
github.com/thales-e-security/pool v0.0.2 // indirect
github.com/ucarion/urlpath v0.0.0-20200424170820-7ccc79b76bbb // indirect
github.com/x448/float16 v0.8.4 // indirect
github.com/xdg-go/pbkdf2 v1.0.0 // indirect
github.com/xdg-go/scram v1.0.2 // indirect
Expand Down Expand Up @@ -266,7 +266,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/denisenkom/go-mssqldb => github.com/gravitational/go-mssqldb v0.11.1-0.20220202000043-bec708e9bfd0
github.com/dgrijalva/jwt-go v3.2.0+incompatible => github.com/golang-jwt/jwt v3.2.1+incompatible
github.com/go-redis/redis/v8 => github.com/gravitational/redis/v8 v8.11.5-0.20220211010318-7af711b76a91
Expand Down
4 changes: 2 additions & 2 deletions go.sum
Original file line number Diff line number Diff line change
Expand Up @@ -500,8 +500,8 @@ github.com/gravitational/go-mssqldb v0.11.1-0.20220202000043-bec708e9bfd0 h1:DC+
github.com/gravitational/go-mssqldb v0.11.1-0.20220202000043-bec708e9bfd0/go.mod h1:iiK0YP1ZeepvmBQk/QpLEhhTNJgfzrpArPY/aFvc9yU=
github.com/gravitational/go-mysql v1.1.1-teleport.2 h1:XZ36BZ7BgslA5ZCyCHjpc1wilFITThIH7cLcbLWKWzM=
github.com/gravitational/go-mysql v1.1.1-teleport.2/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/gosaml2 v0.0.0-20220318224559-f06932032ae2 h1:8z1D1fehTDV20wkiGX+JTnlevvEUeVEh4LCygvOrFBs=
github.com/gravitational/gosaml2 v0.0.0-20220318224559-f06932032ae2/go.mod h1:PiLt5KX4EMjlMIq3WLRR/xb5yqhiwtQhGr8wmU0b08M=
github.com/gravitational/kingpin v2.1.11-0.20190130013101-742f2714c145+incompatible h1:CfyZl3nyo9K5lLqOmqvl9/IElY1UCnOWKZiQxJ8HKdA=
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"
"net/http"
"net/url"
"time"

"github.com/gravitational/teleport"
"github.com/gravitational/teleport/api/constants"
Expand Down Expand Up @@ -78,36 +79,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 @@ -707,19 +694,19 @@ 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 If the default client secret basic is used the Ping OIDC
// 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

0 comments on commit eeacf60

Please sign in to comment.