diff --git a/CHANGELOG.md b/CHANGELOG.md index 74290803..2b75a8af 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -13,6 +13,7 @@ - [#335](https://github.com/pusher/oauth2_proxy/pull/335) OIDC Provider support for empty id_tokens in the access token refresh response (@howzat) - [#363](https://github.com/pusher/oauth2_proxy/pull/363) Extension of Redis Session Store to Support Redis Cluster (@yan-dblinf) - [#353](https://github.com/pusher/oauth2_proxy/pull/353) Fix login page fragment handling after soft reload on Firefox (@ffdybuster) +- [#355](https://github.com/pusher/oauth2_proxy/pull/355) Add Client Secret File support for providers that rotate client secret via file system (@pasha-r) # v5.0.0 @@ -40,6 +41,7 @@ - [#179](https://github.com/pusher/oauth2_proxy/pull/179) Add Nextcloud provider (@Ramblurr) - [#280](https://github.com/pusher/oauth2_proxy/pull/280) whitelisted redirect domains: add support for whitelisting specific ports or allowing wildcard ports (@kamaln7) - [#351](https://github.com/pusher/oauth2_proxy/pull/351) Add DigitalOcean Auth provider (@kamaln7) + # v4.1.0 ## Release Highlights diff --git a/docs/configuration/configuration.md b/docs/configuration/configuration.md index e2a7deb9..7e797264 100644 --- a/docs/configuration/configuration.md +++ b/docs/configuration/configuration.md @@ -29,6 +29,7 @@ An example [oauth2_proxy.cfg]({{ site.gitweb }}/contrib/oauth2_proxy.cfg.example | `-basic-auth-password` | string | the password to set when passing the HTTP Basic Auth header | | | `-client-id` | string | the OAuth Client ID: ie: `"123456.apps.googleusercontent.com"` | | | `-client-secret` | string | the OAuth Client Secret | | +| `-client-secret-file` | string | the file with OAuth Client Secret | | | `-config` | string | path to config file | | | `-cookie-domain` | string | an optional cookie domain to force cookies to (ie: `.yourcompany.com`) | | | `-cookie-expire` | duration | expire timeframe for cookie | 168h0m0s | @@ -329,3 +330,6 @@ nginx.ingress.kubernetes.io/configuration-snippet: | ``` You have to substitute *name* with the actual cookie name you configured via --cookie-name parameter. If you don't set a custom cookie name the variable should be "$upstream_cookie__oauth2_proxy_1" instead of "$upstream_cookie_name_1" and the new cookie-name should be "_oauth2_proxy_1=" instead of "name_1=". + +### Note on rotated Client Secret +If you set up your OAuth2 provider to rotate your client secret, you can use the `client-secret-file` option to reload the secret when it is updated. diff --git a/options.go b/options.go index 5355177b..97dfbf60 100644 --- a/options.go +++ b/options.go @@ -29,18 +29,19 @@ import ( // Options holds Configuration Options that can be set by Command Line Flag, // or Config File type Options struct { - ProxyPrefix string `flag:"proxy-prefix" cfg:"proxy_prefix" env:"OAUTH2_PROXY_PROXY_PREFIX"` - PingPath string `flag:"ping-path" cfg:"ping_path" env:"OAUTH2_PROXY_PING_PATH"` - ProxyWebSockets bool `flag:"proxy-websockets" cfg:"proxy_websockets" env:"OAUTH2_PROXY_PROXY_WEBSOCKETS"` - HTTPAddress string `flag:"http-address" cfg:"http_address" env:"OAUTH2_PROXY_HTTP_ADDRESS"` - HTTPSAddress string `flag:"https-address" cfg:"https_address" env:"OAUTH2_PROXY_HTTPS_ADDRESS"` - ReverseProxy bool `flag:"reverse-proxy" cfg:"reverse_proxy" env:"OAUTH2_PROXY_REVERSE_PROXY"` - ForceHTTPS bool `flag:"force-https" cfg:"force_https" env:"OAUTH2_PROXY_FORCE_HTTPS"` - RedirectURL string `flag:"redirect-url" cfg:"redirect_url" env:"OAUTH2_PROXY_REDIRECT_URL"` - ClientID string `flag:"client-id" cfg:"client_id" env:"OAUTH2_PROXY_CLIENT_ID"` - ClientSecret string `flag:"client-secret" cfg:"client_secret" env:"OAUTH2_PROXY_CLIENT_SECRET"` - TLSCertFile string `flag:"tls-cert-file" cfg:"tls_cert_file" env:"OAUTH2_PROXY_TLS_CERT_FILE"` - TLSKeyFile string `flag:"tls-key-file" cfg:"tls_key_file" env:"OAUTH2_PROXY_TLS_KEY_FILE"` + ProxyPrefix string `flag:"proxy-prefix" cfg:"proxy_prefix" env:"OAUTH2_PROXY_PROXY_PREFIX"` + PingPath string `flag:"ping-path" cfg:"ping_path" env:"OAUTH2_PROXY_PING_PATH"` + ProxyWebSockets bool `flag:"proxy-websockets" cfg:"proxy_websockets" env:"OAUTH2_PROXY_PROXY_WEBSOCKETS"` + HTTPAddress string `flag:"http-address" cfg:"http_address" env:"OAUTH2_PROXY_HTTP_ADDRESS"` + HTTPSAddress string `flag:"https-address" cfg:"https_address" env:"OAUTH2_PROXY_HTTPS_ADDRESS"` + ReverseProxy bool `flag:"reverse-proxy" cfg:"reverse_proxy" env:"OAUTH2_PROXY_REVERSE_PROXY"` + ForceHTTPS bool `flag:"force-https" cfg:"force_https" env:"OAUTH2_PROXY_FORCE_HTTPS"` + RedirectURL string `flag:"redirect-url" cfg:"redirect_url" env:"OAUTH2_PROXY_REDIRECT_URL"` + ClientID string `flag:"client-id" cfg:"client_id" env:"OAUTH2_PROXY_CLIENT_ID"` + ClientSecret string `flag:"client-secret" cfg:"client_secret" env:"OAUTH2_PROXY_CLIENT_SECRET"` + ClientSecretFile string `flag:"client-secret-file" cfg:"client_secret_file" env:"OAUTH2_PROXY_CLIENT_SECRET_FILE"` + TLSCertFile string `flag:"tls-cert-file" cfg:"tls_cert_file" env:"OAUTH2_PROXY_TLS_CERT_FILE"` + TLSKeyFile string `flag:"tls-key-file" cfg:"tls_key_file" env:"OAUTH2_PROXY_TLS_KEY_FILE"` AuthenticatedEmailsFile string `flag:"authenticated-emails-file" cfg:"authenticated_emails_file" env:"OAUTH2_PROXY_AUTHENTICATED_EMAILS_FILE"` KeycloakGroup string `flag:"keycloak-group" cfg:"keycloak_group" env:"OAUTH2_PROXY_KEYCLOAK_GROUP"` @@ -222,8 +223,16 @@ func (o *Options) Validate() error { msgs = append(msgs, "missing setting: client-id") } // login.gov uses a signed JWT to authenticate, not a client-secret - if o.ClientSecret == "" && o.Provider != "login.gov" { - msgs = append(msgs, "missing setting: client-secret") + if o.Provider != "login.gov" { + if o.ClientSecret == "" && o.ClientSecretFile == "" { + msgs = append(msgs, "missing setting: client-secret or client-secret-file") + } + if o.ClientSecret == "" && o.ClientSecretFile != "" { + _, err := ioutil.ReadFile(o.ClientSecretFile) + if err != nil { + msgs = append(msgs, "could not read client secret file: "+o.ClientSecretFile) + } + } } if o.AuthenticatedEmailsFile == "" && len(o.EmailDomains) == 0 && o.HtpasswdFile == "" { msgs = append(msgs, "missing setting for email validation: email-domain or authenticated-emails-file required."+ @@ -392,10 +401,11 @@ func (o *Options) Validate() error { func parseProviderInfo(o *Options, msgs []string) []string { p := &providers.ProviderData{ - Scope: o.Scope, - ClientID: o.ClientID, - ClientSecret: o.ClientSecret, - ApprovalPrompt: o.ApprovalPrompt, + Scope: o.Scope, + ClientID: o.ClientID, + ClientSecret: o.ClientSecret, + ClientSecretFile: o.ClientSecretFile, + ApprovalPrompt: o.ApprovalPrompt, } p.LoginURL, msgs = parseURL(o.LoginURL, "login", msgs) p.RedeemURL, msgs = parseURL(o.RedeemURL, "redeem", msgs) diff --git a/options_test.go b/options_test.go index 171ff360..996acaea 100644 --- a/options_test.go +++ b/options_test.go @@ -3,7 +3,9 @@ package main import ( "crypto" "fmt" + "io/ioutil" "net/url" + "os" "strings" "testing" "time" @@ -37,10 +39,58 @@ func TestNewOptions(t *testing.T) { expected := errorMsg([]string{ "missing setting: cookie-secret", "missing setting: client-id", - "missing setting: client-secret"}) + "missing setting: client-secret or client-secret-file"}) assert.Equal(t, expected, err.Error()) } +func TestClientSecretFileOptionFails(t *testing.T) { + o := NewOptions() + o.CookieSecret = "foobar" + o.ClientID = "bazquux" + o.ClientSecretFile = "xyzzyplugh" + o.EmailDomains = []string{"*"} + err := o.Validate() + assert.NotEqual(t, nil, err) + + p := o.provider.Data() + assert.Equal(t, "xyzzyplugh", p.ClientSecretFile) + assert.Equal(t, "", p.ClientSecret) + + s, err := p.GetClientSecret() + assert.NotEqual(t, nil, err) + assert.Equal(t, "", s) +} + +func TestClientSecretFileOption(t *testing.T) { + var err error + f, err := ioutil.TempFile("", "client_secret_temp_file_") + if err != nil { + t.Fatalf("failed to create temp file: %v", err) + } + f.WriteString("testcase") + if err := f.Close(); err != nil { + t.Fatalf("failed to close temp file: %v", err) + } + clientSecretFileName := f.Name() + defer os.Remove(clientSecretFileName) + + o := NewOptions() + o.CookieSecret = "foobar" + o.ClientID = "bazquux" + o.ClientSecretFile = clientSecretFileName + o.EmailDomains = []string{"*"} + err = o.Validate() + assert.Equal(t, nil, err) + + p := o.provider.Data() + assert.Equal(t, clientSecretFileName, p.ClientSecretFile) + assert.Equal(t, "", p.ClientSecret) + + s, err := p.GetClientSecret() + assert.Equal(t, nil, err) + assert.Equal(t, "testcase", s) +} + func TestGoogleGroupOptions(t *testing.T) { o := testOptions() o.GoogleGroups = []string{"googlegroup"} diff --git a/providers/azure.go b/providers/azure.go index 48cea846..b619e6fe 100644 --- a/providers/azure.go +++ b/providers/azure.go @@ -74,11 +74,16 @@ func (p *AzureProvider) Redeem(redirectURL, code string) (s *sessions.SessionSta err = errors.New("missing code") return } + clientSecret, err := p.GetClientSecret() + if err != nil { + return + } + params := url.Values{} params.Add("redirect_uri", redirectURL) params.Add("client_id", p.ClientID) - params.Add("client_secret", p.ClientSecret) + params.Add("client_secret", clientSecret) params.Add("code", code) params.Add("grant_type", "authorization_code") if p.ProtectedResource != nil && p.ProtectedResource.String() != "" { diff --git a/providers/gitlab.go b/providers/gitlab.go index c32ebe85..396d41fc 100644 --- a/providers/gitlab.go +++ b/providers/gitlab.go @@ -38,10 +38,15 @@ func NewGitLabProvider(p *ProviderData) *GitLabProvider { // Redeem exchanges the OAuth2 authentication token for an ID token func (p *GitLabProvider) Redeem(redirectURL, code string) (s *sessions.SessionState, err error) { + clientSecret, err := p.GetClientSecret() + if err != nil { + return + } + ctx := context.Background() c := oauth2.Config{ ClientID: p.ClientID, - ClientSecret: p.ClientSecret, + ClientSecret: clientSecret, Endpoint: oauth2.Endpoint{ TokenURL: p.RedeemURL.String(), }, @@ -77,9 +82,14 @@ func (p *GitLabProvider) RefreshSessionIfNeeded(s *sessions.SessionState) (bool, } func (p *GitLabProvider) redeemRefreshToken(s *sessions.SessionState) (err error) { + clientSecret, err := p.GetClientSecret() + if err != nil { + return + } + c := oauth2.Config{ ClientID: p.ClientID, - ClientSecret: p.ClientSecret, + ClientSecret: clientSecret, Endpoint: oauth2.Endpoint{ TokenURL: p.RedeemURL.String(), }, diff --git a/providers/google.go b/providers/google.go index dee67e06..12216a35 100644 --- a/providers/google.go +++ b/providers/google.go @@ -102,11 +102,15 @@ func (p *GoogleProvider) Redeem(redirectURL, code string) (s *sessions.SessionSt err = errors.New("missing code") return } + clientSecret, err := p.GetClientSecret() + if err != nil { + return + } params := url.Values{} params.Add("redirect_uri", redirectURL) params.Add("client_id", p.ClientID) - params.Add("client_secret", p.ClientSecret) + params.Add("client_secret", clientSecret) params.Add("code", code) params.Add("grant_type", "authorization_code") var req *http.Request @@ -261,9 +265,14 @@ func (p *GoogleProvider) RefreshSessionIfNeeded(s *sessions.SessionState) (bool, func (p *GoogleProvider) redeemRefreshToken(refreshToken string) (token string, idToken string, expires time.Duration, err error) { // https://developers.google.com/identity/protocols/OAuth2WebServer#refresh + clientSecret, err := p.GetClientSecret() + if err != nil { + return + } + params := url.Values{} params.Add("client_id", p.ClientID) - params.Add("client_secret", p.ClientSecret) + params.Add("client_secret", clientSecret) params.Add("refresh_token", refreshToken) params.Add("grant_type", "refresh_token") var req *http.Request diff --git a/providers/google_test.go b/providers/google_test.go index 37b8326c..0e1de914 100644 --- a/providers/google_test.go +++ b/providers/google_test.go @@ -146,6 +146,18 @@ func TestGoogleProviderGetEmailAddressInvalidEncoding(t *testing.T) { } } +func TestGoogleProviderRedeemFailsNoCLientSecret(t *testing.T) { + p := newGoogleProvider() + p.ProviderData.ClientSecretFile = "srvnoerre" + + session, err := p.Redeem("http://redirect/", "code1234") + assert.NotEqual(t, nil, err) + if session != nil { + t.Errorf("expect nill session %#v", session) + } + assert.Equal(t, "could not read client secret file", err.Error()) +} + func TestGoogleProviderGetEmailAddressInvalidJson(t *testing.T) { p := newGoogleProvider() diff --git a/providers/oidc.go b/providers/oidc.go index 06b3206c..4a994017 100644 --- a/providers/oidc.go +++ b/providers/oidc.go @@ -30,10 +30,15 @@ func NewOIDCProvider(p *ProviderData) *OIDCProvider { // Redeem exchanges the OAuth2 authentication token for an ID token func (p *OIDCProvider) Redeem(redirectURL, code string) (s *sessions.SessionState, err error) { + clientSecret, err := p.GetClientSecret() + if err != nil { + return + } + ctx := context.Background() c := oauth2.Config{ ClientID: p.ClientID, - ClientSecret: p.ClientSecret, + ClientSecret: clientSecret, Endpoint: oauth2.Endpoint{ TokenURL: p.RedeemURL.String(), }, @@ -77,9 +82,14 @@ func (p *OIDCProvider) RefreshSessionIfNeeded(s *sessions.SessionState) (bool, e } func (p *OIDCProvider) redeemRefreshToken(s *sessions.SessionState) (err error) { + clientSecret, err := p.GetClientSecret() + if err != nil { + return + } + c := oauth2.Config{ ClientID: p.ClientID, - ClientSecret: p.ClientSecret, + ClientSecret: clientSecret, Endpoint: oauth2.Endpoint{ TokenURL: p.RedeemURL.String(), }, diff --git a/providers/provider_data.go b/providers/provider_data.go index ff752c34..b264a0bf 100644 --- a/providers/provider_data.go +++ b/providers/provider_data.go @@ -1,6 +1,9 @@ package providers import ( + "errors" + "github.com/pusher/oauth2_proxy/pkg/logger" + "io/ioutil" "net/url" ) @@ -10,6 +13,7 @@ type ProviderData struct { ProviderName string ClientID string ClientSecret string + ClientSecretFile string LoginURL *url.URL RedeemURL *url.URL ProfileURL *url.URL @@ -21,3 +25,17 @@ type ProviderData struct { // Data returns the ProviderData func (p *ProviderData) Data() *ProviderData { return p } + +func (p *ProviderData) GetClientSecret() (ClientSecret string, err error) { + if p.ClientSecret != "" || p.ClientSecretFile == "" { + return p.ClientSecret, nil + } + + // Getting ClientSecret can fail in runtime so we need to report it without returning the file name to the user + fileClientSecret, err := ioutil.ReadFile(p.ClientSecretFile) + if err != nil { + logger.Printf("error reading client secret file %s: %s", p.ClientSecretFile, err) + return "", errors.New("could not read client secret file") + } + return string(fileClientSecret), nil +} diff --git a/providers/provider_default.go b/providers/provider_default.go index d87b939c..707bbcea 100644 --- a/providers/provider_default.go +++ b/providers/provider_default.go @@ -20,11 +20,15 @@ func (p *ProviderData) Redeem(redirectURL, code string) (s *sessions.SessionStat err = errors.New("missing code") return } + clientSecret, err := p.GetClientSecret() + if err != nil { + return + } params := url.Values{} params.Add("redirect_uri", redirectURL) params.Add("client_id", p.ClientID) - params.Add("client_secret", p.ClientSecret) + params.Add("client_secret", clientSecret) params.Add("code", code) params.Add("grant_type", "authorization_code") if p.ProtectedResource != nil && p.ProtectedResource.String() != "" {