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 15, 2022
1 parent d67d3a2 commit 16f8651
Show file tree
Hide file tree
Showing 7 changed files with 1,032 additions and 43 deletions.
5 changes: 5 additions & 0 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,8 @@ build
_obj
_test
tmp
target/
test-logs/

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

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

# build tooling
build.assets/tooling/bin/**
2 changes: 2 additions & 0 deletions constants.go
Original file line number Diff line number Diff line change
Expand Up @@ -441,6 +441,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: 1 addition & 3 deletions go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -147,9 +147,7 @@ require (
github.com/gopherjs/gopherjs v0.0.0-20190430165422-3e4dfb77656c // indirect
github.com/gorilla/handlers v1.5.1 // indirect
github.com/gorilla/mux v1.8.0 // indirect
github.com/gregjones/httpcache v0.0.0-20180305231024-9cad4c3443a7 // indirect
github.com/grpc-ecosystem/go-grpc-middleware/v2 v2.0.0-rc.2.0.20220308023801-e4a6915ea237 // indirect
github.com/hashicorp/go-uuid v1.0.2 // indirect
github.com/imdario/mergo v0.3.5 // indirect
github.com/jackc/chunkreader/v2 v2.0.1 // indirect
github.com/jackc/pgio v1.0.0 // indirect
Expand Down Expand Up @@ -214,7 +212,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-teleport.2
Expand Down
999 changes: 997 additions & 2 deletions go.sum

Large diffs are not rendered by default.

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 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.

8 changes: 2 additions & 6 deletions vendor/modules.txt
Original file line number Diff line number Diff line change
Expand Up @@ -286,7 +286,7 @@ github.com/cloudflare/cfssl/helpers
github.com/cloudflare/cfssl/helpers/derhelpers
github.com/cloudflare/cfssl/log
github.com/cloudflare/cfssl/revoke
# 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 @@ -493,16 +493,12 @@ github.com/gravitational/trace/trail
# github.com/gravitational/ttlmap v0.0.0-20171116003245-91fd36b9004c
## explicit
github.com/gravitational/ttlmap
# github.com/gregjones/httpcache v0.0.0-20180305231024-9cad4c3443a7
## explicit
# github.com/grpc-ecosystem/go-grpc-middleware/providers/openmetrics/v2 v2.0.0-20220308023801-e4a6915ea237
## explicit; go 1.15
github.com/grpc-ecosystem/go-grpc-middleware/providers/openmetrics/v2
# github.com/grpc-ecosystem/go-grpc-middleware/v2 v2.0.0-rc.2.0.20220308023801-e4a6915ea237
## explicit; go 1.14
github.com/grpc-ecosystem/go-grpc-middleware/v2/interceptors
# github.com/hashicorp/go-uuid v1.0.2
## explicit
# github.com/hashicorp/golang-lru v0.5.4
## explicit; go 1.12
github.com/hashicorp/golang-lru
Expand Down Expand Up @@ -1288,7 +1284,7 @@ sigs.k8s.io/structured-merge-diff/v4/value
# sigs.k8s.io/yaml v1.2.0
## explicit; go 1.12
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
# github.com/sirupsen/logrus => github.com/gravitational/logrus v1.4.4-0.20210817004754-047e20245621

0 comments on commit 16f8651

Please sign in to comment.