Merge branch 'opaque-resource-ids' into 'master'

Opaque resource ids

Closes #5

See merge request !3

actions.go

@@ -181,11 +181,11 @@ func (r *ResourceRequestBase) PopulateContext(rctx *RequestContext) error {
 	if err != nil {
 		return err
 	}
-	rctx.Resource = rsrc
+	rctx.Resource = &rsrc.Resource
 
 	// If the resource has an owner, populate the User context field.
-	if owner := rsrc.ID.User(); owner != "" {
-		user, err := getUserOrDie(rctx.Context, rctx.TX, owner)
+	if rsrc.Owner != "" {
+		user, err := getUserOrDie(rctx.Context, rctx.TX, rsrc.Owner)
 		if err != nil {
 			return err
 		}
@@ -197,27 +197,12 @@ func (r *ResourceRequestBase) PopulateContext(rctx *RequestContext) error {
 
 // Authorize the request.
 func (r *ResourceRequestBase) Authorize(rctx *RequestContext) error {
-	if !rctx.isAdmin(rctx.SSO) && !canAccessResource(rctx.SSO.User, rctx.Resource) {
+	if !rctx.isAdmin(rctx.SSO) && !rctx.TX.CanAccessResource(rctx.Context, rctx.SSO.User, rctx.Resource) {
 		return fmt.Errorf("user %s can't access resource %s", rctx.SSO.User, rctx.Resource.ID)
 	}
 	return nil
 }
 
-func canAccessResource(username string, r *Resource) bool {
-	switch r.ID.Type() {
-	case ResourceTypeMailingList:
-		// Check the list owners.
-		for _, a := range r.List.Admins {
-			if a == username {
-				return true
-			}
-		}
-		return false
-	default:
-		return r.ID.User() == username
-	}
-}
-
 // AdminResourceRequestBase is an admin-only version of ResourceRequestBase.
 type AdminResourceRequestBase struct {
 	ResourceRequestBase

actions_create.go

@@ -2,7 +2,7 @@ package accountserver
 
 import (
 	"context"
-	"errors"
+	"fmt"
 	"log"
 
 	"git.autistici.org/ai3/go-common/pwhash"
@@ -12,94 +12,110 @@ import (
 type CreateResourcesRequest struct {
 	AdminRequestBase
 
+	// Username the resources will belong to (optional).
+	Username string `json:"username"`
+
 	// Resources to create. All must either be global resources
 	// (no user ownership), or belong to the same user.
 	Resources []*Resource `json:"resources"`
 }
 
+// PopulateContext extracts information from the request and stores it
+// into the RequestContext.
+func (r *CreateResourcesRequest) PopulateContext(rctx *RequestContext) error {
+	if r.Username != "" {
+		user, err := getUserOrDie(rctx.Context, rctx.TX, r.Username)
+		if err != nil {
+			return err
+		}
+		rctx.User = user
+	}
+	return r.AdminRequestBase.PopulateContext(rctx)
+}
+
 // CreateResourcesResponse is the response type for CreateResourcesRequest.
 type CreateResourcesResponse struct {
 	// Resources that were created.
 	Resources []*Resource `json:"resources"`
 }
 
-func (r *CreateResourcesRequest) getOwner(rctx *RequestContext) (*RawUser, error) {
-	// Fetch the user associated with the first resource (if
-	// any). Since resource validation might reference other
-	// resources, we need to provide it with a view of what the
-	// future resources will be. So we merge the resources from
-	// the database with those from the request, using a local
-	// copy of the User object.
-	if len(r.Resources) > 0 {
-		if owner := r.Resources[0].ID.User(); owner != "" {
-			u, err := getUserOrDie(rctx.Context, rctx.TX, owner)
-			if err != nil {
-				return nil, err
+// Merge two resource lists by ID, return a new list. If a resource is
+// duplicated (detected by type/name matching), return an error.
+func mergeResources(a, b []*Resource) ([]*Resource, error) {
+	tmp := make(map[string]struct{})
+	var out []*Resource
+	for _, l := range [][]*Resource{a, b} {
+		for _, r := range l {
+			key := r.String()
+			if _, seen := tmp[key]; seen {
+				return nil, fmt.Errorf("resource %s already exists", key)
 			}
-			user := *u
-			user.Resources = mergeResources(u.Resources, r.Resources)
-			return &user, nil
+			tmp[key] = struct{}{}
+			out = append(out, r)
 		}
 	}
-	return nil, nil
+	return out, nil
 }
 
 // Validate the request.
 func (r *CreateResourcesRequest) Validate(rctx *RequestContext) error {
-	var owner string
-	user, err := r.getOwner(rctx)
-	if err != nil {
-		return err
-	}
 	var tplUser *User
-	if user != nil {
-		owner = user.Name
-		tplUser = &user.User
+
+	if rctx.User != nil {
+		// To provide resource validators with a view of what the User should be
+		// with the new resources, we merge the ones from the database with the
+		// ones from the request. This is also a good time to check for
+		// uniqueness (even though it would fail at commit time anyway).
+		merged, err := mergeResources(rctx.User.Resources, r.Resources)
+		if err != nil {
+			return err
+		}
+		userCopy := rctx.User.User
+		userCopy.Resources = merged
+		tplUser = &userCopy
 	}
 
 	for _, rsrc := range r.Resources {
-		rctx.resourceTemplates.applyTemplate(rctx.Context, rsrc, tplUser)
-
-		// Check same-user ownership.
-		if rsrc.ID.User() != owner {
-			return errors.New("resources are owned by different users")
+		// Apply resource templates.
+		if err := rctx.resourceTemplates.applyTemplate(rctx.Context, rsrc, tplUser); err != nil {
+			return err
 		}
 
 		// Validate the resource.
 		if err := rctx.resourceValidator.validateResource(rctx.Context, rsrc, tplUser); err != nil {
-			log.Printf("validation error while creating resource %s: %v", rsrc.ID, err)
+			log.Printf("validation error while creating resource %s: %v", rsrc.String(), err)
+			return err
+		}
+	}
+
+	if tplUser != nil {
+		// If the resource has an owner, validate it (checks that the new
+		// resources do not violate user invariants).
+		if err := checkUserInvariants(tplUser); err != nil {
+			log.Printf("validation error while creating resources: %v", err)
 			return err
 		}
 	}
+
 	return nil
 }
 
 // Serve the request.
 func (r *CreateResourcesRequest) Serve(rctx *RequestContext) (interface{}, error) {
-	var resp CreateResourcesResponse
-	for _, rsrc := range r.Resources {
-		if err := rctx.TX.CreateResource(rctx.Context, rsrc); err != nil {
-			return nil, err
-		}
-		rctx.audit.Log(rctx, rsrc.ID, "resource created")
-		resp.Resources = append(resp.Resources, rsrc)
+	var user *User
+	if rctx.User != nil {
+		user = &rctx.User.User
 	}
-	return &resp, nil
-}
-
-// Merge two resource lists by ID (the second one wins), return a new list.
-func mergeResources(a, b []*Resource) []*Resource {
-	tmp := make(map[string]*Resource)
-	for _, l := range [][]*Resource{a, b} {
-		for _, r := range l {
-			tmp[r.ID.String()] = r
-		}
+	rsrcs, err := rctx.TX.CreateResources(rctx.Context, user, r.Resources)
+	if err != nil {
+		return nil, err
 	}
-	out := make([]*Resource, 0, len(tmp))
-	for _, r := range tmp {
-		out = append(out, r)
+	for _, rsrc := range rsrcs {
+		rctx.audit.Log(rctx, rsrc, "resource created")
 	}
-	return out
+	return &CreateResourcesResponse{
+		Resources: rsrcs,
+	}, nil
 }
 
 // CreateUserRequest lets administrators create a new user along with the
@@ -135,7 +151,13 @@ func (r *CreateUserRequest) applyTemplate(rctx *RequestContext) error {
 
 	// Apply templates to all resources in the request.
 	for _, rsrc := range r.User.Resources {
-		rctx.resourceTemplates.applyTemplate(rctx.Context, rsrc, r.User)
+		if err := rctx.resourceTemplates.applyTemplate(rctx.Context, rsrc, r.User); err != nil {
+			return err
+		}
+		// Set the user shard to match the email resource shard.
+		if rsrc.Type == ResourceTypeEmail {
+			r.User.Shard = rsrc.Shard
+		}
 	}
 
 	return nil
@@ -149,36 +171,17 @@ func (r *CreateUserRequest) Validate(rctx *RequestContext) error {
 
 	// Validate the user *and* all resources.  The request must contain at
 	// least one email resource with the same name as the user.
-	if err := rctx.userValidator(rctx.Context, r.User); err != nil {
-		log.Printf("validation error while creating user %+v: %v", r.User, err)
-		return err
-	}
-	var emailCount int
 	for _, rsrc := range r.User.Resources {
 		if err := rctx.resourceValidator.validateResource(rctx.Context, rsrc, r.User); err != nil {
 			log.Printf("validation error while creating resource %+v: %v", rsrc, err)
 			return err
 		}
-		if rsrc.ID.Type() == ResourceTypeEmail {
-			emailCount++
-		}
-	}
-	if emailCount == 0 {
-		return errors.New("missing email resource")
-	}
-	if emailCount > 1 {
-		return errors.New("too many email resources")
 	}
-	email := r.User.GetSingleResourceByType(ResourceTypeEmail)
-	if email.Name != r.User.Name {
-		return errors.New("user and email resource names do not match")
+	if err := rctx.userValidator(rctx.Context, r.User); err != nil {
+		log.Printf("validation error while creating user %+v: %v", r.User, err)
+		return err
 	}
 
-	// Now that validation is done, finalize the object by setting some derived parameters.
-
-	// Set the user shard to the email shard.
-	r.User.Shard = email.Shard
-
 	return nil
 }
 
@@ -200,17 +203,18 @@ func (r *CreateUserRequest) Serve(rctx *RequestContext) (interface{}, error) {
 	var resp CreateUserResponse
 
 	// Create the user first, along with all the resources.
-	if err := rctx.TX.CreateUser(rctx.Context, r.User); err != nil {
+	user, err := rctx.TX.CreateUser(rctx.Context, r.User)
+	if err != nil {
 		return nil, err
 	}
-	resp.User = r.User
+	resp.User = user
 
 	// Now set a password for the user and return it, and
 	// set random passwords for all the resources
 	// (currently, we don't care about those, the user
 	// will reset them later). However, we could return
 	// them in the response as well, if necessary.
-	u := &RawUser{User: *r.User}
+	u := &RawUser{User: *user}
 	newPassword := randomPassword()
 	if err := u.resetPassword(rctx.Context, rctx.TX, newPassword); err != nil {
 		return nil, err
@@ -219,15 +223,15 @@ func (r *CreateUserRequest) Serve(rctx *RequestContext) (interface{}, error) {
 
 	// Fake a RawUser in the RequestContext just for the purpose
 	// of audit logging.
-	rctx.User = &RawUser{User: *r.User}
-	rctx.audit.Log(rctx, ResourceID{}, "user created")
+	rctx.User = u
+	rctx.audit.Log(rctx, nil, "user created")
 
 	for _, rsrc := range r.User.Resources {
-		rctx.audit.Log(rctx, rsrc.ID, "resource created")
+		rctx.audit.Log(rctx, rsrc, "resource created")
 		if resourceHasPassword(rsrc) {
 			if _, err := doResetResourcePassword(rctx.Context, rctx.TX, rsrc); err != nil {
 				// Just log, don't fail.
-				log.Printf("can't set random password for resource %s: %v", rsrc.ID, err)
+				log.Printf("can't set random password for resource %s: %v", rsrc.String(), err)
 			}
 		}
 	}
@@ -235,6 +239,15 @@ func (r *CreateUserRequest) Serve(rctx *RequestContext) (interface{}, error) {
 	return &resp, nil
 }
 
+func resourceHasPassword(r *Resource) bool {
+	switch r.Type {
+	case ResourceTypeDAV, ResourceTypeDatabase, ResourceTypeMailingList:
+		return true
+	default:
+		return false
+	}
+}
+
 func doResetResourcePassword(ctx context.Context, tx TX, rsrc *Resource) (string, error) {
 	newPassword := randomPassword()
 	encPassword := pwhash.Encrypt(newPassword)

actions_resource.go

@@ -13,7 +13,7 @@ func setResourceStatus(rctx *RequestContext, status string) error {
 	if err := rctx.TX.UpdateResource(rctx.Context, rsrc); err != nil {
 		return err
 	}
-	rctx.audit.Log(rctx, rsrc.ID, fmt.Sprintf("status set to %s", status))
+	rctx.audit.Log(rctx, rsrc, fmt.Sprintf("status set to %s", status))
 	return nil
 }
 
@@ -50,18 +50,9 @@ type ResetResourcePasswordResponse struct {
 	Password string `json:"password"`
 }
 
-func resourceHasPassword(r *Resource) bool {
-	switch r.ID.Type() {
-	case ResourceTypeDAV, ResourceTypeDatabase, ResourceTypeMailingList:
-		return true
-	default:
-		return false
-	}
-}
-
 // Validate the request.
-func (r *ResetResourcePasswordRequest) Validate(_ *RequestContext) error {
-	switch r.ResourceID.Type() {
+func (r *ResetResourcePasswordRequest) Validate(rctx *RequestContext) error {
+	switch rctx.Resource.Type {
 	case ResourceTypeDAV, ResourceTypeDatabase, ResourceTypeMailingList:
 	case ResourceTypeEmail:
 		return errors.New("can't reset email passwords with this API")
@@ -118,7 +109,7 @@ func (r *MoveResourceRequest) Serve(rctx *RequestContext) (interface{}, error) {
 	// We need to enforce consistency between email resources and
 	// the user shard, so that temporary data can be colocated
 	// with email storage.
-	if rctx.Resource.ID.Type() == ResourceTypeEmail && rctx.User.Shard != r.Shard {
+	if rctx.Resource.Type == ResourceTypeEmail && rctx.User.Shard != r.Shard {
 		rctx.User.Shard = r.Shard
 		if err := rctx.TX.UpdateUser(rctx.Context, &rctx.User.User); err != nil {
 			return nil, err
@@ -144,7 +135,7 @@ type AddEmailAliasRequest struct {
 
 // Validate the request.
 func (r *AddEmailAliasRequest) Validate(rctx *RequestContext) error {
-	if rctx.Resource.ID.Type() != ResourceTypeEmail {
+	if rctx.Resource.Type != ResourceTypeEmail {
 		return errors.New("this operation only works on email resources")
 	}
 
@@ -168,7 +159,7 @@ func (r *AddEmailAliasRequest) Serve(rctx *RequestContext) (interface{}, error)
 		return nil, err
 	}
 
-	rctx.audit.Log(rctx, r.ResourceID, fmt.Sprintf("added alias %s", r.Addr))
+	rctx.audit.Log(rctx, rctx.Resource, fmt.Sprintf("added alias %s", r.Addr))
 	return nil, nil
 }
 
@@ -180,7 +171,7 @@ type DeleteEmailAliasRequest struct {
 
 // Validate the request.
 func (r *DeleteEmailAliasRequest) Validate(rctx *RequestContext) error {
-	if rctx.Resource.ID.Type() != ResourceTypeEmail {
+	if rctx.Resource.Type != ResourceTypeEmail {
 		return errors.New("this operation only works on email resources")
 	}
 	return nil
@@ -199,6 +190,6 @@ func (r *DeleteEmailAliasRequest) Serve(rctx *RequestContext) (interface{}, erro
 		return nil, err
 	}
 
-	rctx.audit.Log(rctx, r.ResourceID, fmt.Sprintf("removed alias %s", r.Addr))
+	rctx.audit.Log(rctx, rctx.Resource, fmt.Sprintf("removed alias %s", r.Addr))
 	return nil, nil
 }

actions_test.go

@@ -3,15 +3,19 @@ package accountserver
 import (
 	"context"
 	"errors"
+	"fmt"
+	"strings"
 	"testing"
 
 	"git.autistici.org/ai3/go-common/pwhash"
 	sso "git.autistici.org/id/go-sso"
 )
 
+const testUser = "testuser@example.com"
+
 type fakeBackend struct {
 	users                map[string]*User
-	resources            map[string]map[string]*Resource
+	resources            map[string]*Resource
 	passwords            map[string]string
 	recoveryPasswords    map[string]string
 	appSpecificPasswords map[string][]*AppSpecificPasswordInfo
@@ -30,8 +34,16 @@ func (b *fakeBackend) NextUID(_ context.Context) (int, error) {
 	return 42, nil
 }
 
+func (b *fakeBackend) CanAccessResource(_ context.Context, username string, rsrc *Resource) bool {
+	owner := strings.Split(string(rsrc.ID), "/")[0]
+	return owner == username
+}
+
 func (b *fakeBackend) GetUser(_ context.Context, username string) (*RawUser, error) {
-	u := b.users[username]
+	u, ok := b.users[username]
+	if !ok {
+		return nil, errors.New("user not found in fake backend")
+	}
 	return &RawUser{
 		User:             *u,
 		Password:         b.passwords[username],
@@ -45,27 +57,50 @@ func (b *fakeBackend) UpdateUser(_ context.Context, user *User) error {
 	return nil
 }
 
-func (b *fakeBackend) CreateUser(_ context.Context, user *User) error {
+func (b *fakeBackend) CreateUser(_ context.Context, user *User) (*User, error) {
+	for _, r := range user.Resources {
+		r.ID = makeResourceID(user.Name, r.Type, r.Name)
+	}
 	b.users[user.Name] = user
-	return nil
+	return user, nil
 }
 
-func (b *fakeBackend) GetResource(_ context.Context, resourceID ResourceID) (*Resource, error) {
-	return b.resources[resourceID.User()][resourceID.String()], nil
+func (b *fakeBackend) GetResource(_ context.Context, resourceID ResourceID) (*RawResource, error) {
+	owner := strings.Split(resourceID.String(), "/")[0]
+	r := b.resources[resourceID.String()]
+	return &RawResource{Resource: *r, Owner: owner}, nil
 }
 
 func (b *fakeBackend) UpdateResource(_ context.Context, r *Resource) error {
-	b.resources[r.ID.User()][r.ID.String()] = r
+	b.resources[r.ID.String()] = r
 	return nil
 }
 
-func (b *fakeBackend) CreateResource(_ context.Context, r *Resource) error {
-	if _, ok := b.resources[r.ID.User()][r.ID.String()]; ok {
-		return errors.New("resource already exists")
-	}
+func makeResourceID(owner, rtype, rname string) ResourceID {
+	return ResourceID(fmt.Sprintf("%s/%s/%s", owner, rtype, rname))
+}
 
-	b.resources[r.ID.User()][r.ID.String()] = r
-	return nil
+func (b *fakeBackend) CreateResources(_ context.Context, u *User, rsrcs []*Resource) ([]*Resource, error) {
+	var out []*Resource
+	for _, r := range rsrcs {
+		if !r.ID.Empty() {
+			return nil, errors.New("resource ID not empty")
+		}
+
+		var username string
+		if u != nil {
+			username = u.Name
+		}
+		r.ID = makeResourceID(username, r.Type, r.Name)
+
+		if _, ok := b.resources[r.ID.String()]; ok {
+			return nil, errors.New("resource already exists")
+		}
+
+		b.resources[r.ID.String()] = r
+		out = append(out, r)
+	}
+	return out, nil
 }
 
 func (b *fakeBackend) SetUserPassword(_ context.Context, user *User, password string) error {
@@ -123,11 +158,9 @@ func (b *fakeBackend) DeleteUserTOTPSecret(_ context.Context, user *User) error
 
 func (b *fakeBackend) HasAnyResource(_ context.Context, rsrcs []FindResourceRequest) (bool, error) {
 	for _, fr := range rsrcs {
-		for _, ur := range b.resources {
-			for _, r := range ur {
-				if r.ID.Type() == fr.Type && r.ID.Name() == fr.Name {
-					return true, nil
-				}
+		for _, r := range b.resources {
+			if r.Type == fr.Type && r.Name == fr.Name {
+				return true, nil
 			}
 		}
 	}
@@ -160,37 +193,35 @@ func (v *fakeValidator) Validate(tkt, nonce, service string, _ []string) (*sso.T
 
 func (b *fakeBackend) addUser(user *User, pw, rpw string) {
 	b.users[user.Name] = user
-	b.resources[user.Name] = make(map[string]*Resource)
+	//b.resources[user.Name] = make(map[string]*Resource)
 	b.passwords[user.Name] = pwhash.Encrypt(pw)
 	if rpw != "" {
 		b.recoveryPasswords[user.Name] = pwhash.Encrypt(rpw)
 	}
 	for _, r := range user.Resources {
-		b.resources[user.Name][r.ID.String()] = r
+		b.resources[r.ID.String()] = r
 	}
 }
 
 func createFakeBackend() *fakeBackend {
 	fb := &fakeBackend{
-		users: make(map[string]*User),
-		resources: map[string]map[string]*Resource{
-			// For global (user-less) resources, where CreateUser is not called.
-			"": make(map[string]*Resource),
-		},
+		users:                make(map[string]*User),
+		resources:            make(map[string]*Resource),
 		passwords:            make(map[string]string),
 		recoveryPasswords:    make(map[string]string),
 		appSpecificPasswords: make(map[string][]*AppSpecificPasswordInfo),
 		encryptionKeys:       make(map[string][]*UserEncryptionKey),
 	}
 	fb.addUser(&User{
-		Name:   "testuser",
+		Name:   testUser,
 		Status: UserStatusActive,
 		Shard:  "1",
 		UID:    4242,
 		Resources: []*Resource{
 			{
-				ID:     NewResourceID(ResourceTypeEmail, "testuser", "testuser@example.com"),
-				Name:   "testuser@example.com",
+				ID:     makeResourceID(testUser, ResourceTypeEmail, testUser),
+				Name:   testUser,
+				Type:   ResourceTypeEmail,
 				Status: ResourceStatusActive,
 				Shard:  "1",
 				Email: &Email{
@@ -198,10 +229,12 @@ func createFakeBackend() *fakeBackend {
 				},
 			},
 			{
-				ID:     NewResourceID(ResourceTypeDAV, "testuser", "dav1"),
+				ID:     makeResourceID(testUser, ResourceTypeDAV, "dav1"),
 				Name:   "dav1",
+				Type:   ResourceTypeDAV,
 				Status: ResourceStatusActive,
 				DAV: &WebDAV{
+					UID:     4242,
 					Homedir: "/home/dav1",
 				},
 			},
@@ -237,23 +270,28 @@ func testService(admin string) *AccountService {
 	return svc
 }
 
-func TestService_GetUser(t *testing.T) {
-	svc := testService("")
-
+func getUser(t testing.TB, svc *AccountService, username string) *User {
 	req := &GetUserRequest{
 		UserRequestBase: UserRequestBase{
 			RequestBase: RequestBase{
-				SSO: "testuser",
+				SSO: testUser,
 			},
-			Username: "testuser",
+			Username: testUser,
 		},
 	}
 	resp, err := svc.Handle(context.TODO(), req)
 	if err != nil {
 		t.Fatal(err)
 	}
-	if resp.(*User).Name != "testuser" {
-		t.Fatalf("bad response: %+v", resp)
+	return resp.(*User)
+}
+
+func TestService_GetUser(t *testing.T) {
+	svc := testService("")
+
+	user := getUser(t, svc, testUser)
+	if user.Name != testUser {
+		t.Fatalf("bad response: %+v", user)
 	}
 }
 
@@ -266,36 +304,41 @@ func TestService_GetUser_ResourceGroups(t *testing.T) {
 		Status: UserStatusActive,
 		Resources: []*Resource{
 			{