Change pointer booleans to regular boolean configuration

This commit is contained in:
Herman Slatman 2022-04-21 23:45:05 +02:00
parent e9f5a1eb98
commit ef110a94df
No known key found for this signature in database
GPG key ID: F4D8A44EA0A75A4F
6 changed files with 43 additions and 154 deletions

View file

@ -5,7 +5,6 @@ import (
"net/http" "net/http"
"go.step.sm/linkedca" "go.step.sm/linkedca"
"google.golang.org/protobuf/types/known/wrapperspb"
"github.com/smallstep/certificates/acme" "github.com/smallstep/certificates/acme"
"github.com/smallstep/certificates/api/read" "github.com/smallstep/certificates/api/read"
@ -97,8 +96,6 @@ func (par *PolicyAdminResponder) CreateAuthorityPolicy(w http.ResponseWriter, r
return return
} }
applyConditionalDefaults(newPolicy)
newPolicy.Deduplicate() newPolicy.Deduplicate()
adm := linkedca.AdminFromContext(ctx) adm := linkedca.AdminFromContext(ctx)
@ -234,8 +231,6 @@ func (par *PolicyAdminResponder) CreateProvisionerPolicy(w http.ResponseWriter,
return return
} }
applyConditionalDefaults(newPolicy)
newPolicy.Deduplicate() newPolicy.Deduplicate()
prov.Policy = newPolicy prov.Policy = newPolicy
@ -454,14 +449,3 @@ func isBadRequest(err error) bool {
isPolicyError := errors.As(err, &pe) isPolicyError := errors.As(err, &pe)
return isPolicyError && (pe.Typ == authority.AdminLockOut || pe.Typ == authority.EvaluationFailure || pe.Typ == authority.ConfigurationFailure) return isPolicyError && (pe.Typ == authority.AdminLockOut || pe.Typ == authority.EvaluationFailure || pe.Typ == authority.ConfigurationFailure)
} }
// applyConditionalDefaults applies default settings in case they're not provided
// in the request body.
func applyConditionalDefaults(p *linkedca.Policy) {
if p.GetX509() == nil {
return
}
if p.GetX509().GetVerifySubjectCommonName() == nil {
p.X509.VerifySubjectCommonName = &wrapperspb.BoolValue{Value: true}
}
}

View file

@ -12,7 +12,6 @@ import (
"testing" "testing"
"google.golang.org/protobuf/encoding/protojson" "google.golang.org/protobuf/encoding/protojson"
"google.golang.org/protobuf/types/known/wrapperspb"
"go.step.sm/linkedca" "go.step.sm/linkedca"
@ -310,7 +309,7 @@ func TestPolicyAdminResponder_CreateAuthorityPolicy(t *testing.T) {
Allow: &linkedca.X509Names{ Allow: &linkedca.X509Names{
Dns: []string{"*.local"}, Dns: []string{"*.local"},
}, },
VerifySubjectCommonName: &wrapperspb.BoolValue{Value: true}, DisableSubjectCommonNameVerification: false,
}, },
} }
body, err := protojson.Marshal(policy) body, err := protojson.Marshal(policy)
@ -1047,7 +1046,7 @@ func TestPolicyAdminResponder_CreateProvisionerPolicy(t *testing.T) {
Allow: &linkedca.X509Names{ Allow: &linkedca.X509Names{
Dns: []string{"*.local"}, Dns: []string{"*.local"},
}, },
VerifySubjectCommonName: &wrapperspb.BoolValue{Value: true}, DisableSubjectCommonNameVerification: false,
}, },
} }
body, err := protojson.Marshal(policy) body, err := protojson.Marshal(policy)
@ -2077,86 +2076,6 @@ func TestPolicyAdminResponder_DeleteACMEAccountPolicy(t *testing.T) {
} }
} }
func Test_applyConditionalDefaults(t *testing.T) {
tests := []struct {
name string
policy *linkedca.Policy
expected *linkedca.Policy
}{
{
name: "no-x509",
policy: &linkedca.Policy{
Ssh: &linkedca.SSHPolicy{},
},
expected: &linkedca.Policy{
Ssh: &linkedca.SSHPolicy{},
},
},
{
name: "with-x509-verify-subject-common-name",
policy: &linkedca.Policy{
X509: &linkedca.X509Policy{
Allow: &linkedca.X509Names{
Dns: []string{"*.local"},
},
VerifySubjectCommonName: &wrapperspb.BoolValue{Value: true},
},
},
expected: &linkedca.Policy{
X509: &linkedca.X509Policy{
Allow: &linkedca.X509Names{
Dns: []string{"*.local"},
},
VerifySubjectCommonName: &wrapperspb.BoolValue{Value: true},
},
},
},
{
name: "without-x509-verify-subject-common-name",
policy: &linkedca.Policy{
X509: &linkedca.X509Policy{
Allow: &linkedca.X509Names{
Dns: []string{"*.local"},
},
VerifySubjectCommonName: &wrapperspb.BoolValue{Value: false},
},
},
expected: &linkedca.Policy{
X509: &linkedca.X509Policy{
Allow: &linkedca.X509Names{
Dns: []string{"*.local"},
},
VerifySubjectCommonName: &wrapperspb.BoolValue{Value: false},
},
},
},
{
name: "no-x509-verify-subject-common-name",
policy: &linkedca.Policy{
X509: &linkedca.X509Policy{
Allow: &linkedca.X509Names{
Dns: []string{"*.local"},
},
},
},
expected: &linkedca.Policy{
X509: &linkedca.X509Policy{
Allow: &linkedca.X509Names{
Dns: []string{"*.local"},
},
VerifySubjectCommonName: &wrapperspb.BoolValue{Value: true},
},
},
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
applyConditionalDefaults(tt.policy)
assert.Equals(t, tt.expected, tt.policy)
})
}
}
func Test_isBadRequest(t *testing.T) { func Test_isBadRequest(t *testing.T) {
tests := []struct { tests := []struct {
name string name string

View file

@ -350,12 +350,9 @@ func policyToCertificates(p *linkedca.Policy) *authPolicy.Options {
opts.X509.DeniedNames.URIDomains = deny.Uris opts.X509.DeniedNames.URIDomains = deny.Uris
} }
} }
if v := x509.GetAllowWildcardLiteral(); v != nil {
opts.X509.AllowWildcardLiteral = &v.Value opts.X509.AllowWildcardLiteral = x509.AllowWildcardLiteral
} opts.X509.DisableSubjectCommonNameVerification = x509.DisableSubjectCommonNameVerification
if v := x509.GetVerifySubjectCommonName(); v != nil {
opts.X509.VerifySubjectCommonName = &v.Value
}
} }
// fill ssh policy configuration // fill ssh policy configuration

View file

@ -44,10 +44,10 @@ type X509PolicyOptions struct {
// AllowWildcardLiteral indicates if literal wildcard names // AllowWildcardLiteral indicates if literal wildcard names
// such as *.example.com and @example.com are allowed. Defaults // such as *.example.com and @example.com are allowed. Defaults
// to false. // to false.
AllowWildcardLiteral *bool `json:"allow_wildcard_literal,omitempty"` AllowWildcardLiteral bool `json:"allow_wildcard_literal,omitempty"`
// VerifySubjectCommonName indicates if the Subject Common Name // DisableSubjectCommonNameVerification indicates if the Subject Common Name
// is verified in addition to the SANs. Defaults to true. // is verified in addition to the SANs. Defaults to false.
VerifySubjectCommonName *bool `json:"verify_subject_common_name,omitempty"` DisableSubjectCommonNameVerification bool `json:"disable_subject_common_name_verification,omitempty"`
} }
// X509NameOptions models the X509 name policy configuration. // X509NameOptions models the X509 name policy configuration.
@ -83,21 +83,22 @@ func (o *X509PolicyOptions) GetDeniedNameOptions() *X509NameOptions {
return o.DeniedNames return o.DeniedNames
} }
// IsWildcardLiteralAllowed returns whether the authority allows
// literal wildcard domains and other names to be signed.
func (o *X509PolicyOptions) IsWildcardLiteralAllowed() bool { func (o *X509PolicyOptions) IsWildcardLiteralAllowed() bool {
if o == nil { if o == nil {
return true return true
} }
return o.AllowWildcardLiteral != nil && *o.AllowWildcardLiteral return o.AllowWildcardLiteral
} }
// ShouldVerifySubjectCommonName returns whether the authority
// should verify the Subject Common Name in addition to the SANs.
func (o *X509PolicyOptions) ShouldVerifySubjectCommonName() bool { func (o *X509PolicyOptions) ShouldVerifySubjectCommonName() bool {
if o == nil { if o == nil {
return false return false
} }
if o.VerifySubjectCommonName == nil { return !o.DisableSubjectCommonNameVerification
return true
}
return *o.VerifySubjectCommonName
} }
// SSHPolicyOptionsInterface is an interface for providers of // SSHPolicyOptionsInterface is an interface for providers of

