diff --git a/app/controllers/concerns/membership_actions.rb b/app/controllers/concerns/membership_actions.rb
index 3c1156bf551fcc072741918b7ba9889b1e887f58..d6af279ae8ac56766158c01bcdda41b4d73915ee 100644
--- a/app/controllers/concerns/membership_actions.rb
+++ b/app/controllers/concerns/membership_actions.rb
@@ -11,27 +11,30 @@ def request_access
   def approve_access_request
     member = Members::ApproveAccessRequestService.new(membershipable, current_user, params).execute
 
+    ## EE specific
     log_audit_event(member, action: :create)
+    ## EE specific
 
     redirect_to polymorphic_url([membershipable, :members])
   end
 
   def leave
-    @member = membershipable.members.find_by(user_id: current_user) ||
-      membershipable.requesters.find_by(user_id: current_user)
-    Members::DestroyService.new(@member, current_user).execute
+    member = Members::DestroyService.new(membershipable, current_user, user_id: current_user.id).
+      execute(:all)
 
-    source_type = @member.real_source_type.humanize(capitalize: false)
+    source_type = membershipable.class.to_s.humanize(capitalize: false)
     notice =
-      if @member.request?
+      if member.request?
         "Your access request to the #{source_type} has been withdrawn."
       else
-        "You left the \"#{@member.source.human_name}\" #{source_type}."
+        "You left the \"#{membershipable.human_name}\" #{source_type}."
       end
 
-    log_audit_event(@member, action: :destroy) unless @member.request?
+    ## EE specific
+    log_audit_event(member, action: :destroy) unless member.request?
+    ## EE specific
 
-    redirect_path = @member.request? ? @member.source : [:dashboard, @member.real_source_type.tableize]
+    redirect_path = member.request? ? member.source : [:dashboard, membershipable.class.to_s.tableize]
 
     redirect_to redirect_path, notice: notice
   end
@@ -42,8 +45,10 @@ def membershipable
     raise NotImplementedError
   end
 
+  ## EE specific
   def log_audit_event(member, options = {})
     AuditEventService.new(current_user, membershipable, options).
       for_member(member).security_event
   end
+  ## EE specific
 end
diff --git a/app/controllers/groups/group_members_controller.rb b/app/controllers/groups/group_members_controller.rb
index a22ebe5f9916f40672f93eb858b0c4a4a41672fc..9df8d8392ff6f8a37091ed99ca05758bf5c2bf91 100644
--- a/app/controllers/groups/group_members_controller.rb
+++ b/app/controllers/groups/group_members_controller.rb
@@ -50,11 +50,11 @@ def update
   end
 
   def destroy
-    @group_member = @group.members.find_by(id: params[:id]) ||
-      @group.requesters.find_by(id: params[:id])
+    member = Members::DestroyService.new(@group, current_user, id: params[:id]).execute(:all)
 
-    Members::DestroyService.new(@group_member, current_user).execute
-    log_audit_event(@group_member, action: :destroy)
+    ## EE specific
+    log_audit_event(member, action: :destroy)
+    ## EE specific
 
     respond_to do |format|
       format.html { redirect_to group_group_members_path(@group), notice: 'User was successfully removed from group.' }
diff --git a/app/controllers/projects/project_members_controller.rb b/app/controllers/projects/project_members_controller.rb
index 9e3626f1a6e7614fd26c986ce011ebc2924eb27c..535e93ff7fc497b40de608ed47a6dd1cd0784657 100644
--- a/app/controllers/projects/project_members_controller.rb
+++ b/app/controllers/projects/project_members_controller.rb
@@ -65,12 +65,12 @@ def update
   end
 
   def destroy
-    @project_member = @project.members.find_by(id: params[:id]) ||
-      @project.requesters.find_by(id: params[:id])
+    member = Members::DestroyService.new(@project, current_user, params).
+      execute(:all)
 
-    Members::DestroyService.new(@project_member, current_user).execute
-
-    log_audit_event(@project_member, action: :destroy)
+    ## EE specific
+    log_audit_event(member, action: :destroy)
+    ## EE specific
 
     respond_to do |format|
       format.html do
diff --git a/app/models/group.rb b/app/models/group.rb
index 59718668a5296f83457c060ceadd5579cb94b1a2..4a8ccb6dc0631b12511a22d7335239c9d665085d 100644
--- a/app/models/group.rb
+++ b/app/models/group.rb
@@ -117,42 +117,45 @@ def lfs_enabled?
     self[:lfs_enabled]
   end
 
-  def add_users(user_ids, access_level, current_user: nil, skip_notification: false, expires_at: nil, ldap: false)
-    user_ids.each do |user_id|
-      Member.add_user(
-        self.group_members,
-        user_id,
-        access_level,
-        current_user: current_user,
-        skip_notification: skip_notification,
-        expires_at: expires_at,
-        ldap: ldap
-      )
-    end
-  end
-
-  def add_user(user, access_level, current_user: nil, skip_notification: false, expires_at: nil)
-    add_users([user], access_level, current_user: current_user, skip_notification: skip_notification, expires_at: expires_at)
-  end
-
-  def add_owner(user, current_user = nil, skip_notification: false)
-    add_user(user, Gitlab::Access::OWNER, current_user: current_user, skip_notification: skip_notification)
+  def add_users(users, access_level, current_user: nil, expires_at: nil)
+    GroupMember.add_users_to_group(
+      self,
+      users,
+      access_level,
+      current_user: current_user,
+      expires_at: expires_at
+    )
+  end
+
+  def add_user(user, access_level, current_user: nil, expires_at: nil, ldap: false)
+    GroupMember.add_user(
+      self,
+      user,
+      access_level,
+      current_user: current_user,
+      expires_at: expires_at,
+      ldap: ldap
+    )
   end
 
   def add_guest(user, current_user = nil)
-    add_user(user, Gitlab::Access::GUEST, current_user: current_user)
+    add_user(user, :guest, current_user: current_user)
   end
 
   def add_reporter(user, current_user = nil)
-    add_user(user, Gitlab::Access::REPORTER, current_user: current_user)
+    add_user(user, :reporter, current_user: current_user)
   end
 
   def add_developer(user, current_user = nil)
-    add_user(user, Gitlab::Access::DEVELOPER, current_user: current_user)
+    add_user(user, :developer, current_user: current_user)
   end
 
   def add_master(user, current_user = nil)
-    add_user(user, Gitlab::Access::MASTER, current_user: current_user)
+    add_user(user, :master, current_user: current_user)
+  end
+
+  def add_owner(user, current_user = nil)
+    add_user(user, :owner, current_user: current_user)
   end
 
   def has_owner?(user)
diff --git a/app/models/member.rb b/app/models/member.rb
index d968394037a5e1d68a96b76cbd055e113ef3f37e..0bc62b98544119cd921edaee3b2a04bce78a52d5 100644
--- a/app/models/member.rb
+++ b/app/models/member.rb
@@ -81,51 +81,77 @@ def find_by_invite_token(invite_token)
       find_by(invite_token: invite_token)
     end
 
-    # This method is used to find users that have been entered into the "Add members" field.
-    # These can be the User objects directly, their IDs, their emails, or new emails to be invited.
-    def user_for_id(user_id)
-      return user_id if user_id.is_a?(User)
-
-      user = User.find_by(id: user_id)
-      user ||= User.find_by(email: user_id)
-      user ||= user_id
-      user
-    end
-
-    def add_user(members, user_id, access_level, current_user: nil, skip_notification: false, expires_at: nil, ldap: false)
-      user = user_for_id(user_id)
+    def add_user(source, user, access_level, current_user: nil, expires_at: nil, ldap: false)
+      user = retrieve_user(user)
+      access_level = retrieve_access_level(access_level)
 
       # `user` can be either a User object or an email to be invited
-      if user.is_a?(User)
-        member = members.find_or_initialize_by(user_id: user.id)
+      member =
+        if user.is_a?(User)
+          source.members.find_by(user_id: user.id) ||
+          source.requesters.find_by(user_id: user.id) ||
+          source.members.build(user_id: user.id)
+        else
+          source.members.build(invite_email: user)
+        end
+
+      return member unless can_update_member?(current_user, member)
+
+      member.attributes = {
+        created_by: member.created_by || current_user,
+        access_level: access_level,
+        expires_at: expires_at,
+        skip_notification: ldap,
+        ldap: ldap
+      }
+
+      if member.request?
+        ::Members::ApproveAccessRequestService.new(
+          source,
+          current_user,
+          id: member.id,
+          access_level: access_level
+        ).execute
       else
