Merge branch 'expiration-date-on-memberships' into 'master'

Expiration date on memberships

Closes https://gitlab.com/gitlab-org/gitlab-ce/issues/17495

See merge request !5876

CHANGELOG

@@ -134,6 +134,7 @@ v 8.11.0 (unreleased)
   - Change requests_profiles resource constraint to catch virtually any file
   - Bump gitlab_git to lazy load compare commits
   - Reduce number of queries made for merge_requests/:id/diffs
+  - Add the option to set the expiration date for the project membership when giving a user access to a project. !5599 (Adam Niedzielski)
   - Sensible state specific default sort order for issues and merge requests !5453 (tomb0y)
   - Fix bug where destroying a namespace would not always destroy projects
   - Fix RequestProfiler::Middleware error when code is reloaded in development

app/assets/javascripts/dispatcher.js

@@ -129,10 +129,12 @@
           new NotificationsDropdown();
           break;
         case 'groups:group_members:index':
+          new gl.MemberExpirationDate();
           new GroupMembers();
           new UsersSelect();
           break;
         case 'projects:project_members:index':
+          new gl.MemberExpirationDate();
           new ProjectMembers();
           new UsersSelect();
           break;
@@ -174,6 +176,7 @@
           new BuildArtifacts();
           break;
         case 'projects:group_links:index':
+          new gl.MemberExpirationDate();
           new GroupsSelect();
           break;
         case 'search:show':

app/assets/javascripts/member_expiration_date.js

+(function() {
+  // Add datepickers to all `js-access-expiration-date` elements. If those elements are
+  // children of an element with the `clearable-input` class, and have a sibling
+  // `js-clear-input` element, then show that element when there is a value in the
+  // datepicker, and make clicking on that element clear the field.
+  //
+  gl.MemberExpirationDate = function() {
+    function toggleClearInput() {
+      $(this).closest('.clearable-input').toggleClass('has-value', $(this).val() !== '');
+    }
+
+    var inputs = $('.js-access-expiration-date');
+
+    inputs.datepicker({
+      dateFormat: 'yy-mm-dd',
+      minDate: 1,
+      onSelect: toggleClearInput
+    });
+
+    inputs.next('.js-clear-input').on('click', function(event) {
+      event.preventDefault();
+
+      var input = $(this).closest('.clearable-input').find('.js-access-expiration-date');
+      input.datepicker('setDate', null);
+      toggleClearInput.call(input);
+    });
+
+    inputs.on('blur', toggleClearInput);
+
+    inputs.each(toggleClearInput);
+  };
+}).call(this);

app/assets/javascripts/project_members.js

@@ -5,9 +5,6 @@
         return $(this).fadeOut();
       });
     }
-
     return ProjectMembers;
-
   })();
-
 }).call(this);

app/assets/stylesheets/pages/projects.scss

@@ -719,3 +719,29 @@ pre.light-well {
     width: 300px;
   }
 }
+
+.clearable-input {
+  position: relative;
+
+  .clear-icon {
+    @extend .fa-times;
+    display: none;
+    position: absolute;
+    right: 7px;
+    top: 7px;
+    color: $location-icon-color;
+
+    &:before {
+      font-family: FontAwesome;
+      font-weight: normal;
+      font-style: normal;
+    }
+  }
+
+  &.has-value {
+    .clear-icon {
+      cursor: pointer;
+      display: block;
+    }
+  }
+}

app/controllers/admin/groups_controller.rb

@@ -42,7 +42,7 @@ class Admin::GroupsController < Admin::ApplicationController
   end
 
   def members_update
-    @group.add_users(params[:user_ids].split(','), params[:access_level], current_user)
+    @group.add_users(params[:user_ids].split(','), params[:access_level], current_user: current_user)
 
     redirect_to [:admin, @group], notice: 'Users were successfully added.'
   end

app/controllers/groups/group_members_controller.rb

@@ -21,7 +21,12 @@ class Groups::GroupMembersController < Groups::ApplicationController
   end
 
   def create
-    @group.add_users(params[:user_ids].split(','), params[:access_level], current_user)
+    @group.add_users(
+      params[:user_ids].split(','),
+      params[:access_level],
+      current_user: current_user,
+      expires_at: params[:expires_at]
+    )
 
     redirect_to group_group_members_path(@group), notice: 'Users were successfully added.'
   end
@@ -63,7 +68,7 @@ class Groups::GroupMembersController < Groups::ApplicationController
   protected
 
   def member_params
-    params.require(:group_member).permit(:access_level, :user_id)
+    params.require(:group_member).permit(:access_level, :user_id, :expires_at)
   end
 
   # MembershipActions concern

app/controllers/projects/group_links_controller.rb