View file

@ -5,8 +5,6 @@ import (
) )
func TestX509PolicyOptions_IsWildcardLiteralAllowed(t *testing.T) { func TestX509PolicyOptions_IsWildcardLiteralAllowed(t *testing.T) {
trueValue := true
falseValue := false
tests := []struct { tests := []struct {
name string name string
options *X509PolicyOptions options *X509PolicyOptions
@ -18,23 +16,21 @@ func TestX509PolicyOptions_IsWildcardLiteralAllowed(t *testing.T) {
want: true, want: true,
}, },
{ {
name: "nil", name: "not-set",
options: &X509PolicyOptions{ options: &X509PolicyOptions{},
AllowWildcardLiteral: nil, want: false,
},
want: false,
}, },
{ {
name: "set-true", name: "set-true",
options: &X509PolicyOptions{ options: &X509PolicyOptions{
AllowWildcardLiteral: &trueValue, AllowWildcardLiteral: true,
}, },
want: true, want: true,
}, },
{ {
name: "set-false", name: "set-false",
options: &X509PolicyOptions{ options: &X509PolicyOptions{
AllowWildcardLiteral: &falseValue, AllowWildcardLiteral: false,
}, },
want: false, want: false,
}, },
@ -49,8 +45,6 @@ func TestX509PolicyOptions_IsWildcardLiteralAllowed(t *testing.T) {
} }
func TestX509PolicyOptions_ShouldVerifySubjectCommonName(t *testing.T) { func TestX509PolicyOptions_ShouldVerifySubjectCommonName(t *testing.T) {
trueValue := true
falseValue := false
tests := []struct { tests := []struct {
name string name string
options *X509PolicyOptions options *X509PolicyOptions
@ -62,25 +56,23 @@ func TestX509PolicyOptions_ShouldVerifySubjectCommonName(t *testing.T) {
want: false, want: false,
}, },
{ {
name: "nil", name: "not-set",
options: &X509PolicyOptions{ options: &X509PolicyOptions{},
VerifySubjectCommonName: nil, want: true,
},
want: true,
}, },
{ {
name: "set-true", name: "set-true",
options: &X509PolicyOptions{ options: &X509PolicyOptions{
VerifySubjectCommonName: &trueValue, DisableSubjectCommonNameVerification: true,
}, },
want: true, want: false,
}, },
{ {
name: "set-false", name: "set-false",
options: &X509PolicyOptions{ options: &X509PolicyOptions{
VerifySubjectCommonName: &falseValue, DisableSubjectCommonNameVerification: false,
}, },
want: false, want: true,
}, },
} }
for _, tt := range tests { for _, tt := range tests {

View file

@ -7,7 +7,6 @@ import (
"github.com/google/go-cmp/cmp" "github.com/google/go-cmp/cmp"
"github.com/stretchr/testify/assert" "github.com/stretchr/testify/assert"
"google.golang.org/protobuf/types/known/wrapperspb"
"go.step.sm/linkedca" "go.step.sm/linkedca"
@ -193,8 +192,6 @@ func TestAuthority_checkPolicy(t *testing.T) {
} }
func Test_policyToCertificates(t *testing.T) { func Test_policyToCertificates(t *testing.T) {
trueValue := true
falseValue := false
tests := []struct { tests := []struct {
name string name string
policy *linkedca.Policy policy *linkedca.Policy
@ -217,8 +214,8 @@ func Test_policyToCertificates(t *testing.T) {
Allow: &linkedca.X509Names{ Allow: &linkedca.X509Names{
Dns: []string{"*.local"}, Dns: []string{"*.local"},
}, },
AllowWildcardLiteral: &wrapperspb.BoolValue{Value: false}, AllowWildcardLiteral: false,
VerifySubjectCommonName: &wrapperspb.BoolValue{Value: true}, DisableSubjectCommonNameVerification: false,
}, },
}, },
want: &policy.Options{ want: &policy.Options{
@ -226,8 +223,8 @@ func Test_policyToCertificates(t *testing.T) {
AllowedNames: &policy.X509NameOptions{ AllowedNames: &policy.X509NameOptions{
DNSDomains: []string{"*.local"}, DNSDomains: []string{"*.local"},
}, },
AllowWildcardLiteral: &falseValue, AllowWildcardLiteral: false,
VerifySubjectCommonName: &trueValue, DisableSubjectCommonNameVerification: false,
}, },
}, },
}, },
@ -247,8 +244,8 @@ func Test_policyToCertificates(t *testing.T) {
Emails: []string{"badhost.example.com"}, Emails: []string{"badhost.example.com"},
Uris: []string{"https://badhost.local"}, Uris: []string{"https://badhost.local"},
}, },
AllowWildcardLiteral: &wrapperspb.BoolValue{Value: true}, AllowWildcardLiteral: true,
VerifySubjectCommonName: &wrapperspb.BoolValue{Value: true}, DisableSubjectCommonNameVerification: false,
}, },
Ssh: &linkedca.SSHPolicy{ Ssh: &linkedca.SSHPolicy{
Host: &linkedca.SSHHostPolicy{ Host: &linkedca.SSHHostPolicy{
@ -289,8 +286,8 @@ func Test_policyToCertificates(t *testing.T) {
EmailAddresses: []string{"badhost.example.com"}, EmailAddresses: []string{"badhost.example.com"},
URIDomains: []string{"https://badhost.local"}, URIDomains: []string{"https://badhost.local"},
}, },
AllowWildcardLiteral: &trueValue, AllowWildcardLiteral: true,
VerifySubjectCommonName: &trueValue, DisableSubjectCommonNameVerification: false,
}, },
SSH: &policy.SSHPolicyOptions{ SSH: &policy.SSHPolicyOptions{
Host: &policy.SSHHostCertificateOptions{ Host: &policy.SSHHostCertificateOptions{
@ -335,7 +332,6 @@ func TestAuthority_reloadPolicyEngines(t *testing.T) {
sshUserPolicy bool sshUserPolicy bool
sshHostPolicy bool sshHostPolicy bool
} }
trueValue := true
tests := []struct { tests := []struct {
name string name string
config *config.Config config *config.Config
@ -517,8 +513,8 @@ func TestAuthority_reloadPolicyEngines(t *testing.T) {
DeniedNames: &policy.X509NameOptions{ DeniedNames: &policy.X509NameOptions{
DNSDomains: []string{"badhost.local"}, DNSDomains: []string{"badhost.local"},
}, },
AllowWildcardLiteral: &trueValue, AllowWildcardLiteral: true,
VerifySubjectCommonName: &trueValue, DisableSubjectCommonNameVerification: false,
}, },
}, },
}, },
@ -637,8 +633,8 @@ func TestAuthority_reloadPolicyEngines(t *testing.T) {
DeniedNames: &policy.X509NameOptions{ DeniedNames: &policy.X509NameOptions{
DNSDomains: []string{"badhost.local"}, DNSDomains: []string{"badhost.local"},
}, },
AllowWildcardLiteral: &trueValue, AllowWildcardLiteral: true,
VerifySubjectCommonName: &trueValue, DisableSubjectCommonNameVerification: false,
}, },
SSH: &policy.SSHPolicyOptions{ SSH: &policy.SSHPolicyOptions{
Host: &policy.SSHHostCertificateOptions{ Host: &policy.SSHHostCertificateOptions{
@ -770,8 +766,8 @@ func TestAuthority_reloadPolicyEngines(t *testing.T) {
Deny: &linkedca.X509Names{ Deny: &linkedca.X509Names{
Dns: []string{"badhost.local"}, Dns: []string{"badhost.local"},
}, },
AllowWildcardLiteral: &wrapperspb.BoolValue{Value: true}, AllowWildcardLiteral: true,
VerifySubjectCommonName: &wrapperspb.BoolValue{Value: true}, DisableSubjectCommonNameVerification: false,
}, },
Ssh: &linkedca.SSHPolicy{ Ssh: &linkedca.SSHPolicy{
Host: &linkedca.SSHHostPolicy{ Host: &linkedca.SSHHostPolicy{
@ -835,8 +831,8 @@ func TestAuthority_reloadPolicyEngines(t *testing.T) {
Deny: &linkedca.X509Names{ Deny: &linkedca.X509Names{
Dns: []string{"badhost.local"}, Dns: []string{"badhost.local"},
}, },
AllowWildcardLiteral: &wrapperspb.BoolValue{Value: true}, AllowWildcardLiteral: true,
VerifySubjectCommonName: &wrapperspb.BoolValue{Value: true}, DisableSubjectCommonNameVerification: false,
}, },
}, nil }, nil
}, },