-        member = members.build
-        member.invite_email = user
+        member.save
       end
 
-      if can_update_member?(current_user, member) || project_creator?(member, access_level)
-        member.created_by ||= current_user
-        member.access_level = access_level
-        member.expires_at = expires_at
-        member.skip_notification = skip_notification
-        member.ldap = ldap
+      member
+    end
 
-        member.save
-      end
+    def access_levels
+      Gitlab::Access.sym_options
     end
 
     private
 
+    # This method is used to find users that have been entered into the "Add members" field.
+    # These can be the User objects directly, their IDs, their emails, or new emails to be invited.
+    def retrieve_user(user)
+      return user if user.is_a?(User)
+
+      User.find_by(id: user) || User.find_by(email: user) || user
+    end
+
+    def retrieve_access_level(access_level)
+      access_levels.fetch(access_level) { access_level.to_i }
+    end
+
     def can_update_member?(current_user, member)
       # There is no current user for bulk actions, in which case anything is allowed
-      !current_user ||
-        current_user.can?(:update_group_member, member) ||
-        current_user.can?(:update_project_member, member)
+      !current_user || current_user.can?(:"update_#{member.type.underscore}", member)
     end
 
-    def project_creator?(member, access_level)
-      member.new_record? && member.owner? &&
-        access_level.to_i == ProjectMember::MASTER
+    def add_users_to_source(source, users, access_level, current_user: nil, expires_at: nil)
+      users.each do |user|
+        add_user(
+          source,
+          user,
+          access_level,
+          current_user: current_user,
+          expires_at: expires_at
+        )
+      end
     end
   end
 
diff --git a/app/models/members/group_member.rb b/app/models/members/group_member.rb
index 57d7f3d8bab92cddb2fc55567a4ae6f683ca0e4f..ae6c831d5d78f82e560e476ec50984f054eee6da 100644
--- a/app/models/members/group_member.rb
+++ b/app/models/members/group_member.rb
@@ -17,6 +17,22 @@ def self.access_level_roles
     Gitlab::Access.options_with_owner
   end
 
+  def self.access_levels
+    Gitlab::Access.sym_options_with_owner
+  end
+
+  def self.add_users_to_group(group, users, access_level, current_user: nil, expires_at: nil)
+    self.transaction do
+      add_users_to_source(
+        group,
+        users,
+        access_level,
+        current_user: current_user,
+        expires_at: expires_at
+      )
+    end
+  end
+
   def group
     source
   end
diff --git a/app/models/members/project_member.rb b/app/models/members/project_member.rb
index 3dfc01f6b58c2aa5a68b0defa6e7909b28a0c142..610bbd824fcd61565024e3d57fa76ef03f7abfce 100644
--- a/app/models/members/project_member.rb
+++ b/app/models/members/project_member.rb
@@ -35,36 +35,20 @@ class << self
     #     :master
     #   )
     #
-    def add_users_to_projects(project_ids, user_ids, access, current_user: nil, expires_at: nil)
-      access_level = if roles_hash.has_key?(access)
-                       roles_hash[access]
-                     elsif roles_hash.values.include?(access.to_i)
-                       access
-                     else
-                       raise "Non valid access"
-                     end
-
-      users = user_ids.map { |user_id| Member.user_for_id(user_id) }
-
-      ProjectMember.transaction do
+    def add_users_to_projects(project_ids, users, access_level, current_user: nil, expires_at: nil)
+      self.transaction do
         project_ids.each do |project_id|
           project = Project.find(project_id)
 
-          users.each do |user|
-            Member.add_user(
-              project.project_members,
-              user,
-              access_level,
-              current_user: current_user,
-              expires_at: expires_at
-            )
-          end
+          add_users_to_source(
+            project,
+            users,
+            access_level,
+            current_user: current_user,
+            expires_at: expires_at
+          )
         end
       end
-
-      true
-    rescue
-      false
     end
 
     def truncate_teams(project_ids)
@@ -85,13 +69,15 @@ def truncate_team(project)
       truncate_teams [project.id]
     end
 
-    def roles_hash
-      Gitlab::Access.sym_options
-    end
-
     def access_level_roles
       Gitlab::Access.options
     end
+
+    private
+
+    def can_update_member?(current_user, member)
+      super || (member.owner? && member.new_record?)
+    end
   end
 
   def access_field
diff --git a/app/models/project.rb b/app/models/project.rb
index 082e5e4eeacacc7e57e2396b82bbddaf0549c53e..10c85b1f4e9bbc9641debc0f0b1078f92c2c3dff 100644
--- a/app/models/project.rb
+++ b/app/models/project.rb
@@ -151,6 +151,7 @@ def set_last_activity_at
 
   delegate :name, to: :owner, allow_nil: true, prefix: true
   delegate :members, to: :team, prefix: true
+  delegate :add_user, to: :team
 
   # Validations
   validates :creator, presence: true, on: :create
@@ -1127,10 +1128,6 @@ def project_member(user)
     project_members.find_by(user_id: user)
   end
 
-  def add_user(user, access_level, current_user: nil, expires_at: nil)
-    team.add_user(user, access_level, current_user: current_user, expires_at: expires_at)
-  end
-
   def default_branch
     @default_branch ||= repository.root_ref if repository.exists?
   end
diff --git a/app/models/project_team.rb b/app/models/project_team.rb
index bb22f0b74a91953583dd808e6892071a564a6bc3..665a0fa1a2031479156c80b031a5cb80b19e4021 100644
--- a/app/models/project_team.rb
+++ b/app/models/project_team.rb
@@ -33,20 +33,26 @@ def find_member(user_id)
     member
   end
 
-  def add_users(users, access, current_user: nil, expires_at: nil)
+  def add_users(users, access_level, current_user: nil, expires_at: nil)
     return false if group_member_lock
 
     ProjectMember.add_users_to_projects(
       [project.id],
       users,
-      access,
+      access_level,
       current_user: current_user,
       expires_at: expires_at
     )
   end
 
-  def add_user(user, access, current_user: nil, expires_at: nil)
-    add_users([user], access, current_user: current_user, expires_at: expires_at)
+  def add_user(user, access_level, current_user: nil, expires_at: nil)
+    ProjectMember.add_user(
+      project,
+      user,
+      access_level,
+      current_user: current_user,
+      expires_at: expires_at
+    )
   end
 
   # Remove all users from project team
diff --git a/app/services/members/authorized_destroy_service.rb b/app/services/members/authorized_destroy_service.rb
index ca9db59cac706762c7586ffb5f6fb16308ab6d4c..b7a244c2029282cc8ad8cdd720e35414a8bd1926 100644
--- a/app/services/members/authorized_destroy_service.rb
+++ b/app/services/members/authorized_destroy_service.rb
@@ -14,6 +14,8 @@ def execute
       if member.request? && member.user != user
         notification_service.decline_access_request(member)
       end
+
+      member
     end
   end
 end
diff --git a/app/services/members/destroy_service.rb b/app/services/members/destroy_service.rb
index 9a2bf82ef516e2ac9b3f1fa6510eb634e8d3ba45..431da8372c96d0403ecf7c562999daa65dc9c524 100644
--- a/app/services/members/destroy_service.rb
+++ b/app/services/members/destroy_service.rb
@@ -1,17 +1,42 @@
 module Members
   class DestroyService < BaseService
-    attr_accessor :member, :current_user
+    include MembersHelper
 
-    def initialize(member, current_user)
-      @member = member
+    attr_accessor :source
+
+    ALLOWED_SCOPES = %i[members requesters all]
+
+    def initialize(source, current_user, params = {})
+      @source = source
       @current_user = current_user
+      @params = params
     end
 
-    def execute
-      unless member && can?(current_user, "destroy_#{member.type.underscore}".to_sym, member)
-        raise Gitlab::Access::AccessDeniedError
-      end
+    def execute(scope = :members)
+      raise "scope :#{scope} is not allowed!" unless ALLOWED_SCOPES.include?(scope)
+
+      member = find_member!(scope)
+
+      raise Gitlab::Access::AccessDeniedError unless can_destroy_member?(member)
+
       AuthorizedDestroyService.new(member, current_user).execute
     end