@@ -11,7 +11,9 @@ class Projects::GroupLinksController < Projects::ApplicationController
     return render_404 unless can?(current_user, :read_group, group)
 
     project.project_group_links.create(
-      group: group, group_access: params[:link_group_access]
+      group: group,
+      group_access: params[:link_group_access],
+      expires_at: params[:expires_at]
     )
 
     redirect_to namespace_project_group_links_path(project.namespace, project)

app/controllers/projects/project_members_controller.rb

@@ -36,7 +36,12 @@ class Projects::ProjectMembersController < Projects::ApplicationController
   end
 
   def create
-    @project.team.add_users(params[:user_ids].split(','), params[:access_level], current_user)
+    @project.team.add_users(
+      params[:user_ids].split(','),
+      params[:access_level],
+      expires_at: params[:expires_at],
+      current_user: current_user
+    )
 
     redirect_to namespace_project_project_members_path(@project.namespace, @project)
   end
@@ -94,7 +99,7 @@ class Projects::ProjectMembersController < Projects::ApplicationController
   protected
 
   def member_params
-    params.require(:project_member).permit(:user_id, :access_level)
+    params.require(:project_member).permit(:user_id, :access_level, :expires_at)
   end
 
   # MembershipActions concern

app/models/concerns/expirable.rb

+module Expirable
+  extend ActiveSupport::Concern
+
+  included do
+    scope :expired, -> { where('expires_at <= ?', Time.current) }
+  end
+
+  def expires?
+    expires_at.present?
+  end
+
+  def expires_soon?
+    expires_at < 7.days.from_now
+  end
+end

app/models/group.rb

@@ -95,34 +95,40 @@ class Group < Namespace
     end
   end
 
-  def add_users(user_ids, access_level, current_user = nil)
+  def add_users(user_ids, access_level, current_user: nil, expires_at: nil)
     user_ids.each do |user_id|
-      Member.add_user(self.group_members, user_id, access_level, current_user)
+      Member.add_user(
+        self.group_members,
+        user_id,
+        access_level,
+        current_user: current_user,
+        expires_at: expires_at
+      )
     end
   end
 
-  def add_user(user, access_level, current_user = nil)
-    add_users([user], access_level, current_user)
+  def add_user(user, access_level, current_user: nil, expires_at: nil)
+    add_users([user], access_level, current_user: current_user, expires_at: expires_at)
   end
 
   def add_guest(user, current_user = nil)
-    add_user(user, Gitlab::Access::GUEST, current_user)
+    add_user(user, Gitlab::Access::GUEST, current_user: current_user)
   end
 
   def add_reporter(user, current_user = nil)
-    add_user(user, Gitlab::Access::REPORTER, current_user)
+    add_user(user, Gitlab::Access::REPORTER, current_user: current_user)
   end
 
   def add_developer(user, current_user = nil)
-    add_user(user, Gitlab::Access::DEVELOPER, current_user)
+    add_user(user, Gitlab::Access::DEVELOPER, current_user: current_user)
   end
 
   def add_master(user, current_user = nil)
-    add_user(user, Gitlab::Access::MASTER, current_user)
+    add_user(user, Gitlab::Access::MASTER, current_user: current_user)
   end
 
   def add_owner(user, current_user = nil)
-    add_user(user, Gitlab::Access::OWNER, current_user)
+    add_user(user, Gitlab::Access::OWNER, current_user: current_user)
   end
 
   def has_owner?(user)

app/models/member.rb

 class Member < ActiveRecord::Base
   include Sortable
   include Importable
+  include Expirable
   include Gitlab::Access
 
   attr_accessor :raw_invite_token
@@ -73,7 +74,7 @@ class Member < ActiveRecord::Base
       user
     end
 
-    def add_user(members, user_id, access_level, current_user = nil)
+    def add_user(members, user_id, access_level, current_user: nil, expires_at: nil)
       user = user_for_id(user_id)
 
       # `user` can be either a User object or an email to be invited
@@ -87,6 +88,7 @@ class Member < ActiveRecord::Base
       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.save
       end

app/models/members/project_member.rb

@@ -34,7 +34,7 @@ class ProjectMember < Member
     #     :master
     #   )
     #
-    def add_users_to_projects(project_ids, user_ids, access, current_user = nil)
+    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)
@@ -50,7 +50,13 @@ class ProjectMember < Member
           project = Project.find(project_id)
 
           users.each do |user|
-            Member.add_user(project.project_members, user, access_level, current_user)
+            Member.add_user(
+              project.project_members,
+              user,
+              access_level,
+              current_user: current_user,
+              expires_at: expires_at
+            )
           end
         end
       end

app/models/project.rb

@@ -1003,8 +1003,8 @@ class Project < ActiveRecord::Base
     project_members.find_by(user_id: user)
   end
 
