diff options
| author | Dmitriy Zaporozhets <dmitriy.zaporozhets@gmail.com> | 2015-04-15 10:48:28 +0000 |
|---|---|---|
| committer | Dmitriy Zaporozhets <dmitriy.zaporozhets@gmail.com> | 2015-04-15 10:48:28 +0000 |
| commit | e3d818a4e8f51c00aaba1924690f728aa468d7f2 (patch) | |
| tree | 2893b6d4528cbb0580d42de94725ff00fcfd5786 | |
| parent | b75866120b2391d0f2f01d03601a19baa9fa9b4e (diff) | |
| parent | 3d0c0bd922dadbf09c46f2b8acb58d44565394ce (diff) | |
| download | gitlab-ce-e3d818a4e8f51c00aaba1924690f728aa468d7f2.tar.gz | |
Merge branch 'invitation' into 'master'
Allow users to be invited.
Addresses private issue https://dev.gitlab.org/gitlab/gitlabhq/issues/2058.
The "Add members" panes for both Group Members and Project Members have gained a line of text by the People field.
Entering an email address that is not already a member will give you the option to invite them.
Choosing the option will add them to the People field. This works the right way (TM) in combination with adding existing users as members.
The invited member will be shown in the members list as such. The access level can be changed, and the invite can be revoked by deleting the member.
The invited user will receive an email with an "Accept invitation" link.
If they're not already logged in, clicking this link will redirect them to the sign in/up page with a helpful notice.
Signing in or signing up will redirect them back to the invite detail page, where they can actually accept the invitation, which will update the member record in question to point to the user in question.
Accepting the invitation will redirect them to the group (or project) with an appropriate notice.
As currently, they will also receive this information by email.
At the same time, the person who initially invited the email address is sent a notification as well, so they know of the new member and to tell them what name the user signed up with.
The member row on the Members page will now have been updated with the new user account.
See merge request !500
64 files changed, 1042 insertions, 179 deletions
diff --git a/CHANGELOG b/CHANGELOG index a207a59db63..b0c175bf437 100644 --- a/CHANGELOG +++ b/CHANGELOG @@ -1,6 +1,7 @@ Please view this file on the master branch, on stable branches it's out of date. v 7.10.0 (unreleased) + - Allow users to be invited by email to join a group or project. - Fix broken file browsing with a submodule that contains a relative link (Stan Hu) - Fix persistent XSS vulnerability around profile website URLs. - Fix project import URL regex to prevent arbitary local repos from being imported. diff --git a/app/assets/javascripts/users_select.js.coffee b/app/assets/javascripts/users_select.js.coffee index ccd85f2455d..aeeed9ca3cc 100644 --- a/app/assets/javascripts/users_select.js.coffee +++ b/app/assets/javascripts/users_select.js.coffee @@ -8,6 +8,7 @@ class @UsersSelect @groupId = $(select).data('group-id') showNullUser = $(select).data('null-user') showAnyUser = $(select).data('any-user') + showEmailUser = $(select).data('email-user') firstUser = $(select).data('first-user') $(select).select2 @@ -19,20 +20,6 @@ class @UsersSelect data = { results: users } if query.term.length == 0 - anyUser = { - name: 'Any', - avatar: null, - username: 'none', - id: null - } - - nullUser = { - name: 'Unassigned', - avatar: null, - username: 'none', - id: 0 - } - if firstUser # Move current user to the front of the list for obj, index in data.results @@ -40,11 +27,34 @@ class @UsersSelect data.results.splice(index, 1) data.results.unshift(obj) break + if showNullUser + nullUser = { + name: 'Unassigned', + avatar: null, + username: 'none', + id: 0 + } data.results.unshift(nullUser) + if showAnyUser + anyUser = { + name: 'Any', + avatar: null, + username: 'none', + id: null + } data.results.unshift(anyUser) + if showEmailUser && data.results.length == 0 && query.term.match(/^[^@]+@[^@]+$/) + emailUser = { + name: "Invite \"#{query.term}\"", + avatar: null, + username: query.term, + id: query.term + } + data.results.unshift(emailUser) + query.callback(data) initSelection: (element, callback) => diff --git a/app/controllers/admin/groups_controller.rb b/app/controllers/admin/groups_controller.rb index 9d9adaa467f..22d045fc388 100644 --- a/app/controllers/admin/groups_controller.rb +++ b/app/controllers/admin/groups_controller.rb @@ -41,7 +41,7 @@ class Admin::GroupsController < Admin::ApplicationController end def members_update - @group.add_users(params[:user_ids].split(','), params[:access_level]) + @group.add_users(params[:user_ids].split(','), params[:access_level], current_user) redirect_to [:admin, @group], notice: 'Users were successfully added.' end diff --git a/app/controllers/confirmations_controller.rb b/app/controllers/confirmations_controller.rb index bc98eab133c..af1faca93f6 100644 --- a/app/controllers/confirmations_controller.rb +++ b/app/controllers/confirmations_controller.rb @@ -4,11 +4,11 @@ class ConfirmationsController < Devise::ConfirmationsController def after_confirmation_path_for(resource_name, resource) if signed_in?(resource_name) - signed_in_root_path(resource) + after_sign_in_path_for(resource) else sign_in(resource) if signed_in?(resource_name) - signed_in_root_path(resource) + after_sign_in_path_for(resource) else new_session_path(resource_name) end diff --git a/app/controllers/groups/application_controller.rb b/app/controllers/groups/application_controller.rb index a73b8fa212a..469a6813ee2 100644 --- a/app/controllers/groups/application_controller.rb +++ b/app/controllers/groups/application_controller.rb @@ -13,7 +13,7 @@ class Groups::ApplicationController < ApplicationController end def authorize_admin_group! - unless can?(current_user, :manage_group, group) + unless can?(current_user, :admin_group, group) return render_404 end end diff --git a/app/controllers/groups/group_members_controller.rb b/app/controllers/groups/group_members_controller.rb index 2df51c97a22..265cf4f0f4a 100644 --- a/app/controllers/groups/group_members_controller.rb +++ b/app/controllers/groups/group_members_controller.rb @@ -11,6 +11,7 @@ class Groups::GroupMembersController < Groups::ApplicationController def index @project = @group.projects.find(params[:project_id]) if params[:project_id] @members = @group.group_members + @members = @members.non_invite unless can?(current_user, :admin_group, @group) if params[:search].present? users = @group.users.search(params[:search]).to_a @@ -22,7 +23,7 @@ class Groups::GroupMembersController < Groups::ApplicationController end def create - @group.add_users(params[:user_ids].split(','), params[:access_level]) + @group.add_users(params[:user_ids].split(','), params[:access_level], current_user) redirect_to group_group_members_path(@group), notice: 'Users were successfully added.' end @@ -38,7 +39,7 @@ class Groups::GroupMembersController < Groups::ApplicationController if can?(current_user, :destroy_group_member, @group_member) # May fail if last owner. @group_member.destroy respond_to do |format| - format.html { redirect_to group_group_members_path(@group), notice: 'User was successfully removed from group.' } + format.html { redirect_to group_group_members_path(@group), notice: 'User was successfully removed from group.' } format.js { render nothing: true } end else @@ -46,12 +47,26 @@ class Groups::GroupMembersController < Groups::ApplicationController end end + def resend_invite + redirect_path = group_group_members_path(@group) + + @group_member = @group.group_members.find(params[:id]) + + if @group_member.invite? + @group_member.resend_invite + + redirect_to redirect_path, notice: 'The invitation was successfully resent.' + else + redirect_to redirect_path, alert: 'The invitation has already been accepted.' + end + end + def leave @group_member = @group.group_members.where(user_id: current_user.id).first if can?(current_user, :destroy_group_member, @group_member) @group_member.destroy - redirect_to(dashboard_groups_path, info: "You left #{group.name} group.") + redirect_to(dashboard_groups_path, notice: "You left #{group.name} group.") else return render_403 end diff --git a/app/controllers/groups/milestones_controller.rb b/app/controllers/groups/milestones_controller.rb index c46b8fff88f..546ff2cc71f 100644 --- a/app/controllers/groups/milestones_controller.rb +++ b/app/controllers/groups/milestones_controller.rb @@ -51,6 +51,6 @@ class Groups::MilestonesController < ApplicationController end def authorize_group_milestone! - return render_404 unless can?(current_user, :manage_group, group) + return render_404 unless can?(current_user, :admin_group, group) end end diff --git a/app/controllers/invites_controller.rb b/app/controllers/invites_controller.rb new file mode 100644 index 00000000000..1f97ff16c55 --- /dev/null +++ b/app/controllers/invites_controller.rb @@ -0,0 +1,83 @@ +class InvitesController < ApplicationController + before_filter :member + skip_before_filter :authenticate_user!, only: :decline + + respond_to :html + + layout 'navless' + + def show + + end + + def accept + if member.accept_invite!(current_user) + label, path = source_info(member.source) + + redirect_to path, notice: "You have been granted #{member.human_access} access to #{label}." + else + redirect_to :back, alert: "The invitation could not be accepted." + end + end + + def decline + if member.decline_invite! + label, _ = source_info(member.source) + + path = + if current_user + dashboard_path + else + new_user_session_path + end + + redirect_to path, notice: "You have declined the invitation to join #{label}." + else + redirect_to :back, alert: "The invitation could not be declined." + end + end + + private + + def member + return @member if defined?(@member) + + @token = params[:id] + @member = Member.find_by_invite_token(@token) + + unless @member + render_404 and return + end + + @member + end + + def authenticate_user! + return if current_user + + notice = "To accept this invitation, sign in" + notice << " or create an account" if current_application_settings.signup_enabled? + notice << "." + + store_location_for :user, request.fullpath + redirect_to new_user_session_path, notice: notice + end + + def source_info(source) + case source + when Project + project = member.source + label = "project #{project.name_with_namespace}" + path = namespace_project_path(project.namespace, project) + when Group + group = member.source + label = "group #{group.name}" + path = group_path(group) + else + label = "who knows what" + path = dashboard_path + end + + [label, path] + end +end diff --git a/app/controllers/projects/project_members_controller.rb b/app/controllers/projects/project_members_controller.rb index 4ab15db01f7..72967a26ff1 100644 --- a/app/controllers/projects/project_members_controller.rb +++ b/app/controllers/projects/project_members_controller.rb @@ -6,6 +6,7 @@ class Projects::ProjectMembersController < Projects::ApplicationController def index @project_members = @project.project_members + @project_members = @project_members.non_invite unless can?(current_user, :admin_project, @project) if params[:search].present? users = @project.users.search(params[:search]).to_a @@ -17,6 +18,7 @@ class Projects::ProjectMembersController < Projects::ApplicationController @group = @project.group if @group @group_members = @group.group_members + @group_members = @group_members.non_invite unless can?(current_user, :admin_group, @group) if params[:search].present? users = @group.users.search(params[:search]).to_a @@ -34,30 +36,42 @@ class Projects::ProjectMembersController < Projects::ApplicationController end def create - users = User.where(id: params[:user_ids].split(',')) - @project.team << [users, params[:access_level]] + @project.team.add_users(params[:user_ids].split(','), params[:access_level], current_user) redirect_to namespace_project_project_members_path(@project.namespace, @project) end def update - @project_member = @project.project_members.find_by(user_id: member) + @project_member = @project.project_members.find(params[:id]) @project_member.update_attributes(member_params) end def destroy - @project_member = @project.project_members.find_by(user_id: member) + @project_member = @project.project_members.find(params[:id]) @project_member.destroy respond_to do |format| format.html do - redirect_to namespace_project_project_members_path(@project.namespace, - @project) + redirect_to namespace_project_project_members_path(@project.namespace, @project) end format.js { render nothing: true } end end + def resend_invite + redirect_path = namespace_project_project_members_path(@project.namespace, @project) + + @project_member = @project.project_members.find(params[:id]) + + if @project_member.invite? + @project_member.resend_invite + + redirect_to redirect_path, notice: 'The invitation was successfully resent.' + else + redirect_to redirect_path, alert: 'The invitation has already been accepted.' + end + end + def leave @project.project_members.find_by(user_id: current_user).destroy @@ -69,7 +83,7 @@ class Projects::ProjectMembersController < Projects::ApplicationController def apply_import giver = Project.find(params[:source_project_id]) - status = @project.team.import(giver) + status = @project.team.import(giver, current_user) notice = status ? "Successfully imported" : "Import failed" redirect_to(namespace_project_project_members_path(project.namespace, project), @@ -78,10 +92,6 @@ class Projects::ProjectMembersController < Projects::ApplicationController protected - def member - @member ||= User.find_by(username: params[:id]) - end - def member_params params.require(:project_member).permit(:user_id, :access_level) end diff --git a/app/helpers/groups_helper.rb b/app/helpers/groups_helper.rb index 2d0d0b494f6..add0a776a63 100644 --- a/app/helpers/groups_helper.rb +++ b/app/helpers/groups_helper.rb @@ -1,6 +1,10 @@ module GroupsHelper - def remove_user_from_group_message(group, user) - "Are you sure you want to remove \"#{user.name}\" from \"#{group.name}\"?" + def remove_user_from_group_message(group, member) + if member.user + "Are you sure you want to remove \"#{member.user.name}\" from \"#{group.name}\"?" + else + "Are you sure you want to revoke the invitation for \"#{member.invite_email}\" to join \"#{group.name}\"?" + end end def leave_group_message(group) diff --git a/app/helpers/projects_helper.rb b/app/helpers/projects_helper.rb index ebbd2bfd77d..c2a7732e6f0 100644 --- a/app/helpers/projects_helper.rb +++ b/app/helpers/projects_helper.rb @@ -1,6 +1,10 @@ module ProjectsHelper - def remove_from_project_team_message(project, user) - "You are going to remove #{user.name} from #{project.name} project team. Are you sure?" + def remove_from_project_team_message(project, member) + if member.user + "You are going to remove #{member.user.name} from #{project.name} project team. Are you sure?" + else + "You are going to revoke the invitation for #{member.invite_email} to join #{project.name} project team. Are you sure?" + end end def link_to_project(project) diff --git a/app/helpers/selects_helper.rb b/app/helpers/selects_helper.rb index 54e0f4f9b3e..bec8f2f1aa7 100644 --- a/app/helpers/selects_helper.rb +++ b/app/helpers/selects_helper.rb @@ -8,6 +8,7 @@ module SelectsHelper null_user = opts[:null_user] || false any_user = opts[:any_user] || false + email_user = opts[:email_user] || false first_user = opts[:first_user] && current_user ? current_user.username : false html = { @@ -15,6 +16,7 @@ module SelectsHelper 'data-placeholder' => placeholder, 'data-null-user' => null_user, 'data-any-user' => any_user, + 'data-email-user' => email_user, 'data-first-user' => first_user } diff --git a/app/mailers/emails/groups.rb b/app/mailers/emails/groups.rb index 626eb593d51..1c43f95dc8c 100644 --- a/app/mailers/emails/groups.rb +++ b/app/mailers/emails/groups.rb @@ -3,10 +3,50 @@ module Emails def group_access_granted_email(group_member_id) @group_member = GroupMember.find(group_member_id) @group = @group_member.group + @target_url = group_url(@group) @current_user = @group_member.user - mail(to: @group_member.user.email, + + mail(to: @group_member.user.notification_email, subject: subject("Access to group was granted")) end + + def group_member_invited_email(group_member_id, token) + @group_member = GroupMember.find group_member_id + @group = @group_member.group + @token = token + + @target_url = group_url(@group) + @current_user = @group_member.user + + mail(to: @group_member.invite_email, + subject: "Invitation to join group #{@group.name}") + end + + def group_invite_accepted_email(group_member_id) + @group_member = GroupMember.find group_member_id + return if @group_member.created_by.nil? + + @group = @group_member.group + + @target_url = group_url(@group) + @current_user = @group_member.created_by + + mail(to: @group_member.created_by.notification_email, + subject: subject("Invitation accepted")) + end + + def group_invite_declined_email(group_id, invite_email, access_level, created_by_id) + return if created_by_id.nil? + + @group = Group.find(group_id) + @current_user = @created_by = User.find(created_by_id) + @access_level = access_level + @invite_email = invite_email + + @target_url = group_url(@group) + mail(to: @created_by.notification_email, + subject: subject("Invitation declined")) + end end end diff --git a/app/mailers/emails/projects.rb b/app/mailers/emails/projects.rb index 20a863c3742..2584e9d48b1 100644 --- a/app/mailers/emails/projects.rb +++ b/app/mailers/emails/projects.rb @@ -1,14 +1,55 @@ module Emails module Projects - def project_access_granted_email(user_project_id) - @project_member = ProjectMember.find user_project_id + def project_access_granted_email(project_member_id) + @project_member = ProjectMember.find project_member_id @project = @project_member.project + @target_url = namespace_project_url(@project.namespace, @project) @current_user = @project_member.user - mail(to: @project_member.user.email, + + mail(to: @project_member.user.notification_email, subject: subject("Access to project was granted")) end + def project_member_invited_email(project_member_id, token) + @project_member = ProjectMember.find project_member_id + @project = @project_member.project + @token = token + + @target_url = namespace_project_url(@project.namespace, @project) + @current_user = @project_member.user + + mail(to: @project_member.invite_email, + subject: "Invitation to join project #{@project.name_with_namespace}") + end + + def project_invite_accepted_email(project_member_id) + @project_member = ProjectMember.find project_member_id + return if @project_member.created_by.nil? + + @project = @project_member.project + + @target_url = namespace_project_url(@project.namespace, @project) + @current_user = @project_member.created_by + + mail(to: @project_member.created_by.notification_email, + subject: subject("Invitation accepted")) + end + + def project_invite_declined_email(project_id, invite_email, access_level, created_by_id) + return if created_by_id.nil? + + @project = Project.find(project_id) + @current_user = @created_by = User.find(created_by_id) + @access_level = access_level + @invite_email = invite_email + + @target_url = namespace_project_url(@project.namespace, @project) + + mail(to: @created_by.notification_email, + subject: subject("Invitation declined")) + end + def project_was_moved_email(project_id, user_id) @current_user = @user = User.find user_id @project = Project.find project_id diff --git a/app/models/ability.rb b/app/models/ability.rb index d2b39f667f2..85a15596f8d 100644 --- a/app/models/ability.rb +++ b/app/models/ability.rb @@ -198,11 +198,11 @@ class Ability ]) end - # Only group owner and administrators can manage group + # Only group owner and administrators can admin group if group.has_owner?(user) || user.admin? rules.push(*[ - :manage_group, - :manage_namespace + :admin_group, + :admin_namespace ]) end @@ -212,11 +212,11 @@ class Ability def namespace_abilities(user, namespace) rules = [] - # Only namespace owner and administrators can manage it + # Only namespace owner and administrators can admin it if namespace.owner == user || user.admin? rules.push(*[ :create_projects, - :manage_namespace + :admin_namespace ]) end @@ -254,7 +254,7 @@ class Ability rules = [] target_user = subject.user group = subject.group - can_manage = group_abilities(user, group).include?(:manage_group) + can_manage = group_abilities(user, group).include?(:admin_group) if can_manage && (user != target_user) rules << :modify_group_member rules << :destroy_group_member diff --git a/app/models/group.rb b/app/models/group.rb index da9621a2a1a..1386a9eccc9 100644 --- a/app/models/group.rb +++ b/app/models/group.rb @@ -46,19 +46,18 @@ class Group < Namespace @owners ||= group_members.owners.map(&:user) end - def add_users(user_ids, access_level) - user_ids.compact.each do |user_id| - user = self.group_members.find_or_initialize_by(user_id: user_id) - user.update_attributes(access_level: access_level) + def add_users(user_ids, access_level, current_user = nil) + user_ids.each do |user_id| + Member.add_user(self.group_members, user_id, access_level, current_user) end end - def add_user(user, access_level) - self.group_members.create(user_id: user.id, access_level: access_level) + def add_user(user, access_level, current_user = nil) + add_users([user], access_level, current_user) end - def add_owner(user) - self.add_user(user, Gitlab::Access::OWNER) + def add_owner(user, current_user = nil) + self.add_user(user, Gitlab::Access::OWNER, current_user) end def has_owner?(user) diff --git a/app/models/member.rb b/app/models/member.rb index fe3d2f40e87..d151c7b2390 100644 --- a/app/models/member.rb +++ b/app/models/member.rb @@ -11,6 +11,10 @@ # type :string(255) # created_at :datetime # updated_at :datetime +# created_by_id :integer +# invite_email :string +# invite_token :string +# invite_accepted_at :datetime # class Member < ActiveRecord::Base @@ -18,19 +22,151 @@ class Member < ActiveRecord::Base include Notifiable include Gitlab::Access + attr_accessor :raw_invite_token + + belongs_to :created_by, class_name: "User" belongs_to :user belongs_to :source, polymorphic: true - validates :user, presence: true + validates :user, presence: true, unless: :invite? validates :source, presence: true - validates :user_id, uniqueness: { scope: [:source_type, :source_id], message: "already exists in source" } + validates :user_id, uniqueness: { scope: [:source_type, :source_id], + message: "already exists in source", + allow_nil: true } validates :access_level, inclusion: { in: Gitlab::Access.all_values }, presence: true + validates :invite_email, presence: { if: :invite? }, + email: { strict_mode: true, allow_nil: true }, + uniqueness: { scope: [:source_type, :source_id], allow_nil: true } + scope :invite, -> { where(user_id: nil) } + scope :non_invite, -> { where("user_id IS NOT NULL") } scope :guests, -> { where(access_level: GUEST) } scope :reporters, -> { where(access_level: REPORTER) } scope :developers, -> { where(access_level: DEVELOPER) } scope :masters, -> { where(access_level: MASTER) } scope :owners, -> { where(access_level: OWNER) } + before_validation :generate_invite_token, on: :create, if: -> (member) { member.invite_email.present? } + after_create :send_invite, if: :invite? + after_create :post_create_hook, unless: :invite? + after_update :post_update_hook, unless: :invite? + after_destroy :post_destroy_hook, unless: :invite? + delegate :name, :username, :email, to: :user, prefix: true + + class << self + def find_by_invite_token(invite_token) + invite_token = Devise.token_generator.digest(self, :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) + user = user_for_id(user_id) + + # `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) + else + member = members.build + member.invite_email = user + end + + member.created_by ||= current_user + member.access_level = access_level + + member.save + end + end + + def invite? + self.invite_token.present? + end + + def accept_invite!(new_user) + return false unless invite? + + self.invite_token = nil + self.invite_accepted_at = Time.now.utc + + self.user = new_user + + saved = self.save + + after_accept_invite if saved + + saved + end + + def decline_invite! + return false unless invite? + + destroyed = self.destroy + + after_decline_invite if destroyed + + destroyed + end + + def generate_invite_token + raw, enc = Devise.token_generator.generate(self.class, :invite_token) + @raw_invite_token = raw + self.invite_token = enc + end + + def generate_invite_token! + generate_invite_token && save(validate: false) + end + + def resend_invite + return unless invite? + + generate_invite_token! unless @raw_invite_token + + send_invite + end + + private + + def send_invite + # override in subclass + end + + def post_create_hook + system_hook_service.execute_hooks_for(self, :create) + end + + def post_update_hook + # override in subclass + end + + def post_destroy_hook + system_hook_service.execute_hooks_for(self, :destroy) + end + + def after_accept_invite + post_create_hook + end + + def after_decline_invite + # override in subclass + end + + def system_hook_service + SystemHooksService.new + end + + def notification_service + NotificationService.new + end end diff --git a/app/models/members/group_member.rb b/app/models/members/group_member.rb index 28d0b4483b4..84c91372b3f 100644 --- a/app/models/members/group_member.rb +++ b/app/models/members/group_member.rb @@ -27,10 +27,6 @@ class GroupMember < Member scope :with_group, ->(group) { where(source_id: group.id) } scope :with_user, ->(user) { where(user_id: user.id) } - after_create :post_create_hook - after_update :notify_update - after_destroy :post_destroy_hook - def self.access_level_roles Gitlab::Access.options_with_owner end @@ -43,26 +39,37 @@ class GroupMember < Member access_level end + private + + def send_invite + notification_service.invite_group_member(self, @raw_invite_token) + + super + end + def post_create_hook notification_service.new_group_member(self) - system_hook_service.execute_hooks_for(self, :create) + + super end - def notify_update + def post_update_hook if access_level_changed? notification_service.update_group_member(self) end - end - def post_destroy_hook - system_hook_service.execute_hooks_for(self, :destroy) + super end - def system_hook_service - SystemHooksService.new + def after_accept_invite + notification_service.accept_group_invite(self) + + super end - def notification_service - NotificationService.new + def after_decline_invite + notification_service.decline_group_invite(self) + + super end end diff --git a/app/models/members/project_member.rb b/app/models/members/project_member.rb index 6b13e0ff30b..0a3b4d2182b 100644 --- a/app/models/members/project_member.rb +++ b/app/models/members/project_member.rb @@ -27,10 +27,6 @@ class ProjectMember < Member validates_format_of :source_type, with: /\AProject\z/ default_scope { where(source_type: SOURCE_TYPE) } - after_create :post_create_hook - after_update :post_update_hook - after_destroy :post_destroy_hook - scope :in_project, ->(project) { where(source_id: project.id) } scope :in_projects, ->(projects) { where(source_id: projects.pluck(:id)) } scope :with_user, ->(user) { where(user_id: user.id) } @@ -55,7 +51,7 @@ class ProjectMember < Member # :master # ) # - def add_users_into_projects(project_ids, user_ids, access) + def add_users_into_projects(project_ids, user_ids, access, current_user = nil) access_level = if roles_hash.has_key?(access) roles_hash[access] elsif roles_hash.values.include?(access.to_i) @@ -64,12 +60,14 @@ class ProjectMember < Member raise "Non valid access" end + users = user_ids.map { |user_id| Member.user_for_id(user_id) } + ProjectMember.transaction do project_ids.each do |project_id| - user_ids.each do |user_id| - member = ProjectMember.new(access_level: access_level, user_id: user_id) - member.source_id = project_id - member.save + project = Project.find(project_id) + + users.each do |user| + Member.add_user(project.project_members, user, access_level, current_user) end end end @@ -82,6 +80,7 @@ class ProjectMember < Member def truncate_teams(project_ids) ProjectMember.transaction do members = ProjectMember.where(source_id: project_ids) + members.each do |member| member.destroy end @@ -109,41 +108,58 @@ class ProjectMember < Member access_level end + def project + source + end + def owner? project.owner == user end + private + + def send_invite + notification_service.invite_project_member(self, @raw_invite_token) + + super + end + def post_create_hook unless owner? event_service.join_project(self.project, self.user) notification_service.new_project_member(self) end - system_hook_service.execute_hooks_for(self, :create) + super end def post_update_hook - notification_service.update_project_member(self) if self.access_level_changed? + if access_level_changed? + notification_service.update_project_member(self) + end + + super end def post_destroy_hook event_service.leave_project(self.project, self.user) - system_hook_service.execute_hooks_for(self, :destroy) - end - def event_service - EventCreateService.new + super end - def notification_service - NotificationService.new + def after_accept_invite + notification_service.accept_project_invite(self) + + super end - def system_hook_service - SystemHooksService.new + def after_decline_invite + notification_service.decline_project_invite(self) + + super end - def project - source + def event_service + EventCreateService.new end end diff --git a/app/models/project_team.rb b/app/models/project_team.rb index d4a07caf9ef..56e49af2324 100644 --- a/app/models/project_team.rb +++ b/app/models/project_team.rb @@ -12,12 +12,12 @@ class ProjectTeam # @team << [@users, :master] # def <<(args) - users = args.first + users, access, current_user = *args if users.respond_to?(:each) - add_users(users, args.second) + add_users(users, access, current_user) else - add_user(users, args.second) + add_user(users, access, current_user) end end @@ -43,22 +43,19 @@ class ProjectTeam member end - def add_user(user, access) - add_users_ids([user.id], access) - end - - def add_users(users, access) - add_users_ids(users.map(&:id), access) - end - - def add_users_ids(user_ids, access) + def add_users(users, access, current_user = nil) ProjectMember.add_users_into_projects( [project.id], - user_ids, - access + users, + access, + current_user ) end + def add_user(user, access, current_user = nil) + add_users([user], access, current_user) + end + # Remove all users from project team def truncate ProjectMember.truncate_team(project) @@ -88,7 +85,7 @@ class ProjectTeam @masters ||= fetch_members(:masters) end - def import(source_project) + def import(source_project, current_user = nil) target_project = project source_members = source_project.project_members.to_a @@ -96,13 +93,14 @@ class ProjectTeam source_members.reject! do |member| # Skip if user already present in team - target_user_ids.include?(member.user_id) + !member.invite? && target_user_ids.include?(member.user_id) end source_members.map! do |member| new_member = member.dup new_member.id = nil new_member.source = target_project + new_member.created_by = current_user new_member end diff --git a/app/services/notification_service.rb b/app/services/notification_service.rb index 42547f6f481..203e654c18f 100644 --- a/app/services/notification_service.rb +++ b/app/services/notification_service.rb @@ -165,6 +165,18 @@ class NotificationService end end + def invite_project_member(project_member, token) + mailer.project_member_invited_email(project_member.id, token) + end + + def accept_project_invite(project_member) + mailer.project_invite_accepted_email(project_member.id) + end + + def decline_project_invite(project_member) + mailer.project_invite_declined_email(project_member.project.id, project_member.invite_email, project_member.access_level, project_member.created_by_id) + end + def new_project_member(project_member) mailer.project_access_granted_email(project_member.id) end @@ -173,6 +185,18 @@ class NotificationService mailer.project_access_granted_email(project_member.id) end + def invite_group_member(group_member, token) + mailer.group_member_invited_email(group_member.id, token) + end + + def accept_group_invite(group_member) + mailer.group_invite_accepted_email(group_member.id) + end + + def decline_group_invite(group_member) + mailer.group_invite_declined_email(group_member.group.id, group_member.invite_email, group_member.access_level, group_member.created_by_id) + end + def new_group_member(group_member) mailer.group_access_granted_email(group_member.id) end diff --git a/app/services/projects/create_service.rb b/app/services/projects/create_service.rb index 7ffd0b3882a..a7afcf8f64b 100644 --- a/app/services/projects/create_service.rb +++ b/app/services/projects/create_service.rb @@ -83,7 +83,7 @@ module Projects system_hook_service.execute_hooks_for(@project, :create) unless @project.group - @project.team << [current_user, :master] + @project.team << [current_user, :master, current_user] end @project.update_column(:last_activity_at, @project.created_at) diff --git a/app/services/projects/fork_service.rb b/app/services/projects/fork_service.rb index 4ec98696a65..1e4deb6ed39 100644 --- a/app/services/projects/fork_service.rb +++ b/app/services/projects/fork_service.rb @@ -38,7 +38,7 @@ module Projects #First save the DB entries as they can be rolled back if the repo fork fails project.build_forked_project_link(forked_to_project_id: project.id, forked_from_project_id: @from_project.id) if project.save - project.team << [@current_user, :master] + project.team << [@current_user, :master, @current_user] end #Now fork the repo diff --git a/app/views/admin/groups/show.html.haml b/app/views/admin/groups/show.html.haml index 7d292118075..14996dcd6a2 100644 --- a/app/views/admin/groups/show.html.haml +++ b/app/views/admin/groups/show.html.haml @@ -60,7 +60,7 @@ = form_tag members_update_admin_group_path(@group), id: "new_project_member", class: "bulk_import", method: :put do %div - = users_select_tag(:user_ids, multiple: true) + = users_select_tag(:user_ids, multiple: true, email_user: true) %div.prepend-top-10 = select_tag :access_level, options_for_select(GroupMember.access_level_roles), class: "project-access-select select2" %hr @@ -74,13 +74,18 @@ %ul.well-list.group-users-list - @members.each do |member| - user = member.user - %li{class: dom_class(member), id: dom_id(user)} + %li{class: dom_class(member), id: (dom_id(user) if user)} .list-item-name - %strong - = link_to user.name, admin_user_path(user) + - if user + %strong + = link_to user.name, admin_user_path(user) + - else + %strong + = member.invite_email + (invited) %span.pull-right.light = member.human_access - = link_to group_group_member_path(@group, member), data: { confirm: remove_user_from_group_message(@group, user) }, method: :delete, remote: true, class: "btn-xs btn btn-remove", title: 'Remove user from group' do + = link_to group_group_member_path(@group, member), data: { confirm: remove_user_from_group_message(@group, member) }, method: :delete, remote: true, class: "btn-xs btn btn-remove", title: 'Remove user from group' do %i.fa.fa-minus.fa-inverse .panel-footer = paginate @members, param_name: 'members_page', theme: 'gitlab' diff --git a/app/views/admin/projects/show.html.haml b/app/views/admin/projects/show.html.haml index b0b23132560..78684c692c7 100644 --- a/app/views/admin/projects/show.html.haml +++ b/app/views/admin/projects/show.html.haml @@ -124,14 +124,19 @@ - user = project_member.user %li.project_member .list-item-name - %strong - = link_to user.name, admin_user_path(user) + - if user + %strong + = link_to user.name, admin_user_path(user) + - else + %strong + = project_member.invite_email + (invited) .pull-right - if project_member.owner? %span.light Owner - else %span.light= project_member.human_access - = link_to namespace_project_project_member_path(@project.namespace, @project, user), data: { confirm: remove_from_project_team_message(@project, user)}, method: :delete, remote: true, class: "btn btn-sm btn-remove" do + = link_to namespace_project_project_member_path(@project.namespace, @project, project_member), data: { confirm: remove_from_project_team_message(@project, project_member)}, method: :delete, remote: true, class: "btn btn-sm btn-remove" do %i.fa.fa-times .panel-footer = paginate @project_members, param_name: 'project_members_page', theme: 'gitlab' diff --git a/app/views/admin/users/show.html.haml b/app/views/admin/users/show.html.haml index 0a2934d3bda..3524f04c5ed 100644 --- a/app/views/admin/users/show.html.haml +++ b/app/views/admin/users/show.html.haml @@ -182,7 +182,7 @@ .pull-right %span.light= group_member.human_access - unless group_member.owner? - = link_to group_group_member_path(group, group_member), data: { confirm: remove_user_from_group_message(group, @user) }, method: :delete, remote: true, class: "btn-xs btn btn-remove", title: 'Remove user from group' do + = link_to group_group_member_path(group, group_member), data: { confirm: remove_user_from_group_message(group, group_member) }, method: :delete, remote: true, class: "btn-xs btn btn-remove", title: 'Remove user from group' do %i.fa.fa-times.fa-inverse - else .nothing-here-block This user has no groups. @@ -221,7 +221,7 @@ %span.light= member.human_access - if member.respond_to? :project - = link_to namespace_project_project_member_path(project.namespace, project, @user), data: { confirm: remove_from_project_team_message(project, @user) }, remote: true, method: :delete, class: "btn-xs btn btn-remove", title: 'Remove user from project' do + = link_to namespace_project_project_member_path(project.namespace, project, member), data: { confirm: remove_from_project_team_message(project, member) }, remote: true, method: :delete, class: "btn-xs btn btn-remove", title: 'Remove user from project' do %i.fa.fa-times #ssh-keys.tab-pane = render 'profiles/keys/key_table', admin: true diff --git a/app/views/dashboard/groups/index.html.haml b/app/views/dashboard/groups/index.html.haml index 165db214d75..0cb7f764fab 100644 --- a/app/views/dashboard/groups/index.html.haml +++ b/app/views/dashboard/groups/index.html.haml @@ -17,7 +17,7 @@ - group = group_member.group %li .pull-right - - if can?(current_user, :manage_group, group) + - if can?(current_user, :admin_group, group) = link_to edit_group_path(group), class: "btn-sm btn btn-grouped" do %i.fa.fa-cogs Settings diff --git a/app/views/groups/group_members/_group_member.html.haml b/app/views/groups/group_members/_group_member.html.haml index 2462c952090..56b1948a474 100644 --- a/app/views/groups/group_members/_group_member.html.haml +++ b/app/views/groups/group_members/_group_member.html.haml @@ -1,17 +1,32 @@ - user = member.user -- return unless user +- return unless user || member.invite? - show_roles = true if show_roles.nil? %li{class: "#{dom_class(member)} js-toggle-container", id: dom_id(member)} %span{class: ("list-item-name" if show_controls)} - = image_tag avatar_icon(user.email, 16), class: "avatar s16", alt: '' - %strong= user.name - %span.cgray= user.username - - if user == current_user - %span.label.label-success It's you - - if user.blocked? - %label.label.label-danger - %strong Blocked + - if member.user + = image_tag avatar_icon(user.email, 16), class: "avatar s16", alt: '' + %strong= user.name + %span.cgray= user.username + - if user == current_user + %span.label.label-success It's you + - if user.blocked? + %label.label.label-danger + %strong Blocked + - else + = image_tag avatar_icon(member.invite_email, 16), class: "avatar s16", alt: '' + %strong + = member.invite_email + %span.cgray + invited + - if member.created_by + by + = link_to member.created_by.name, user_path(member.created_by) + = time_ago_with_tooltip(member.created_at) + + - if show_controls && can?(current_user, :admin_group, @group) + = link_to resend_invite_group_group_member_path(@group, member), method: :post, class: "btn-xs btn", title: 'Resend invite' do + Resend invite - if show_roles %span.pull-right @@ -27,7 +42,7 @@ = link_to leave_group_group_members_path(@group), data: { confirm: leave_group_message(@group.name)}, method: :delete, class: "btn-xs btn btn-remove", title: 'Remove user from group' do %i.fa.fa-minus.fa-inverse - else - = link_to group_group_member_path(@group, member), data: { confirm: remove_user_from_group_message(@group, user) }, method: :delete, remote: true, class: "btn-xs btn btn-remove", title: 'Remove user from group' do + = link_to group_group_member_path(@group, member), data: { confirm: remove_user_from_group_message(@group, member) }, method: :delete, remote: true, class: "btn-xs btn btn-remove", title: 'Remove user from group' do %i.fa.fa-minus.fa-inverse .edit-member.hide.js-toggle-content diff --git a/app/views/groups/group_members/_new_group_member.html.haml b/app/views/groups/group_members/_new_group_member.html.haml index a52b8197384..3361d7e2a8d 100644 --- a/app/views/groups/group_members/_new_group_member.html.haml +++ b/app/views/groups/group_members/_new_group_member.html.haml @@ -1,7 +1,10 @@ = form_for @group_member, url: group_group_members_path(@group), html: { class: 'form-horizontal users-group-form' } do |f| .form-group = f.label :user_ids, "People", class: 'control-label' - .col-sm-10= users_select_tag(:user_ids, multiple: true, class: 'input-large', scope: :all) + .col-sm-10 + = users_select_tag(:user_ids, multiple: true, class: 'input-large', scope: :all, email_user: true) + .help-block + Search for existing users or invite new ones using their email address. .form-group = f.label :access_level, "Group Access", class: 'control-label' diff --git a/app/views/groups/group_members/index.html.haml b/app/views/groups/group_members/index.html.haml index 0d501fe7bd3..c0c9cd170ad 100644 --- a/app/views/groups/group_members/index.html.haml +++ b/app/views/groups/group_members/index.html.haml @@ -16,7 +16,7 @@ = search_field_tag :search, params[:search], { placeholder: 'Find existing member by name', class: 'form-control search-text-input input-mn-300' } = button_tag 'Search', class: 'btn' - - if current_user && current_user.can?(:manage_group, @group) + - if current_user && current_user.can?(:admin_group, @group) .pull-right = button_tag class: 'btn btn-new js-toggle-button', type: 'button' do Add members diff --git a/app/views/groups/milestones/_milestone.html.haml b/app/views/groups/milestones/_milestone.html.haml index 94fc43a581e..30093d2d05d 100644 --- a/app/views/groups/milestones/_milestone.html.haml +++ b/app/views/groups/milestones/_milestone.html.haml @@ -1,6 +1,6 @@ %li{class: "milestone milestone-#{milestone.closed? ? 'closed' : 'open'}", id: dom_id(milestone.milestones.first) } .pull-right - - if can?(current_user, :manage_group, @group) + - if can?(current_user, :admin_group, @group) - if milestone.closed? = link_to 'Reopen Milestone', group_milestone_path(@group, milestone.safe_title, title: milestone.title, milestone: {state_event: :activate }), method: :put, class: "btn btn-sm btn-grouped btn-reopen" - else diff --git a/app/views/groups/milestones/show.html.haml b/app/views/groups/milestones/show.html.haml index fea70f5cbc3..fb32f2caa4c 100644 --- a/app/views/groups/milestones/show.html.haml +++ b/app/views/groups/milestones/show.html.haml @@ -6,7 +6,7 @@ Open Milestone #{@group_milestone.title} .pull-right - - if can?(current_user, :manage_group, @group) + - if can?(current_user, :admin_group, @group) - if @group_milestone.active? = link_to 'Close Milestone', group_milestone_path(@group, @group_milestone.safe_title, title: @group_milestone.title, milestone: {state_event: :close }), method: :put, class: "btn btn-sm btn-close" - else diff --git a/app/views/groups/projects.html.haml b/app/views/groups/projects.html.haml index dd1fa3840d5..0d547984cc9 100644 --- a/app/views/groups/projects.html.haml +++ b/app/views/groups/projects.html.haml @@ -2,7 +2,7 @@ .panel-heading %strong= @group.name projects: - - if can? current_user, :manage_group, @group + - if can? current_user, :admin_group, @group .panel-head-actions = link_to new_project_path(namespace_id: @group.id), class: "btn btn-sm btn-success" do %i.fa.fa-plus diff --git a/app/views/invites/show.html.haml b/app/views/invites/show.html.haml new file mode 100644 index 00000000000..ab0ecffe4d2 --- /dev/null +++ b/app/views/invites/show.html.haml @@ -0,0 +1,29 @@ +%h3.page-title Invitation + +%p + You have been invited + - if inviter = @member.created_by + by + = link_to inviter.name, user_url(inviter) + to join + - case @member.source + - when Project + - project = @member.source + project + %strong + = link_to project.name_with_namespace, namespace_project_url(project.namespace, project) + - when Group + - group = @member.source + group + %strong + = link_to group.name, group_url(group) + as #{@member.human_access}. + +- if @member.source.users.include?(current_user) + %p + However, you are already a member of this #{@member.source.is_a?(Group) ? "group" : "project"}. + Sign in using a different account to accept the invitation. +- else + .actions + = link_to "Accept invitation", accept_invite_url(@token), method: :post, class: "btn btn-success" + = link_to "Decline", decline_invite_url(@token), method: :post, class: "btn btn-danger prepend-left-10" diff --git a/app/views/layouts/_head_panel.html.haml b/app/views/layouts/_head_panel.html.haml index b1c2e1a7b19..d58582c107a 100644 --- a/app/views/layouts/_head_panel.html.haml +++ b/app/views/layouts/_head_panel.html.haml @@ -39,7 +39,7 @@ = link_to profile_path, title: "Profile settings", class: 'has_bottom_tooltip', 'data-original-title' => 'Profile settings"' do %i.fa.fa-user %li - = link_to destroy_user_session_path, class: "logout", method: :delete, title: "Logout", class: 'has_bottom_tooltip', 'data-original-title' => 'Logout' do + = link_to destroy_user_session_path, class: "logout", method: :delete, title: "Sign out", class: 'has_bottom_tooltip', 'data-original-title' => 'Sign out' do %i.fa.fa-sign-out %li.hidden-xs = link_to current_user, class: "profile-pic has_bottom_tooltip", id: 'profile-pic', 'data-original-title' => 'Your profile' do diff --git a/app/views/layouts/nav/_group.html.haml b/app/views/layouts/nav/_group.html.haml index 32fe0e37df8..f0d92b7a12c 100644 --- a/app/views/layouts/nav/_group.html.haml +++ b/app/views/layouts/nav/_group.html.haml @@ -30,7 +30,7 @@ %span Members - - if can?(current_user, :manage_group, @group) + - if can?(current_user, :admin_group, @group) = nav_link(html_options: { class: "#{"active" if group_settings_page?} separate-item" }) do = link_to edit_group_path(@group), title: 'Settings', class: "tab no-highlight" do %i.fa.fa-cogs diff --git a/app/views/notify/group_invite_accepted_email.html.haml b/app/views/notify/group_invite_accepted_email.html.haml new file mode 100644 index 00000000000..55efad384a7 --- /dev/null +++ b/app/views/notify/group_invite_accepted_email.html.haml @@ -0,0 +1,6 @@ +%p + #{@group_member.invite_email}, now known as + #{link_to @group_member.user.name, user_url(@group_member.user)}, + has accepted your invitation to join group + #{link_to @group.name, group_url(@group)}. + diff --git a/app/views/notify/group_invite_accepted_email.text.erb b/app/views/notify/group_invite_accepted_email.text.erb new file mode 100644 index 00000000000..f8b70f7a5a6 --- /dev/null +++ b/app/views/notify/group_invite_accepted_email.text.erb @@ -0,0 +1,3 @@ +<%= @group_member.invite_email %>, now known as <%= @group_member.user.name %>, has accepted your invitation to join group <%= @group.name %>. + +<%= group_url(@group) %> diff --git a/app/views/notify/group_invite_declined_email.html.haml b/app/views/notify/group_invite_declined_email.html.haml new file mode 100644 index 00000000000..f9525d84fac --- /dev/null +++ b/app/views/notify/group_invite_declined_email.html.haml @@ -0,0 +1,5 @@ +%p + #{@invite_email} + has declined your invitation to join group + #{link_to @group.name, group_url(@group)}. + diff --git a/app/views/notify/group_invite_declined_email.text.erb b/app/views/notify/group_invite_declined_email.text.erb new file mode 100644 index 00000000000..6c19a288d15 --- /dev/null +++ b/app/views/notify/group_invite_declined_email.text.erb @@ -0,0 +1,3 @@ +<%= @invite_email %> has declined your invitation to join group <%= @group.name %>. + +<%= group_url(@group) %> diff --git a/app/views/notify/group_member_invited_email.html.haml b/app/views/notify/group_member_invited_email.html.haml new file mode 100644 index 00000000000..163e88bfea3 --- /dev/null +++ b/app/views/notify/group_member_invited_email.html.haml @@ -0,0 +1,14 @@ +%p + You have been invited + - if inviter = @group_member.created_by + by + = link_to inviter.name, user_url(inviter) + to join group + = link_to @group.name, group_url(@group) + as #{@group_member.human_access}. + +%p + = link_to 'Accept invitation', invite_url(@token) + or + = link_to 'decline', decline_invite_url(@token) + diff --git a/app/views/notify/group_member_invited_email.text.erb b/app/views/notify/group_member_invited_email.text.erb new file mode 100644 index 00000000000..28ce4819b14 --- /dev/null +++ b/app/views/notify/group_member_invited_email.text.erb @@ -0,0 +1,4 @@ +You have been invited <%= "by #{@group_member.created_by.name} " if @group_member.created_by %>to join group <%= @group.name %> as <%= @group_member.human_access %>. + +Accept invitation: <%= invite_url(@token) %> +Decline invitation: <%= decline_invite_url(@token) %> diff --git a/app/views/notify/project_invite_accepted_email.html.haml b/app/views/notify/project_invite_accepted_email.html.haml new file mode 100644 index 00000000000..7e58d30b10a --- /dev/null +++ b/app/views/notify/project_invite_accepted_email.html.haml @@ -0,0 +1,6 @@ +%p + #{@project_member.invite_email}, now known as + #{link_to @project_member.user.name, user_url(@project_member.user)}, + has accepted your invitation to join project + #{link_to @project.name_with_namespace, namespace_project_url(@project.namespace, @project)}. + diff --git a/app/views/notify/project_invite_accepted_email.text.erb b/app/views/notify/project_invite_accepted_email.text.erb new file mode 100644 index 00000000000..fcbe752114d --- /dev/null +++ b/app/views/notify/project_invite_accepted_email.text.erb @@ -0,0 +1,3 @@ +<%= @project_member.invite_email %>, now known as <%= @project_member.user.name %>, has accepted your invitation to join project <%= @project.name_with_namespace %>. + +<%= namespace_project_url(@project.namespace, @project) %> diff --git a/app/views/notify/project_invite_declined_email.html.haml b/app/views/notify/project_invite_declined_email.html.haml new file mode 100644 index 00000000000..c2d7e6f6e3a --- /dev/null +++ b/app/views/notify/project_invite_declined_email.html.haml @@ -0,0 +1,5 @@ +%p + #{@invite_email} + has declined your invitation to join project + #{link_to @project.name_with_namespace, namespace_project_url(@project.namespace, @project)}. + diff --git a/app/views/notify/project_invite_declined_email.text.erb b/app/views/notify/project_invite_declined_email.text.erb new file mode 100644 index 00000000000..484687fa51c --- /dev/null +++ b/app/views/notify/project_invite_declined_email.text.erb @@ -0,0 +1,3 @@ +<%= @invite_email %> has declined your invitation to join project <%= @project.name_with_namespace %>. + +<%= namespace_project_url(@project.namespace, @project) %> diff --git a/app/views/notify/project_member_invited_email.html.haml b/app/views/notify/project_member_invited_email.html.haml new file mode 100644 index 00000000000..79eb89616de --- /dev/null +++ b/app/views/notify/project_member_invited_email.html.haml @@ -0,0 +1,13 @@ +%p + You have been invited + - if inviter = @project_member.created_by + by + = link_to inviter.name, user_url(inviter) + to join project + = link_to @project.name_with_namespace, namespace_project_url(@project.namespace, @project) + as #{@project_member.human_access}. + +%p + = link_to 'Accept invitation', invite_url(@token) + or + = link_to 'decline', decline_invite_url(@token) diff --git a/app/views/notify/project_member_invited_email.text.erb b/app/views/notify/project_member_invited_email.text.erb new file mode 100644 index 00000000000..e0706272115 --- /dev/null +++ b/app/views/notify/project_member_invited_email.text.erb @@ -0,0 +1,4 @@ +You have been invited <%= "by #{@project_member.created_by.name} " if @project_member.created_by %>to join project <%= @project.name_with_namespace %> as <%= @project_member.human_access %>. + +Accept invitation: <%= invite_url(@token) %> +Decline invitation: <%= decline_invite_url(@token) %> diff --git a/app/views/projects/project_members/_new_project_member.html.haml b/app/views/projects/project_members/_new_project_member.html.haml index 5daae2708e6..d708b01a114 100644 --- a/app/views/projects/project_members/_new_project_member.html.haml +++ b/app/views/projects/project_members/_new_project_member.html.haml @@ -1,7 +1,10 @@ = form_for @project_member, as: :project_member, url: namespace_project_project_members_path(@project.namespace, @project), html: { class: 'form-horizontal users-project-form' } do |f| .form-group = f.label :user_ids, "People", class: 'control-label' - .col-sm-10= users_select_tag(:user_ids, multiple: true, class: 'input-large', scope: :all) + .col-sm-10 + = users_select_tag(:user_ids, multiple: true, class: 'input-large', scope: :all, email_user: true) + .help-block + Search for existing users or invite new ones using their email address. .form-group = f.label :access_level, "Project Access", class: 'control-label' diff --git a/app/views/projects/project_members/_project_member.html.haml b/app/views/projects/project_members/_project_member.html.haml index 4f053977215..635e4d70941 100644 --- a/app/views/projects/project_members/_project_member.html.haml +++ b/app/views/projects/project_members/_project_member.html.haml @@ -1,16 +1,32 @@ - user = member.user -- return unless user +- return unless user || member.invite? %li{class: "#{dom_class(member)} js-toggle-container project_member_row access-#{member.human_access.downcase}", id: dom_id(member)} %span.list-item-name - = image_tag avatar_icon(user.email, 16), class: "avatar s16", alt: '' - %strong= user.name - %span.cgray= user.username - - if user == current_user - %span.label.label-success It's you - - if user.blocked? - %label.label.label-danger - %strong Blocked + - if member.user + = image_tag avatar_icon(user.email, 16), class: "avatar s16", alt: '' + %strong + = link_to user.name, user_path(user) + %span.cgray= user.username + - if user == current_user + %span.label.label-success It's you + - if user.blocked? + %label.label.label-danger + %strong Blocked + - else + = image_tag avatar_icon(member.invite_email, 16), class: "avatar s16", alt: '' + %strong + = member.invite_email + %span.cgray + invited + - if member.created_by + by + = link_to member.created_by.name, user_path(member.created_by) + = time_ago_with_tooltip(member.created_at) + + - if current_user_can_admin_project + = link_to resend_invite_namespace_project_project_member_path(@project.namespace, @project, member), method: :post, class: "btn-xs btn", title: 'Resend invite' do + Resend invite - if current_user_can_admin_project - unless @project.personal? && user == current_user @@ -25,12 +41,12 @@ = link_to leave_namespace_project_project_members_path(@project.namespace, @project), data: { confirm: "Leave project?"}, method: :delete, class: "btn-xs btn btn-remove", title: 'Leave project' do %i.fa.fa-minus.fa-inverse - else - = link_to namespace_project_project_member_path(@project.namespace, @project, user), data: { confirm: remove_from_project_team_message(@project, user) }, method: :delete, remote: true, class: "btn-xs btn btn-remove", title: 'Remove user from team' do + = link_to namespace_project_project_member_path(@project.namespace, @project, member), data: { confirm: remove_from_project_team_message(@project, member) }, method: :delete, remote: true, class: "btn-xs btn btn-remove", title: 'Remove user from team' do %i.fa.fa-minus.fa-inverse .edit-member.hide.js-toggle-content %br - = form_for member, as: :project_member, url: namespace_project_project_member_path(@project.namespace, @project, member.user), remote: true do |f| + = form_for member, as: :project_member, url: namespace_project_project_member_path(@project.namespace, @project, member), remote: true do |f| .prepend-top-10 = f.select :access_level, options_for_select(ProjectMember.access_roles, member.access_level), {}, class: 'form-control' .prepend-top-10 diff --git a/config/routes.rb b/config/routes.rb index 5d176590d70..4f33b11d220 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -53,6 +53,16 @@ Gitlab::Application.routes.draw do end get '/s/:username' => 'snippets#user_index', as: :user_snippets, constraints: { username: /.*/ } + # + # Invites + # + + resources :invites, only: [:show], constraints: { id: /[A-Za-z0-9_-]+/ } do + member do + post :accept + match :decline, via: [:get, :post] + end + end # # Import @@ -260,6 +270,7 @@ Gitlab::Application.routes.draw do scope module: :groups do resources :group_members, only: [:index, :create, :update, :destroy] do + post :resend_invite, on: :member delete :leave, on: :collection end @@ -486,6 +497,10 @@ Gitlab::Application.routes.draw do get :import post :apply_import end + + member do + post :resend_invite + end end resources :notes, only: [:index, :create, :destroy, :update], constraints: { id: /\d+/ } do diff --git a/db/migrate/20150406133311_add_invite_data_to_member.rb b/db/migrate/20150406133311_add_invite_data_to_member.rb new file mode 100644 index 00000000000..3452fd45c4f --- /dev/null +++ b/db/migrate/20150406133311_add_invite_data_to_member.rb @@ -0,0 +1,12 @@ +class AddInviteDataToMember < ActiveRecord::Migration + def change + add_column :members, :created_by_id, :integer + add_column :members, :invite_email, :string + add_column :members, :invite_token, :string + add_column :members, :invite_accepted_at, :datetime + + change_column :members, :user_id, :integer, null: true + + add_index :members, :invite_token, unique: true + end +end diff --git a/db/schema.rb b/db/schema.rb index a54fb4b7098..0b91f9c467e 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -163,15 +163,20 @@ ActiveRecord::Schema.define(version: 20150413192223) do t.integer "access_level", null: false t.integer "source_id", null: false t.string "source_type", null: false - t.integer "user_id", null: false + t.integer "user_id" t.integer "notification_level", null: false t.string "type" t.datetime "created_at" t.datetime "updated_at" + t.integer "created_by_id" + t.string "invite_email" + t.string "invite_token" + t.datetime "invite_accepted_at" end add_index "members", ["access_level"], name: "index_members_on_access_level", using: :btree add_index "members", ["created_at", "id"], name: "index_members_on_created_at_and_id", using: :btree + add_index "members", ["invite_token"], name: "index_members_on_invite_token", unique: true, using: :btree add_index "members", ["source_id", "source_type"], name: "index_members_on_source_id_and_source_type", using: :btree add_index "members", ["type"], name: "index_members_on_type", using: :btree add_index "members", ["user_id"], name: "index_members_on_user_id", using: :btree diff --git a/features/groups.feature b/features/groups.feature index 65d06a0daf9..415e43d6ae7 100644 --- a/features/groups.feature +++ b/features/groups.feature @@ -55,6 +55,13 @@ Feature: Groups When I select "Mike" as "Reporter" Then I should see "Mike" in team list as "Reporter" + @javascript + Scenario: Invite user to group + When I visit group "Owned" members page + And I click link "Add members" + When I select "sjobs@apple.com" as "Reporter" + Then I should see "sjobs@apple.com" in team list as invited "Reporter" + # Leave @javascript diff --git a/features/invites.feature b/features/invites.feature new file mode 100644 index 00000000000..dc8eefaeaed --- /dev/null +++ b/features/invites.feature @@ -0,0 +1,45 @@ +Feature: Invites + Background: + Given "John Doe" is owner of group "Owned" + And "John Doe" has invited "user@example.com" to group "Owned" + + Scenario: Viewing invitation when signed out + When I visit the invitation page + Then I should be redirected to the sign in page + And I should see a notice telling me to sign in + + Scenario: Signing in to view invitation + When I visit the invitation page + And I sign in as "Mary Jane" + Then I should be redirected to the invitation page + + Scenario: Viewing invitation when signed in + Given I sign in as "Mary Jane" + And I visit the invitation page + Then I should see the invitation details + And I should see an "Accept invitation" button + And I should see a "Decline" button + + Scenario: Viewing invitation as an existing member + Given I sign in as "John Doe" + And I visit the invitation page + Then I should see a message telling me I'm already a member + + Scenario: Accepting the invitation + Given I sign in as "Mary Jane" + And I visit the invitation page + And I click the "Accept invitation" button + Then I should be redirected to the group page + And I should see a notice telling me I have access + + Scenario: Declining the application when signed in + Given I sign in as "Mary Jane" + And I visit the invitation page + And I click the "Decline" button + Then I should be redirected to the dashboard + And I should see a notice telling me I have declined + + Scenario: Declining the application when signed out + When I visit the invitation's decline page + Then I should be redirected to the sign in page + And I should see a notice telling me I have declined diff --git a/features/project/team_management.feature b/features/project/team_management.feature index 22393622bb9..6cda225ea7b 100644 --- a/features/project/team_management.feature +++ b/features/project/team_management.feature @@ -18,6 +18,12 @@ Feature: Project Team Management Then I should see "Mike" in team list as "Reporter" @javascript + Scenario: Invite user to project + Given I click link "Add members" + And I select "sjobs@apple.com" as "Reporter" + Then I should see "sjobs@apple.com" in team list as invited "Reporter" + + @javascript Scenario: Update user access Given I should see "Sam" in team list as "Developer" And I change "Sam" role to "Reporter" diff --git a/features/steps/groups.rb b/features/steps/groups.rb index ec5213e4b93..228b83e5fd0 100644 --- a/features/steps/groups.rb +++ b/features/steps/groups.rb @@ -31,6 +31,23 @@ class Spinach::Features::Groups < Spinach::FeatureSteps end end + step 'I select "sjobs@apple.com" as "Reporter"' do + within ".users-group-form" do + select2("sjobs@apple.com", from: "#user_ids", multiple: true) + select "Reporter", from: "access_level" + end + + click_button "Add users to group" + end + + step 'I should see "sjobs@apple.com" in team list as invited "Reporter"' do + within '.well-list' do + page.should have_content('sjobs@apple.com') + page.should have_content('invited') + page.should have_content('Reporter') + end + end + step 'I should see group "Owned" projects list' do Group.find_by(name: "Owned").projects.each do |project| page.should have_link project.name diff --git a/features/steps/invites.rb b/features/steps/invites.rb new file mode 100644 index 00000000000..d051cc3edc8 --- /dev/null +++ b/features/steps/invites.rb @@ -0,0 +1,80 @@ +class Spinach::Features::Invites < Spinach::FeatureSteps + include SharedAuthentication + include SharedUser + include SharedGroup + + step '"John Doe" has invited "user@example.com" to group "Owned"' do + user = User.find_by(name: "John Doe") + group = Group.find_by(name: "Owned") + group.add_user("user@example.com", Gitlab::Access::DEVELOPER, user) + end + + step 'I visit the invitation page' do + group = Group.find_by(name: "Owned") + invite = group.group_members.invite.last + invite.generate_invite_token! + @raw_invite_token = invite.raw_invite_token + visit invite_path(@raw_invite_token) + end + + step 'I should be redirected to the sign in page' do + expect(current_path).to eq(new_user_session_path) + end + + step 'I should see a notice telling me to sign in' do + expect(page).to have_content "To accept this invitation, sign in" + end + + step 'I should be redirected to the invitation page' do + expect(current_path).to eq(invite_path(@raw_invite_token)) + end + + step 'I should see the invitation details' do + expect(page).to have_content("You have been invited by John Doe to join group Owned as Developer.") + end + + step "I should see a message telling me I'm already a member" do + expect(page).to have_content("However, you are already a member of this group.") + end + + step 'I should see an "Accept invitation" button' do + expect(page).to have_link("Accept invitation") + end + + step 'I should see a "Decline" button' do + expect(page).to have_link("Decline") + end + + step 'I click the "Accept invitation" button' do + page.click_link "Accept invitation" + end + + step 'I should be redirected to the group page' do + group = Group.find_by(name: "Owned") + expect(current_path).to eq(group_path(group)) + end + + step 'I should see a notice telling me I have access' do + expect(page).to have_content("You have been granted Developer access to group Owned.") + end + + step 'I click the "Decline" button' do + page.click_link "Decline" + end + + step 'I should be redirected to the dashboard' do + expect(current_path).to eq(dashboard_path) + end + + step 'I should see a notice telling me I have declined' do + expect(page).to have_content("You have declined the invitation to join group Owned.") + end + + step "I visit the invitation's decline page" do + group = Group.find_by(name: "Owned") + invite = group.group_members.invite.last + invite.generate_invite_token! + @raw_invite_token = invite.raw_invite_token + visit decline_invite_path(@raw_invite_token) + end +end diff --git a/features/steps/project/team_management.rb b/features/steps/project/team_management.rb index 0eefe2b5688..e95621071c4 100644 --- a/features/steps/project/team_management.rb +++ b/features/steps/project/team_management.rb @@ -35,6 +35,22 @@ class Spinach::Features::ProjectTeamManagement < Spinach::FeatureSteps end end + step 'I select "sjobs@apple.com" as "Reporter"' do + within ".users-project-form" do + select2("sjobs@apple.com", from: "#user_ids", multiple: true) + select "Reporter", from: "access_level" + end + click_button "Add users to project" + end + + step 'I should see "sjobs@apple.com" in team list as invited "Reporter"' do + within ".access-reporter" do + page.should have_content('sjobs@apple.com') + page.should have_content('invited') + page.should have_content('Reporter') + end + end + step 'I should see "Sam" in team list as "Developer"' do within ".access-developer" do page.should have_content('Sam') diff --git a/lib/api/group_members.rb b/lib/api/group_members.rb index ed54c7f6ff0..ab9b7c602b5 100644 --- a/lib/api/group_members.rb +++ b/lib/api/group_members.rb @@ -9,8 +9,7 @@ module API # GET /groups/:id/members get ":id/members" do group = find_group(params[:id]) - members = group.group_members - users = (paginate members).collect(&:user) + users = group.users present users, with: Entities::GroupMember, group: group end @@ -24,7 +23,7 @@ module API # POST /groups/:id/members post ":id/members" do group = find_group(params[:id]) - authorize! :manage_group, group + authorize! :admin_group, group required_attributes! [:user_id, :access_level] unless validate_access_level?(params[:access_level]) @@ -35,7 +34,7 @@ module API render_api_error!("Already exists", 409) end - group.add_users([params[:user_id]], params[:access_level]) + group.add_users([params[:user_id]], params[:access_level], current_user) member = group.group_members.find_by(user_id: params[:user_id]) present member.user, with: Entities::GroupMember, group: group end @@ -50,7 +49,7 @@ module API # PUT /groups/:id/members/:user_id put ':id/members/:user_id' do group = find_group(params[:id]) - authorize! :manage_group, group + authorize! :admin_group, group required_attributes! [:access_level] group_member = group.group_members.find_by(user_id: params[:user_id]) @@ -74,7 +73,7 @@ module API # DELETE /groups/:id/members/:user_id delete ":id/members/:user_id" do group = find_group(params[:id]) - authorize! :manage_group, group + authorize! :admin_group, group member = group.group_members.find_by(user_id: params[:user_id]) if member.nil? diff --git a/lib/api/groups.rb b/lib/api/groups.rb index a92abd4b690..8cb9f920975 100644 --- a/lib/api/groups.rb +++ b/lib/api/groups.rb @@ -61,7 +61,7 @@ module API # DELETE /groups/:id delete ":id" do group = find_group(params[:id]) - authorize! :manage_group, group + authorize! :admin_group, group group.destroy end diff --git a/spec/models/member_spec.rb b/spec/models/member_spec.rb new file mode 100644 index 00000000000..56d030a03b3 --- /dev/null +++ b/spec/models/member_spec.rb @@ -0,0 +1,148 @@ +require 'spec_helper' + +describe Member do + describe "Associations" do + it { is_expected.to belong_to(:user) } + end + + describe "Validation" do + subject { Member.new(access_level: Member::GUEST) } + + it { is_expected.to validate_presence_of(:user) } + it { is_expected.to validate_presence_of(:source) } + it { is_expected.to validate_inclusion_of(:access_level).in_array(Gitlab::Access.values) } + + context "when an invite email is provided" do + let(:member) { build(:project_member, invite_email: "user@example.com", user: nil) } + + it "doesn't require a user" do + expect(member).to be_valid + end + + it "requires a valid invite email" do + member.invite_email = "nope" + + expect(member).not_to be_valid + end + + it "requires a unique invite email scoped to this source" do + create(:project_member, source: member.source, invite_email: member.invite_email) + + expect(member).not_to be_valid + end + + it "is valid otherwise" do + expect(member).to be_valid + end + end + + context "when an invite email is not provided" do + let(:member) { build(:project_member) } + + it "requires a user" do + member.user = nil + + expect(member).not_to be_valid + end + + it "is valid otherwise" do + expect(member).to be_valid + end + end + end + + describe "Delegate methods" do + it { is_expected.to respond_to(:user_name) } + it { is_expected.to respond_to(:user_email) } + end + + describe ".add_user" do + let!(:user) { create(:user) } + let(:project) { create(:project) } + + 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) + + expect(project.users).to include(user) + end + 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) + + expect(project.users).to include(user) + end + 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) + + expect(project.users).to include(user) + end + end + + 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) + + expect(project.project_members.invite.pluck(:invite_email)).to include("user@example.com") + end + end + end + + describe "#accept_invite!" do + let!(:member) { create(:project_member, invite_email: "user@example.com", user: nil) } + let(:user) { create(:user) } + + it "resets the invite token" do + member.accept_invite!(user) + + expect(member.invite_token).to be_nil + end + + it "sets the invite accepted timestamp" do + member.accept_invite!(user) + + expect(member.invite_accepted_at).not_to be_nil + end + + it "sets the user" do + member.accept_invite!(user) + + expect(member.user).to eq(user) + end + + it "calls #after_accept_invite" do + expect(member).to receive(:after_accept_invite) + + member.accept_invite!(user) + end + end + + describe "#decline_invite!" do + let!(:member) { create(:project_member, invite_email: "user@example.com", user: nil) } + + it "destroys the member" do + member.decline_invite! + + expect(member).to be_destroyed + end + + it "calls #after_decline_invite" do + expect(member).to receive(:after_decline_invite) + + member.decline_invite! + end + end + + describe "#generate_invite_token" do + let!(:member) { create(:project_member, invite_email: "user@example.com", user: nil) } + + it "sets the invite token" do + expect { member.generate_invite_token }.to change { member.invite_token} + end + end +end diff --git a/spec/models/members_spec.rb b/spec/models/members_spec.rb deleted file mode 100644 index dfd3f7feb6b..00000000000 --- a/spec/models/members_spec.rb +++ /dev/null @@ -1,20 +0,0 @@ -require 'spec_helper' - -describe Member do - describe "Associations" do - it { is_expected.to belong_to(:user) } - end - - describe "Validation" do - subject { Member.new(access_level: Member::GUEST) } - - it { is_expected.to validate_presence_of(:user) } - it { is_expected.to validate_presence_of(:source) } - it { is_expected.to validate_inclusion_of(:access_level).in_array(Gitlab::Access.values) } - end - - describe "Delegate methods" do - it { is_expected.to respond_to(:user_name) } - it { is_expected.to respond_to(:user_email) } - end -end diff --git a/spec/requests/api/repositories_spec.rb b/spec/requests/api/repositories_spec.rb index 729970153d1..09a79553f72 100644 --- a/spec/requests/api/repositories_spec.rb +++ b/spec/requests/api/repositories_spec.rb @@ -11,8 +11,6 @@ describe API::API, api: true do let!(:master) { create(:project_member, user: user, project: project, access_level: ProjectMember::MASTER) } let!(:guest) { create(:project_member, user: user2, project: project, access_level: ProjectMember::GUEST) } - before { project.team << [user, :reporter] } - describe "GET /projects/:id/repository/tags" do it "should return an array of project tags" do get api("/projects/#{project.id}/repository/tags", user) |