+
+    private
+
+    def find_member!(scope)
+      condition = params[:user_id] ? { user_id: params[:user_id] } : { id: params[:id] }
+      case scope
+      when :all
+        source.members.find_by(condition) ||
+          source.requesters.find_by!(condition)
+      else
+        source.public_send(scope).find_by!(condition)
+      end
+    end
+
+    def can_destroy_member?(member)
+      member && can?(current_user, action_member_permission(:destroy, member), member)
+    end
   end
 end
diff --git a/db/fixtures/development/06_teams.rb b/db/fixtures/development/06_teams.rb
index 3e8cdcd67b4a66ab88eb6927453d98782b85bfa2..84b1e4b79e96f21f7b05f80b8848700bc0e723ee 100644
--- a/db/fixtures/development/06_teams.rb
+++ b/db/fixtures/development/06_teams.rb
@@ -1,11 +1,8 @@
 Gitlab::Seeder.quiet do
   Group.all.each do |group|
     User.all.sample(4).each do |user|
-      if group.add_users([user.id], Gitlab::Access.values.sample)
-        print '.'
-      else
-        print 'F'
-      end
+      group.add_users([user.id], Gitlab::Access.values.sample)
+      print '.'
     end
   end
 
diff --git a/lib/api/access_requests.rb b/lib/api/access_requests.rb
index 7b9de7c9598bf39c2b82b797e3652874d4c5b2f5..d3db77408302c92ec80204ffaafa2978df3355e5 100644
--- a/lib/api/access_requests.rb
+++ b/lib/api/access_requests.rb
@@ -75,9 +75,8 @@ class AccessRequests < Grape::API
           required_attributes! [:user_id]
           source = find_source(source_type, params[:id])
 
-          access_requester = source.requesters.find_by!(user_id: params[:user_id])
-
-          ::Members::DestroyService.new(access_requester, current_user).execute
+          ::Members::DestroyService.new(source, current_user, params).
+            execute(:requesters)
         end
       end
     end
diff --git a/lib/api/members.rb b/lib/api/members.rb
index 99cdf6e8c2a9b118a5548cb8066dc2158a4c97a3..3d359c3fbd00bb12f275c70453ccc16a971b179f 100644
--- a/lib/api/members.rb
+++ b/lib/api/members.rb
@@ -65,13 +65,6 @@ class Members < Grape::API
           end
           ## EE specific
 
-          access_requester = source.requesters.find_by(user_id: params[:user_id])
-          if access_requester
-            # We pass current_user = access_requester so that the requester doesn't
-            # receive a "access denied" email
-            ::Members::DestroyService.new(access_requester, access_requester.user).execute
-          end
-
           member = source.members.find_by(user_id: params[:user_id])
 
           # This is to ensure back-compatibility but 409 behavior should be used
@@ -79,18 +72,12 @@ class Members < Grape::API
           conflict!('Member already exists') if source_type == 'group' && member
 
           unless member
-            source.add_user(params[:user_id], params[:access_level], current_user: current_user, expires_at: params[:expires_at])
-            member = source.members.find_by(user_id: params[:user_id])
+            member = source.add_user(params[:user_id], params[:access_level], current_user: current_user, expires_at: params[:expires_at])
           end
 
-          if member
+          if member.persisted? && member.valid?
             present member.user, with: Entities::Member, member: member
           else
-            # Since `source.add_user` doesn't return a member object, we have to
-            # build a new one and populate its errors in order to render them.
-            member = source.members.build(attributes_for_keys([:user_id, :access_level, :expires_at]))
-            member.valid? # populate the errors
-
             # This is to ensure back-compatibility but 400 behavior should be used
             # for all validation errors in 9.0!
             render_api_error!('Access level is not known', 422) if member.errors.key?(:access_level)
@@ -153,7 +140,7 @@ class Members < Grape::API
           if member.nil?
             { message: "Access revoked", id: params[:user_id].to_i }
           else
-            ::Members::DestroyService.new(member, current_user).execute
+            ::Members::DestroyService.new(source, current_user, params).execute
 
             present member.user, with: Entities::Member, member: member
           end
diff --git a/lib/ee/gitlab/ldap/sync/group.rb b/lib/ee/gitlab/ldap/sync/group.rb
index c7acd2ce8a6ff2a0a496e5a1d6938fa112b74ff7..30ad92f40562a6779e9f49f2a576829d0a4dee4a 100644
--- a/lib/ee/gitlab/ldap/sync/group.rb
+++ b/lib/ee/gitlab/ldap/sync/group.rb
@@ -99,6 +99,8 @@ def dn_for_uid(uid)
           def update_existing_group_membership(group, access_levels)
             logger.debug { "Updating existing membership for '#{group.name}' group" }
 
+            first_group_owner = group.members.owners.first.try(:user)
+
             select_and_preload_group_members(group).each do |member|
               user = member.user
               identity = user.identities.select(:id, :extern_uid)
@@ -137,7 +139,12 @@ def update_existing_group_membership(group, access_levels)
 
                 next if member.ldap? && member.override?
 
-                add_or_update_user_membership(user, group, desired_access)
+                add_or_update_user_membership(
+                  user,
+                  group,
+                  desired_access,
+                  current_user: first_group_owner
+                )
               elsif group.last_owner?(user)
                 warn_cannot_remove_last_owner(user, group)
               else
@@ -149,11 +156,18 @@ def update_existing_group_membership(group, access_levels)
           def add_new_members(group, access_levels)
             logger.debug { "Adding new members to '#{group.name}' group" }
 
+            first_group_owner = group.members.owners.first.try(:user)
+
             access_levels.each do |member_dn, access_level|
               user = ::Gitlab::LDAP::User.find_by_uid_and_provider(member_dn, provider)
 
               if user.present?
-                add_or_update_user_membership(user, group, access_level)
+                add_or_update_user_membership(
+                  user,
+                  group,
+                  access_level,
+                  current_user: first_group_owner
+                )
               else
                 logger.debug do
                   <<-MSG.strip_heredoc.tr("\n", ' ')
@@ -167,23 +181,19 @@ def add_new_members(group, access_levels)
             end
           end
 
-          def add_or_update_user_membership(user, group, access)
+          def add_or_update_user_membership(user, group, access, current_user: nil)
             # Prevent the last owner of a group from being demoted
             if access < ::Gitlab::Access::OWNER && group.last_owner?(user)
               warn_cannot_remove_last_owner(user, group)
             else
-              # Temporarily handle access requests until
-              # gitlab-org/gitlab-ee#825 is properly resolved.
-              member = group.requesters.find_by(user_id: user.id)
-              if member.present?
-                member.access_level = access
-                member.requested_at = nil
-                member.save
-              else
-                # If you pass the user object, instead of just user ID,
-                # it saves an extra user database query.
-                group.add_users([user], access, skip_notification: true, ldap: true)
-              end
+              # If you pass the user object, instead of just user ID,
+              # it saves an extra user database query.
+              group.add_user(
+                user,
+                access,
+                current_user: current_user,
+                ldap: true
+              )
             end
           end
 
diff --git a/lib/gitlab/access.rb b/lib/gitlab/access.rb
index a533bac26925ff589a89a4c312df8040769e28bd..9b484a2ecfd9ddfbb3a31a9f36357cf6fee801ea 100644
--- a/lib/gitlab/access.rb
+++ b/lib/gitlab/access.rb
@@ -53,6 +53,10 @@ def sym_options
         }
       end
 
