Skip to content

Commit

Permalink
fix(config): syncing for postgres hook without secrets (#2931)
Browse files Browse the repository at this point in the history 
* fix(config): syncing for postgres hook without secrets

* chore: apply pr comments

* chore: stricter hook secrets validation

* chore: update unit tests and error handling

---------

Co-authored-by: Qiao Han <[email protected]>
  • Loading branch information
@avallete @sweatybridge
avallete and sweatybridge authored Dec 2, 2024
1 parent 7ff60a7 commit 2ff5960
Show file tree
Hide file tree
Showing 5 changed files with 197 additions and 90 deletions.
55 changes: 39 additions & 16 deletions pkg/config/auth.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -262,53 +262,76 @@ func (a *auth) FromRemoteAuthConfig(remoteConfig v1API.AuthConfigResponse) {
func (h hook) toAuthConfigBody(body *v1API.UpdateAuthConfigBody) {
if body.HookCustomAccessTokenEnabled = &h.CustomAccessToken.Enabled; *body.HookCustomAccessTokenEnabled {
body.HookCustomAccessTokenUri = &h.CustomAccessToken.URI
body.HookCustomAccessTokenSecrets = &h.CustomAccessToken.Secrets
if len(h.CustomAccessToken.Secrets) > 0 {
body.HookCustomAccessTokenSecrets = &h.CustomAccessToken.Secrets
}
}
if body.HookSendEmailEnabled = &h.SendEmail.Enabled; *body.HookSendEmailEnabled {
body.HookSendEmailUri = &h.SendEmail.URI
body.HookSendEmailSecrets = &h.SendEmail.Secrets
if len(h.SendEmail.Secrets) > 0 {
body.HookSendEmailSecrets = &h.SendEmail.Secrets
}
}
if body.HookSendSmsEnabled = &h.SendSMS.Enabled; *body.HookSendSmsEnabled {
body.HookSendSmsUri = &h.SendSMS.URI
body.HookSendSmsSecrets = &h.SendSMS.Secrets
if len(h.SendSMS.Secrets) > 0 {
body.HookSendSmsSecrets = &h.SendSMS.Secrets
}
}
// Enterprise and team only features
if body.HookMfaVerificationAttemptEnabled = &h.MFAVerificationAttempt.Enabled; *body.HookMfaVerificationAttemptEnabled {
body.HookMfaVerificationAttemptUri = &h.MFAVerificationAttempt.URI
body.HookMfaVerificationAttemptSecrets = &h.MFAVerificationAttempt.Secrets
if len(h.MFAVerificationAttempt.Secrets) > 0 {
body.HookMfaVerificationAttemptSecrets = &h.MFAVerificationAttempt.Secrets
}
}
if body.HookPasswordVerificationAttemptEnabled = &h.PasswordVerificationAttempt.Enabled; *body.HookPasswordVerificationAttemptEnabled {
body.HookPasswordVerificationAttemptUri = &h.PasswordVerificationAttempt.URI
body.HookPasswordVerificationAttemptSecrets = &h.PasswordVerificationAttempt.Secrets
if len(h.PasswordVerificationAttempt.Secrets) > 0 {
body.HookPasswordVerificationAttemptSecrets = &h.PasswordVerificationAttempt.Secrets
}
}
}

func (h *hook) fromAuthConfig(remoteConfig v1API.AuthConfigResponse) {
// Ignore disabled hooks because their envs are not loaded
if h.CustomAccessToken.Enabled {
h.CustomAccessToken.URI = cast.Val(remoteConfig.HookCustomAccessTokenUri, "")
h.CustomAccessToken.Secrets = hashPrefix + cast.Val(remoteConfig.HookCustomAccessTokenSecrets, "")
if remoteConfig.HookCustomAccessTokenSecrets != nil {
h.CustomAccessToken.Secrets = hashPrefix + cast.Val(remoteConfig.HookCustomAccessTokenSecrets, "")
}
}
h.CustomAccessToken.Enabled = cast.Val(remoteConfig.HookCustomAccessTokenEnabled, false)

if h.SendEmail.Enabled {
h.SendEmail.URI = cast.Val(remoteConfig.HookSendEmailUri, "")
h.SendEmail.Secrets = hashPrefix + cast.Val(remoteConfig.HookSendEmailSecrets, "")
if remoteConfig.HookSendEmailSecrets != nil {
h.SendEmail.Secrets = hashPrefix + cast.Val(remoteConfig.HookSendEmailSecrets, "")
}
}
h.SendEmail.Enabled = cast.Val(remoteConfig.HookSendEmailEnabled, false)

if h.SendSMS.Enabled {
h.SendSMS.URI = cast.Val(remoteConfig.HookSendSmsUri, "")
h.SendSMS.Secrets = hashPrefix + cast.Val(remoteConfig.HookSendSmsSecrets, "")
if remoteConfig.HookSendSmsSecrets != nil {
h.SendSMS.Secrets = hashPrefix + cast.Val(remoteConfig.HookSendSmsSecrets, "")
}
}
h.SendSMS.Enabled = cast.Val(remoteConfig.HookSendSmsEnabled, false)

// Enterprise and team only features
if h.MFAVerificationAttempt.Enabled {
h.MFAVerificationAttempt.URI = cast.Val(remoteConfig.HookMfaVerificationAttemptUri, "")
h.MFAVerificationAttempt.Secrets = hashPrefix + cast.Val(remoteConfig.HookMfaVerificationAttemptSecrets, "")
if remoteConfig.HookMfaVerificationAttemptSecrets != nil {
h.MFAVerificationAttempt.Secrets = hashPrefix + cast.Val(remoteConfig.HookMfaVerificationAttemptSecrets, "")
}
}
h.MFAVerificationAttempt.Enabled = cast.Val(remoteConfig.HookMfaVerificationAttemptEnabled, false)

if h.PasswordVerificationAttempt.Enabled {
h.PasswordVerificationAttempt.URI = cast.Val(remoteConfig.HookPasswordVerificationAttemptUri, "")
h.PasswordVerificationAttempt.Secrets = hashPrefix + cast.Val(remoteConfig.HookPasswordVerificationAttemptSecrets, "")
if remoteConfig.HookPasswordVerificationAttemptSecrets != nil {
h.PasswordVerificationAttempt.Secrets = hashPrefix + cast.Val(remoteConfig.HookPasswordVerificationAttemptSecrets, "")
}
}
h.PasswordVerificationAttempt.Enabled = cast.Val(remoteConfig.HookPasswordVerificationAttemptEnabled, false)
}
Expand Down Expand Up @@ -851,19 +874,19 @@ func (a *auth) HashSecrets(key string) {
case a.Sms.Vonage.Enabled:
a.Sms.Vonage.ApiSecret = hash(a.Sms.Vonage.ApiSecret)
}
if a.Hook.MFAVerificationAttempt.Enabled {
if a.Hook.MFAVerificationAttempt.Enabled && len(a.Hook.MFAVerificationAttempt.Secrets) > 0 {
a.Hook.MFAVerificationAttempt.Secrets = hash(a.Hook.MFAVerificationAttempt.Secrets)
}
if a.Hook.PasswordVerificationAttempt.Enabled {
if a.Hook.PasswordVerificationAttempt.Enabled && len(a.Hook.PasswordVerificationAttempt.Secrets) > 0 {
a.Hook.PasswordVerificationAttempt.Secrets = hash(a.Hook.PasswordVerificationAttempt.Secrets)
}
if a.Hook.CustomAccessToken.Enabled {
if a.Hook.CustomAccessToken.Enabled && len(a.Hook.CustomAccessToken.Secrets) > 0 {
a.Hook.CustomAccessToken.Secrets = hash(a.Hook.CustomAccessToken.Secrets)
}
if a.Hook.SendSMS.Enabled {
if a.Hook.SendSMS.Enabled && len(a.Hook.SendSMS.Secrets) > 0 {
a.Hook.SendSMS.Secrets = hash(a.Hook.SendSMS.Secrets)
}
if a.Hook.SendEmail.Enabled {
if a.Hook.SendEmail.Enabled && len(a.Hook.SendEmail.Secrets) > 0 {
a.Hook.SendEmail.Secrets = hash(a.Hook.SendEmail.Secrets)
}
for name, provider := range a.External {
Expand Down
88 changes: 65 additions & 23 deletions pkg/config/auth_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -121,29 +121,47 @@ func TestHookDiff(t *testing.T) {
t.Run("local and remote enabled", func(t *testing.T) {
c := newWithDefaults()
c.Hook = hook{
CustomAccessToken: hookConfig{Enabled: true},
SendSMS: hookConfig{Enabled: true},
SendEmail: hookConfig{Enabled: true},
MFAVerificationAttempt: hookConfig{Enabled: true},
PasswordVerificationAttempt: hookConfig{Enabled: true},
CustomAccessToken: hookConfig{
Enabled: true,
URI: "http://example.com",
Secrets: "test-secret",
},
SendSMS: hookConfig{
Enabled: true,
URI: "http://example.com",
Secrets: "test-secret",
},
SendEmail: hookConfig{
Enabled: true,
URI: "https://example.com",
Secrets: "test-secret",
},
MFAVerificationAttempt: hookConfig{
Enabled: true,
URI: "https://example.com",
Secrets: "test-secret",
},
PasswordVerificationAttempt: hookConfig{
Enabled: true,
URI: "pg-functions://verifyPassword",
},
}
// Run test
diff, err := c.DiffWithRemote("", v1API.AuthConfigResponse{
HookCustomAccessTokenEnabled: cast.Ptr(true),
HookCustomAccessTokenUri: cast.Ptr(""),
HookCustomAccessTokenSecrets: cast.Ptr("b613679a0814d9ec772f95d778c35fc5ff1697c493715653c6c712144292c5ad"),
HookCustomAccessTokenUri: cast.Ptr("http://example.com"),
HookCustomAccessTokenSecrets: cast.Ptr("ce62bb9bcced294fd4afe668f8ab3b50a89cf433093c526fffa3d0e46bf55252"),
HookSendEmailEnabled: cast.Ptr(true),
HookSendEmailUri: cast.Ptr(""),
HookSendEmailSecrets: cast.Ptr("b613679a0814d9ec772f95d778c35fc5ff1697c493715653c6c712144292c5ad"),
HookSendEmailUri: cast.Ptr("https://example.com"),
HookSendEmailSecrets: cast.Ptr("ce62bb9bcced294fd4afe668f8ab3b50a89cf433093c526fffa3d0e46bf55252"),
HookSendSmsEnabled: cast.Ptr(true),
HookSendSmsUri: cast.Ptr(""),
HookSendSmsSecrets: cast.Ptr("b613679a0814d9ec772f95d778c35fc5ff1697c493715653c6c712144292c5ad"),
HookSendSmsUri: cast.Ptr("http://example.com"),
HookSendSmsSecrets: cast.Ptr("ce62bb9bcced294fd4afe668f8ab3b50a89cf433093c526fffa3d0e46bf55252"),
HookMfaVerificationAttemptEnabled: cast.Ptr(true),
HookMfaVerificationAttemptUri: cast.Ptr(""),
HookMfaVerificationAttemptSecrets: cast.Ptr("b613679a0814d9ec772f95d778c35fc5ff1697c493715653c6c712144292c5ad"),
HookMfaVerificationAttemptUri: cast.Ptr("https://example.com"),
HookMfaVerificationAttemptSecrets: cast.Ptr("ce62bb9bcced294fd4afe668f8ab3b50a89cf433093c526fffa3d0e46bf55252"),
HookPasswordVerificationAttemptEnabled: cast.Ptr(true),
HookPasswordVerificationAttemptUri: cast.Ptr(""),
HookPasswordVerificationAttemptSecrets: cast.Ptr("b613679a0814d9ec772f95d778c35fc5ff1697c493715653c6c712144292c5ad"),
HookPasswordVerificationAttemptUri: cast.Ptr("pg-functions://verifyPassword"),
})
// Check error
assert.NoError(t, err)
Expand All @@ -153,17 +171,41 @@ func TestHookDiff(t *testing.T) {
t.Run("local enabled and disabled", func(t *testing.T) {
c := newWithDefaults()
c.Hook = hook{
CustomAccessToken: hookConfig{Enabled: true},
MFAVerificationAttempt: hookConfig{Enabled: false},
CustomAccessToken: hookConfig{
Enabled: true,
URI: "http://example.com",
Secrets: "test-secret",
},
SendSMS: hookConfig{
Enabled: false,
URI: "https://example.com",
Secrets: "test-secret",
},
SendEmail: hookConfig{
Enabled: true,
URI: "pg-functions://sendEmail",
},
MFAVerificationAttempt: hookConfig{
Enabled: false,
URI: "pg-functions://verifyMFA",
},
PasswordVerificationAttempt: hookConfig{Enabled: false},
}
// Run test
diff, err := c.DiffWithRemote("", v1API.AuthConfigResponse{
HookCustomAccessTokenEnabled: cast.Ptr(false),
HookCustomAccessTokenUri: cast.Ptr(""),
HookCustomAccessTokenSecrets: cast.Ptr("b613679a0814d9ec772f95d778c35fc5ff1697c493715653c6c712144292c5ad"),
HookMfaVerificationAttemptEnabled: cast.Ptr(true),
HookMfaVerificationAttemptUri: cast.Ptr(""),
HookMfaVerificationAttemptSecrets: cast.Ptr("b613679a0814d9ec772f95d778c35fc5ff1697c493715653c6c712144292c5ad"),
HookCustomAccessTokenEnabled: cast.Ptr(false),
HookCustomAccessTokenUri: cast.Ptr(""),
HookCustomAccessTokenSecrets: cast.Ptr("b613679a0814d9ec772f95d778c35fc5ff1697c493715653c6c712144292c5ad"),
HookSendEmailEnabled: cast.Ptr(false),
HookSendEmailUri: cast.Ptr(""),
HookSendSmsEnabled: cast.Ptr(true),
HookSendSmsUri: cast.Ptr("http://example.com"),
HookSendSmsSecrets: cast.Ptr("ce62bb9bcced294fd4afe668f8ab3b50a89cf433093c526fffa3d0e46bf55252"),
HookMfaVerificationAttemptEnabled: cast.Ptr(true),
HookMfaVerificationAttemptUri: cast.Ptr("pg-functions://verifyMFA"),
HookPasswordVerificationAttemptEnabled: cast.Ptr(true),
HookPasswordVerificationAttemptUri: cast.Ptr("https://example.com"),
HookPasswordVerificationAttemptSecrets: cast.Ptr("ce62bb9bcced294fd4afe668f8ab3b50a89cf433093c526fffa3d0e46bf55252"),
})
// Check error
assert.NoError(t, err)
Expand Down
32 changes: 20 additions & 12 deletions pkg/config/config.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -993,14 +993,25 @@ func (h *hookConfig) validate(hookType string) (err error) {
return nil
}
if h.URI == "" {
return errors.Errorf("missing required field in config: auth.hook.%s.uri", hookType)
} else if parsed, err := url.Parse(h.URI); err != nil {
return errors.Errorf("Missing required field in config: auth.hook.%s.uri", hookType)
}
parsed, err := url.Parse(h.URI)
if err != nil {
return errors.Errorf("failed to parse template url: %w", err)
} else if !(parsed.Scheme == "http" || parsed.Scheme == "https" || parsed.Scheme == "pg-functions") {
return errors.Errorf("Invalid HTTP hook config: auth.hook.%v should be a Postgres function URI, or a HTTP or HTTPS URL", hookType)
}
if h.Secrets, err = maybeLoadEnv(h.Secrets); err != nil {
return errors.Errorf("missing required field in config: auth.hook.%s.secrets", hookType)
switch strings.ToLower(parsed.Scheme) {
case "http", "https":
if len(h.Secrets) == 0 {
return errors.Errorf("Missing required field in config: auth.hook.%s.secrets", hookType)
} else if h.Secrets, err = maybeLoadEnv(h.Secrets); err != nil {
return err
}
case "pg-functions":
if len(h.Secrets) > 0 {
return errors.Errorf("Invalid hook config: auth.hook.%s.secrets is unsupported for pg-functions URI", hookType)
}
default:
return errors.Errorf("Invalid hook config: auth.hook.%v should be a HTTP, HTTPS, or pg-functions URI", hookType)
}
return nil
}
Expand Down Expand Up @@ -1070,19 +1081,16 @@ func (c *tpaCognito) issuerURL() string {
return fmt.Sprintf("https://cognito-idp.%s.amazonaws.com/%s", c.UserPoolRegion, c.UserPoolID)
}

func (c *tpaCognito) validate() error {
func (c *tpaCognito) validate() (err error) {
if c.UserPoolID == "" {
return errors.New("Invalid config: auth.third_party.cognito is enabled but without a user_pool_id.")
}
var err error
if c.UserPoolID, err = maybeLoadEnv(c.UserPoolID); err != nil {
} else if c.UserPoolID, err = maybeLoadEnv(c.UserPoolID); err != nil {
return err
}

if c.UserPoolRegion == "" {
return errors.New("Invalid config: auth.third_party.cognito is enabled but without a user_pool_region.")
}
if c.UserPoolRegion, err = maybeLoadEnv(c.UserPoolRegion); err != nil {
} else if c.UserPoolRegion, err = maybeLoadEnv(c.UserPoolRegion); err != nil {
return err
}

Expand Down
87 changes: 53 additions & 34 deletions pkg/config/config_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -205,55 +205,74 @@ func TestSigningJWT(t *testing.T) {

func TestValidateHookURI(t *testing.T) {
tests := []struct {
name string
uri string
hookName string
shouldErr bool
errorMsg string
hookConfig
name string
errorMsg string
}{
{
name: "valid http URL",
uri: "http://example.com",
hookName: "testHook",
shouldErr: false,
name: "valid http URL",
hookConfig: hookConfig{
Enabled: true,
URI: "http://example.com",
Secrets: "test-secret",
},
},
{
name: "valid https URL",
uri: "https://example.com",
hookName: "testHook",
shouldErr: false,
name: "valid https URL",
hookConfig: hookConfig{
Enabled: true,
URI: "https://example.com",
Secrets: "test-secret",
},
},
{
name: "valid pg-functions URI",
hookConfig: hookConfig{
Enabled: true,
URI: "pg-functions://functionName",
},
},
{
name: "invalid URI with unsupported scheme",
hookConfig: hookConfig{
Enabled: true,
URI: "ftp://example.com",
Secrets: "test-secret",
},
errorMsg: "Invalid hook config: auth.hook.invalid URI with unsupported scheme should be a HTTP, HTTPS, or pg-functions URI",
},
{
name: "valid pg-functions URI",
uri: "pg-functions://functionName",
hookName: "pgHook",
shouldErr: false,
name: "invalid URI with parsing error",
hookConfig: hookConfig{
Enabled: true,
URI: "http://a b.com",
Secrets: "test-secret",
},
errorMsg: "failed to parse template url: parse \"http://a b.com\": invalid character \" \" in host name",
},
{
name: "invalid URI with unsupported scheme",
uri: "ftp://example.com",
hookName: "malformedHook",
shouldErr: true,
errorMsg: "Invalid HTTP hook config: auth.hook.malformedHook should be a Postgres function URI, or a HTTP or HTTPS URL",
name: "valid http URL with missing secrets",
hookConfig: hookConfig{
Enabled: true,
URI: "http://example.com",
},
errorMsg: "Missing required field in config: auth.hook.valid http URL with missing secrets.secrets",
},
{
name: "invalid URI with parsing error",
uri: "http://a b.com",
hookName: "errorHook",
shouldErr: true,
errorMsg: "failed to parse template url: parse \"http://a b.com\": invalid character \" \" in host name",
name: "valid pg-functions URI with unsupported secrets",
hookConfig: hookConfig{
Enabled: true,
URI: "pg-functions://functionName",
Secrets: "test-secret",
},
errorMsg: "Invalid hook config: auth.hook.valid pg-functions URI with unsupported secrets.secrets is unsupported for pg-functions URI",
},
}

for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
h := hookConfig{
Enabled: true,
URI: tt.uri,
Secrets: "test-secret",
}
err := h.validate(tt.hookName)
if tt.shouldErr {
err := tt.hookConfig.validate(tt.name)
if len(tt.errorMsg) > 0 {
assert.Error(t, err, "Expected an error for %v", tt.name)
assert.EqualError(t, err, tt.errorMsg, "Expected error message does not match for %v", tt.name)
} else {
Expand Down
Loading

0 comments on commit 2ff5960

Please sign in to comment.