-  def add_user(user, access_level, current_user = nil)
-    team.add_user(user, access_level, current_user)
+  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

app/models/project_group_link.rb

 class ProjectGroupLink < ActiveRecord::Base
+  include Expirable
+
   GUEST     = 10
   REPORTER  = 20
   DEVELOPER = 30
@@ -26,7 +28,7 @@ class ProjectGroupLink < ActiveRecord::Base
     self.class.access_options.key(self.group_access)
   end
 
-  private 
+  private
 
   def different_group
     if self.group && self.project && self.project.group == self.group

app/models/project_team.rb

@@ -15,9 +15,9 @@ class ProjectTeam
     users, access, current_user = *args
 
     if users.respond_to?(:each)
-      add_users(users, access, current_user)
+      add_users(users, access, current_user: current_user)
     else
-      add_user(users, access, current_user)
+      add_user(users, access, current_user: current_user)
     end
   end
 
@@ -33,17 +33,18 @@ class ProjectTeam
     member
   end
 
-  def add_users(users, access, current_user = nil)
+  def add_users(users, access, current_user: nil, expires_at: nil)
     ProjectMember.add_users_to_projects(
       [project.id],
       users,
       access,
-      current_user
+      current_user: current_user,
+      expires_at: expires_at
     )
   end
 
-  def add_user(user, access, current_user = nil)
-    add_users([user], access, current_user)
+  def add_user(user, access, current_user: nil, expires_at: nil)
+    add_users([user], access, current_user: current_user, expires_at: expires_at)
   end
 
   # Remove all users from project team

app/services/members/authorized_destroy_service.rb

+module Members
+  class AuthorizedDestroyService < BaseService
+    attr_accessor :member, :user
+
+    def initialize(member, user = nil)
+      @member, @user = member, user
+    end
+
+    def execute
+      return false if member.is_a?(GroupMember) && member.source.last_owner?(member.user)
+
+      member.destroy
+
+      if member.request? && member.user != user
+        notification_service.decline_access_request(member)
+      end
+    end
+  end
+end

app/services/members/destroy_service.rb

@@ -11,12 +11,7 @@ module Members
       unless member && can?(current_user, "destroy_#{member.type.underscore}".to_sym, member)
         raise Gitlab::Access::AccessDeniedError
       end
-
-      member.destroy
-
-      if member.request? && member.user != current_user
-        notification_service.decline_access_request(member)
-      end
+      AuthorizedDestroyService.new(member, current_user).execute
     end
   end
 end

app/views/groups/group_members/_new_group_member.html.haml

@@ -14,5 +14,14 @@
         Read more about role permissions
         %strong= link_to "here", help_page_path("user/permissions"), class: "vlink"
 
+  .form-group
+    = f.label :expires_at, 'Access expiration date', class: 'control-label'
+    .col-sm-10
+      .clearable-input
+        = text_field_tag :expires_at, nil, class: 'form-control js-access-expiration-date', placeholder: 'Select access expiration date'
+        %i.clear-icon.js-clear-input
+      .help-block
+        On this date, the user(s) will automatically lose access to this group and all of its projects.
+
   .form-actions
     = f.submit 'Add users to group', class: "btn btn-create"

app/views/groups/group_members/update.js.haml

 :plain
   $("##{dom_id(@group_member)}").replaceWith('#{escape_javascript(render('shared/members/member', member: @group_member))}');
+  new MemberExpirationDate();

app/views/projects/group_links/index.html.haml

@@ -17,6 +17,13 @@
         .select-wrapper
           = select_tag :link_group_access, options_for_select(ProjectGroupLink.access_options, ProjectGroupLink.default_access), class: "form-control select-control"
           %span.caret
+      .form-group
+        = label_tag :expires_at, 'Access expiration date', class: 'label-light'
+        .clearable-input
+          = text_field_tag :expires_at, nil, class: 'form-control js-access-expiration-date', placeholder: 'Select access expiration date'
+          %i.clear-icon.js-clear-input
+        .help-block
+          On this date, all users in the group will automatically lose access to this project.
       = submit_tag "Share", class: "btn btn-create"
   .col-lg-9.col-lg-offset-3
     %hr
@@ -35,6 +42,10 @@
                 = group.name
               %br
               up to #{group_link.human_access}
+              - if group_link.expires?
+                ·
+                %span{ class: ('text-warning' if group_link.expires_soon?) }
+                  expires in #{distance_of_time_in_words_to_now(group_link.expires_at)}
             .pull-right
               = link_to namespace_project_group_link_path(@project.namespace, @project, group_link), method: :delete, class: "btn btn-transparent" do
                 %span.sr-only disable sharing