+      def sym_options_with_owner
+        sym_options.merge(owner: OWNER)
+      end
+
       def protection_options
         {
           "Not protected: Both developers and masters can push new commits, force push, or delete the branch." => PROTECTION_NONE,
diff --git a/spec/controllers/groups/group_members_controller_spec.rb b/spec/controllers/groups/group_members_controller_spec.rb
index 92b97bf3d0cc1962d8ac582f5e5ae37864d20b30..a0870891cf4a4ca7d8386bc69ab4903bdf0b28ac 100644
--- a/spec/controllers/groups/group_members_controller_spec.rb
+++ b/spec/controllers/groups/group_members_controller_spec.rb
@@ -87,10 +87,10 @@
     context 'when member is not found' do
       before { sign_in(user) }
 
-      it 'returns 403' do
+      it 'returns 404' do
         delete :leave, group_id: group
 
-        expect(response).to have_http_status(403)
+        expect(response).to have_http_status(404)
       end
     end
 
diff --git a/spec/controllers/projects/project_members_controller_spec.rb b/spec/controllers/projects/project_members_controller_spec.rb
index 19d05aef0a6fcb78ed8a86951c0e8eb8570452ac..c660b887e69cbef03f7466983f2ad1ad4bafa1bf 100644
--- a/spec/controllers/projects/project_members_controller_spec.rb
+++ b/spec/controllers/projects/project_members_controller_spec.rb
@@ -135,11 +135,11 @@
     context 'when member is not found' do
       before { sign_in(user) }
 
-      it 'returns 403' do
+      it 'returns 404' do
         delete :leave, namespace_id: project.namespace,
                        project_id: project
 
-        expect(response).to have_http_status(403)
+        expect(response).to have_http_status(404)
       end
     end
 
diff --git a/spec/controllers/projects/templates_controller_spec.rb b/spec/controllers/projects/templates_controller_spec.rb
index 7b3a26d7ca773695830c09c0d26a323d954d799a..19a152bcb055b778a2ba8358e38e50848a382387 100644
--- a/spec/controllers/projects/templates_controller_spec.rb
+++ b/spec/controllers/projects/templates_controller_spec.rb
@@ -13,7 +13,7 @@
   end
 
   before do
-    project.team.add_user(user, Gitlab::Access::MASTER)
+    project.add_user(user, Gitlab::Access::MASTER)
     project.repository.commit_file(user, file_path_1, "something valid", "test 3", "master", false)
   end
 
diff --git a/spec/factories/project_members.rb b/spec/factories/project_members.rb
index cf3659ba2750654a6086ba48cc7d0d2ccd698a24..1ddb305a8af4a7c351041ec2e7dcc664f83eb411 100644
--- a/spec/factories/project_members.rb
+++ b/spec/factories/project_members.rb
@@ -4,24 +4,9 @@
     project
     master
 
-    trait :guest do
-      access_level ProjectMember::GUEST
-    end
-
-    trait :reporter do
-      access_level ProjectMember::REPORTER
-    end
-
-    trait :developer do
-      access_level ProjectMember::DEVELOPER
-    end
-
-    trait :master do
-      access_level ProjectMember::MASTER
-    end
-
-    trait :owner do
-      access_level ProjectMember::OWNER
-    end
+    trait(:guest)     { access_level ProjectMember::GUEST }
+    trait(:reporter)  { access_level ProjectMember::REPORTER }
+    trait(:developer) { access_level ProjectMember::DEVELOPER }
+    trait(:master)    { access_level ProjectMember::MASTER }
   end
 end
diff --git a/spec/features/projects/members/owner_cannot_leave_project_spec.rb b/spec/features/projects/members/owner_cannot_leave_project_spec.rb
index 67811b1048e6c5df768e7e358a23153158447dde..6e948b7a61636643a2a6388c5de1f5d5aba62515 100644
--- a/spec/features/projects/members/owner_cannot_leave_project_spec.rb
+++ b/spec/features/projects/members/owner_cannot_leave_project_spec.rb
@@ -1,12 +1,10 @@
 require 'spec_helper'
 
 feature 'Projects > Members > Owner cannot leave project', feature: true do
-  let(:owner) { create(:user) }
   let(:project) { create(:project) }
 
   background do
-    project.team << [owner, :owner]
-    login_as(owner)
+    login_as(project.owner)
     visit namespace_project_path(project.namespace, project)
   end
 
diff --git a/spec/features/projects/members/owner_cannot_request_access_to_his_project_spec.rb b/spec/features/projects/members/owner_cannot_request_access_to_his_project_spec.rb
index 0e54c4fdf20b4c7bdcdf0df8dc6365942a8a9ecc..4ca9272b9c10736f4584ea23c280b79bc898a26a 100644
--- a/spec/features/projects/members/owner_cannot_request_access_to_his_project_spec.rb
+++ b/spec/features/projects/members/owner_cannot_request_access_to_his_project_spec.rb
@@ -1,12 +1,10 @@
 require 'spec_helper'
 
 feature 'Projects > Members > Owner cannot request access to his project', feature: true do
-  let(:owner) { create(:user) }
   let(:project) { create(:project) }
 
   background do
-    project.team << [owner, :owner]
-    login_as(owner)
+    login_as(project.owner)
     visit namespace_project_path(project.namespace, project)
   end
 
diff --git a/spec/features/projects_spec.rb b/spec/features/projects_spec.rb
index 2242cb6236a602d1b025bfde7f76d216c1580912..c30d38b650891b5cbfbcf5f448d1f9cd2cb770fe 100644
--- a/spec/features/projects_spec.rb
+++ b/spec/features/projects_spec.rb
@@ -82,7 +82,7 @@
 
     before do
       login_with(user)
-      project.team.add_user(user, Gitlab::Access::MASTER)
+      project.add_user(user, Gitlab::Access::MASTER)
       visit namespace_project_path(project.namespace, project)
     end
 
@@ -101,8 +101,8 @@
     context 'on issues page', js: true do
       before do
         login_with(user)
-        project.team.add_user(user, Gitlab::Access::MASTER)
-        project2.team.add_user(user, Gitlab::Access::MASTER)
+        project.add_user(user, Gitlab::Access::MASTER)
+        project2.add_user(user, Gitlab::Access::MASTER)
         visit namespace_project_issue_path(project.namespace, project, issue)
       end
 
diff --git a/spec/finders/joined_groups_finder_spec.rb b/spec/finders/joined_groups_finder_spec.rb
index f90a8e007c8e10b3dc212ba7dc2358e1d4929e1f..29a47e005a669313151a1be73d5b71fae22562e9 100644
--- a/spec/finders/joined_groups_finder_spec.rb
+++ b/spec/finders/joined_groups_finder_spec.rb
@@ -43,7 +43,7 @@
       context 'if profile visitor is in one of the private group projects' do
         before do
           project = create(:project, :private, group: private_group, name: 'B', path: 'B')
-          project.team.add_user(profile_visitor, Gitlab::Access::DEVELOPER)
+          project.add_user(profile_visitor, Gitlab::Access::DEVELOPER)
         end
 
         it 'shows group' do
diff --git a/spec/finders/projects_finder_spec.rb b/spec/finders/projects_finder_spec.rb
index 7a3a74335e899a0c2dcef9506c2b418970910ec0..13bda5f7c5ad5ea79f6544cfcc14b293f3b60fcb 100644
--- a/spec/finders/projects_finder_spec.rb
+++ b/spec/finders/projects_finder_spec.rb
@@ -38,7 +38,7 @@
 
       describe 'with private projects' do
         before do
-          private_project.team.add_user(user, Gitlab::Access::MASTER)
+          private_project.add_user(user, Gitlab::Access::MASTER)
         end
 
         it do
diff --git a/spec/lib/ee/gitlab/ldap/sync/group_spec.rb b/spec/lib/ee/gitlab/ldap/sync/group_spec.rb
index 3b2c2f7fdc89631c141c90c04d3f8cabf728a644..748cbae47848bef3565696fef73174b13efa1ee5 100644
--- a/spec/lib/ee/gitlab/ldap/sync/group_spec.rb
+++ b/spec/lib/ee/gitlab/ldap/sync/group_spec.rb
@@ -112,8 +112,13 @@ def execute
   end
 
   describe '#update_permissions' do
-    before { group.start_ldap_sync }
-    after { group.finish_ldap_sync }
+    before do
+      group.start_ldap_sync
+    end
+
+    after do
+      group.finish_ldap_sync
+    end
 
     let(:group) do
       create(:group_with_ldap_group_link,
@@ -142,17 +147,15 @@ def execute
         end
 
         it 'converts an existing membership access request to a real member' do
-          group.members.create(
-            user: user,
-            access_level: ::Gitlab::Access::MASTER,
-            requested_at: DateTime.now
-          )
+          group.add_owner(create(:user))
+          access_requester = group.request_access(user)
+          access_requester.update(access_level: ::Gitlab::Access::MASTER)
           # Validate that the user is properly created as a requester first.
-          expect(group.requesters.pluck(:user_id)).to include(user.id)
+          expect(group.requesters.pluck(:id)).to include(access_requester.id)
 
           sync_group.update_permissions
 
-          expect(group.members.pluck(:user_id)).to include(user.id)
+          expect(group.members.pluck(:id)).to include(access_requester.id)
           expect(group.members.find_by(user_id: user.id).access_level)
             .to eq(::Gitlab::Access::DEVELOPER)
         end
@@ -160,7 +163,7 @@ def execute
         it 'downgrades existing member access' do
           # Create user with higher access
           group.add_users([user],
-                          ::Gitlab::Access::MASTER, skip_notification: true)
+                          ::Gitlab::Access::MASTER)
 
           sync_group.update_permissions
 
@@ -171,7 +174,7 @@ def execute
         it 'upgrades existing member access' do
           # Create user with lower access
           group.add_users([user],
-                          ::Gitlab::Access::GUEST, skip_notification: true)
+                          ::Gitlab::Access::GUEST)
 
           sync_group.update_permissions
 
