Skip to content

Commit

Permalink
Fix Okta OIDC (gravitational#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 authored and Gregy committed Apr 14, 2022
1 parent 9d3da3a commit 8775157
Show file tree
Hide file tree
Showing 4 changed files with 22 additions and 30 deletions.
2 changes: 2 additions & 0 deletions constants.go
Original file line number Diff line number Diff line change
Expand Up @@ -465,6 +465,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
3 changes: 2 additions & 1 deletion go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -85,6 +85,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 @@ -268,7 +269,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
6 changes: 4 additions & 2 deletions go.sum
Original file line number Diff line number Diff line change
Expand Up @@ -456,8 +456,10 @@ 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=
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 @@ -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 8775157

Please sign in to comment.