Commit 39eac7b0b92ac65edf22ce860174e6049af13032
Exists in
spb-stable
and in
2 other branches
Merge branch 'api_ldap' into 'master'
Check user access during API calls
Showing
5 changed files
with
36 additions
and
12 deletions
Show diff stats
CHANGELOG
| ... | ... | @@ -16,6 +16,7 @@ v 6.9.0 |
| 16 | 16 | - Two Step MR creation process |
| 17 | 17 | - Remove unwanted files from satellite working directory with git clean -fdx |
| 18 | 18 | - Accept merge request via API (sponsored by O'Reilly Media) |
| 19 | + - Add more access checks during API calls | |
| 19 | 20 | |
| 20 | 21 | v 6.8.0 |
| 21 | 22 | - Ability to at mention users that are participating in issue and merge req. discussion | ... | ... |
lib/api/helpers.rb
| ... | ... | @@ -8,6 +8,11 @@ module API |
| 8 | 8 | def current_user |
| 9 | 9 | private_token = (params[PRIVATE_TOKEN_PARAM] || env[PRIVATE_TOKEN_HEADER]).to_s |
| 10 | 10 | @current_user ||= User.find_by(authentication_token: private_token) |
| 11 | + | |
| 12 | + unless @current_user && Gitlab::UserAccess.allowed?(@current_user) | |
| 13 | + return nil | |
| 14 | + end | |
| 15 | + | |
| 11 | 16 | identifier = sudo_identifier() |
| 12 | 17 | |
| 13 | 18 | # If the sudo is the current user do nothing | ... | ... |
lib/gitlab/git_access.rb
| ... | ... | @@ -61,18 +61,7 @@ module Gitlab |
| 61 | 61 | private |
| 62 | 62 | |
| 63 | 63 | def user_allowed?(user) |
| 64 | - return false if user.blocked? | |
| 65 | - | |
| 66 | - if Gitlab.config.ldap.enabled | |
| 67 | - if user.ldap_user? | |
| 68 | - # Check if LDAP user exists and match LDAP user_filter | |
| 69 | - Gitlab::LDAP::Access.open do |adapter| | |
| 70 | - return false unless adapter.allowed?(user) | |
| 71 | - end | |
| 72 | - end | |
| 73 | - end | |
| 74 | - | |
| 75 | - true | |
| 64 | + Gitlab::UserAccess.allowed?(user) | |
| 76 | 65 | end |
| 77 | 66 | end |
| 78 | 67 | end | ... | ... |
| ... | ... | @@ -0,0 +1,18 @@ |
| 1 | +module Gitlab | |
| 2 | + module UserAccess | |
| 3 | + def self.allowed?(user) | |
| 4 | + return false if user.blocked? | |
| 5 | + | |
| 6 | + if Gitlab.config.ldap.enabled | |
| 7 | + if user.ldap_user? | |
| 8 | + # Check if LDAP user exists and match LDAP user_filter | |
| 9 | + Gitlab::LDAP::Access.open do |adapter| | |
| 10 | + return false unless adapter.allowed?(user) | |
| 11 | + end | |
| 12 | + end | |
| 13 | + end | |
| 14 | + | |
| 15 | + true | |
| 16 | + end | |
| 17 | + end | |
| 18 | +end | ... | ... |
spec/requests/api/api_helpers_spec.rb
| ... | ... | @@ -39,6 +39,17 @@ describe API, api: true do |
| 39 | 39 | end |
| 40 | 40 | |
| 41 | 41 | describe ".current_user" do |
| 42 | + it "should return nil for an invalid token" do | |
| 43 | + env[API::APIHelpers::PRIVATE_TOKEN_HEADER] = 'invalid token' | |
| 44 | + current_user.should be_nil | |
| 45 | + end | |
| 46 | + | |
| 47 | + it "should return nil for a user without access" do | |
| 48 | + env[API::APIHelpers::PRIVATE_TOKEN_HEADER] = user.private_token | |
| 49 | + Gitlab::UserAccess.stub(allowed?: false) | |
| 50 | + current_user.should be_nil | |
| 51 | + end | |
| 52 | + | |
| 42 | 53 | it "should leave user as is when sudo not specified" do |
| 43 | 54 | env[API::APIHelpers::PRIVATE_TOKEN_HEADER] = user.private_token |
| 44 | 55 | current_user.should == user | ... | ... |