Commit 56476f18 authored by Robert Speicher's avatar 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
parents 3a85c93a 08dc3822
...@@ -33,6 +33,7 @@ v 8.2.0 ...@@ -33,6 +33,7 @@ v 8.2.0
- Allow to define cache in `.gitlab-ci.yml` - Allow to define cache in `.gitlab-ci.yml`
- Fix: 500 error returned if destroy request without HTTP referer (Kazuki Shimizu) - Fix: 500 error returned if destroy request without HTTP referer (Kazuki Shimizu)
- Remove deprecated CI events from project settings page - 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 - [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) - Fix omniauth documentation setting for omnibus configuration (Jon Cairns)
- Add "New file" link to dropdown on project page - Add "New file" link to dropdown on project page
......
class SnippetsController < ApplicationController class SnippetsController < ApplicationController
before_action :snippet, only: [:show, :edit, :destroy, :update, :raw] before_action :snippet, only: [:show, :edit, :destroy, :update, :raw]
# Allow read snippet
before_action :authorize_read_snippet!, only: [:show]
# Allow modify snippet # Allow modify snippet
before_action :authorize_update_snippet!, only: [:edit, :update] before_action :authorize_update_snippet!, only: [:edit, :update]
...@@ -79,10 +82,14 @@ class SnippetsController < ApplicationController ...@@ -79,10 +82,14 @@ class SnippetsController < ApplicationController
[Snippet::PUBLIC, Snippet::INTERNAL]). [Snippet::PUBLIC, Snippet::INTERNAL]).
find(params[:id]) find(params[:id])
else else
PersonalSnippet.are_public.find(params[:id]) PersonalSnippet.find(params[:id])
end end
end end
def authorize_read_snippet!
authenticate_user! unless can?(current_user, :read_personal_snippet, @snippet)
end
def authorize_update_snippet! def authorize_update_snippet!
return render_404 unless can?(current_user, :update_personal_snippet, @snippet) return render_404 unless can?(current_user, :update_personal_snippet, @snippet)
end end
......
class Ability class Ability
class << self class << self
def allowed(user, subject) def allowed(user, subject)
return not_auth_abilities(user, subject) if user.nil? return anonymous_abilities(user, subject) if user.nil?
return [] unless user.kind_of?(User) return [] unless user.is_a?(User)
return [] if user.blocked? return [] if user.blocked?
case subject.class.name case subject.class.name
...@@ -20,15 +20,25 @@ class Ability ...@@ -20,15 +20,25 @@ class Ability
end.concat(global_abilities(user)) end.concat(global_abilities(user))
end end
# List of possible abilities # List of possible abilities for anonymous user
# for non-authenticated user def anonymous_abilities(user, subject)
def not_auth_abilities(user, subject) case true
project = if subject.kind_of?(Project) 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 subject
elsif subject.respond_to?(:project)
subject.project
else else
nil subject.project
end end
if project && project.public? if project && project.public?
...@@ -48,19 +58,29 @@ class Ability ...@@ -48,19 +58,29 @@ class Ability
rules - project_disabled_features_rules(project) rules - project_disabled_features_rules(project)
else else
group = if subject.kind_of?(Group) []
subject end
elsif subject.respond_to?(:group) end
subject.group
else
nil
end
if group && group.public_profile? def anonymous_group_abilities(subject)
[:read_group] group = if subject.is_a?(Group)
else subject
[] else
end 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
end end
...@@ -280,7 +300,7 @@ class Ability ...@@ -280,7 +300,7 @@ class Ability
end end
end end
[:note, :project_snippet, :personal_snippet].each do |name| [:note, :project_snippet].each do |name|
define_method "#{name}_abilities" do |user, subject| define_method "#{name}_abilities" do |user, subject|
rules = [] rules = []
...@@ -300,6 +320,24 @@ class Ability ...@@ -300,6 +320,24 @@ class Ability
end end
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) def group_member_abilities(user, subject)
rules = [] rules = []
target_user = subject.user 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 ...@@ -165,6 +165,18 @@ FactoryGirl.define do
title title
content content
file_name 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 end
factory :snippet do 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