@@ -182,8 +185,7 @@ def execute
         it 'sets an existing member ldap attribute to true' do
           group.add_users(
             [user],
-            ::Gitlab::Access::DEVELOPER,
-            skip_notification: true
+            ::Gitlab::Access::DEVELOPER
           )
 
           sync_group.update_permissions
@@ -213,7 +215,7 @@ def execute
 
         it 'removes the user from the group' do
           group.add_users([user],
-                          Gitlab::Access::MASTER, skip_notification: true)
+                          Gitlab::Access::MASTER)
 
           sync_group.update_permissions
 
@@ -222,7 +224,7 @@ def execute
 
         it 'refuses to delete the last owner' do
           group.add_users([user],
-                          Gitlab::Access::OWNER, skip_notification: true)
+                          Gitlab::Access::OWNER)
 
           sync_group.update_permissions
 
@@ -242,7 +244,7 @@ def execute
           create(:identity, user: user1, extern_uid: user_dn(user1.username))
           create(:identity, user: user2, extern_uid: user_dn(user2.username))
           group.add_users([user1, user2],
-                          Gitlab::Access::OWNER, skip_notification: true)
+                          Gitlab::Access::OWNER)
 
           sync_group.update_permissions
 
diff --git a/spec/lib/gitlab/template/issue_template_spec.rb b/spec/lib/gitlab/template/issue_template_spec.rb
index f770857e958817206b6687da1a5f1cd5ef66c73f..d2d334e64131d41034b0eec7daebf65e77f3804b 100644
--- a/spec/lib/gitlab/template/issue_template_spec.rb
+++ b/spec/lib/gitlab/template/issue_template_spec.rb
@@ -10,7 +10,7 @@
   let(:file_path_3) { '.gitlab/issue_templates/feature_proposal.md' }
 
   before do
-    project.team.add_user(user, Gitlab::Access::MASTER)
+    project.add_user(user, Gitlab::Access::MASTER)
     project.repository.commit_file(user, file_path_1, "something valid", "test 3", "master", false)
     project.repository.commit_file(user, file_path_2, "template_test", "test 1", "master", false)
     project.repository.commit_file(user, file_path_3, "feature_proposal", "test 2", "master", false)
@@ -53,7 +53,7 @@
 
     context 'when repo is bare or empty' do
       let(:empty_project) { create(:empty_project) }
-      before { empty_project.team.add_user(user, Gitlab::Access::MASTER) }
+      before { empty_project.add_user(user, Gitlab::Access::MASTER) }
 
       it "returns empty array" do
         templates = subject.by_category('', empty_project)
@@ -78,7 +78,7 @@
     context "when repo is empty" do
       let(:empty_project) { create(:empty_project) }
 
-      before { empty_project.team.add_user(user, Gitlab::Access::MASTER) }
+      before { empty_project.add_user(user, Gitlab::Access::MASTER) }
 
       it "raises file not found" do
         issue_template = subject.new('.gitlab/issue_templates/not_existent.md', empty_project)
diff --git a/spec/lib/gitlab/template/merge_request_template_spec.rb b/spec/lib/gitlab/template/merge_request_template_spec.rb
index bb0f68043fa0ef7ae0f5b8c6b4c419dc87b6684c..ddf68c4cf78bfc0ab8d16b706fac05e8f0af25b6 100644
--- a/spec/lib/gitlab/template/merge_request_template_spec.rb
+++ b/spec/lib/gitlab/template/merge_request_template_spec.rb
@@ -10,7 +10,7 @@
   let(:file_path_3) { '.gitlab/merge_request_templates/feature_proposal.md' }
 
   before do
-    project.team.add_user(user, Gitlab::Access::MASTER)
+    project.add_user(user, Gitlab::Access::MASTER)
     project.repository.commit_file(user, file_path_1, "something valid", "test 3", "master", false)
     project.repository.commit_file(user, file_path_2, "template_test", "test 1", "master", false)
     project.repository.commit_file(user, file_path_3, "feature_proposal", "test 2", "master", false)
@@ -53,7 +53,7 @@
 
     context 'when repo is bare or empty' do
       let(:empty_project) { create(:empty_project) }
-      before { empty_project.team.add_user(user, Gitlab::Access::MASTER) }
+      before { empty_project.add_user(user, Gitlab::Access::MASTER) }
 
       it "returns empty array" do
         templates = subject.by_category('', empty_project)
@@ -78,7 +78,7 @@
     context "when repo is empty" do
       let(:empty_project) { create(:empty_project) }
 
-      before { empty_project.team.add_user(user, Gitlab::Access::MASTER) }
+      before { empty_project.add_user(user, Gitlab::Access::MASTER) }
 
       it "raises file not found" do
         issue_template = subject.new('.gitlab/merge_request_templates/not_existent.md', empty_project)
diff --git a/spec/mailers/notify_spec.rb b/spec/mailers/notify_spec.rb
index 55c01311565f52dfa488acdcba01562201586678..9ff34ade10802994f83782df9b1bf3be4f45cd14 100644
--- a/spec/mailers/notify_spec.rb
+++ b/spec/mailers/notify_spec.rb
@@ -551,21 +551,22 @@
       end
     end
 
