Commit 64e3cba9 authored by Robert Speicher's avatar Robert Speicher Committed by Robert Speicher

Merge branch 'dbalexandre/gitlab-ce-fix-personal-snippet-access-workflow' into 'master'

Improve personal snippet access workflow.

Replaces !1709

Fixes #3258

See merge request !1817
parent 463a3cfe
......@@ -28,6 +28,7 @@ v 8.2.0
- Allow to define cache in `.gitlab-ci.yml`
- Fix: 500 error returned if destroy request without HTTP referer (Kazuki Shimizu)
- Remove deprecated CI events from project settings page
- Improve personal snippet access workflow (Douglas Alexandre)
- [API] Add ability to fetch the commit ID of the last commit that actually touched a file
- Fix omniauth documentation setting for omnibus configuration (Jon Cairns)
- Add "New file" link to dropdown on project page
......
class SnippetsController < ApplicationController
before_action :snippet, only: [:show, :edit, :destroy, :update, :raw]
# Allow read snippet
before_action :authorize_read_snippet!, only: [:show]
# Allow modify snippet
before_action :authorize_update_snippet!, only: [:edit, :update]
......@@ -79,10 +82,14 @@ class SnippetsController < ApplicationController
[Snippet::PUBLIC, Snippet::INTERNAL]).
find(params[:id])
else
PersonalSnippet.are_public.find(params[:id])
PersonalSnippet.find(params[:id])
end
end
def authorize_read_snippet!
authenticate_user! unless can?(current_user, :read_personal_snippet, @snippet)
end
def authorize_update_snippet!
return render_404 unless can?(current_user, :update_personal_snippet, @snippet)
end
......
class Ability
class << self
def allowed(user, subject)
return not_auth_abilities(user, subject) if user.nil?
return [] unless user.kind_of?(User)
return anonymous_abilities(user, subject) if user.nil?
return [] unless user.is_a?(User)
return [] if user.blocked?
case subject.class.name
......@@ -20,15 +20,25 @@ class Ability
end.concat(global_abilities(user))
end
# List of possible abilities
# for non-authenticated user
def not_auth_abilities(user, subject)
project = if subject.kind_of?(Project)
# List of possible abilities for anonymous user
def anonymous_abilities(user, subject)
case true
when subject.is_a?(PersonalSnippet)
anonymous_personal_snippet_abilities(subject)
when subject.is_a?(Project) || subject.respond_to?(:project)
anonymous_project_abilities(subject)
when subject.is_a?(Group) || subject.respond_to?(:group)
anonymous_group_abilities(subject)
else
[]
end
end
def anonymous_project_abilities(subject)
project = if subject.is_a?(Project)
subject
elsif subject.respond_to?(:project)
subject.project
else
nil
subject.project
end
if project && project.public?
......@@ -48,19 +58,29 @@ class Ability
rules - project_disabled_features_rules(project)
else
group = if subject.kind_of?(Group)
subject
elsif subject.respond_to?(:group)
subject.group
else
nil
end
[]
end
end
if group && group.public_profile?
[:read_group]
else
[]
end
def anonymous_group_abilities(subject)
group = if subject.is_a?(Group)
subject
else
subject.group
end
if group && group.public_profile?
[:read_group]
else
[]
end
end
def anonymous_personal_snippet_abilities(snippet)
if snippet.public?
[:read_personal_snippet]
else
[]
end
end
......@@ -280,7 +300,7 @@ class Ability
end
end
[:note, :project_snippet, :personal_snippet].each do |name|
[:note, :project_snippet].each do |name|
define_method "#{name}_abilities" do |user, subject|
rules = []
......@@ -300,6 +320,24 @@ class Ability
end
end
def personal_snippet_abilities(user, snippet)
rules = []
if snippet.author == user
rules += [
:read_personal_snippet,
:update_personal_snippet,
:admin_personal_snippet
]
end
if snippet.public? || snippet.internal?
rules << :read_personal_snippet
end
rules
end
def group_member_abilities(user, subject)
rules = []
target_user = subject.user
......
require 'spec_helper'
describe SnippetsController do
describe 'GET #show' do
let(:user) { create(:user) }
context 'when the personal snippet is private' do
let(:personal_snippet) { create(:personal_snippet, :private, author: user) }
context 'when signed in' do
before do
sign_in(user)
end
context 'when signed in user is not the author' do
let(:other_author) { create(:author) }
let(:other_personal_snippet) { create(:personal_snippet, :private, author: other_author) }
it 'responds with status 404' do
get :show, id: other_personal_snippet.to_param
expect(response.status).to eq(404)
end
end
context 'when signed in user is the author' do
it 'renders the snippet' do
get :show, id: personal_snippet.to_param
expect(assigns(:snippet)).to eq(personal_snippet)
expect(response.status).to eq(200)
end
end
end
context 'when not signed in' do
it 'redirects to the sign in page' do
get :show, id: personal_snippet.to_param
expect(response).to redirect_to(new_user_session_path)
end
end
end
context 'when the personal snippet is internal' do
let(:personal_snippet) { create(:personal_snippet, :internal, author: user) }
context 'when signed in' do
before do
sign_in(user)
end
it 'renders the snippet' do
get :show, id: personal_snippet.to_param
expect(assigns(:snippet)).to eq(personal_snippet)
expect(response.status).to eq(200)
end
end
context 'when not signed in' do
it 'redirects to the sign in page' do
get :show, id: personal_snippet.to_param
expect(response).to redirect_to(new_user_session_path)
end
end
end
context 'when the personal snippet is public' do
let(:personal_snippet) { create(:personal_snippet, :public, author: user) }
context 'when signed in' do
before do
sign_in(user)
end
it 'renders the snippet' do
get :show, id: personal_snippet.to_param
expect(assigns(:snippet)).to eq(personal_snippet)
expect(response.status).to eq(200)
end
end
context 'when not signed in' do
it 'renders the snippet' do
get :show, id: personal_snippet.to_param
expect(assigns(:snippet)).to eq(personal_snippet)
expect(response.status).to eq(200)
end
end
end
context 'when the personal snippet does not exist' do
context 'when signed in' do
before do
sign_in(user)
end
it 'responds with status 404' do
get :show, id: 'doesntexist'
expect(response.status).to eq(404)
end
end
context 'when not signed in' do
it 'responds with status 404' do
get :show, id: 'doesntexist'
expect(response.status).to eq(404)
end
end
end
end
end
......@@ -165,6 +165,18 @@ FactoryGirl.define do
title
content
file_name
trait :public do
visibility_level Gitlab::VisibilityLevel::PUBLIC
end
trait :internal do
visibility_level Gitlab::VisibilityLevel::INTERNAL
end
trait :private do
visibility_level Gitlab::VisibilityLevel::PRIVATE
end
end
factory :snippet do
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment