Skip to content

Commit

Permalink
config: remove deprecated client_ca option (#4918)
Browse files Browse the repository at this point in the history 
The client_ca and client_ca_file settings were deprecated in v0.23.
Remove these options and add a link to the corresponding explanation on
the Upgrading docs page.
  • Loading branch information
@kenjenkins
kenjenkins committed Jan 30, 2024
1 parent 6a833b3 commit e83b14b
Show file tree
Hide file tree
Showing 4 changed files with 5 additions and 65 deletions.
1 change: 0 additions & 1 deletion config/config_source.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -270,7 +270,6 @@ func getAllConfigFilePaths(cfg *Config) []string {
fs := []string{
cfg.Options.CAFile,
cfg.Options.CertFile,
cfg.Options.ClientCAFile,
cfg.Options.ClientSecretFile,
cfg.Options.CookieSecretFile,
cfg.Options.DataBrokerStorageCAFile,
Expand Down
24 changes: 0 additions & 24 deletions config/options.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -254,15 +254,6 @@ type Options struct {
DataBrokerStorageCAFile string `mapstructure:"databroker_storage_ca_file" yaml:"databroker_storage_ca_file,omitempty"`
DataBrokerStorageCertSkipVerify bool `mapstructure:"databroker_storage_tls_skip_verify" yaml:"databroker_storage_tls_skip_verify,omitempty"`

// ClientCA is the base64-encoded certificate authority to validate client mTLS certificates against.
//
// Deprecated: Use DownstreamMTLS.CA instead.
ClientCA string `mapstructure:"client_ca" yaml:"client_ca,omitempty"`
// ClientCAFile points to a file that contains the certificate authority to validate client mTLS certificates against.
//
// Deprecated: Use DownstreamMTLS.CAFile instead.
ClientCAFile string `mapstructure:"client_ca_file" yaml:"client_ca_file,omitempty"`

// DownstreamMTLS holds all downstream mTLS settings.
DownstreamMTLS DownstreamMTLSSettings `mapstructure:"downstream_mtls" yaml:"downstream_mtls,omitempty"`

Expand Down Expand Up @@ -700,21 +691,6 @@ func (o *Options) Validate() error {
}
}

if o.ClientCA != "" {
log.Warn(context.Background()).Msg("config: client_ca is deprecated, set " +
"downstream_mtls.ca instead")
if o.DownstreamMTLS.CA == "" {
o.DownstreamMTLS.CA = o.ClientCA
}
}
if o.ClientCAFile != "" {
log.Warn(context.Background()).Msg("config: client_ca_file is deprecated, set " +
"downstream_mtls.ca_file instead")
if o.DownstreamMTLS.CAFile == "" {
o.DownstreamMTLS.CAFile = o.ClientCAFile
}
}

if err := o.DownstreamMTLS.validate(); err != nil {
return fmt.Errorf("config: bad downstream mTLS settings: %w", err)
}
Expand Down
2 changes: 2 additions & 0 deletions config/options_check.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,8 @@ var reKeyPath = regexp.MustCompile(`\[\d+\]`)
var (
// options that were deprecated in the config
removedConfigFields = map[string]string{
"client_ca": "https://www.pomerium.com/docs/deploy/core/upgrading#new-downstream-mtls-settings",
"client_ca_file": "https://www.pomerium.com/docs/deploy/core/upgrading#new-downstream-mtls-settings",
"idp_service_account": "https://docs.pomerium.com/docs/overview/upgrading#idp-directory-sync",
"idp_refresh_directory_timeout": "https://docs.pomerium.com/docs/overview/upgrading#idp-directory-sync",
"idp_refresh_directory_interval": "https://docs.pomerium.com/docs/overview/upgrading#idp-directory-sync",
Expand Down
43 changes: 3 additions & 40 deletions config/options_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,6 @@ import (

"github.com/pomerium/csrf"
"github.com/pomerium/pomerium/internal/identity/oauth/apple"
"github.com/pomerium/pomerium/internal/testutil"
"github.com/pomerium/pomerium/pkg/cryptutil"
"github.com/pomerium/pomerium/pkg/grpc/config"
)
Expand Down Expand Up @@ -470,9 +469,9 @@ func Test_NewOptionsFromConfigEnvVar(t *testing.T) {
{"bad cert files", map[string]string{"INSECURE_SERVER": "true", "SHARED_SECRET": "YixWi1MYh77NMECGGIJQevoonYtVF+ZPRkQZrrmeRqM=", "CERTIFICATES": "./test-data/example-cert.pem"}, true},
{"good cert file", map[string]string{"CERTIFICATE_FILE": "./testdata/example-cert.pem", "CERTIFICATE_KEY_FILE": "./testdata/example-key.pem", "INSECURE_SERVER": "true", "SHARED_SECRET": "YixWi1MYh77NMECGGIJQevoonYtVF+ZPRkQZrrmeRqM="}, false},
{"bad cert file", map[string]string{"CERTIFICATE_FILE": "./testdata/example-cert-bad.pem", "CERTIFICATE_KEY_FILE": "./testdata/example-key-bad.pem", "INSECURE_SERVER": "true", "SHARED_SECRET": "YixWi1MYh77NMECGGIJQevoonYtVF+ZPRkQZrrmeRqM="}, true},
{"good client ca file", map[string]string{"CLIENT_CA_FILE": "./testdata/ca.pem", "INSECURE_SERVER": "true", "SHARED_SECRET": "YixWi1MYh77NMECGGIJQevoonYtVF+ZPRkQZrrmeRqM="}, false},
{"bad client ca file", map[string]string{"CLIENT_CA_FILE": "./testdata/bad-ca.pem", "INSECURE_SERVER": "true", "SHARED_SECRET": "YixWi1MYh77NMECGGIJQevoonYtVF+ZPRkQZrrmeRqM="}, true},
{"bad client ca b64", map[string]string{"CLIENT_CA": "bad cert", "INSECURE_SERVER": "true", "SHARED_SECRET": "YixWi1MYh77NMECGGIJQevoonYtVF+ZPRkQZrrmeRqM="}, true},
{"good client ca file", map[string]string{"DOWNSTREAM_MTLS_CA_FILE": "./testdata/ca.pem", "INSECURE_SERVER": "true", "SHARED_SECRET": "YixWi1MYh77NMECGGIJQevoonYtVF+ZPRkQZrrmeRqM="}, false},
{"bad client ca file", map[string]string{"DOWNSTREAM_MTLS_CA_FILE": "./testdata/bad-ca.pem", "INSECURE_SERVER": "true", "SHARED_SECRET": "YixWi1MYh77NMECGGIJQevoonYtVF+ZPRkQZrrmeRqM="}, true},
{"bad client ca b64", map[string]string{"DOWNSTREAM_MTLS_CA": "bad cert", "INSECURE_SERVER": "true", "SHARED_SECRET": "YixWi1MYh77NMECGGIJQevoonYtVF+ZPRkQZrrmeRqM="}, true},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
Expand Down Expand Up @@ -709,42 +708,6 @@ func TestCompareByteSliceSlice(t *testing.T) {
}
}

func TestDeprecatedClientCAOptions(t *testing.T) {
fakeCACert := []byte("--- FAKE CA CERT ---")
caFile := filepath.Join(t.TempDir(), "CA.pem")
os.WriteFile(caFile, fakeCACert, 0o644)

t.Run("CA", func(t *testing.T) {
o := NewDefaultOptions()
o.AutocertOptions.Enable = true // suppress an unrelated warning
o.ClientCA = "LS0tIEZBS0UgQ0EgQ0VSVCAtLS0="

var err error
logOutput := testutil.CaptureLogs(t, func() {
err = o.Validate()
})

require.NoError(t, err)
assert.Equal(t, "LS0tIEZBS0UgQ0EgQ0VSVCAtLS0=", o.DownstreamMTLS.CA)
assert.Contains(t, logOutput, `{"level":"warn","message":"config: client_ca is deprecated, set downstream_mtls.ca instead"}`)
})

t.Run("CAFile", func(t *testing.T) {
o := NewDefaultOptions()
o.AutocertOptions.Enable = true // suppress an unrelated warning
o.ClientCAFile = caFile

var err error
logOutput := testutil.CaptureLogs(t, func() {
err = o.Validate()
})

require.NoError(t, err)
assert.Equal(t, caFile, o.DownstreamMTLS.CAFile)
assert.Contains(t, logOutput, `{"level":"warn","message":"config: client_ca_file is deprecated, set downstream_mtls.ca_file instead"}`)
})
}

func TestHasAnyDownstreamMTLSClientCA(t *testing.T) {
t.Parallel()

Expand Down

0 comments on commit e83b14b

Please sign in to comment.