Commit 7a56075efac308a3bd08e022e56e96d41c4c05b5
1 parent
e86e8818
Exists in
master
and in
4 other branches
Some of the requested updates, rebase on master
Change-Id: I305266fe9acbbb5136adeeb52e7e4e1d6629a30a
Showing
4 changed files
with
38 additions
and
27 deletions
Show diff stats
app/models/user.rb
| @@ -190,6 +190,14 @@ class User < ActiveRecord::Base | @@ -190,6 +190,14 @@ class User < ActiveRecord::Base | ||
| 190 | def search query | 190 | def search query |
| 191 | where("name LIKE :query OR email LIKE :query OR username LIKE :query", query: "%#{query}%") | 191 | where("name LIKE :query OR email LIKE :query OR username LIKE :query", query: "%#{query}%") |
| 192 | end | 192 | end |
| 193 | + | ||
| 194 | + def by_username_or_id(name_or_id) | ||
| 195 | + if (name_or_id.is_a?(Integer)) | ||
| 196 | + User.find_by_id(name_or_id) | ||
| 197 | + else | ||
| 198 | + User.find_by_username(name_or_id) | ||
| 199 | + end | ||
| 200 | + end | ||
| 193 | end | 201 | end |
| 194 | 202 | ||
| 195 | # | 203 | # |
lib/api/helpers.rb
| @@ -12,25 +12,18 @@ module API | @@ -12,25 +12,18 @@ module API | ||
| 12 | if (identifier && !(@current_user.id == identifier || @current_user.username == identifier)) | 12 | if (identifier && !(@current_user.id == identifier || @current_user.username == identifier)) |
| 13 | render_api_error!('403 Forbidden: Must be admin to use sudo', 403) unless @current_user.is_admin? | 13 | render_api_error!('403 Forbidden: Must be admin to use sudo', 403) unless @current_user.is_admin? |
| 14 | begin | 14 | begin |
| 15 | - | ||
| 16 | - if (identifier.is_a?(Integer)) | ||
| 17 | - user = User.find_by_id(identifier) | ||
| 18 | - else | ||
| 19 | - user = User.find_by_username(identifier) | ||
| 20 | - end | ||
| 21 | - if user.nil? | ||
| 22 | - not_found!("No user id or username for: #{identifier}") | ||
| 23 | - end | ||
| 24 | - @current_user = user | 15 | + @current_user = User.by_username_or_id(identifier) |
| 25 | rescue => ex | 16 | rescue => ex |
| 26 | not_found!("No user id or username for: #{identifier}") | 17 | not_found!("No user id or username for: #{identifier}") |
| 27 | end | 18 | end |
| 19 | + not_found!("No user id or username for: #{identifier}") if current_user.nil? | ||
| 28 | end | 20 | end |
| 29 | @current_user | 21 | @current_user |
| 30 | end | 22 | end |
| 31 | 23 | ||
| 32 | def sudo_identifier() | 24 | def sudo_identifier() |
| 33 | - identifier = params[SUDO_PARAM] == nil ? env[SUDO_HEADER] : params[SUDO_PARAM] | 25 | + identifier ||= params[SUDO_PARAM] ||= env[SUDO_HEADER] |
| 26 | + # Regex for integers | ||
| 34 | if (!!(identifier =~ /^[0-9]+$/)) | 27 | if (!!(identifier =~ /^[0-9]+$/)) |
| 35 | identifier.to_i | 28 | identifier.to_i |
| 36 | else | 29 | else |
| @@ -129,10 +122,10 @@ module API | @@ -129,10 +122,10 @@ module API | ||
| 129 | 122 | ||
| 130 | def abilities | 123 | def abilities |
| 131 | @abilities ||= begin | 124 | @abilities ||= begin |
| 132 | - abilities = Six.new | ||
| 133 | - abilities << Ability | ||
| 134 | - abilities | ||
| 135 | - end | 125 | + abilities = Six.new |
| 126 | + abilities << Ability | ||
| 127 | + abilities | ||
| 128 | + end | ||
| 136 | end | 129 | end |
| 137 | end | 130 | end |
| 138 | end | 131 | end |
spec/models/user_spec.rb
| @@ -208,4 +208,14 @@ describe User do | @@ -208,4 +208,14 @@ describe User do | ||
| 208 | user.can_create_group.should == false | 208 | user.can_create_group.should == false |
| 209 | end | 209 | end |
| 210 | end | 210 | end |
| 211 | + | ||
| 212 | + describe 'by_username_or_id' do | ||
| 213 | + let(:user1){create(:user, username: 'foo')} | ||
| 214 | + it "should get the correct user" do | ||
| 215 | + User.by_username_or_id(user1.id).should == user1 | ||
| 216 | + User.by_username_or_id('foo').should == user1 | ||
| 217 | + User.by_username_or_id(-1).should be_nil | ||
| 218 | + User.by_username_or_id('bar').should be_nil | ||
| 219 | + end | ||
| 220 | + end | ||
| 211 | end | 221 | end |
spec/requests/api/api_helpers_spec.rb
| 1 | require 'spec_helper' | 1 | require 'spec_helper' |
| 2 | 2 | ||
| 3 | -describe Gitlab::API do | ||
| 4 | - include Gitlab::APIHelpers | 3 | +describe API do |
| 4 | + include API::APIHelpers | ||
| 5 | include ApiHelpers | 5 | include ApiHelpers |
| 6 | let(:user) { create(:user) } | 6 | let(:user) { create(:user) } |
| 7 | let(:admin) { create(:admin) } | 7 | let(:admin) { create(:admin) } |
| @@ -13,27 +13,27 @@ describe Gitlab::API do | @@ -13,27 +13,27 @@ describe Gitlab::API do | ||
| 13 | def set_env(token_usr, identifier) | 13 | def set_env(token_usr, identifier) |
| 14 | clear_env | 14 | clear_env |
| 15 | clear_param | 15 | clear_param |
| 16 | - env[Gitlab::APIHelpers::PRIVATE_TOKEN_HEADER] = token_usr.private_token | ||
| 17 | - env[Gitlab::APIHelpers::SUDO_HEADER] = identifier | 16 | + env[API::APIHelpers::PRIVATE_TOKEN_HEADER] = token_usr.private_token |
| 17 | + env[API::APIHelpers::SUDO_HEADER] = identifier | ||
| 18 | end | 18 | end |
| 19 | 19 | ||
| 20 | 20 | ||
| 21 | def set_param(token_usr, identifier) | 21 | def set_param(token_usr, identifier) |
| 22 | clear_env | 22 | clear_env |
| 23 | clear_param | 23 | clear_param |
| 24 | - params[Gitlab::APIHelpers::PRIVATE_TOKEN_PARAM] = token_usr.private_token | ||
| 25 | - params[Gitlab::APIHelpers::SUDO_PARAM] = identifier | 24 | + params[API::APIHelpers::PRIVATE_TOKEN_PARAM] = token_usr.private_token |
| 25 | + params[API::APIHelpers::SUDO_PARAM] = identifier | ||
| 26 | end | 26 | end |
| 27 | 27 | ||
| 28 | 28 | ||
| 29 | def clear_env | 29 | def clear_env |
| 30 | - env.delete(Gitlab::APIHelpers::PRIVATE_TOKEN_HEADER) | ||
| 31 | - env.delete(Gitlab::APIHelpers::SUDO_HEADER) | 30 | + env.delete(API::APIHelpers::PRIVATE_TOKEN_HEADER) |
| 31 | + env.delete(API::APIHelpers::SUDO_HEADER) | ||
| 32 | end | 32 | end |
| 33 | 33 | ||
| 34 | def clear_param | 34 | def clear_param |
| 35 | - params.delete(Gitlab::APIHelpers::PRIVATE_TOKEN_PARAM) | ||
| 36 | - params.delete(Gitlab::APIHelpers::SUDO_PARAM) | 35 | + params.delete(API::APIHelpers::PRIVATE_TOKEN_PARAM) |
| 36 | + params.delete(API::APIHelpers::SUDO_PARAM) | ||
| 37 | end | 37 | end |
| 38 | 38 | ||
| 39 | def error!(message, status) | 39 | def error!(message, status) |
| @@ -42,10 +42,10 @@ describe Gitlab::API do | @@ -42,10 +42,10 @@ describe Gitlab::API do | ||
| 42 | 42 | ||
| 43 | describe ".current_user" do | 43 | describe ".current_user" do |
| 44 | it "should leave user as is when sudo not specified" do | 44 | it "should leave user as is when sudo not specified" do |
| 45 | - env[Gitlab::APIHelpers::PRIVATE_TOKEN_HEADER] = user.private_token | 45 | + env[API::APIHelpers::PRIVATE_TOKEN_HEADER] = user.private_token |
| 46 | current_user.should == user | 46 | current_user.should == user |
| 47 | clear_env | 47 | clear_env |
| 48 | - params[Gitlab::APIHelpers::PRIVATE_TOKEN_PARAM] = user.private_token | 48 | + params[API::APIHelpers::PRIVATE_TOKEN_PARAM] = user.private_token |
| 49 | current_user.should == user | 49 | current_user.should == user |
| 50 | end | 50 | end |
| 51 | 51 |