Merge pull request #627 from butonic/add-basic-auth-option

add enable basic auth option and check permissions

.drone.star

@@ -362,7 +362,7 @@ def localApiTests(ctx, coreBranch = 'master', coreCommit = '', storage = 'ownclo
           'OCIS_SKELETON_STRATEGY': '%s' % ('copy' if storage == 'owncloud' else 'upload'),
           'TEST_OCIS':'true',
           'BEHAT_FILTER_TAGS': '~@skipOnOcis-%s-Storage' % ('OC' if storage == 'owncloud' else 'OCIS'),
-          'PATH_TO_CORE': '/srv/app/testrunner'
+          'PATH_TO_CORE': '/srv/app/testrunner',
         },
         'commands': [
           'cd ocis',
@@ -419,7 +419,7 @@ def coreApiTests(ctx, coreBranch = 'master', coreCommit = '', part_number = 1, n
           'BEHAT_FILTER_TAGS': '~@notToImplementOnOCIS&&~@toImplementOnOCIS&&~comments-app-required&&~@federation-app-required&&~@notifications-app-required&&~systemtags-app-required&&~@local_storage&&~@skipOnOcis-%s-Storage' % ('OC' if storage == 'owncloud' else 'OCIS'),
           'DIVIDE_INTO_NUM_PARTS': number_of_parts,
           'RUN_PART': part_number,
-          'EXPECTED_FAILURES_FILE': '/drone/src/ocis/tests/acceptance/expected-failures-on-%s-storage.txt' % (storage.upper())
+          'EXPECTED_FAILURES_FILE': '/drone/src/ocis/tests/acceptance/expected-failures-on-%s-storage.txt' % (storage.upper()),
         },
         'commands': [
           'cd /srv/app/testrunner',
@@ -1406,6 +1406,7 @@ def ocisServer(storage):
         'STORAGE_DATAGATEWAY_PUBLIC_URL': 'https://ocis-server:9200/data',
         'STORAGE_USERS_DATA_SERVER_URL': 'http://ocis-server:9158/data',
         'STORAGE_FRONTEND_PUBLIC_URL': 'https://ocis-server:9200',
+        'PROXY_ENABLE_BASIC_AUTH': True,
         'PHOENIX_WEB_CONFIG': '/drone/src/ocis/tests/config/drone/ocis-config.json',
         'KONNECTD_IDENTIFIER_REGISTRATION_CONF': '/drone/src/ocis/tests/config/drone/identifier-registration.yml',
         'KONNECTD_ISS': 'https://ocis-server:9200',

accounts/pkg/proto/v0/accounts.pb.micro_test.go

@@ -630,7 +630,7 @@ func TestListAccounts(t *testing.T) {
 	assert.NoError(t, err)
 
 	assert.IsType(t, &proto.ListAccountsResponse{}, resp)
-	assert.Equal(t, 8, len(resp.Accounts))
+	assert.Equal(t, 9, len(resp.Accounts))
 
 	assertResponseContainsUser(t, resp, getAccount("user1"))
 	assertResponseContainsUser(t, resp, getAccount("user2"))
@@ -642,8 +642,8 @@ func TestListWithoutUserCreation(t *testing.T) {
 	resp, err := listAccounts(t)
 	assert.NoError(t, err)
 
-	// Only 5 default users
-	assert.Equal(t, 6, len(resp.Accounts))
+	// Only 7 default users
+	assert.Equal(t, 7, len(resp.Accounts))
 	cleanUp(t)
 }
 

accounts/pkg/service/v0/accounts.go

@@ -16,8 +16,10 @@ import (
 	"github.com/golang/protobuf/ptypes/empty"
 	fieldmask_utils "github.com/mennanov/fieldmask-utils"
 	merrors "github.com/micro/go-micro/v2/errors"
+	"github.com/micro/go-micro/v2/metadata"
 	"github.com/owncloud/ocis/accounts/pkg/proto/v0"
 	"github.com/owncloud/ocis/accounts/pkg/storage"
+	"github.com/owncloud/ocis/ocis-pkg/middleware"
 	"github.com/owncloud/ocis/ocis-pkg/roles"
 	settings "github.com/owncloud/ocis/settings/pkg/proto/v0"
 	settings_svc "github.com/owncloud/ocis/settings/pkg/service/v0"
@@ -64,17 +66,32 @@ func (s Service) hasAccountManagementPermissions(ctx context.Context) bool {
 	roleIDs, ok := roles.ReadRoleIDsFromContext(ctx)
 	if !ok {
 		/**
-		 * FIXME: with this we are skipping permission checks on all requests that are coming in without roleIDs in the
-		 * metadata context. This is a huge security impairment, as that's the case not only for grpc requests but also
-		 * for unauthenticated http requests and http requests coming in without hitting the ocis-proxy first.
+		* FIXME: with this we are skipping permission checks on all requests that are coming in without roleIDs in the
+		* metadata context. This is a huge security impairment, as that's the case not only for grpc requests but also
+		* for unauthenticated http requests and http requests coming in without hitting the ocis-proxy first.
 		 */
+		// TODO add system role for internal requests.
+		// - at least the proxy needs to look up account info
+		// - glauth needs to make bind requests
+		// tracked as OCIS-454
 		return true
 	}
 
 	// check if permission is present in roles of the authenticated account
 	return s.RoleManager.FindPermissionByID(ctx, roleIDs, AccountManagementPermissionID) != nil
 }
 
+func (s Service) hasSelfManagementPermissions(ctx context.Context) bool {
+	// get roles from context
+	roleIDs, ok := roles.ReadRoleIDsFromContext(ctx)
+	if !ok {
+		return false
+	}
+
+	// check if permission is present in roles of the authenticated account
+	return s.RoleManager.FindPermissionByID(ctx, roleIDs, SelfManagementPermissionID) != nil
+}
+
 // serviceUserToIndex temporarily adds a service user to the index, which is supposed to be removed before the lock on the handler function is released
 func (s Service) serviceUserToIndex() (teardownServiceUser func()) {
 	if s.Config.ServiceUser.Username != "" && s.Config.ServiceUser.UUID != "" {
@@ -105,9 +122,12 @@ func (s Service) getInMemoryServiceUser() proto.Account {
 // ListAccounts implements the AccountsServiceHandler interface
 // the query contains account properties
 func (s Service) ListAccounts(ctx context.Context, in *proto.ListAccountsRequest, out *proto.ListAccountsResponse) (err error) {
-	if !s.hasAccountManagementPermissions(ctx) {
+	hasSelf := s.hasSelfManagementPermissions(ctx)
+	hasManagement := s.hasAccountManagementPermissions(ctx)
+	if !hasSelf && !hasManagement {
 		return merrors.Forbidden(s.id, "no permission for ListAccounts")
 	}
+	onlySelf := hasSelf && !hasManagement
 
 	accLock.Lock()
 	defer accLock.Unlock()
@@ -146,6 +166,15 @@ func (s Service) ListAccounts(ctx context.Context, in *proto.ListAccountsRequest
 		return nil
 	}
 
+	if onlySelf {
+		// limit list to own account id
+		if aid, ok := metadata.Get(ctx, middleware.AccountID); ok {
+			in.Query = "id eq '" + aid + "'"
+		} else {
+			return merrors.InternalServerError(s.id, "account id not in context")
+		}
+	}
+
 	if in.Query == "" {
 		err = s.repo.LoadAccounts(ctx, &out.Accounts)
 		if err != nil {
@@ -202,9 +231,12 @@ func (s Service) findAccountsByQuery(ctx context.Context, query string) ([]strin
 
 // GetAccount implements the AccountsServiceHandler interface
 func (s Service) GetAccount(ctx context.Context, in *proto.GetAccountRequest, out *proto.Account) (err error) {
-	if !s.hasAccountManagementPermissions(ctx) {
+	hasSelf := s.hasSelfManagementPermissions(ctx)
+	hasManagement := s.hasAccountManagementPermissions(ctx)
+	if !hasSelf && !hasManagement {
 		return merrors.Forbidden(s.id, "no permission for GetAccount")
 	}
+	onlySelf := hasSelf && !hasManagement
 
 	accLock.Lock()
 	defer accLock.Unlock()
@@ -213,6 +245,17 @@ func (s Service) GetAccount(ctx context.Context, in *proto.GetAccountRequest, ou
 		return merrors.InternalServerError(s.id, "could not clean up account id: %v", err.Error())
 	}
 
+	if onlySelf {
+		// limit get to own account id
+		if aid, ok := metadata.Get(ctx, middleware.AccountID); ok {
+			if id != aid {
+				return merrors.Forbidden(s.id, "no permission for GetAccount of another user")
+			}
+		} else {
+			return merrors.InternalServerError(s.id, "account id not in context")
+		}
+	}
+
 	if err = s.repo.LoadAccount(ctx, id, out); err != nil {
 		if storage.IsNotFoundErr(err) {
 			return merrors.NotFound(s.id, "account not found: %v", err.Error())
@@ -268,7 +311,7 @@ func (s Service) CreateAccount(ctx context.Context, in *proto.CreateAccountReque
 		return merrors.InternalServerError(s.id, "could not check if account exists: %v", err.Error())
 	}
 	if exists {
-		return merrors.BadRequest(s.id, "account already exists")
+		return merrors.Conflict(s.id, "account already exists")
 	}
 
 	if out.PasswordProfile != nil {
@@ -298,7 +341,7 @@ func (s Service) CreateAccount(ctx context.Context, in *proto.CreateAccountReque
 	indexResults, err := s.index.Add(out)
 	if err != nil {
 		s.rollbackCreateAccount(ctx, out)
-		return merrors.BadRequest(s.id, "Account already exists %v", err.Error())
+		return merrors.Conflict(s.id, "Account already exists %v", err.Error())
 
 	}
 	s.log.Debug().Interface("account", out).Msg("account after indexing")
@@ -370,9 +413,12 @@ func (s Service) rollbackCreateAccount(ctx context.Context, acc *proto.Account)
 // read only fields are ignored
 // TODO how can we unset specific values? using the update mask
 func (s Service) UpdateAccount(ctx context.Context, in *proto.UpdateAccountRequest, out *proto.Account) (err error) {
-	if !s.hasAccountManagementPermissions(ctx) {
+	hasSelf := s.hasSelfManagementPermissions(ctx)
+	hasManagement := s.hasAccountManagementPermissions(ctx)
+	if !hasSelf && !hasManagement {
 		return merrors.Forbidden(s.id, "no permission for UpdateAccount")
 	}
+	onlySelf := hasSelf && !hasManagement
 
 	accLock.Lock()
 	defer accLock.Unlock()
@@ -388,14 +434,24 @@ func (s Service) UpdateAccount(ctx context.Context, in *proto.UpdateAccountReque
 		return merrors.InternalServerError(s.id, "could not clean up account id: %v", err.Error())
 	}
 
+	if onlySelf {
+		// limit update to own account id
+		if aid, ok := metadata.Get(ctx, middleware.AccountID); ok {
+			if id != aid {
+				return merrors.Forbidden(s.id, "no permission to UpdateAccount of another user")
+			}
+		} else {
+			return merrors.InternalServerError(s.id, "account id not in context")
+		}
+	}
+
 	if err = s.repo.LoadAccount(ctx, id, out); err != nil {
 		if storage.IsNotFoundErr(err) {
 			return merrors.NotFound(s.id, "account not found: %v", err.Error())
 		}
 
 		s.log.Error().Err(err).Str("id", id).Msg("could not load account")
 		return merrors.InternalServerError(s.id, "could not load account: %v", err.Error())
-
 	}
 
 	t := time.Now()
@@ -404,9 +460,15 @@ func (s Service) UpdateAccount(ctx context.Context, in *proto.UpdateAccountReque
 		Nanos:   int32(t.Nanosecond()),
 	}
 
-	validMask, err := validateUpdate(in.UpdateMask, updatableAccountPaths)
-	if err != nil {
-		return merrors.BadRequest(s.id, "%s", err)
+	var validMask fieldmask_utils.FieldFilterContainer
+	if onlySelf {
+		if validMask, err = validateUpdate(in.UpdateMask, selfUpdatableAccountPaths); err != nil {
+			return merrors.BadRequest(s.id, "%s", err)
+		}
+	} else {
+		if validMask, err = validateUpdate(in.UpdateMask, updatableAccountPaths); err != nil {
+			return merrors.BadRequest(s.id, "%s", err)
+		}
 	}
 
 	if _, exists := validMask.Filter("PreferredName"); exists {
@@ -490,6 +552,14 @@ func (s Service) UpdateAccount(ctx context.Context, in *proto.UpdateAccountReque
 	return
 }
 
+// whitelist of all paths/fields which can be updated by users themself
+var selfUpdatableAccountPaths = map[string]struct{}{
+	"DisplayName":              {},
+	"Description":              {},
+	"Mail":                     {}, // read only?,
+	"PasswordProfile.Password": {},
+}
+
 // whitelist of all paths/fields which can be updated by clients
 var updatableAccountPaths = map[string]struct{}{
 	"AccountEnabled":                   {},

accounts/pkg/service/v0/groups.go

@@ -50,25 +50,30 @@ func (s Service) deflateMembers(g *proto.Group) {
 }
 
 // ListGroups implements the GroupsServiceHandler interface
-func (s Service) ListGroups(c context.Context, in *proto.ListGroupsRequest, out *proto.ListGroupsResponse) (err error) {
-	var searchResults []string
-
-	out.Groups = make([]*proto.Group, 0)
+func (s Service) ListGroups(ctx context.Context, in *proto.ListGroupsRequest, out *proto.ListGroupsResponse) (err error) {
 	if in.Query == "" {
-		searchResults, _ = s.index.FindByPartial(&proto.Group{}, "DisplayName", "*")
-	}
+		err = s.repo.LoadGroups(ctx, &out.Groups)
+		if err != nil {
+			s.log.Err(err).Msg("failed to load all groups from storage")
+			return merrors.InternalServerError(s.id, "failed to load all groups")
+		}
+		for i := range out.Groups {
+			a := out.Groups[i]
+
+			// TODO add accounts only if requested
+			// if in.FieldMask ...
+			s.expandMembers(a)
 
-	/*
-		var startsWithIDQuery = regexp.MustCompile(`^startswith\(id,'(.*)'\)$`)
-		match := startsWithIDQuery.FindStringSubmatch(in.Query)
-		if len(match) == 2 {
-			searchResults = []string{match[1]}
 		}
-	*/
+		return nil
+	}
+
+	searchResults, err := s.findGroupsByQuery(ctx, in.Query)
+	out.Groups = make([]*proto.Group, 0, len(searchResults))
 
 	for _, hit := range searchResults {
 		g := &proto.Group{}
-		if err = s.repo.LoadGroup(c, hit, g); err != nil {
+		if err = s.repo.LoadGroup(ctx, hit, g); err != nil {
 			s.log.Error().Err(err).Str("group", hit).Msg("could not load group, skipping")
 			continue
 		}
@@ -83,6 +88,9 @@ func (s Service) ListGroups(c context.Context, in *proto.ListGroupsRequest, out
 
 	return
 }
+func (s Service) findGroupsByQuery(ctx context.Context, query string) ([]string, error) {
+	return s.index.Query(&proto.Group{}, query)
+}
 
 // GetGroup implements the GroupsServiceHandler interface
 func (s Service) GetGroup(c context.Context, in *proto.GetGroupRequest, out *proto.Group) (err error) {
@@ -249,8 +257,11 @@ func (s Service) AddMember(c context.Context, in *proto.AddMemberRequest, out *p
 			alreadyRelated = true
 		}
 	}
+	aref := &proto.Account{
+		Id: a.Id,
+	}
 	if !alreadyRelated {
-		g.Members = append(g.Members, a)
+		g.Members = append(g.Members, aref)
 	}
 
 	// check if we need to add the group to the account
@@ -261,8 +272,12 @@ func (s Service) AddMember(c context.Context, in *proto.AddMemberRequest, out *p
 			break
 		}
 	}
+	// only store the reference to prevent recurision when marshaling json
+	gref := &proto.Group{
+		Id: g.Id,
+	}
 	if !alreadyRelated {
-		a.MemberOf = append(a.MemberOf, g)
+		a.MemberOf = append(a.MemberOf, gref)
 	}
 
 	if err = s.repo.WriteAccount(c, a); err != nil {

accounts/pkg/service/v0/permissions.go

@@ -11,13 +11,17 @@ import (
 
 const (
 	// AccountManagementPermissionID is the hardcoded setting UUID for the account management permission
-	AccountManagementPermissionID   string = "8e587774-d929-4215-910b-a317b1e80f73"
+	AccountManagementPermissionID string = "8e587774-d929-4215-910b-a317b1e80f73"
 	// AccountManagementPermissionName is the hardcoded setting name for the account management permission
 	AccountManagementPermissionName string = "account-management"
 	// GroupManagementPermissionID is the hardcoded setting UUID for the group management permission
-	GroupManagementPermissionID     string = "522adfbe-5908-45b4-b135-41979de73245"
+	GroupManagementPermissionID string = "522adfbe-5908-45b4-b135-41979de73245"
 	// GroupManagementPermissionName is the hardcoded setting name for the group management permission
-	GroupManagementPermissionName   string = "group-management"
+	GroupManagementPermissionName string = "group-management"
+	// SelfManagementPermissionID is the hardcoded setting UUID for the self management permission
+	SelfManagementPermissionID string = "e03070e9-4362-4cc6-a872-1c7cb2eb2b8e"
+	// SelfManagementPermissionName is the hardcoded setting name for the self management permission
+	SelfManagementPermissionName string = "self-management"
 )
 
 // RegisterPermissions registers permissions for account management and group management with the settings service.
@@ -78,5 +82,24 @@ func generateAccountManagementPermissionsRequests() []settings.AddSettingToBundl
 				},
 			},
 		},
+		{
+			BundleId: ssvc.BundleUUIDRoleUser,
+			Setting: &settings.Setting{
+				Id:          SelfManagementPermissionID,
+				Name:        SelfManagementPermissionName,
+				DisplayName: "Self Management",
+				Description: "This permission gives access to self management.",
+				Resource: &settings.Resource{
+					Type: settings.Resource_TYPE_USER,
+					Id:   "me",
+				},
+				Value: &settings.Setting_PermissionValue{
+					PermissionValue: &settings.Permission{
+						Operation:  settings.Permission_OPERATION_READWRITE,
+						Constraint: settings.Permission_CONSTRAINT_OWN,
+					},
+				},
+			},
+		},
 	}
 }