-    def invite_to_project(project:, email:, inviter:)
-      Member.add_user(
-        project.project_members,
-        'toto@example.com',
-        Gitlab::Access::DEVELOPER,
-        current_user: inviter
+    def invite_to_project(project, inviter:)
+      create(
+        :project_member,
+        :developer,
+        project: project,
+        invite_token: '1234',
+        invite_email: 'toto@example.com',
+        user: nil,
+        created_by: inviter
       )
-
-      project.project_members.invite.last
     end
 
     describe 'project invitation' do
       let(:project) { create(:project) }
       let(:master) { create(:user).tap { |u| project.team << [u, :master] } }
-      let(:project_member) { invite_to_project(project: project, email: 'toto@example.com', inviter: master) }
+      let(:project_member) { invite_to_project(project, inviter: master) }
 
       subject { Notify.member_invited_email('project', project_member.id, project_member.invite_token) }
 
@@ -584,10 +585,10 @@ def invite_to_project(project:, email:, inviter:)
 
     describe 'project invitation accepted' do
       let(:project) { create(:project) }
-      let(:invited_user) { create(:user) }
+      let(:invited_user) { create(:user, name: 'invited user') }
       let(:master) { create(:user).tap { |u| project.team << [u, :master] } }
       let(:project_member) do
-        invitee = invite_to_project(project: project, email: 'toto@example.com', inviter: master)
+        invitee = invite_to_project(project, inviter: master)
         invitee.accept_invite!(invited_user)
         invitee
       end
@@ -611,7 +612,7 @@ def invite_to_project(project:, email:, inviter:)
       let(:project) { create(:project) }
       let(:master) { create(:user).tap { |u| project.team << [u, :master] } }
       let(:project_member) do
-        invitee = invite_to_project(project: project, email: 'toto@example.com', inviter: master)
+        invitee = invite_to_project(project, inviter: master)
         invitee.decline_invite!
         invitee
       end
@@ -803,21 +804,22 @@ def invite_to_project(project:, email:, inviter:)
       end
     end
 
-    def invite_to_group(group:, email:, inviter:)
-      Member.add_user(
-        group.group_members,
-        'toto@example.com',
-        Gitlab::Access::DEVELOPER,
-        current_user: inviter
+    def invite_to_group(group, inviter:)
+      create(
+        :group_member,
+        :developer,
+        group: group,
+        invite_token: '1234',
+        invite_email: 'toto@example.com',
+        user: nil,
+        created_by: inviter
       )
-
-      group.group_members.invite.last
     end
 
     describe 'group invitation' do
       let(:group) { create(:group) }
       let(:owner) { create(:user).tap { |u| group.add_user(u, Gitlab::Access::OWNER) } }
-      let(:group_member) { invite_to_group(group: group, email: 'toto@example.com', inviter: owner) }
+      let(:group_member) { invite_to_group(group, inviter: owner) }
 
       subject { Notify.member_invited_email('group', group_member.id, group_member.invite_token) }
 
@@ -836,10 +838,10 @@ def invite_to_group(group:, email:, inviter:)
 
     describe 'group invitation accepted' do
       let(:group) { create(:group) }
-      let(:invited_user) { create(:user) }
+      let(:invited_user) { create(:user, name: 'invited user') }
       let(:owner) { create(:user).tap { |u| group.add_user(u, Gitlab::Access::OWNER) } }
       let(:group_member) do
-        invitee = invite_to_group(group: group, email: 'toto@example.com', inviter: owner)
+        invitee = invite_to_group(group, inviter: owner)
         invitee.accept_invite!(invited_user)
         invitee
       end
@@ -863,7 +865,7 @@ def invite_to_group(group:, email:, inviter:)
       let(:group) { create(:group) }
       let(:owner) { create(:user).tap { |u| group.add_user(u, Gitlab::Access::OWNER) } }
       let(:group_member) do
-        invitee = invite_to_group(group: group, email: 'toto@example.com', inviter: owner)
+        invitee = invite_to_group(group, inviter: owner)
         invitee.decline_invite!
         invitee
       end
diff --git a/spec/models/issue_spec.rb b/spec/models/issue_spec.rb
index 3259f79529647c10c17959bb9de5a3a2ab37ad10..3b8b743af2d475c8136df5547f7f896edfe53b62 100644
--- a/spec/models/issue_spec.rb
+++ b/spec/models/issue_spec.rb
@@ -494,7 +494,7 @@
 
     context 'with an admin user' do
       let(:project) { create(:empty_project) }
-      let(:user) { create(:user, admin: true) }
+      let(:user) { create(:admin) }
 
       it 'returns true for a regular issue' do
         issue = build(:issue, project: project)
diff --git a/spec/models/member_spec.rb b/spec/models/member_spec.rb
index 0b1634f654af88cd808b5b359530877918414b7d..a9d3fcc6587bbfd206ed9f90011aad53855d1246 100644
--- a/spec/models/member_spec.rb
+++ b/spec/models/member_spec.rb
@@ -74,22 +74,17 @@
       @blocked_master = project.members.find_by(user_id: @blocked_user.id, access_level: Gitlab::Access::MASTER)
       @blocked_developer = project.members.find_by(user_id: @blocked_user.id, access_level: Gitlab::Access::DEVELOPER)
 
-      Member.add_user(
-        project.members,
-        'toto1@example.com',
-        Gitlab::Access::DEVELOPER,
-        current_user: @master_user
-      )
-      @invited_member = project.members.invite.find_by_invite_email('toto1@example.com')
+      @invited_member = create(:project_member, :developer,
+                              project: project,
+                              invite_token: '1234',
+                              invite_email: 'toto1@example.com')
 
       accepted_invite_user = build(:user, state: :active)
-      Member.add_user(
-        project.members,
-        'toto2@example.com',
-        Gitlab::Access::DEVELOPER,
-        current_user: @master_user
-      )
-      @accepted_invite_member = project.members.invite.find_by_invite_email('toto2@example.com').tap { |u| u.accept_invite!(accepted_invite_user) }
+      @accepted_invite_member = create(:project_member, :developer,
+                                      project: project,
+                                      invite_token: '1234',
+                                      invite_email: 'toto2@example.com').
+                                      tap { |u| u.accept_invite!(accepted_invite_user) }
 
       requested_user = create(:user).tap { |u| project.request_access(u) }
       @requested_member = project.requesters.find_by(user_id: requested_user.id)
@@ -176,39 +171,209 @@
     it { is_expected.to respond_to(:user_email) }
   end
 
-  describe ".add_user" do
-    let!(:user)    { create(:user) }
-    let(:project) { create(:project) }
+  describe '.add_user' do
+    %w[project group].each do |source_type|
+      context "when source is a #{source_type}" do
+        let!(:source) { create(source_type) }
+        let!(:user) { create(:user) }
+        let!(:admin) { create(:admin) }
 
-    context "when called with a user id" do
-      it "adds the user as a member" do
-        Member.add_user(project.project_members, user.id, ProjectMember::MASTER)
+        it 'returns a <Source>Member object' do
+          member = described_class.add_user(source, user, :master)
 
-        expect(project.users).to include(user)
-      end
-    end
+          expect(member).to be_a "#{source_type.classify}Member".constantize
+          expect(member).to be_persisted
+        end
 
-    context "when called with a user object" do
-      it "adds the user as a member" do
-        Member.add_user(project.project_members, user, ProjectMember::MASTER)
+        it 'sets members.created_by to the given current_user' do
+          member = described_class.add_user(source, user, :master, current_user: admin)
 
-        expect(project.users).to include(user)
-      end
-    end
+          expect(member.created_by).to eq(admin)
+        end
 
-    context "when called with a known user email" do
-      it "adds the user as a member" do
-        Member.add_user(project.project_members, user.email, ProjectMember::MASTER)
+        it 'sets members.expires_at to the given expires_at' do
+          member = described_class.add_user(source, user, :master, expires_at: Date.new(2016, 9, 22))
 
-        expect(project.users).to include(user)
-      end
-    end
+          expect(member.expires_at).to eq(Date.new(2016, 9, 22))
+        end
+
+        described_class.access_levels.each do |sym_key, int_access_level|
+          it "accepts the :#{sym_key} symbol as access level" do
+            expect(source.users).not_to include(user)
+
+            member = described_class.add_user(source, user.id, sym_key)
+
+            expect(member.access_level).to eq(int_access_level)
+            expect(source.users.reload).to include(user)
+          end
+
+          it "accepts the #{int_access_level} integer as access level" do
+            expect(source.users).not_to include(user)
+
+            member = described_class.add_user(source, user.id, int_access_level)
+
+            expect(member.access_level).to eq(int_access_level)
+            expect(source.users.reload).to include(user)
+          end
+        end
+
+        context 'with no current_user' do
+          context 'when called with a known user id' do
+            it 'adds the user as a member' do
+              expect(source.users).not_to include(user)
+
+              described_class.add_user(source, user.id, :master)
+
+              expect(source.users.reload).to include(user)
+            end
+          end
+
+          context 'when called with an unknown user id' do
+            it 'adds the user as a member' do
+              expect(source.users).not_to include(user)
+
+              described_class.add_user(source, 42, :master)
+
+              expect(source.users.reload).not_to include(user)
+            end
+          end
+
+          context 'when called with a user object' do
+            it 'adds the user as a member' do
+              expect(source.users).not_to include(user)
+
+              described_class.add_user(source, user, :master)
+
+              expect(source.users.reload).to include(user)
+            end
+          end
+
+          context 'when called with a requester user object' do
+            before do
+              source.request_access(user)
+            end
+
+            it 'adds the requester as a member' do
+              expect(source.users).not_to include(user)
+              expect(source.requesters.exists?(user_id: user)).to be_truthy
+
+              expect { described_class.add_user(source, user, :master) }.
+                to raise_error(Gitlab::Access::AccessDeniedError)
+
+              expect(source.users.reload).not_to include(user)
+              expect(source.requesters.reload.exists?(user_id: user)).to be_truthy
+            end
+          end
+
+          context 'when called with a known user email' do
+            it 'adds the user as a member' do
+              expect(source.users).not_to include(user)
+
+              described_class.add_user(source, user.email, :master)
+
+              expect(source.users.reload).to include(user)
+            end
+          end
+
+          context 'when called with an unknown user email' do
+            it 'creates an invited member' do
+              expect(source.users).not_to include(user)
+
+              described_class.add_user(source, 'user@example.com', :master)
+
+              expect(source.members.invite.pluck(:invite_email)).to include('user@example.com')
+            end
+          end
+        end
+
+        context 'when current_user can update member' do
+          it 'creates the member' do
+            expect(source.users).not_to include(user)
+
+            described_class.add_user(source, user, :master, current_user: admin)
+
+            expect(source.users.reload).to include(user)
+          end
+
+          context 'when called with a requester user object' do
+            before do
+              source.request_access(user)
+            end
+
+            it 'adds the requester as a member' do
+              expect(source.users).not_to include(user)
+              expect(source.requesters.exists?(user_id: user)).to be_truthy
+
+              described_class.add_user(source, user, :master, current_user: admin)
+
+              expect(source.users.reload).to include(user)
+              expect(source.requesters.reload.exists?(user_id: user)).to be_falsy
+            end
+          end
+        end
+
+        context 'when current_user cannot update member' do
+          it 'does not create the member' do
+            expect(source.users).not_to include(user)
+
+            member = described_class.add_user(source, user, :master, current_user: user)
+
+            expect(source.users.reload).not_to include(user)
+            expect(member).not_to be_persisted
+          end
+
+          context 'when called with a requester user object' do
+            before do
+              source.request_access(user)
+            end
+
+            it 'does not destroy the requester' do
+              expect(source.users).not_to include(user)
+              expect(source.requesters.exists?(user_id: user)).to be_truthy
+
+              described_class.add_user(source, user, :master, current_user: user)
+
+              expect(source.users.reload).not_to include(user)
+              expect(source.requesters.exists?(user_id: user)).to be_truthy
+            end
+          end
+        end
+
+        context 'when member already exists' do
+          before do
+            source.add_user(user, :developer)
+          end
+
+          context 'with no current_user' do
+            it 'updates the member' do
+              expect(source.users).to include(user)
+
+              described_class.add_user(source, user, :master)
+
+              expect(source.members.find_by(user_id: user).access_level).to eq(Gitlab::Access::MASTER)
+            end
+          end
+
+          context 'when current_user can update member' do
+            it 'updates the member' do
+              expect(source.users).to include(user)
+
+              described_class.add_user(source, user, :master, current_user: admin)
+
+              expect(source.members.find_by(user_id: user).access_level).to eq(Gitlab::Access::MASTER)
+            end
+          end
+
+          context 'when current_user cannot update member' do
+            it 'does not update the member' do
+              expect(source.users).to include(user)
 
-    context "when called with an unknown user email" do
-      it "adds a member invite" do
-        Member.add_user(project.project_members, "user@example.com", ProjectMember::MASTER)
+              described_class.add_user(source, user, :master, current_user: user)
 
-        expect(project.project_members.invite.pluck(:invite_email)).to include("user@example.com")
+              expect(source.members.find_by(user_id: user).access_level).to eq(Gitlab::Access::DEVELOPER)
+            end
+          end
+        end
       end
     end
   end
diff --git a/spec/models/members/group_member_spec.rb b/spec/models/members/group_member_spec.rb
index 56fa7fa61347cc6c73f12866ab0754008173be57..370aeb9e0a920adf692e3eeb358748bdaf4422e4 100644
--- a/spec/models/members/group_member_spec.rb
+++ b/spec/models/members/group_member_spec.rb
@@ -1,6 +1,33 @@
 require 'spec_helper'
 
 describe GroupMember, models: true do
+  describe '.access_level_roles' do
+    it 'returns Gitlab::Access.options_with_owner' do
+      expect(described_class.access_level_roles).to eq(Gitlab::Access.options_with_owner)
+    end
+  end
+
+  describe '.access_levels' do
+    it 'returns Gitlab::Access.options_with_owner' do
+      expect(described_class.access_levels).to eq(Gitlab::Access.sym_options_with_owner)
+    end
+  end
+
+  describe '.add_users_to_group' do
+    it 'adds the given users to the given group' do
+      group = create(:group)
+      users = create_list(:user, 2)
+
+      described_class.add_users_to_group(
+        group,
+        [users.first.id, users.second],
+        described_class::MASTER
+      )
+
+      expect(group.users).to include(users.first, users.second)
+    end
+  end
+
   describe 'notifications' do
     describe "#after_create" do
       it "sends email to user" do
diff --git a/spec/models/members/project_member_spec.rb b/spec/models/members/project_member_spec.rb
index 805c15a4e5ecd85d0d0a6f995d04f40e36ba2e9c..d85a1c1e3b2e32d830b640af55a22d245ec6bec2 100644
--- a/spec/models/members/project_member_spec.rb
+++ b/spec/models/members/project_member_spec.rb
@@ -15,6 +15,26 @@
     it { is_expected.to include_module(Gitlab::ShellAdapter) }
   end
 
+  describe '.access_level_roles' do
+    it 'returns Gitlab::Access.options' do
+      expect(described_class.access_level_roles).to eq(Gitlab::Access.options)
+    end
+  end
+
+  describe '.add_user' do
+    context 'when called with the project owner' do
+      it 'adds the user as a member' do
+        project = create(:empty_project)
+
+        expect(project.users).not_to include(project.owner)
+
+        described_class.add_user(project, project.owner, :master, current_user: project.owner)
+
+        expect(project.users.reload).to include(project.owner)
+      end
+    end
+  end
+
   describe '#real_source_type' do
     subject { create(:project_member).real_source_type }
 
@@ -50,7 +70,7 @@
     end
   end
 
-  describe :import_team do
+  describe '.import_team' do
     before do
       @project_1 = create :project
       @project_2 = create :project
@@ -81,25 +101,21 @@
   end
 
   describe '.add_users_to_projects' do
-    before do
-      @project_1 = create :project
-      @project_2 = create :project
+    it 'adds the given users to the given projects' do
+      projects = create_list(:empty_project, 2)
+      users = create_list(:user, 2)
 
-      @user_1 = create :user
-      @user_2 = create :user
-
-      ProjectMember.add_users_to_projects(
-        [@project_1.id, @project_2.id],
-        [@user_1.id, @user_2.id],
-        ProjectMember::MASTER
-      )
-    end
+      described_class.add_users_to_projects(
+        [projects.first.id, projects.second],
+        [users.first.id, users.second],
+        described_class::MASTER)
 
-    it { expect(@project_1.users).to include(@user_1) }
-    it { expect(@project_1.users).to include(@user_2) }
+      expect(projects.first.users).to include(users.first)
+      expect(projects.first.users).to include(users.second)
 
-    it { expect(@project_2.users).to include(@user_1) }
-    it { expect(@project_2.users).to include(@user_2) }
+      expect(projects.second.users).to include(users.first)
+      expect(projects.second.users).to include(users.second)
+    end
   end
 
   describe '.truncate_teams' do
diff --git a/spec/models/project_spec.rb b/spec/models/project_spec.rb
index 8e63baafb9ce36ba41f1b0e230a85f5331266515..14465bac3521353860bfeb92b6e80fbd48499e1f 100644
--- a/spec/models/project_spec.rb
+++ b/spec/models/project_spec.rb
@@ -960,7 +960,7 @@ def create_pipeline
 
     describe 'when a user has access to a project' do
       before do
-        project.team.add_user(user, Gitlab::Access::MASTER)
+        project.add_user(user, Gitlab::Access::MASTER)
       end
 
       it { is_expected.to eq([project]) }
diff --git a/spec/requests/api/access_requests_spec.rb b/spec/requests/api/access_requests_spec.rb
index d78494b76fac2a281852498c867b575a40a2454e..aeb76a7a6e7496a7e5b4bf2ffce10ef6787d0bd7 100644
--- a/spec/requests/api/access_requests_spec.rb
+++ b/spec/requests/api/access_requests_spec.rb
@@ -181,7 +181,7 @@
       end
 
       context 'when authenticated as the access requester' do
-        it 'returns 200' do
+        it 'deletes the access requester' do
           expect do
             delete api("/#{source_type.pluralize}/#{source.id}/access_requests/#{access_requester.id}", access_requester)
 
@@ -191,7 +191,7 @@
       end
 
       context 'when authenticated as a master/owner' do
-        it 'returns 200' do
+        it 'deletes the access requester' do
           expect do
             delete api("/#{source_type.pluralize}/#{source.id}/access_requests/#{access_requester.id}", master)
 
@@ -199,6 +199,16 @@
           end.to change { source.requesters.count }.by(-1)
         end
 
+        context 'user_id matches a member, not an access requester' do
+          it 'returns 404' do
+            expect do
+              delete api("/#{source_type.pluralize}/#{source.id}/access_requests/#{developer.id}", master)
+
+              expect(response).to have_http_status(404)
+            end.not_to change { source.requesters.count }
+          end
+        end
+
         context 'user_id does not match an existing access requester' do
           it 'returns 404' do
             expect do
diff --git a/spec/requests/api/ldap_group_links_spec.rb b/spec/requests/api/ldap_group_links_spec.rb
index ec9b1263e53ec4a9d6a825ef8e33f468c531fb42..64f96cdf720554110e2c13d01ce4e78ad55c8bbf 100644
--- a/spec/requests/api/ldap_group_links_spec.rb
+++ b/spec/requests/api/ldap_group_links_spec.rb
@@ -16,7 +16,7 @@
 
   before do
     group_with_ldap_links.add_owner owner
-    group_with_ldap_links.add_user user, group_access: Gitlab::Access::DEVELOPER
+    group_with_ldap_links.add_user user, Gitlab::Access::DEVELOPER
   end
 
   describe "POST /groups/:id/ldap_group_links" do
diff --git a/spec/requests/api/merge_requests_spec.rb b/spec/requests/api/merge_requests_spec.rb
index 626af06821b104d3cfb519a518badf3c233655aa..4b6825d9fbec06159439c71f55f87f9aa70e7bbf 100644
--- a/spec/requests/api/merge_requests_spec.rb
+++ b/spec/requests/api/merge_requests_spec.rb
@@ -15,7 +15,7 @@
   let(:milestone)   { create(:milestone, title: '1.0.0', project: project) }
 
   before do
-    project.team << [user, :reporters]
+    project.team << [user, :reporter]
   end
 
   describe "GET /projects/:id/merge_requests" do
@@ -299,7 +299,7 @@
       let!(:unrelated_project) { create(:project,  namespace: create(:user).namespace, creator_id: user2.id) }
 
       before :each do |each|
-        fork_project.team << [user2, :reporters]
+        fork_project.team << [user2, :reporter]
       end
 
       it "returns merge_request" do
diff --git a/spec/services/members/destroy_service_spec.rb b/spec/services/members/destroy_service_spec.rb
index 2395445e7fddcb1372b5010dea71680d712f174b..9995f3488af78d6e4b7ea0639836d95bbe814868 100644
--- a/spec/services/members/destroy_service_spec.rb
+++ b/spec/services/members/destroy_service_spec.rb
@@ -2,70 +2,111 @@
 
 describe Members::DestroyService, services: true do
   let(:user) { create(:user) }
-  let(:project) { create(:project) }
-  let!(:member) { create(:project_member, source: project) }
+  let(:member_user) { create(:user) }
+  let(:project) { create(:project, :public) }
+  let(:group) { create(:group, :public) }
 
-  context 'when member is nil' do
-    before do
-      project.team << [user, :developer]
+  shared_examples 'a service raising ActiveRecord::RecordNotFound' do
+    it 'raises ActiveRecord::RecordNotFound' do
+      expect { described_class.new(source, user, params).execute }.to raise_error(ActiveRecord::RecordNotFound)
     end
+  end
 
-    it 'does not destroy the member' do
-      expect { destroy_member(nil, user) }.to raise_error(Gitlab::Access::AccessDeniedError)
+  shared_examples 'a service raising Gitlab::Access::AccessDeniedError' do
+    it 'raises Gitlab::Access::AccessDeniedError' do
+      expect { described_class.new(source, user, params).execute }.to raise_error(Gitlab::Access::AccessDeniedError)
     end
   end
 
-  context 'when current user cannot destroy the given member' do
-    before do
-      project.team << [user, :developer]
+  shared_examples 'a service destroying a member' do
+    it 'destroys the member' do
+      expect { described_class.new(source, user, params).execute }.to change { source.members.count }.by(-1)
+    end
+
+    context 'when the given member is an access requester' do
+      before do
+        source.members.find_by(user_id: member_user).destroy
+        source.request_access(member_user)
+      end
+      let(:access_requester) { source.requesters.find_by(user_id: member_user) }
+
+      it_behaves_like 'a service raising ActiveRecord::RecordNotFound'
+
+      %i[requesters all].each do |scope|
+        context "and #{scope} scope is passed" do
+          it 'destroys the access requester' do
+            expect { described_class.new(source, user, params).execute(scope) }.to change { source.requesters.count }.by(-1)
+          end
+
+          it 'calls Member#after_decline_request' do
+            expect_any_instance_of(NotificationService).to receive(:decline_access_request).with(access_requester)
+
+            described_class.new(source, user, params).execute(scope)
+          end
+
+          context 'when current user is the member' do
+            it 'does not call Member#after_decline_request' do
+              expect_any_instance_of(NotificationService).not_to receive(:decline_access_request).with(access_requester)
+
+              described_class.new(source, member_user, params).execute(scope)
+            end
+          end
+        end
+      end
     end
+  end
+
+  context 'when no member are found' do
+    let(:params) { { user_id: 42 } }
 
-    it 'does not destroy the member' do
-      expect { destroy_member(member, user) }.to raise_error(Gitlab::Access::AccessDeniedError)
+    it_behaves_like 'a service raising ActiveRecord::RecordNotFound' do
+      let(:source) { project }
+    end
+
+    it_behaves_like 'a service raising ActiveRecord::RecordNotFound' do
+      let(:source) { group }
     end
   end
 
-  context 'when current user can destroy the given member' do
+  context 'when a member is found' do
     before do
-      project.team << [user, :master]
+      project.team << [member_user, :developer]
+      group.add_developer(member_user)
     end
+    let(:params) { { user_id: member_user.id } }
 
-    it 'destroys the member' do
-      destroy_member(member, user)
+    context 'when current user cannot destroy the given member' do
+      it_behaves_like 'a service raising Gitlab::Access::AccessDeniedError' do
+        let(:source) { project }
+      end
 
-      expect(member).to be_destroyed
+      it_behaves_like 'a service raising Gitlab::Access::AccessDeniedError' do
+        let(:source) { group }
+      end
     end
 
-    context 'when the given member is a requester' do
+    context 'when current user can destroy the given member' do
       before do
-        member.update_column(:requested_at, Time.now)
+        project.team << [user, :master]
+        group.add_owner(user)
       end
 
-      it 'calls Member#after_decline_request' do
-        expect_any_instance_of(NotificationService).to receive(:decline_access_request).with(member)
-
-        destroy_member(member, user)
+      it_behaves_like 'a service destroying a member' do
+        let(:source) { project }
       end
 
-      context 'when current user is the member' do
-        it 'does not call Member#after_decline_request' do
-          expect_any_instance_of(NotificationService).not_to receive(:decline_access_request).with(member)
-
-          destroy_member(member, member.user)
-        end
+      it_behaves_like 'a service destroying a member' do
+        let(:source) { group }
       end
 
-      context 'when current user is the member and ' do
-        it 'does not call Member#after_decline_request' do
-          expect_any_instance_of(NotificationService).not_to receive(:decline_access_request).with(member)
+      context 'when given a :id' do
+        let(:params) { { id: project.members.find_by!(user_id: user.id).id } }
 
-          destroy_member(member, member.user)
+        it 'destroys the member' do
+          expect { described_class.new(project, user, params).execute }.
+            to change { project.members.count }.by(-1)
         end
       end
     end
   end
-
-  def destroy_member(member, user)
-    Members::DestroyService.new(member, user).execute
-  end
 end
diff --git a/spec/services/projects/update_service_spec.rb b/spec/services/projects/update_service_spec.rb
index ec68875bf3e6ec90b63ff50f2febf47ef6c38a61..8618d60fe656d9d084a2b2e006f6e9c6f6525650 100644
--- a/spec/services/projects/update_service_spec.rb
+++ b/spec/services/projects/update_service_spec.rb
@@ -140,7 +140,7 @@
   end
 
   describe 'repository_storage' do
-    let(:admin_user) { create(:user, admin: true) }
+    let(:admin_user) { create(:admin) }
     let(:user) { create(:user) }
     let(:project) { create(:project, repository_storage: 'a') }
     let(:opts) { { repository_storage: 'b' } }