app/views/projects/project_members/_new_project_member.html.haml

@@ -14,5 +14,14 @@
         Read more about role permissions
         %strong= link_to "here", help_page_path("user/permissions"), class: "vlink"
 
+  .form-group
+    = f.label :expires_at, 'Access expiration date', class: 'control-label'
+    .col-sm-10
+      .clearable-input
+        = text_field_tag :expires_at, nil, class: 'form-control js-access-expiration-date', placeholder: 'Select access expiration date'
+        %i.clear-icon.js-clear-input
+      .help-block
+        On this date, the user(s) will automatically lose access to this project.
+
   .form-actions
     = f.submit 'Add users to project', class: "btn btn-create"

app/views/projects/project_members/index.html.haml

 - page_title "Members"
 
-.project-members-page.prepend-top-default
+.project-members-page.js-project-members-page.prepend-top-default
   - if can?(current_user, :admin_project_member, @project)
     .panel.panel-default
       .panel-heading

app/views/projects/project_members/update.js.haml

 :plain
   $("##{dom_id(@project_member)}").replaceWith('#{escape_javascript(render('shared/members/member', member: @project_member))}');
+  new MemberExpirationDate();

app/views/shared/members/_member.html.haml

@@ -16,7 +16,7 @@
           = button_tag icon('pencil'),
                        type: 'button',
                        class: 'btn inline js-toggle-button',
-                       title: 'Edit access level'
+                       title: 'Edit'
 
           - if member.request?
             = link_to icon('check inverse'), polymorphic_path([:approve_access_request, member]),
@@ -59,6 +59,10 @@
           = time_ago_with_tooltip(member.requested_at)
         - else
           Joined #{time_ago_with_tooltip(member.created_at)}
+        - if member.expires?
+          ·
+          %span{ class: ('text-warning' if member.expires_soon?) }
+            Expires in #{distance_of_time_in_words_to_now(member.expires_at)}
 
     - else
       = image_tag avatar_icon(member.invite_email, 40), class: "avatar s40", alt: ''
@@ -73,8 +77,16 @@
   - if show_roles
     .edit-member.hide.js-toggle-content
       %br
-      = form_for member, remote: true do |f|
-        .prepend-top-10
-          = f.select :access_level, options_for_select(member.class.access_level_roles, member.access_level), {}, class: 'form-control'
+      = form_for member, remote: true, html: { class: 'form-horizontal' }  do |f|
+        .form-group
+          = label_tag "member_access_level_#{member.id}", 'Project access', class: 'control-label'
+          .col-sm-10
+            = f.select :access_level, options_for_select(member.class.access_level_roles, member.access_level), {}, class: 'form-control', id: "member_access_level_#{member.id}"
+        .form-group
+          = label_tag "member_expires_at_#{member.id}", 'Access expiration date', class: 'control-label'
+          .col-sm-10
+            .clearable-input
+              = f.text_field :expires_at, class: 'form-control js-access-expiration-date', placeholder: 'Select access expiration date', id: "member_expires_at_#{member.id}"
+              %i.clear-icon.js-clear-input
         .prepend-top-10
           = f.submit 'Save', class: 'btn btn-save btn-sm'

app/workers/remove_expired_group_links_worker.rb

+class RemoveExpiredGroupLinksWorker
+  include Sidekiq::Worker
+
+  def perform
+    ProjectGroupLink.expired.destroy_all
+  end
+end

app/workers/remove_expired_members_worker.rb

+class RemoveExpiredMembersWorker
+  include Sidekiq::Worker
+
+  def perform
+    Member.expired.find_each do |member|
+      begin
+        Members::AuthorizedDestroyService.new(member).execute
+      rescue => ex
+        logger.error("Expired Member ID=#{member.id} cannot be removed - #{ex}")
+      end
+    end
+  end
+end

config/initializers/1_settings.rb

@@ -293,6 +293,12 @@ Settings.cron_jobs['import_export_project_cleanup_worker']['job_class'] = 'Impor
 Settings.cron_jobs['requests_profiles_worker'] ||= Settingslogic.new({})
 Settings.cron_jobs['requests_profiles_worker']['cron'] ||= '0 0 * * *'
 Settings.cron_jobs['requests_profiles_worker']['job_class'] = 'RequestsProfilesWorker'
+Settings.cron_jobs['remove_expired_members_worker'] ||= Settingslogic.new({})
+Settings.cron_jobs['remove_expired_members_worker']['cron'] ||= '10 0 * * *'
+Settings.cron_jobs['remove_expired_members_worker']['job_class'] = 'RemoveExpiredMembersWorker'
+Settings.cron_jobs['remove_expired_group_links_worker'] ||= Settingslogic.new({})
+Settings.cron_jobs['remove_expired_group_links_worker']['cron'] ||= '10 0 * * *'
+Settings.cron_jobs['remove_expired_group_links_worker']['job_class'] = 'RemoveExpiredGroupLinksWorker'
 
 #
 # GitLab Shell

