Commit a0c1b63b authored by ale's avatar ale
Browse files

Plug validators back in

The new Backend/TX split makes it a bit harder to test the validators,
but do so anyway.
parent 6fd238b5
...@@ -96,8 +96,8 @@ func newAccountServiceWithSSO(backend Backend, config *Config, ssoValidator sso. ...@@ -96,8 +96,8 @@ func newAccountServiceWithSSO(backend Backend, config *Config, ssoValidator sso.
validationConfig := config.validationConfig() validationConfig := config.validationConfig()
domainBackend := config.domainBackend() domainBackend := config.domainBackend()
s.dataValidators = map[string]ValidatorFunc{ s.dataValidators = map[string]ValidatorFunc{
ResourceTypeEmail: validHostedEmail(validationConfig, domainBackend, nil), ResourceTypeEmail: validHostedEmail(validationConfig, domainBackend, backend),
ResourceTypeMailingList: validHostedMailingList(validationConfig, domainBackend, nil), ResourceTypeMailingList: validHostedMailingList(validationConfig, domainBackend, backend),
} }
return s return s
......
...@@ -121,6 +121,10 @@ const ( ...@@ -121,6 +121,10 @@ const (
ResourceStatusInactive = "inactive" ResourceStatusInactive = "inactive"
) )
// Resource ID. This is a a unique primary key in the resources space,
// with a path-like representation. It must make sense to the database
// backend and be reversible (i.e. there must be a bidirectional
// mapping between database objects and resource IDs).
type ResourceID struct { type ResourceID struct {
Parts []string Parts []string
} }
......
...@@ -18,11 +18,6 @@ type domainBackend interface { ...@@ -18,11 +18,6 @@ type domainBackend interface {
IsAvailableDomain(context.Context, string, string) bool IsAvailableDomain(context.Context, string, string) bool
} }
// The checkBackend verifies if specific resources already exist or are available.
type checkBackend interface {
HasAnyResource(context.Context, []string) (bool, error)
}
type validationConfig struct { type validationConfig struct {
forbiddenUsernames stringSet forbiddenUsernames stringSet
} }
...@@ -183,16 +178,21 @@ func splitEmailAddr(addr string) (string, string) { ...@@ -183,16 +178,21 @@ func splitEmailAddr(addr string) (string, string) {
// Returns all the possible resources in the email and mailing list // Returns all the possible resources in the email and mailing list
// namespaces that might conflict with the given email address. // namespaces that might conflict with the given email address.
func relatedEmails(ctx context.Context, be domainBackend, addr string) []string { func relatedEmails(ctx context.Context, be domainBackend, addr string) []FindResourceRequest {
resourceIDs := []string{fmt.Sprintf("%s/%s", ResourceTypeEmail, addr)} rel := []FindResourceRequest{
{Type: ResourceTypeEmail, Name: addr},
}
user, _ := splitEmailAddr(addr) user, _ := splitEmailAddr(addr)
// Mailing lists must have unique names regardless of the domain, so we // Mailing lists must have unique names regardless of the domain, so we
// add potential conflicts for mailing lists with the same name over all // add potential conflicts for mailing lists with the same name over all
// list-enabled domains. // list-enabled domains.
for _, d := range be.GetAvailableDomains(ctx, ResourceTypeMailingList) { for _, d := range be.GetAvailableDomains(ctx, ResourceTypeMailingList) {
resourceIDs = append(resourceIDs, fmt.Sprintf("%s/%s@%s", ResourceTypeMailingList, user, d)) rel = append(rel, FindResourceRequest{
Type: ResourceTypeMailingList,
Name: fmt.Sprintf("%s@%s", user, d),
})
} }
return resourceIDs return rel
} }
func splitSubsite(value string) (string, string) { func splitSubsite(value string) (string, string) {
...@@ -235,17 +235,24 @@ func isAvailableMailingListDomain(be domainBackend) ValidatorFunc { ...@@ -235,17 +235,24 @@ func isAvailableMailingListDomain(be domainBackend) ValidatorFunc {
} }
} }
func isAvailableEmailAddr(be domainBackend, cb checkBackend) ValidatorFunc { func isAvailableEmailAddr(be domainBackend, cb Backend) ValidatorFunc {
return func(ctx context.Context, value string) error { return func(ctx context.Context, value string) error {
rel := relatedEmails(ctx, be, value) rel := relatedEmails(ctx, be, value)
if ok, _ := cb.HasAnyResource(ctx, rel); ok {
// Run the presence check in a new transaction. Unavailability
// of the server results in a validation error (fail close).
tx, err := cb.NewTransaction()
if err != nil {
return err
}
if ok, _ := tx.HasAnyResource(ctx, rel); ok {
return errors.New("address unavailable") return errors.New("address unavailable")
} }
return nil return nil
} }
} }
func validHostedEmail(config *validationConfig, be domainBackend, cb checkBackend) ValidatorFunc { func validHostedEmail(config *validationConfig, be domainBackend, cb Backend) ValidatorFunc {
return allOf( return allOf(
validateUsernameAndDomain( validateUsernameAndDomain(
allOf(matchUsernameRx(), minLength(4), notInSet(config.forbiddenUsernames)), allOf(matchUsernameRx(), minLength(4), notInSet(config.forbiddenUsernames)),
...@@ -255,7 +262,7 @@ func validHostedEmail(config *validationConfig, be domainBackend, cb checkBacken ...@@ -255,7 +262,7 @@ func validHostedEmail(config *validationConfig, be domainBackend, cb checkBacken
) )
} }
func validHostedMailingList(config *validationConfig, be domainBackend, cb checkBackend) ValidatorFunc { func validHostedMailingList(config *validationConfig, be domainBackend, cb Backend) ValidatorFunc {
return allOf( return allOf(
validateUsernameAndDomain( validateUsernameAndDomain(
allOf(matchUsernameRx(), minLength(4), notInSet(config.forbiddenUsernames)), allOf(matchUsernameRx(), minLength(4), notInSet(config.forbiddenUsernames)),
......
...@@ -2,6 +2,7 @@ package accountserver ...@@ -2,6 +2,7 @@ package accountserver
import ( import (
"context" "context"
"fmt"
"testing" "testing"
) )
...@@ -59,19 +60,37 @@ func TestValidator_MatchUsername(t *testing.T) { ...@@ -59,19 +60,37 @@ func TestValidator_MatchUsername(t *testing.T) {
runValidationTest(t, matchUsernameRx(), td) runValidationTest(t, matchUsernameRx(), td)
} }
type fakeCheckBackend map[string]struct{} type fakeCheckBackend struct {
// We need this just to satisfy the Backend interface.
*fakeBackend
func newFakeCheckBackend(rids ...string) fakeCheckBackend { m map[string]struct{}
f := make(fakeCheckBackend) }
for _, rid := range rids {
f[rid] = struct{}{} type fakeCheckTX struct {
TX
m map[string]struct{}
}
func newFakeCheckBackend(seeds []FindResourceRequest) *fakeCheckBackend {
f := &fakeCheckBackend{
fakeBackend: createFakeBackend(),
m: make(map[string]struct{}),
}
for _, s := range seeds {
f.m[fmt.Sprintf("%s/%s", s.Type, s.Name)] = struct{}{}
} }
return f return f
} }
func (f fakeCheckBackend) HasAnyResource(_ context.Context, ids []string) (bool, error) { func (f *fakeCheckBackend) NewTransaction() (TX, error) {
for _, id := range ids { tx, _ := f.fakeBackend.NewTransaction()
if _, ok := f[id]; ok { return &fakeCheckTX{TX: tx, m: f.m}, nil
}
func (f *fakeCheckTX) HasAnyResource(_ context.Context, reqs []FindResourceRequest) (bool, error) {
for _, req := range reqs {
if _, ok := f.m[fmt.Sprintf("%s/%s", req.Type, req.Name)]; ok {
return true, nil return true, nil
} }
} }
...@@ -105,7 +124,9 @@ func TestValidator_HostedEmail(t *testing.T) { ...@@ -105,7 +124,9 @@ func TestValidator_HostedEmail(t *testing.T) {
{"existing@example.com", false}, {"existing@example.com", false},
} }
vc := newTestValidationConfig("forbidden") vc := newTestValidationConfig("forbidden")
cb := newFakeCheckBackend("email/existing@example.com") cb := newFakeCheckBackend([]FindResourceRequest{
{Type: ResourceTypeEmail, Name: "existing@example.com"},
})
db := newFakeDomainBackend("example.com") db := newFakeDomainBackend("example.com")
runValidationTest(t, validHostedEmail(vc, db, cb), td) runValidationTest(t, validHostedEmail(vc, db, cb), td)
} }
...@@ -121,7 +142,9 @@ func TestValidator_HostedMailingList(t *testing.T) { ...@@ -121,7 +142,9 @@ func TestValidator_HostedMailingList(t *testing.T) {
{"existing@domain2.com", false}, {"existing@domain2.com", false},
} }
vc := newTestValidationConfig("forbidden") vc := newTestValidationConfig("forbidden")
cb := newFakeCheckBackend("list/existing@domain2.com") cb := newFakeCheckBackend([]FindResourceRequest{
{Type: ResourceTypeMailingList, Name: "existing@domain2.com"},
})
db := newFakeDomainBackend("domain1.com", "domain2.com") db := newFakeDomainBackend("domain1.com", "domain2.com")
runValidationTest(t, validHostedMailingList(vc, db, cb), td) runValidationTest(t, validHostedMailingList(vc, db, cb), td)
} }
Supports Markdown
0% or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment