Handle authentication errors in PKCE flow (#1039)

* handle authentication errors in PKCE flow

* remove shadowing and replace TokenEndpoint for PKCE config

---------

Co-authored-by: Maycon Santos <mlsmaycon@gmail.com>
This commit is contained in:
Bethuel Mmbaga 2023-07-31 15:22:38 +03:00 committed by GitHub
parent 64f6343fcc
commit 48098c994d
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
2 changed files with 37 additions and 30 deletions

View File

@ -25,6 +25,8 @@ var _ OAuthFlow = &PKCEAuthorizationFlow{}
const ( const (
queryState = "state" queryState = "state"
queryCode = "code" queryCode = "code"
queryError = "error"
queryErrorDesc = "error_description"
defaultPKCETimeoutSeconds = 300 defaultPKCETimeoutSeconds = 300
) )
@ -141,9 +143,13 @@ func (p *PKCEAuthorizationFlow) startServer(tokenChan chan<- *oauth2.Token, errC
tokenValidatorFunc := func() (*oauth2.Token, error) { tokenValidatorFunc := func() (*oauth2.Token, error) {
query := req.URL.Query() query := req.URL.Query()
state := query.Get(queryState) if authError := query.Get(queryError); authError != "" {
authErrorDesc := query.Get(queryErrorDesc)
return nil, fmt.Errorf("%s.%s", authError, authErrorDesc)
}
// Prevent timing attacks on state // Prevent timing attacks on state
if subtle.ConstantTimeCompare([]byte(p.state), []byte(state)) == 0 { if state := query.Get(queryState); subtle.ConstantTimeCompare([]byte(p.state), []byte(state)) == 0 {
return nil, fmt.Errorf("invalid state") return nil, fmt.Errorf("invalid state")
} }
@ -161,12 +167,13 @@ func (p *PKCEAuthorizationFlow) startServer(tokenChan chan<- *oauth2.Token, errC
token, err := tokenValidatorFunc() token, err := tokenValidatorFunc()
if err != nil { if err != nil {
errChan <- fmt.Errorf("PKCE authorization flow failed: %v", err)
renderPKCEFlowTmpl(w, err) renderPKCEFlowTmpl(w, err)
errChan <- fmt.Errorf("PKCE authorization flow failed: %v", err)
return
} }
tokenChan <- token
renderPKCEFlowTmpl(w, nil) renderPKCEFlowTmpl(w, nil)
tokenChan <- token
}) })
if err := server.ListenAndServe(); err != nil { if err := server.ListenAndServe(); err != nil {

View File

@ -371,24 +371,24 @@ func handlerFunc(gRPCHandler *grpc.Server, httpHandler http.Handler) http.Handle
} }
func loadMgmtConfig(mgmtConfigPath string) (*server.Config, error) { func loadMgmtConfig(mgmtConfigPath string) (*server.Config, error) {
config := &server.Config{} loadedConfig := &server.Config{}
_, err := util.ReadJson(mgmtConfigPath, config) _, err := util.ReadJson(mgmtConfigPath, loadedConfig)
if err != nil { if err != nil {
return nil, err return nil, err
} }
if mgmtLetsencryptDomain != "" { if mgmtLetsencryptDomain != "" {
config.HttpConfig.LetsEncryptDomain = mgmtLetsencryptDomain loadedConfig.HttpConfig.LetsEncryptDomain = mgmtLetsencryptDomain
} }
if mgmtDataDir != "" { if mgmtDataDir != "" {
config.Datadir = mgmtDataDir loadedConfig.Datadir = mgmtDataDir
} }
if certKey != "" && certFile != "" { if certKey != "" && certFile != "" {
config.HttpConfig.CertFile = certFile loadedConfig.HttpConfig.CertFile = certFile
config.HttpConfig.CertKey = certKey loadedConfig.HttpConfig.CertKey = certKey
} }
oidcEndpoint := config.HttpConfig.OIDCConfigEndpoint oidcEndpoint := loadedConfig.HttpConfig.OIDCConfigEndpoint
if oidcEndpoint != "" { if oidcEndpoint != "" {
// if OIDCConfigEndpoint is specified, we can load DeviceAuthEndpoint and TokenEndpoint automatically // if OIDCConfigEndpoint is specified, we can load DeviceAuthEndpoint and TokenEndpoint automatically
log.Infof("loading OIDC configuration from the provided IDP configuration endpoint %s", oidcEndpoint) log.Infof("loading OIDC configuration from the provided IDP configuration endpoint %s", oidcEndpoint)
@ -399,45 +399,45 @@ func loadMgmtConfig(mgmtConfigPath string) (*server.Config, error) {
log.Infof("loaded OIDC configuration from the provided IDP configuration endpoint: %s", oidcEndpoint) log.Infof("loaded OIDC configuration from the provided IDP configuration endpoint: %s", oidcEndpoint)
log.Infof("overriding HttpConfig.AuthIssuer with a new value %s, previously configured value: %s", log.Infof("overriding HttpConfig.AuthIssuer with a new value %s, previously configured value: %s",
oidcConfig.Issuer, config.HttpConfig.AuthIssuer) oidcConfig.Issuer, loadedConfig.HttpConfig.AuthIssuer)
config.HttpConfig.AuthIssuer = oidcConfig.Issuer loadedConfig.HttpConfig.AuthIssuer = oidcConfig.Issuer
log.Infof("overriding HttpConfig.AuthKeysLocation (JWT certs) with a new value %s, previously configured value: %s", log.Infof("overriding HttpConfig.AuthKeysLocation (JWT certs) with a new value %s, previously configured value: %s",
oidcConfig.JwksURI, config.HttpConfig.AuthKeysLocation) oidcConfig.JwksURI, loadedConfig.HttpConfig.AuthKeysLocation)
config.HttpConfig.AuthKeysLocation = oidcConfig.JwksURI loadedConfig.HttpConfig.AuthKeysLocation = oidcConfig.JwksURI
if !(config.DeviceAuthorizationFlow == nil || strings.ToLower(config.DeviceAuthorizationFlow.Provider) == string(server.NONE)) { if !(loadedConfig.DeviceAuthorizationFlow == nil || strings.ToLower(loadedConfig.DeviceAuthorizationFlow.Provider) == string(server.NONE)) {
log.Infof("overriding DeviceAuthorizationFlow.TokenEndpoint with a new value: %s, previously configured value: %s", log.Infof("overriding DeviceAuthorizationFlow.TokenEndpoint with a new value: %s, previously configured value: %s",
oidcConfig.TokenEndpoint, config.DeviceAuthorizationFlow.ProviderConfig.TokenEndpoint) oidcConfig.TokenEndpoint, loadedConfig.DeviceAuthorizationFlow.ProviderConfig.TokenEndpoint)
config.DeviceAuthorizationFlow.ProviderConfig.TokenEndpoint = oidcConfig.TokenEndpoint loadedConfig.DeviceAuthorizationFlow.ProviderConfig.TokenEndpoint = oidcConfig.TokenEndpoint
log.Infof("overriding DeviceAuthorizationFlow.DeviceAuthEndpoint with a new value: %s, previously configured value: %s", log.Infof("overriding DeviceAuthorizationFlow.DeviceAuthEndpoint with a new value: %s, previously configured value: %s",
oidcConfig.DeviceAuthEndpoint, config.DeviceAuthorizationFlow.ProviderConfig.DeviceAuthEndpoint) oidcConfig.DeviceAuthEndpoint, loadedConfig.DeviceAuthorizationFlow.ProviderConfig.DeviceAuthEndpoint)
config.DeviceAuthorizationFlow.ProviderConfig.DeviceAuthEndpoint = oidcConfig.DeviceAuthEndpoint loadedConfig.DeviceAuthorizationFlow.ProviderConfig.DeviceAuthEndpoint = oidcConfig.DeviceAuthEndpoint
u, err := url.Parse(oidcEndpoint) u, err := url.Parse(oidcEndpoint)
if err != nil { if err != nil {
return nil, err return nil, err
} }
log.Infof("overriding DeviceAuthorizationFlow.ProviderConfig.Domain with a new value: %s, previously configured value: %s", log.Infof("overriding DeviceAuthorizationFlow.ProviderConfig.Domain with a new value: %s, previously configured value: %s",
u.Host, config.DeviceAuthorizationFlow.ProviderConfig.Domain) u.Host, loadedConfig.DeviceAuthorizationFlow.ProviderConfig.Domain)
config.DeviceAuthorizationFlow.ProviderConfig.Domain = u.Host loadedConfig.DeviceAuthorizationFlow.ProviderConfig.Domain = u.Host
if config.DeviceAuthorizationFlow.ProviderConfig.Scope == "" { if loadedConfig.DeviceAuthorizationFlow.ProviderConfig.Scope == "" {
config.DeviceAuthorizationFlow.ProviderConfig.Scope = server.DefaultDeviceAuthFlowScope loadedConfig.DeviceAuthorizationFlow.ProviderConfig.Scope = server.DefaultDeviceAuthFlowScope
} }
} }
if config.PKCEAuthorizationFlow != nil { if loadedConfig.PKCEAuthorizationFlow != nil {
log.Infof("overriding PKCEAuthorizationFlow.TokenEndpoint with a new value: %s, previously configured value: %s", log.Infof("overriding PKCEAuthorizationFlow.TokenEndpoint with a new value: %s, previously configured value: %s",
oidcConfig.TokenEndpoint, config.PKCEAuthorizationFlow.ProviderConfig.TokenEndpoint) oidcConfig.TokenEndpoint, loadedConfig.PKCEAuthorizationFlow.ProviderConfig.TokenEndpoint)
config.DeviceAuthorizationFlow.ProviderConfig.TokenEndpoint = oidcConfig.TokenEndpoint loadedConfig.PKCEAuthorizationFlow.ProviderConfig.TokenEndpoint = oidcConfig.TokenEndpoint
log.Infof("overriding PKCEAuthorizationFlow.AuthorizationEndpoint with a new value: %s, previously configured value: %s", log.Infof("overriding PKCEAuthorizationFlow.AuthorizationEndpoint with a new value: %s, previously configured value: %s",
oidcConfig.AuthorizationEndpoint, config.PKCEAuthorizationFlow.ProviderConfig.AuthorizationEndpoint) oidcConfig.AuthorizationEndpoint, loadedConfig.PKCEAuthorizationFlow.ProviderConfig.AuthorizationEndpoint)
config.PKCEAuthorizationFlow.ProviderConfig.AuthorizationEndpoint = oidcConfig.AuthorizationEndpoint loadedConfig.PKCEAuthorizationFlow.ProviderConfig.AuthorizationEndpoint = oidcConfig.AuthorizationEndpoint
} }
} }
return config, err return loadedConfig, err
} }
// OIDCConfigResponse used for parsing OIDC config response // OIDCConfigResponse used for parsing OIDC config response