db/migrate/20160801163421_add_expires_at_to_member.rb

+# See http://doc.gitlab.com/ce/development/migration_style_guide.html
+# for more information on how to write migrations for GitLab.
+
+class AddExpiresAtToMember < ActiveRecord::Migration
+  include Gitlab::Database::MigrationHelpers
+
+  # Set this constant to true if this migration requires downtime.
+  DOWNTIME = false
+
+  # When a migration requires downtime you **must** uncomment the following
+  # constant and define a short and easy to understand explanation as to why the
+  # migration requires downtime.
+  # DOWNTIME_REASON = ''
+
+  # When using the methods "add_concurrent_index" or "add_column_with_default"
+  # you must disable the use of transactions as these methods can not run in an
+  # existing transaction. When using "add_concurrent_index" make sure that this
+  # method is the _only_ method called in the migration, any other changes
+  # should go in a separate migration. This ensures that upon failure _only_ the
+  # index creation fails and can be retried or reverted easily.
+  #
+  # To disable transactions uncomment the following line and remove these
+  # comments:
+  # disable_ddl_transaction!
+
+  def change
+    add_column :members, :expires_at, :date
+  end
+end

db/migrate/20160818205718_add_expires_at_to_project_group_links.rb

+# See http://doc.gitlab.com/ce/development/migration_style_guide.html
+# for more information on how to write migrations for GitLab.
+
+class AddExpiresAtToProjectGroupLinks < ActiveRecord::Migration
+  include Gitlab::Database::MigrationHelpers
+
+  # Set this constant to true if this migration requires downtime.
+  DOWNTIME = false
+
+  # When a migration requires downtime you **must** uncomment the following
+  # constant and define a short and easy to understand explanation as to why the
+  # migration requires downtime.
+  # DOWNTIME_REASON = ''
+
+  # When using the methods "add_concurrent_index" or "add_column_with_default"
+  # you must disable the use of transactions as these methods can not run in an
+  # existing transaction. When using "add_concurrent_index" make sure that this
+  # method is the _only_ method called in the migration, any other changes
+  # should go in a separate migration. This ensures that upon failure _only_ the
+  # index creation fails and can be retried or reverted easily.
+  #
+  # To disable transactions uncomment the following line and remove these
+  # comments:
+  # disable_ddl_transaction!
+
+  def change
+    add_column :project_group_links, :expires_at, :date
+  end
+end

db/schema.rb

@@ -11,7 +11,7 @@
 #
 # It's strongly recommended that you check this file into your version control system.
 
-ActiveRecord::Schema.define(version: 20160817154936) do
+ActiveRecord::Schema.define(version: 20160818205718) do
 
   # These are extensions that must be enabled in order to support this database
   enable_extension "plpgsql"
@@ -568,6 +568,7 @@ ActiveRecord::Schema.define(version: 20160817154936) do
     t.string   "invite_token"
     t.datetime "invite_accepted_at"
     t.datetime "requested_at"
+    t.date     "expires_at"
   end
 
   add_index "members", ["access_level"], name: "index_members_on_access_level", using: :btree
@@ -783,6 +784,7 @@ ActiveRecord::Schema.define(version: 20160817154936) do
     t.datetime "created_at"
     t.datetime "updated_at"
     t.integer  "group_access", default: 30, null: false
+    t.date     "expires_at"
   end
 
   create_table "project_import_data", force: :cascade do |t|
@@ -1149,4 +1151,4 @@ ActiveRecord::Schema.define(version: 20160817154936) do
   add_foreign_key "protected_branch_merge_access_levels", "protected_branches"
   add_foreign_key "protected_branch_push_access_levels", "protected_branches"
   add_foreign_key "u2f_registrations", "users"
-end
+end
\ No newline at end of file

doc/api/members.md

@@ -86,7 +86,8 @@ Example response:
   "name": "Raymond Smith",
   "state": "active",
   "created_at": "2012-10-22T14:13:35Z",
-  "access_level": 30
+  "access_level": 30,
+  "expires_at": null
 }
 ```
 
@@ -106,6 +107,7 @@ POST /projects/:id/members
 | `id`      | integer/string  | yes | The group/project ID or path |
 | `user_id` | integer         | yes | The user ID of the new member |
 | `access_level` | integer | yes | A valid access level |
+| `expires_at` | string | no | A date string in the format YEAR-MONTH-DAY |
 
 ```bash
 curl --request POST --header "PRIVATE-TOKEN: 9koXpg98eAheJpvBs5tK" https://gitlab.example.com/api/v3/groups/:id/members/:user_id?access_level=30
