Support for client secret file. (#355)

* added ClientSecretFile in ProviderData

* add documentation notes on client secret file

* added Changelog entry for Client Secret File PR

* fixing configuration.md

* addressing PR issue of ClientSecret property naming

* Update providers/provider_data.go

Co-Authored-By: Joel Speed <Joel.speed@hotmail.co.uk>

* corrected changelog entry

* fixed typo in GetClientSecret

Co-authored-by: Joel Speed <Joel.speed@hotmail.co.uk>

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

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.

options.go

@@ -39,6 +39,7 @@ type Options struct {
 	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"`
 
@@ -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" {
+	if o.Provider != "login.gov" {
-		msgs = append(msgs, "missing setting: client-secret")
+		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."+
@@ -395,6 +404,7 @@ func parseProviderInfo(o *Options, msgs []string) []string {
 		Scope:            o.Scope,
 		ClientID:         o.ClientID,
 		ClientSecret:     o.ClientSecret,
+		ClientSecretFile: o.ClientSecretFile,
 		ApprovalPrompt:   o.ApprovalPrompt,
 	}
 	p.LoginURL, msgs = parseURL(o.LoginURL, "login", msgs)

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"}

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() != "" {

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(),
 		},

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

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()
 

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(),
 		},

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
+}

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() != "" {