Skip to content

Commit a2cfbe3

Browse files
hslatmanhslatman
committed
Fix (part of) PR comments
1 parent 3eecc4f commit a2cfbe3

File tree

11 files changed

+343
-796
lines changed

11 files changed

+343
-796
lines changed

‎api/read/read.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,7 @@ func JSON(r io.Reader, v interface{}) error {
2424
}
2525

2626
// ProtoJSON reads JSON from the request body and stores it in the value
27-
// pointed to by v.
27+
// pointed to by m.
2828
func ProtoJSON(r io.Reader, m proto.Message) error {
2929
data, err := io.ReadAll(r)
3030
if err != nil {

‎authority/admin/api/policy.go

Lines changed: 3 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -74,8 +74,7 @@ func (par *PolicyAdminResponder) CreateAuthorityPolicy(w http.ResponseWriter, r
7474
}
7575

7676
if policy != nil {
77-
adminErr := admin.NewError(admin.ErrorBadRequestType, "authority already has a policy")
78-
adminErr.Status = http.StatusConflict
77+
adminErr := admin.NewError(admin.ErrorConflictType, "authority already has a policy")
7978
render.Error(w, adminErr)
8079
return
8180
}
@@ -197,8 +196,7 @@ func (par *PolicyAdminResponder) CreateProvisionerPolicy(w http.ResponseWriter,
197196

198197
policy := prov.GetPolicy()
199198
if policy != nil {
200-
adminErr := admin.NewError(admin.ErrorBadRequestType, "provisioner %s already has a policy", prov.Name)
201-
adminErr.Status = http.StatusConflict
199+
adminErr := admin.NewError(admin.ErrorConflictType, "provisioner %s already has a policy", prov.Name)
202200
render.Error(w, adminErr)
203201
return
204202
}
@@ -307,8 +305,7 @@ func (par *PolicyAdminResponder) CreateACMEAccountPolicy(w http.ResponseWriter,
307305

308306
policy := eak.GetPolicy()
309307
if policy != nil {
310-
adminErr := admin.NewError(admin.ErrorBadRequestType, "ACME EAK %s already has a policy", eak.Id)
311-
adminErr.Status = http.StatusConflict
308+
adminErr := admin.NewError(admin.ErrorConflictType, "ACME EAK %s already has a policy", eak.Id)
312309
render.Error(w, adminErr)
313310
return
314311
}

‎authority/admin/api/policy_test.go

Lines changed: 3 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -154,9 +154,8 @@ func TestPolicyAdminResponder_CreateAuthorityPolicy(t *testing.T) {
154154
},
155155
"fail/existing-policy": func(t *testing.T) test {
156156
ctx := context.Background()
157-
err := admin.NewError(admin.ErrorBadRequestType, "authority already has a policy")
157+
err := admin.NewError(admin.ErrorConflictType, "authority already has a policy")
158158
err.Message = "authority already has a policy"
159-
err.Status = http.StatusConflict
160159
return test{
161160
ctx: ctx,
162161
auth: &mockAdminAuthority{
@@ -864,9 +863,8 @@ func TestPolicyAdminResponder_CreateProvisionerPolicy(t *testing.T) {
864863
Policy: policy,
865864
}
866865
ctx := linkedca.NewContextWithProvisioner(context.Background(), prov)
867-
err := admin.NewError(admin.ErrorBadRequestType, "provisioner provName already has a policy")
866+
err := admin.NewError(admin.ErrorConflictType, "provisioner provName already has a policy")
868867
err.Message = "provisioner provName already has a policy"
869-
err.Status = http.StatusConflict
870868
return test{
871869
ctx: ctx,
872870
err: err,
@@ -1466,9 +1464,8 @@ func TestPolicyAdminResponder_CreateACMEAccountPolicy(t *testing.T) {
14661464
}
14671465
ctx := linkedca.NewContextWithProvisioner(context.Background(), prov)
14681466
ctx = linkedca.NewContextWithExternalAccountKey(ctx, eak)
1469-
err := admin.NewError(admin.ErrorBadRequestType, "ACME EAK eakID already has a policy")
1467+
err := admin.NewError(admin.ErrorConflictType, "ACME EAK eakID already has a policy")
14701468
err.Message = "ACME EAK eakID already has a policy"
1471-
err.Status = http.StatusConflict
14721469
return test{
14731470
ctx: ctx,
14741471
err: err,

‎authority/admin/errors.go

Lines changed: 11 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -24,10 +24,12 @@ const (
2424
ErrorBadRequestType
2525
// ErrorNotImplementedType not implemented.
2626
ErrorNotImplementedType
27-
// ErrorUnauthorizedType internal server error.
27+
// ErrorUnauthorizedType unauthorized.
2828
ErrorUnauthorizedType
2929
// ErrorServerInternalType internal server error.
3030
ErrorServerInternalType
31+
// ErrorConflictType conflict.
32+
ErrorConflictType
3133
)
3234

3335
// String returns the string representation of the admin problem type,
@@ -48,6 +50,8 @@ func (ap ProblemType) String() string {
4850
return "unauthorized"
4951
case ErrorServerInternalType:
5052
return "internalServerError"
53+
case ErrorConflictType:
54+
return "conflict"
5155
default:
5256
return fmt.Sprintf("unsupported error type '%d'", int(ap))
5357
}
@@ -64,7 +68,7 @@ var (
6468
errorServerInternalMetadata = errorMetadata{
6569
typ: ErrorServerInternalType.String(),
6670
details: "the server experienced an internal error",
67-
status: 500,
71+
status: http.StatusInternalServerError,
6872
}
6973
errorMap = map[ProblemType]errorMetadata{
7074
ErrorNotFoundType: {
@@ -98,6 +102,11 @@ var (
98102
status: http.StatusUnauthorized,
99103
},
100104
ErrorServerInternalType: errorServerInternalMetadata,
105+
ErrorConflictType: {
106+
typ: ErrorConflictType.String(),
107+
details: "conflict",
108+
status: http.StatusConflict,
109+
},
101110
}
102111
)
103112

‎authority/policy.go

Lines changed: 14 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -318,11 +318,10 @@ func policyToCertificates(p *linkedca.Policy) *authPolicy.Options {
318318
opts := &authPolicy.Options{}
319319

320320
// fill x509 policy configuration
321-
if p.GetX509() != nil {
321+
if x509 := p.GetX509(); x509 != nil {
322322
opts.X509 = &authPolicy.X509PolicyOptions{}
323-
if p.GetX509().GetAllow() != nil {
323+
if allow := x509.GetAllow(); allow != nil {
324324
opts.X509.AllowedNames = &authPolicy.X509NameOptions{}
325-
allow := p.GetX509().GetAllow()
326325
if allow.Dns != nil {
327326
opts.X509.AllowedNames.DNSDomains = allow.Dns
328327
}
@@ -336,9 +335,8 @@ func policyToCertificates(p *linkedca.Policy) *authPolicy.Options {
336335
opts.X509.AllowedNames.URIDomains = allow.Uris
337336
}
338337
}
339-
if p.GetX509().GetDeny() != nil {
338+
if deny := x509.GetDeny(); deny != nil {
340339
opts.X509.DeniedNames = &authPolicy.X509NameOptions{}
341-
deny := p.GetX509().GetDeny()
342340
if deny.Dns != nil {
343341
opts.X509.DeniedNames.DNSDomains = deny.Dns
344342
}
@@ -352,22 +350,21 @@ func policyToCertificates(p *linkedca.Policy) *authPolicy.Options {
352350
opts.X509.DeniedNames.URIDomains = deny.Uris
353351
}
354352
}
355-
if p.GetX509().GetAllowWildcardLiteral() != nil {
356-
opts.X509.AllowWildcardLiteral = &p.GetX509().GetAllowWildcardLiteral().Value
353+
if v := x509.GetAllowWildcardLiteral(); v != nil {
354+
opts.X509.AllowWildcardLiteral = &v.Value
357355
}
358-
if p.GetX509().GetVerifySubjectCommonName() != nil {
359-
opts.X509.VerifySubjectCommonName = &p.GetX509().VerifySubjectCommonName.Value
356+
if v := x509.GetVerifySubjectCommonName(); v != nil {
357+
opts.X509.VerifySubjectCommonName = &v.Value
360358
}
361359
}
362360

363361
// fill ssh policy configuration
364-
if p.GetSsh() != nil {
362+
if ssh := p.GetSsh(); ssh != nil {
365363
opts.SSH = &authPolicy.SSHPolicyOptions{}
366-
if p.GetSsh().GetHost() != nil {
364+
if host := ssh.GetHost(); host != nil {
367365
opts.SSH.Host = &authPolicy.SSHHostCertificateOptions{}
368-
if p.GetSsh().GetHost().GetAllow() != nil {
366+
if allow := host.GetAllow(); allow != nil {
369367
opts.SSH.Host.AllowedNames = &authPolicy.SSHNameOptions{}
370-
allow := p.GetSsh().GetHost().GetAllow()
371368
if allow.Dns != nil {
372369
opts.SSH.Host.AllowedNames.DNSDomains = allow.Dns
373370
}
@@ -378,9 +375,8 @@ func policyToCertificates(p *linkedca.Policy) *authPolicy.Options {
378375
opts.SSH.Host.AllowedNames.Principals = allow.Principals
379376
}
380377
}
381-
if p.GetSsh().GetHost().GetDeny() != nil {
378+
if deny := host.GetDeny(); deny != nil {
382379
opts.SSH.Host.DeniedNames = &authPolicy.SSHNameOptions{}
383-
deny := p.GetSsh().GetHost().GetDeny()
384380
if deny.Dns != nil {
385381
opts.SSH.Host.DeniedNames.DNSDomains = deny.Dns
386382
}
@@ -392,21 +388,19 @@ func policyToCertificates(p *linkedca.Policy) *authPolicy.Options {
392388
}
393389
}
394390
}
395-
if p.GetSsh().GetUser() != nil {
391+
if user := ssh.GetUser(); user != nil {
396392
opts.SSH.User = &authPolicy.SSHUserCertificateOptions{}
397-
if p.GetSsh().GetUser().GetAllow() != nil {
393+
if allow := user.GetAllow(); allow != nil {
398394
opts.SSH.User.AllowedNames = &authPolicy.SSHNameOptions{}
399-
allow := p.GetSsh().GetUser().GetAllow()
400395
if allow.Emails != nil {
401396
opts.SSH.User.AllowedNames.EmailAddresses = allow.Emails
402397
}
403398
if allow.Principals != nil {
404399
opts.SSH.User.AllowedNames.Principals = allow.Principals
405400
}
406401
}
407-
if p.GetSsh().GetUser().GetDeny() != nil {
402+
if deny := user.GetDeny(); deny != nil {
408403
opts.SSH.User.DeniedNames = &authPolicy.SSHNameOptions{}
409-
deny := p.GetSsh().GetUser().GetDeny()
410404
if deny.Emails != nil {
411405
opts.SSH.User.DeniedNames.EmailAddresses = deny.Emails
412406
}

‎authority/policy/options.go

Lines changed: 14 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -67,24 +67,20 @@ func (o *X509NameOptions) HasNames() bool {
6767
len(o.URIDomains) > 0
6868
}
6969

70-
// GetDeniedNameOptions returns the x509 denied name policy configuration
71-
func (o *X509PolicyOptions) GetDeniedNameOptions() *X509NameOptions {
70+
// GetAllowedNameOptions returns x509 allowed name policy configuration
71+
func (o *X509PolicyOptions) GetAllowedNameOptions() *X509NameOptions {
7272
if o == nil {
7373
return nil
7474
}
75-
return o.DeniedNames
75+
return o.AllowedNames
7676
}
7777

78-
// GetAllowedUserNameOptions returns the SSH allowed user name policy
79-
// configuration.
80-
func (o *SSHPolicyOptions) GetAllowedUserNameOptions() *SSHNameOptions {
78+
// GetDeniedNameOptions returns the x509 denied name policy configuration
79+
func (o *X509PolicyOptions) GetDeniedNameOptions() *X509NameOptions {
8180
if o == nil {
8281
return nil
8382
}
84-
if o.User == nil {
85-
return nil
86-
}
87-
return o.User.AllowedNames
83+
return o.DeniedNames
8884
}
8985

9086
func (o *X509PolicyOptions) IsWildcardLiteralAllowed() bool {
@@ -122,21 +118,19 @@ type SSHPolicyOptions struct {
122118
Host *SSHHostCertificateOptions `json:"host,omitempty"`
123119
}
124120

125-
// GetAllowedNameOptions returns x509 allowed name policy configuration
126-
func (o *X509PolicyOptions) GetAllowedNameOptions() *X509NameOptions {
127-
if o == nil {
121+
// GetAllowedUserNameOptions returns the SSH allowed user name policy
122+
// configuration.
123+
func (o *SSHPolicyOptions) GetAllowedUserNameOptions() *SSHNameOptions {
124+
if o == nil || o.User == nil {
128125
return nil
129126
}
130-
return o.AllowedNames
127+
return o.User.AllowedNames
131128
}
132129

133130
// GetDeniedUserNameOptions returns the SSH denied user name policy
134131
// configuration.
135132
func (o *SSHPolicyOptions) GetDeniedUserNameOptions() *SSHNameOptions {
136-
if o == nil {
137-
return nil
138-
}
139-
if o.User == nil {
133+
if o == nil || o.User == nil {
140134
return nil
141135
}
142136
return o.User.DeniedNames
@@ -145,10 +139,7 @@ func (o *SSHPolicyOptions) GetDeniedUserNameOptions() *SSHNameOptions {
145139
// GetAllowedHostNameOptions returns the SSH allowed host name policy
146140
// configuration.
147141
func (o *SSHPolicyOptions) GetAllowedHostNameOptions() *SSHNameOptions {
148-
if o == nil {
149-
return nil
150-
}
151-
if o.Host == nil {
142+
if o == nil || o.Host == nil {
152143
return nil
153144
}
154145
return o.Host.AllowedNames
@@ -157,10 +148,7 @@ func (o *SSHPolicyOptions) GetAllowedHostNameOptions() *SSHNameOptions {
157148
// GetDeniedHostNameOptions returns the SSH denied host name policy
158149
// configuration.
159150
func (o *SSHPolicyOptions) GetDeniedHostNameOptions() *SSHNameOptions {
160-
if o == nil {
161-
return nil
162-
}
163-
if o.Host == nil {
151+
if o == nil || o.Host == nil {
164152
return nil
165153
}
166154
return o.Host.DeniedNames

‎authority/policy/policy.go

Lines changed: 16 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -28,20 +28,20 @@ func NewX509PolicyEngine(policyOptions X509PolicyOptionsInterface) (X509Policy,
2828
allowed := policyOptions.GetAllowedNameOptions()
2929
if allowed != nil && allowed.HasNames() {
3030
options = append(options,
31-
policy.WithPermittedDNSDomains(allowed.DNSDomains),
32-
policy.WithPermittedIPsOrCIDRs(allowed.IPRanges),
33-
policy.WithPermittedEmailAddresses(allowed.EmailAddresses),
34-
policy.WithPermittedURIDomains(allowed.URIDomains),
31+
policy.WithPermittedDNSDomains(allowed.DNSDomains...),
32+
policy.WithPermittedIPsOrCIDRs(allowed.IPRanges...),
33+
policy.WithPermittedEmailAddresses(allowed.EmailAddresses...),
34+
policy.WithPermittedURIDomains(allowed.URIDomains...),
3535
)
3636
}
3737

3838
denied := policyOptions.GetDeniedNameOptions()
3939
if denied != nil && denied.HasNames() {
4040
options = append(options,
41-
policy.WithExcludedDNSDomains(denied.DNSDomains),
42-
policy.WithExcludedIPsOrCIDRs(denied.IPRanges),
43-
policy.WithExcludedEmailAddresses(denied.EmailAddresses),
44-
policy.WithExcludedURIDomains(denied.URIDomains),
41+
policy.WithExcludedDNSDomains(denied.DNSDomains...),
42+
policy.WithExcludedIPsOrCIDRs(denied.IPRanges...),
43+
policy.WithExcludedEmailAddresses(denied.EmailAddresses...),
44+
policy.WithExcludedURIDomains(denied.URIDomains...),
4545
)
4646
}
4747

@@ -114,19 +114,19 @@ func newSSHPolicyEngine(policyOptions SSHPolicyOptionsInterface, typ sshPolicyEn
114114

115115
if allowed != nil && allowed.HasNames() {
116116
options = append(options,
117-
policy.WithPermittedDNSDomains(allowed.DNSDomains),
118-
policy.WithPermittedIPsOrCIDRs(allowed.IPRanges),
119-
policy.WithPermittedEmailAddresses(allowed.EmailAddresses),
120-
policy.WithPermittedPrincipals(allowed.Principals),
117+
policy.WithPermittedDNSDomains(allowed.DNSDomains...),
118+
policy.WithPermittedIPsOrCIDRs(allowed.IPRanges...),
119+
policy.WithPermittedEmailAddresses(allowed.EmailAddresses...),
120+
policy.WithPermittedPrincipals(allowed.Principals...),
121121
)
122122
}
123123

124124
if denied != nil && denied.HasNames() {
125125
options = append(options,
126-
policy.WithExcludedDNSDomains(denied.DNSDomains),
127-
policy.WithExcludedIPsOrCIDRs(denied.IPRanges),
128-
policy.WithExcludedEmailAddresses(denied.EmailAddresses),
129-
policy.WithExcludedPrincipals(denied.Principals),
126+
policy.WithExcludedDNSDomains(denied.DNSDomains...),
127+
policy.WithExcludedIPsOrCIDRs(denied.IPRanges...),
128+
policy.WithExcludedEmailAddresses(denied.EmailAddresses...),
129+
policy.WithExcludedPrincipals(denied.Principals...),
130130
)
131131
}
132132

0 commit comments

Comments
 (0)