refactor(configuration): remove ptr for duoapi and notifier (#3200)

This adds to the ongoing effort to remove all pointers to structs in the configuration without breaking backwards compatibility.
pull/3202/head^2
James Elliott 2022-04-16 09:34:26 +10:00 committed by GitHub
parent c9568caf4d
commit 4710de33a4
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
19 changed files with 279 additions and 108 deletions

View File

@ -158,6 +158,7 @@ webauthn:
## Parameters used to contact the Duo API. Those are generated when you protect an application of type
## "Partner Auth API" in the management panel.
duo_api:
disable: false
hostname: api-123456789.example.com
integration_key: ABCDEF
## Secret can also be set using a secret: https://www.authelia.com/docs/configuration/secrets.html

View File

@ -21,6 +21,7 @@ section of the configuration.
The configuration is as follows:
```yaml
duo_api:
disable: false
hostname: api-123456789.example.com
integration_key: ABCDEF
secret_key: 1234567890abcdefghifjkl
@ -32,6 +33,19 @@ variable as described [here](./secrets.md).
## Options
### disable:
<div markdown="1">
type: boolean
{: .label .label-config .label-purple }
default: false
{: .label .label-config .label-blue }
required: no
{: .label .label-config .label-green }
</div>
Disables Duo. If the hostname, integration_key, and secret_key are all empty strings or undefined this is automatically
true.
### hostname
<div markdown="1">
type: string

View File

@ -158,6 +158,7 @@ webauthn:
## Parameters used to contact the Duo API. Those are generated when you protect an application of type
## "Partner Auth API" in the management panel.
duo_api:
disable: false
hostname: api-123456789.example.com
integration_key: ABCDEF
## Secret can also be set using a secret: https://www.authelia.com/docs/configuration/secrets.html

View File

@ -273,7 +273,7 @@ func TestShouldHandleErrInvalidatorWhenSMTPSenderBlank(t *testing.T) {
assert.Equal(t, "", config.Notifier.SMTP.Sender.Name)
assert.Equal(t, "", config.Notifier.SMTP.Sender.Address)
validator.ValidateNotifier(config.Notifier, val)
validator.ValidateNotifier(&config.Notifier, val)
require.Len(t, val.Errors(), 1)
assert.Len(t, val.Warnings(), 0)

View File

@ -12,12 +12,12 @@ type Configuration struct {
AuthenticationBackend AuthenticationBackendConfiguration `koanf:"authentication_backend"`
Session SessionConfiguration `koanf:"session"`
TOTP TOTPConfiguration `koanf:"totp"`
DuoAPI *DuoAPIConfiguration `koanf:"duo_api"`
DuoAPI DuoAPIConfiguration `koanf:"duo_api"`
AccessControl AccessControlConfiguration `koanf:"access_control"`
NTP NTPConfiguration `koanf:"ntp"`
Regulation RegulationConfiguration `koanf:"regulation"`
Storage StorageConfiguration `koanf:"storage"`
Notifier *NotifierConfiguration `koanf:"notifier"`
Notifier NotifierConfiguration `koanf:"notifier"`
Server ServerConfiguration `koanf:"server"`
Webauthn WebauthnConfiguration `koanf:"webauthn"`
PasswordPolicy PasswordPolicyConfiguration `koanf:"password_policy"`

View File

@ -2,8 +2,9 @@ package schema
// DuoAPIConfiguration represents the configuration related to Duo API.
type DuoAPIConfiguration struct {
Disable bool `koanf:"disable"`
Hostname string `koanf:"hostname"`
EnableSelfEnrollment bool `koanf:"enable_self_enrollment"`
IntegrationKey string `koanf:"integration_key"`
SecretKey string `koanf:"secret_key"`
EnableSelfEnrollment bool `koanf:"enable_self_enrollment"`
}

View File

@ -37,6 +37,8 @@ func ValidateConfiguration(config *schema.Configuration, validator *schema.Struc
ValidateLog(config, validator)
ValidateDuo(config, validator)
ValidateTOTP(config, validator)
ValidateWebauthn(config, validator)
@ -55,7 +57,7 @@ func ValidateConfiguration(config *schema.Configuration, validator *schema.Struc
ValidateStorage(config.Storage, validator)
ValidateNotifier(config.Notifier, validator)
ValidateNotifier(&config.Notifier, validator)
ValidateIdentityProviders(&config.IdentityProviders, validator)

View File

@ -32,7 +32,7 @@ func newDefaultConfig() schema.Configuration {
config.Storage.Local = &schema.LocalStorageConfiguration{
Path: "abc",
}
config.Notifier = &schema.NotifierConfiguration{
config.Notifier = schema.NotifierConfiguration{
FileSystem: &schema.FileSystemNotifierConfiguration{
Filename: "/tmp/file",
},
@ -48,7 +48,8 @@ func TestShouldEnsureNotifierConfigIsProvided(t *testing.T) {
ValidateConfiguration(&config, validator)
require.Len(t, validator.Errors(), 0)
config.Notifier = nil
config.Notifier.SMTP = nil
config.Notifier.FileSystem = nil
ValidateConfiguration(&config, validator)
require.Len(t, validator.Errors(), 1)

View File

@ -249,6 +249,10 @@ const (
errFmtPasswordPolicyZXCVBNMinScoreInvalid = "password_policy: zxcvbn: option 'min_score' is invalid: must be between 1 and 4 but it's configured as %d"
)
const (
errFmtDuoMissingOption = "duo_api: option '%s' is required when duo is enabled but it is missing"
)
// Error constants.
const (
/*

View File

@ -0,0 +1,34 @@
package validator
import (
"fmt"
"github.com/authelia/authelia/v4/internal/configuration/schema"
)
// ValidateDuo validates and updates the Duo configuration.
func ValidateDuo(config *schema.Configuration, validator *schema.StructValidator) {
if config.DuoAPI.Disable {
return
}
if config.DuoAPI.Hostname == "" && config.DuoAPI.IntegrationKey == "" && config.DuoAPI.SecretKey == "" {
config.DuoAPI.Disable = true
}
if config.DuoAPI.Disable {
return
}
if config.DuoAPI.Hostname == "" {
validator.Push(fmt.Errorf(errFmtDuoMissingOption, "hostname"))
}
if config.DuoAPI.IntegrationKey == "" {
validator.Push(fmt.Errorf(errFmtDuoMissingOption, "integration_key"))
}
if config.DuoAPI.SecretKey == "" {
validator.Push(fmt.Errorf(errFmtDuoMissingOption, "secret_key"))
}
}

View File

@ -0,0 +1,105 @@
package validator
import (
"fmt"
"testing"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
"github.com/authelia/authelia/v4/internal/configuration/schema"
)
func TestValidateDuo(t *testing.T) {
testCases := []struct {
desc string
have *schema.Configuration
expected schema.DuoAPIConfiguration
errs []string
}{
{
desc: "ShouldDisableDuo",
have: &schema.Configuration{},
expected: schema.DuoAPIConfiguration{Disable: true},
},
{
desc: "ShouldNotDisableDuo",
have: &schema.Configuration{DuoAPI: schema.DuoAPIConfiguration{
Hostname: "test",
IntegrationKey: "test",
SecretKey: "test",
}},
expected: schema.DuoAPIConfiguration{
Hostname: "test",
IntegrationKey: "test",
SecretKey: "test",
},
},
{
desc: "ShouldDetectMissingSecretKey",
have: &schema.Configuration{DuoAPI: schema.DuoAPIConfiguration{
Hostname: "test",
IntegrationKey: "test",
}},
expected: schema.DuoAPIConfiguration{
Hostname: "test",
IntegrationKey: "test",
},
errs: []string{
"duo_api: option 'secret_key' is required when duo is enabled but it is missing",
},
},
{
desc: "ShouldDetectMissingIntegrationKey",
have: &schema.Configuration{DuoAPI: schema.DuoAPIConfiguration{
Hostname: "test",
SecretKey: "test",
}},
expected: schema.DuoAPIConfiguration{
Hostname: "test",
SecretKey: "test",
},
errs: []string{
"duo_api: option 'integration_key' is required when duo is enabled but it is missing",
},
},
{
desc: "ShouldDetectMissingHostname",
have: &schema.Configuration{DuoAPI: schema.DuoAPIConfiguration{
IntegrationKey: "test",
SecretKey: "test",
}},
expected: schema.DuoAPIConfiguration{
IntegrationKey: "test",
SecretKey: "test",
},
errs: []string{
"duo_api: option 'hostname' is required when duo is enabled but it is missing",
},
},
}
for _, tc := range testCases {
t.Run(tc.desc, func(t *testing.T) {
val := schema.NewStructValidator()
ValidateDuo(tc.have, val)
assert.Equal(t, tc.expected.Disable, tc.have.DuoAPI.Disable)
assert.Equal(t, tc.expected.Hostname, tc.have.DuoAPI.Hostname)
assert.Equal(t, tc.expected.IntegrationKey, tc.have.DuoAPI.IntegrationKey)
assert.Equal(t, tc.expected.SecretKey, tc.have.DuoAPI.SecretKey)
assert.Equal(t, tc.expected.EnableSelfEnrollment, tc.have.DuoAPI.EnableSelfEnrollment)
require.Len(t, val.Errors(), len(tc.errs))
if len(tc.errs) != 0 {
for i, err := range tc.errs {
t.Run(fmt.Sprintf("Err%d", i+1), func(t *testing.T) {
assert.EqualError(t, val.Errors()[i], err)
})
}
}
})
}
}

View File

@ -12,7 +12,7 @@ import (
// ValidateNotifier validates and update notifier configuration.
func ValidateNotifier(config *schema.NotifierConfiguration, validator *schema.StructValidator) {
if config == nil || (config.SMTP == nil && config.FileSystem == nil) {
if config.SMTP == nil && config.FileSystem == nil {
validator.Push(fmt.Errorf(errFmtNotifierNotConfigured))
return

View File

@ -30,7 +30,9 @@ func (s *SecondFactorAvailableMethodsFixture) TearDownTest() {
func (s *SecondFactorAvailableMethodsFixture) TestShouldHaveAllConfiguredMethods() {
s.mock.Ctx.Configuration = schema.Configuration{
DuoAPI: &schema.DuoAPIConfiguration{},
DuoAPI: schema.DuoAPIConfiguration{
Disable: false,
},
TOTP: schema.TOTPConfiguration{
Disable: false,
},
@ -58,7 +60,9 @@ func (s *SecondFactorAvailableMethodsFixture) TestShouldHaveAllConfiguredMethods
func (s *SecondFactorAvailableMethodsFixture) TestShouldRemoveTOTPFromAvailableMethodsWhenDisabled() {
s.mock.Ctx.Configuration = schema.Configuration{
DuoAPI: &schema.DuoAPIConfiguration{},
DuoAPI: schema.DuoAPIConfiguration{
Disable: false,
},
TOTP: schema.TOTPConfiguration{
Disable: true,
},
@ -86,7 +90,9 @@ func (s *SecondFactorAvailableMethodsFixture) TestShouldRemoveTOTPFromAvailableM
func (s *SecondFactorAvailableMethodsFixture) TestShouldRemoveWebauthnFromAvailableMethodsWhenDisabled() {
s.mock.Ctx.Configuration = schema.Configuration{
DuoAPI: &schema.DuoAPIConfiguration{},
DuoAPI: schema.DuoAPIConfiguration{
Disable: false,
},
TOTP: schema.TOTPConfiguration{
Disable: false,
},
@ -114,7 +120,9 @@ func (s *SecondFactorAvailableMethodsFixture) TestShouldRemoveWebauthnFromAvaila
func (s *SecondFactorAvailableMethodsFixture) TestShouldRemoveDuoFromAvailableMethodsWhenNotConfigured() {
s.mock.Ctx.Configuration = schema.Configuration{
DuoAPI: nil,
DuoAPI: schema.DuoAPIConfiguration{
Disable: true,
},
TOTP: schema.TOTPConfiguration{
Disable: false,
},
@ -142,7 +150,9 @@ func (s *SecondFactorAvailableMethodsFixture) TestShouldRemoveDuoFromAvailableMe
func (s *SecondFactorAvailableMethodsFixture) TestShouldRemoveAllMethodsWhenNoTwoFactorACLRulesConfigured() {
s.mock.Ctx.Configuration = schema.Configuration{
DuoAPI: &schema.DuoAPIConfiguration{},
DuoAPI: schema.DuoAPIConfiguration{
Disable: false,
},
TOTP: schema.TOTPConfiguration{
Disable: false,
},
@ -170,7 +180,9 @@ func (s *SecondFactorAvailableMethodsFixture) TestShouldRemoveAllMethodsWhenNoTw
func (s *SecondFactorAvailableMethodsFixture) TestShouldRemoveAllMethodsWhenAllDisabledOrNotConfigured() {
s.mock.Ctx.Configuration = schema.Configuration{
DuoAPI: nil,
DuoAPI: schema.DuoAPIConfiguration{
Disable: true,
},
TOTP: schema.TOTPConfiguration{
Disable: true,
},

View File

@ -80,7 +80,7 @@ func ResetPasswordPOST(ctx *middlewares.AutheliaCtx) {
bufHTML := new(bytes.Buffer)
disableHTML := false
if ctx.Configuration.Notifier != nil && ctx.Configuration.Notifier.SMTP != nil {
if ctx.Configuration.Notifier.SMTP != nil {
disableHTML = ctx.Configuration.Notifier.SMTP.DisableHTMLEmails
}

View File

@ -101,8 +101,6 @@ func TestUserInfoEndpoint_SetCorrectMethod(t *testing.T) {
mock := mocks.NewMockAutheliaCtx(t)
mock.Ctx.Configuration.DuoAPI = &schema.DuoAPIConfiguration{}
// Set the initial user session.
userSession := mock.Ctx.GetSession()
userSession.Username = testUsername
@ -172,9 +170,7 @@ func TestUserInfoEndpoint_SetDefaultMethod(t *testing.T) {
HasWebauthn: false,
HasDuo: false,
},
config: &schema.Configuration{
DuoAPI: &schema.DuoAPIConfiguration{},
},
config: &schema.Configuration{},
loadErr: nil,
saveErr: nil,
},
@ -192,9 +188,7 @@ func TestUserInfoEndpoint_SetDefaultMethod(t *testing.T) {
HasWebauthn: false,
HasDuo: true,
},
config: &schema.Configuration{
DuoAPI: &schema.DuoAPIConfiguration{},
},
config: &schema.Configuration{},
loadErr: nil,
saveErr: nil,
},
@ -212,6 +206,7 @@ func TestUserInfoEndpoint_SetDefaultMethod(t *testing.T) {
HasWebauthn: false,
HasDuo: true,
},
config: &schema.Configuration{DuoAPI: schema.DuoAPIConfiguration{Disable: true}},
loadErr: nil,
saveErr: nil,
},
@ -233,7 +228,6 @@ func TestUserInfoEndpoint_SetDefaultMethod(t *testing.T) {
TOTP: schema.TOTPConfiguration{
Disable: true,
},
DuoAPI: &schema.DuoAPIConfiguration{},
},
loadErr: nil,
saveErr: nil,
@ -252,15 +246,14 @@ func TestUserInfoEndpoint_SetDefaultMethod(t *testing.T) {
HasWebauthn: true,
HasDuo: true,
},
config: &schema.Configuration{
DuoAPI: &schema.DuoAPIConfiguration{},
},
config: &schema.Configuration{},
loadErr: nil,
saveErr: errors.New("could not save"),
},
}
for _, resp := range expectedResponses {
t.Run(resp.description, func(t *testing.T) {
if resp.api == nil {
resp.api = &resp.db
}
@ -323,19 +316,19 @@ func TestUserInfoEndpoint_SetDefaultMethod(t *testing.T) {
mock.GetResponseData(t, &actualPreferences)
t.Run(fmt.Sprintf("%s/%s", resp.description, "expected method"), func(t *testing.T) {
t.Run("expected method", func(t *testing.T) {
assert.Equal(t, resp.api.Method, actualPreferences.Method)
})
t.Run(fmt.Sprintf("%s/%s", resp.description, "registered webauthn"), func(t *testing.T) {
t.Run("registered webauthn", func(t *testing.T) {
assert.Equal(t, resp.api.HasWebauthn, actualPreferences.HasWebauthn)
})
t.Run(fmt.Sprintf("%s/%s", resp.description, "registered totp"), func(t *testing.T) {
t.Run("registered totp", func(t *testing.T) {
assert.Equal(t, resp.api.HasTOTP, actualPreferences.HasTOTP)
})
t.Run(fmt.Sprintf("%s/%s", resp.description, "registered duo"), func(t *testing.T) {
t.Run("registered duo", func(t *testing.T) {
assert.Equal(t, resp.api.HasDuo, actualPreferences.HasDuo)
})
} else {
@ -350,6 +343,7 @@ func TestUserInfoEndpoint_SetDefaultMethod(t *testing.T) {
}
mock.Close()
})
}
}
@ -420,7 +414,7 @@ func (s *SaveSuite) TestShouldReturnError500WhenBadMethodProvided() {
MethodPreferencePOST(s.mock.Ctx)
s.mock.Assert200KO(s.T(), "Operation failed.")
assert.Equal(s.T(), "unknown or unavailable method 'abc', it should be one of totp, webauthn", s.mock.Hook.LastEntry().Message)
assert.Equal(s.T(), "unknown or unavailable method 'abc', it should be one of totp, webauthn, mobile_push", s.mock.Hook.LastEntry().Message)
assert.Equal(s.T(), logrus.ErrorLevel, s.mock.Hook.LastEntry().Level)
}

View File

@ -67,7 +67,7 @@ func (ctx *AutheliaCtx) AvailableSecondFactorMethods() (methods []string) {
methods = append(methods, model.SecondFactorMethodWebauthn)
}
if ctx.Configuration.DuoAPI != nil {
if !ctx.Configuration.DuoAPI.Disable {
methods = append(methods, model.SecondFactorMethodDuo)
}

View File

@ -121,9 +121,11 @@ func TestShouldReturnCorrectSecondFactorMethods(t *testing.T) {
mock := mocks.NewMockAutheliaCtx(t)
defer mock.Close()
mock.Ctx.Configuration.DuoAPI.Disable = true
assert.Equal(t, []string{model.SecondFactorMethodTOTP, model.SecondFactorMethodWebauthn}, mock.Ctx.AvailableSecondFactorMethods())
mock.Ctx.Configuration.DuoAPI = &schema.DuoAPIConfiguration{}
mock.Ctx.Configuration.DuoAPI.Disable = false
assert.Equal(t, []string{model.SecondFactorMethodTOTP, model.SecondFactorMethodWebauthn, model.SecondFactorMethodDuo}, mock.Ctx.AvailableSecondFactorMethods())
@ -135,7 +137,7 @@ func TestShouldReturnCorrectSecondFactorMethods(t *testing.T) {
assert.Equal(t, []string{model.SecondFactorMethodDuo}, mock.Ctx.AvailableSecondFactorMethods())
mock.Ctx.Configuration.DuoAPI = nil
mock.Ctx.Configuration.DuoAPI.Disable = true
assert.Equal(t, []string{}, mock.Ctx.AvailableSecondFactorMethods())
}

View File

@ -73,7 +73,7 @@ func IdentityVerificationStart(args IdentityVerificationStartArgs, delayFunc Tim
bufHTML := new(bytes.Buffer)
disableHTML := false
if ctx.Configuration.Notifier != nil && ctx.Configuration.Notifier.SMTP != nil {
if ctx.Configuration.Notifier.SMTP != nil {
disableHTML = ctx.Configuration.Notifier.SMTP.DisableHTMLEmails
}

View File

@ -87,7 +87,7 @@ func getHandler(config schema.Configuration, providers middlewares.Providers) fa
resetPasswordCustomURL := config.AuthenticationBackend.PasswordReset.CustomURL.String()
duoSelfEnrollment := f
if config.DuoAPI != nil {
if !config.DuoAPI.Disable {
duoSelfEnrollment = strconv.FormatBool(config.DuoAPI.EnableSelfEnrollment)
}
@ -184,7 +184,7 @@ func getHandler(config schema.Configuration, providers middlewares.Providers) fa
}
// Configure DUO api endpoint only if configuration exists.
if config.DuoAPI != nil {
if !config.DuoAPI.Disable {
var duoAPI duo.API
if os.Getenv("ENVIRONMENT") == dev {
duoAPI = duo.NewDuoAPI(duoapi.NewDuoApi(