@@ -141,6 +143,7 @@ PUT /projects/:id/members/:user_id
 | `id`      | integer/string | yes | The group/project ID or path |
 | `user_id` | integer | yes   | The user ID of the member |
 | `access_level` | integer | yes | A valid access level |
+| `expires_at` | string | no | A date string in the format YEAR-MONTH-DAY |
 
 ```bash
 curl --request PUT --header "PRIVATE-TOKEN: 9koXpg98eAheJpvBs5tK" https://gitlab.example.com/api/v3/groups/:id/members/:user_id?access_level=40

doc/workflow/share_projects_with_other_groups.md

 # Share Projects with other Groups
 
-In GitLab Enterprise Edition you can share projects with other groups.
-This makes it possible to add a group of users to a project with a single action.
+You can share projects with other groups. This makes it possible to add a group of users
+to a project with a single action.
 
 ## Groups as collections of users
 
-In GitLab Community Edition groups are used primarily to [create collections of projects](groups.md).
-In GitLab Enterprise Edition you can also take advantage of the fact that groups define collections of _users_, namely the group members.
+Groups are used primarily to [create collections of projects](groups.md), but you can also
+take advantage of the fact that groups define collections of _users_, namely the group
+members.
 
 ## Sharing a project with a group of users
 
-The primary mechanism to give a group of users, say 'Engineering', access to a project, say 'Project Acme', in GitLab is to make the 'Engineering' group the owner of 'Project Acme'.
-But what if 'Project Acme' already belongs to another group, say 'Open Source'?
-This is where the (Enterprise Edition only) group sharing feature can be of use.
+The primary mechanism to give a group of users, say 'Engineering', access to a project,
+say 'Project Acme', in GitLab is to make the 'Engineering' group the owner of 'Project
+Acme'.  But what if 'Project Acme' already belongs to another group, say 'Open Source'?
+This is where the group sharing feature can be of use.
 
 To share 'Project Acme' with the 'Engineering' group, go to the project settings page for 'Project Acme' and use the left navigation menu to go to the 'Groups' section.
 
-![The 'Groups' section in the project settings screen (Enterprise Edition only)](groups/share_project_with_groups.png)
+![The 'Groups' section in the project settings screen](groups/share_project_with_groups.png)
 
 Now you can add the 'Engineering' group with the maximum access level of your choice.
 After sharing 'Project Acme' with 'Engineering', the project is listed on the group dashboard.

features/steps/group/members.rb

@@ -116,8 +116,8 @@ class Spinach::Features::GroupMembers < Spinach::FeatureSteps
     member = mary_jane_member
 
     page.within "#group_member_#{member.id}" do
-      click_button "Edit access level"
-      select 'Developer', from: 'group_member_access_level'
+      click_button 'Edit'
+      select 'Developer', from: "member_access_level_#{member.id}"
       click_on 'Save'
     end
   end

features/steps/project/team_management.rb

@@ -65,8 +65,8 @@ class Spinach::Features::ProjectTeamManagement < Spinach::FeatureSteps
     user = User.find_by(name: 'Dmitriy')
     project_member = project.project_members.find_by(user_id: user.id)
     page.within "#project_member_#{project_member.id}" do
-      click_button "Edit access level"
-      select "Reporter", from: "project_member_access_level"
+      click_button 'Edit'
+      select "Reporter", from: "member_access_level_#{project_member.id}"
       click_button "Save"
     end
   end

lib/api/entities.rb

@@ -97,6 +97,10 @@ module API
         member = options[:member] || options[:members].find { |m| m.user_id == user.id }
         member.access_level
       end
+      expose :expires_at do |user, options|
+        member = options[:member] || options[:members].find { |m| m.user_id == user.id }
+        member.expires_at
+      end
     end
 
     class AccessRequester < UserBasic

lib/api/members.rb

@@ -49,6 +49,7 @@ module API
         #   id (required) - The group/project ID
         #   user_id (required) - The user ID of the new member
         #   access_level (required) - A valid access level
+        #   expires_at (optional) - Date string in the format YEAR-MONTH-DAY
         #
         # Example Request:
         #   POST /groups/:id/members
@@ -72,7 +73,7 @@ module API
           conflict!('Member already exists') if source_type == 'group' && member
 
           unless member
-            source.add_user(params[:user_id], params[:access_level], current_user)
+            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])
           end
 
@@ -81,7 +82,7 @@ module API
           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]))
+            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
@@ -97,6 +98,7 @@ module API
         #   id (required) - The group/project ID
         #   user_id (required) - The user ID of the member
         #   access_level (required) - A valid access level
+        #   expires_at (optional) - Date string in the format YEAR-MONTH-DAY
         #
         # Example Request:
         #   PUT /groups/:id/members/:user_id
@@ -107,8 +109,9 @@ module API
           required_attributes! [:user_id, :access_level]
 
           member = source.members.find_by!(user_id: params[:user_id])
+          attrs = attributes_for_keys [:access_level, :expires_at]
 
-          if member.update_attributes(access_level: params[:access_level])
+          if member.update_attributes(attrs)
             present member.user, with: Entities::Member, member: member
           else
             # This is to ensure back-compatibility but 400 behavior should be used

spec/features/projects/group_links_spec.rb

+require 'spec_helper'
+
+feature 'Project group links', feature: true, js: true do
+  include Select2Helper
+
+  let(:master) { create(:user) }
+  let(:project) { create(:project) }
+  let!(:group) { create(:group) }
+
+  background do
+    project.team << [master, :master]
+    login_as(master)
+  end
+
+  context 'setting an expiration date for a group link' do
+    before do
+      visit namespace_project_group_links_path(project.namespace, project)
+
+      select2 group.id, from: '#link_group_id'
+      fill_in 'expires_at', with: (Time.current + 4.5.days).strftime('%Y-%m-%d')
+      page.find('body').click
+      click_on 'Share'
+    end
+
+    it 'shows the expiration time with a warning class' do
+      page.within('.enabled-groups') do
+        expect(page).to have_content('expires in 4 days')
+        expect(page).to have_selector('.text-warning')
+      end
+    end
+  end
+end

spec/features/projects/members/master_adds_member_with_expiration_date_spec.rb

+require 'spec_helper'
+
+feature 'Projects > Members > Master adds member with expiration date', feature: true, js: true do
+  include Select2Helper
+  include ActiveSupport::Testing::TimeHelpers
+
+  let(:master) { create(:user) }
+  let(:project) { create(:project) }
+  let!(:new_member) { create(:user) }
+
+  background do
+    project.team << [master, :master]
+    login_as(master)
+  end
+
+  scenario 'expiration date is displayed in the members list' do
+    travel_to Time.zone.parse('2016-08-06 08:00') do
+      visit namespace_project_project_members_path(project.namespace, project)
+
+      page.within '.users-project-form' do
+        select2(new_member.id, from: '#user_ids', multiple: true)
+        fill_in 'expires_at', with: '2016-08-10'
+        click_on 'Add users to project'
+      end
+
+      page.within '.project_member:first-child' do
+        expect(page).to have_content('Expires in 4 days')
+      end
+    end
+  end
+
+  scenario 'change expiration date' do
+    travel_to Time.zone.parse('2016-08-06 08:00') do
+      project.team.add_users([new_member.id], :developer, expires_at: '2016-09-06')
+      visit namespace_project_project_members_path(project.namespace, project)
+
+      page.within '.project_member:first-child' do
+        click_on 'Edit'
+        fill_in 'Access expiration date', with: '2016-08-09'
+        click_on 'Save'
+        expect(page).to have_content('Expires in 3 days')
+      end
+    end
+  end
+end

spec/mailers/notify_spec.rb

@@ -493,7 +493,12 @@ describe Notify do
     end
 
     def invite_to_project(project:, email:, inviter:)
-      ProjectMember.add_user(project.project_members, 'toto@example.com', Gitlab::Access::DEVELOPER, inviter)
+      Member.add_user(
+        project.project_members,
+        'toto@example.com',
+        Gitlab::Access::DEVELOPER,
+        current_user: inviter
+      )
 
       project.project_members.invite.last
     end
@@ -740,7 +745,12 @@ describe Notify do
     end
 
     def invite_to_group(group:, email:, inviter:)
-      GroupMember.add_user(group.group_members, 'toto@example.com', Gitlab::Access::DEVELOPER, inviter)
+      Member.add_user(
+        group.group_members,
+        'toto@example.com',
+        Gitlab::Access::DEVELOPER,
+        current_user: inviter
+      )
 
       group.group_members.invite.last
     end

spec/models/member_spec.rb

@@ -65,11 +65,21 @@ describe Member, models: true do
       @master_user = create(:user).tap { |u| project.team << [u, :master] }
       @master = project.members.find_by(user_id: @master_user.id)
 
-      ProjectMember.add_user(project.members, 'toto1@example.com', Gitlab::Access::DEVELOPER, @master_user)
+      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')
 
       accepted_invite_user = build(:user)
-      ProjectMember.add_user(project.members, 'toto2@example.com', Gitlab::Access::DEVELOPER, @master_user)
+      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) }
 
       requested_user = create(:user).tap { |u| project.request_access(u) }

spec/requests/api/members_spec.rb

@@ -122,12 +122,13 @@ describe API::Members, api: true  do
         it 'creates a new member' do
           expect do
             post api("/#{source_type.pluralize}/#{source.id}/members", master),
-                 user_id: stranger.id, access_level: Member::DEVELOPER
+                 user_id: stranger.id, access_level: Member::DEVELOPER, expires_at: '2016-08-05'
 
             expect(response).to have_http_status(201)
           end.to change { source.members.count }.by(1)
           expect(json_response['id']).to eq(stranger.id)
           expect(json_response['access_level']).to eq(Member::DEVELOPER)
+          expect(json_response['expires_at']).to eq('2016-08-05')
         end
       end
 
@@ -183,11 +184,12 @@ describe API::Members, api: true  do
       context 'when authenticated as a master/owner' do
         it 'updates the member' do
           put api("/#{source_type.pluralize}/#{source.id}/members/#{developer.id}", master),
-              access_level: Member::MASTER
+              access_level: Member::MASTER, expires_at: '2016-08-05'
 
           expect(response).to have_http_status(200)
           expect(json_response['id']).to eq(developer.id)
           expect(json_response['access_level']).to eq(Member::MASTER)
+          expect(json_response['expires_at']).to eq('2016-08-05')
         end
       end
 

spec/workers/remove_expired_group_links_worker_spec.rb

+require 'spec_helper'
+
+describe RemoveExpiredGroupLinksWorker do
+  describe '#perform' do
+    let!(:expired_project_group_link) { create(:project_group_link, expires_at: 1.hour.ago) }
+    let!(:project_group_link_expiring_in_future) { create(:project_group_link, expires_at: 10.days.from_now) }
+    let!(:non_expiring_project_group_link) { create(:project_group_link, expires_at: nil) }
+
+    it 'removes expired group links' do
+      expect { subject.perform }.to change { ProjectGroupLink.count }.by(-1)
+      expect(ProjectGroupLink.find_by(id: expired_project_group_link.id)).to be_nil
+    end
+
+    it 'leaves group links that expire in the future' do
+      subject.perform
+      expect(project_group_link_expiring_in_future.reload).to be_present
+    end
+
+    it 'leaves group links that do not expire at all' do
+      subject.perform
+      expect(non_expiring_project_group_link.reload).to be_present
+    end
+  end
+end

spec/workers/remove_expired_members_worker_spec.rb

+require 'spec_helper'
+
+describe RemoveExpiredMembersWorker do
+  let(:worker) { RemoveExpiredMembersWorker.new }
+
+  describe '#perform' do
+    context 'project members' do
+      let!(:expired_project_member) { create(:project_member, expires_at: 1.hour.ago, access_level: GroupMember::DEVELOPER) }
+      let!(:project_member_expiring_in_future) { create(:project_member, expires_at: 10.days.from_now, access_level: GroupMember::DEVELOPER) }
+      let!(:non_expiring_project_member) { create(:project_member, expires_at: nil, access_level: GroupMember::DEVELOPER) }
+
+      it 'removes expired members' do
+        expect { worker.perform }.to change { Member.count }.by(-1)
+        expect(Member.find_by(id: expired_project_member.id)).to be_nil
+      end
+
+      it 'leaves members that expire in the future' do
+        worker.perform
+        expect(project_member_expiring_in_future.reload).to be_present
+      end
+
+      it 'leaves members that do not expire at all' do
+        worker.perform
+        expect(non_expiring_project_member.reload).to be_present
+      end
+    end
+
+    context 'group members' do
+      let!(:expired_group_member) { create(:group_member, expires_at: 1.hour.ago, access_level: GroupMember::DEVELOPER) }
+      let!(:group_member_expiring_in_future) { create(:group_member, expires_at: 10.days.from_now, access_level: GroupMember::DEVELOPER) }
+      let!(:non_expiring_group_member) { create(:group_member, expires_at: nil, access_level: GroupMember::DEVELOPER) }
+
+      it 'removes expired members' do
+        expect { worker.perform }.to change { Member.count }.by(-1)
+        expect(Member.find_by(id: expired_group_member.id)).to be_nil
+      end
+
+      it 'leaves members that expire in the future' do
+        worker.perform
+        expect(group_member_expiring_in_future.reload).to be_present
+      end
+
+      it 'leaves members that do not expire at all' do
+        worker.perform
+        expect(non_expiring_group_member.reload).to be_present
+      end
+    end
+
+    context 'when the last group owner expires' do
+      let!(:expired_group_owner) { create(:group_member, expires_at: 1.hour.ago, access_level: GroupMember::OWNER) }
+
+      it 'does not delete the owner' do
+        worker.perform
+        expect(expired_group_owner.reload).to be_present
+      end
+    end
+  end
+end