authorize: store policy evaluator on success only (#1206)

Currently, when option changes, whether the option is good or bad, we
always store new policy evaluator.

When options is bad, policy evaluator will be nil. That can lead to panic
at runtime if a Check request were called after Authorize.OnConfigChange
ran with bad option.

We already have an error message if new policy evaluator fails, so we
must only update it on success only.

authorize/authorize_test.go

@@ -3,6 +3,9 @@ package authorize
 import (
 	"testing"
 
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
+
 	"github.com/pomerium/pomerium/config"
 )
 
@@ -23,7 +26,9 @@ func TestNew(t *testing.T) {
 
 	}
 	for _, tt := range tests {
+		tt := tt
 		t.Run(tt.name, func(t *testing.T) {
+			t.Parallel()
 			o := &config.Options{
 				AuthenticateURL: mustParseURL("https://authN.example.com"),
 				DataBrokerURL:   mustParseURL("https://cache.example.com"),
@@ -41,6 +46,46 @@ func TestNew(t *testing.T) {
 	}
 }
 
+func TestAuthorize_OnConfigChange(t *testing.T) {
+	t.Parallel()
+	policies := testPolicies(t)
+	tests := []struct {
+		name           string
+		SharedKey      string
+		Policies       []config.Policy
+		expectedChange bool
+	}{
+		{"good", "gXK6ggrlIW2HyKyUF9rUO4azrDgxhDPWqw9y+lJU7B8=", policies, true},
+		{"bad option", "gXK6ggrlIW2HyKyUF9rUO4azrDgxhDPWqw9y+lJU7B8=", policies, false},
+	}
+	for _, tc := range tests {
+		tc := tc
+		t.Run(tc.name, func(t *testing.T) {
+			t.Parallel()
+			o := &config.Options{
+				AuthenticateURL: mustParseURL("https://authN.example.com"),
+				DataBrokerURL:   mustParseURL("https://cache.example.com"),
+				SharedKey:       tc.SharedKey,
+				Policies:        tc.Policies,
+			}
+			a, err := New(o)
+			require.NoError(t, err)
+			require.NotNil(t, a)
+
+			oldPe := a.pe
+			cfg := &config.Config{Options: o}
+			assertFunc := assert.True
+			o.SigningKey = "bad-share-key"
+			if tc.expectedChange {
+				o.SigningKey = "LS0tLS1CRUdJTiBFQyBQUklWQVRFIEtFWS0tLS0tCk1IY0NBUUVFSUhHNHZDWlJxUFgwNGtmSFQxeVVDM1pUQkF6MFRYWkNtZ043clpDcFE3cHJvQW9HQ0NxR1NNNDkKQXdFSG9VUURRZ0FFbzQzdjAwQlR4c3pKZWpmdHhBOWNtVGVUSmtQQXVtOGt1b0UwVlRUZnlId2k3SHJlN2FRUgpHQVJ6Nm0wMjVRdGFiRGxqeDd5MjIyY1gxblhCQXo3MlF3PT0KLS0tLS1FTkQgRUMgUFJJVkFURSBLRVktLS0tLQo="
+				assertFunc = assert.False
+			}
+			a.OnConfigChange(cfg)
+			assertFunc(t, oldPe == a.pe)
+		})
+	}
+}
+
 func testPolicies(t *testing.T) []config.Policy {
 	testPolicy := config.Policy{From: "https://pomerium.io", To: "http://httpbin.org", AllowedUsers: []string{"test@gmail.com"}}
 	err := testPolicy.Validate()