diff options
author | Rémy Coutable <remy@gitlab.com> | 2016-09-19 16:04:04 +0300 |
---|---|---|
committer | Rémy Coutable <remy@rymai.me> | 2016-09-19 16:56:29 +0300 |
commit | d14c8b16c6f8d38055a608ec325781f0ef657eb2 (patch) | |
tree | aa76db95f915caf96a34b9e36beb6bc2a2e66b69 | |
parent | a73c6c42a8257e07e0982a19b003f4c5852eaede (diff) |
Merge branch '18302-use-rails-cookie-in-api' into 'master'
Allow the Rails cookie to be used for API authentication
Makes the Rails cookie into a valid authentication token for the Grape
API, and uses it instead of token authentication in frontend code that
uses the API.
Rendering the private token into client-side javascript is a security
risk; it may be stolen through XSS or other attacks. In general,
re-using API code in the frontend is more desirable than implementing
endless actions that return JSON.
Closes #18302
See merge request !1995
Signed-off-by: Rémy Coutable <remy@rymai.me>
-rw-r--r-- | CHANGELOG | 3 | ||||
-rw-r--r-- | app/assets/javascripts/api.js.coffee | 7 | ||||
-rw-r--r-- | doc/api/README.md | 16 | ||||
-rw-r--r-- | lib/api/api_guard.rb | 56 | ||||
-rw-r--r-- | lib/api/helpers.rb | 23 | ||||
-rw-r--r-- | lib/gitlab/gon_helper.rb | 1 | ||||
-rw-r--r-- | spec/requests/api/api_helpers_spec.rb | 25 |
7 files changed, 73 insertions, 58 deletions
diff --git a/CHANGELOG b/CHANGELOG index db99c37af4a..45ce2d2bdd6 100644 --- a/CHANGELOG +++ b/CHANGELOG @@ -1,5 +1,8 @@ Please view this file on the master branch, on stable branches it's out of date. +v 8.9.10 + - Allow the Rails cookie to be used for API authentication. + v 8.9.9 - Exclude some pending or inactivated rows in Member scopes diff --git a/app/assets/javascripts/api.js.coffee b/app/assets/javascripts/api.js.coffee index cf46f15a156..2c471c4beef 100644 --- a/app/assets/javascripts/api.js.coffee +++ b/app/assets/javascripts/api.js.coffee @@ -15,8 +15,6 @@ $.ajax( url: url - data: - private_token: gon.api_token dataType: "json" ).done (group) -> callback(group) @@ -29,7 +27,6 @@ $.ajax( url: url data: - private_token: gon.api_token search: query per_page: 20 dataType: "json" @@ -43,7 +40,6 @@ $.ajax( url: url data: - private_token: gon.api_token search: query per_page: 20 dataType: "json" @@ -57,7 +53,6 @@ $.ajax( url: url data: - private_token: gon.api_token search: query order_by: order per_page: 20 @@ -69,7 +64,6 @@ url = Api.buildUrl(Api.labelsPath) url = url.replace(':id', project_id) - data.private_token = gon.api_token $.ajax( url: url type: "POST" @@ -88,7 +82,6 @@ $.ajax( url: url data: - private_token: gon.api_token search: query per_page: 20 dataType: "json" diff --git a/doc/api/README.md b/doc/api/README.md index 288f7f9ee69..9e5b45a301b 100644 --- a/doc/api/README.md +++ b/doc/api/README.md @@ -46,11 +46,12 @@ The following documentation is for the [internal CI API](ci/README.md): ## Authentication -All API requests require authentication via a token. There are three types of tokens -available: private tokens, OAuth 2 tokens, and personal access tokens. +All API requests require authentication via a session cookie or token. There are +three types of tokens available: private tokens, OAuth 2 tokens, and personal +access tokens. -If a token is invalid or omitted, an error message will be returned with -status code `401`: +If authentication information is invalid or omitted, an error message will be +returned with status code `401`: ```json { @@ -89,6 +90,13 @@ that needs access to the GitLab API. Once you have your token, pass it to the API using either the `private_token` parameter or the `PRIVATE-TOKEN` header. + +### Session cookie + +When signing in to GitLab as an ordinary user, a `_gitlab_session` cookie is +set. The API will use this cookie for authentication if it is present, but using +the API to generate a new session cookie is currently not supported. + ## Basic Usage API requests should be prefixed with `api` and the API version. The API version diff --git a/lib/api/api_guard.rb b/lib/api/api_guard.rb index 7e67edb203a..8cc7a26f1fa 100644 --- a/lib/api/api_guard.rb +++ b/lib/api/api_guard.rb @@ -33,46 +33,29 @@ module API # # If the token is revoked, then it raises RevokedError. # - # If the token is not found (nil), then it raises TokenNotFoundError. + # If the token is not found (nil), then it returns nil # # Arguments: # # scopes: (optional) scopes required for this guard. # Defaults to empty array. # - def doorkeeper_guard!(scopes: []) - if (access_token = find_access_token).nil? - raise TokenNotFoundError - - else - case validate_access_token(access_token, scopes) - when Oauth2::AccessTokenValidationService::INSUFFICIENT_SCOPE - raise InsufficientScopeError.new(scopes) - when Oauth2::AccessTokenValidationService::EXPIRED - raise ExpiredError - when Oauth2::AccessTokenValidationService::REVOKED - raise RevokedError - when Oauth2::AccessTokenValidationService::VALID - @current_user = User.find(access_token.resource_owner_id) - end - end - end - def doorkeeper_guard(scopes: []) - if access_token = find_access_token - case validate_access_token(access_token, scopes) - when Oauth2::AccessTokenValidationService::INSUFFICIENT_SCOPE - raise InsufficientScopeError.new(scopes) + access_token = find_access_token + return nil unless access_token + + case validate_access_token(access_token, scopes) + when Oauth2::AccessTokenValidationService::INSUFFICIENT_SCOPE + raise InsufficientScopeError.new(scopes) - when Oauth2::AccessTokenValidationService::EXPIRED - raise ExpiredError + when Oauth2::AccessTokenValidationService::EXPIRED + raise ExpiredError - when Oauth2::AccessTokenValidationService::REVOKED - raise RevokedError + when Oauth2::AccessTokenValidationService::REVOKED + raise RevokedError - when Oauth2::AccessTokenValidationService::VALID - @current_user = User.find(access_token.resource_owner_id) - end + when Oauth2::AccessTokenValidationService::VALID + @current_user = User.find(access_token.resource_owner_id) end end @@ -96,19 +79,6 @@ module API end module ClassMethods - # Installs the doorkeeper guard on the whole Grape API endpoint. - # - # Arguments: - # - # scopes: (optional) scopes required for this guard. - # Defaults to empty array. - # - def guard_all!(scopes: []) - before do - guard! scopes: scopes - end - end - private def install_error_responders(base) diff --git a/lib/api/helpers.rb b/lib/api/helpers.rb index 77e407b54c5..3a107bc11e4 100644 --- a/lib/api/helpers.rb +++ b/lib/api/helpers.rb @@ -9,13 +9,30 @@ module API [ true, 1, '1', 't', 'T', 'true', 'TRUE', 'on', 'ON' ].include?(value) end + def private_token + params[PRIVATE_TOKEN_PARAM] || env[PRIVATE_TOKEN_HEADER] + end + + def warden + env['warden'] + end + + # Check the Rails session for valid authentication details + def find_user_from_warden + warden ? warden.authenticate : nil + end + def find_user_by_private_token - token_string = (params[PRIVATE_TOKEN_PARAM] || env[PRIVATE_TOKEN_HEADER]).to_s - User.find_by_authentication_token(token_string) || User.find_by_personal_access_token(token_string) + token = private_token + return nil unless token.present? + + User.find_by_authentication_token(token) || User.find_by_personal_access_token(token) end def current_user - @current_user ||= (find_user_by_private_token || doorkeeper_guard) + @current_user ||= find_user_by_private_token + @current_user ||= doorkeeper_guard + @current_user ||= find_user_from_warden unless @current_user && Gitlab::UserAccess.allowed?(@current_user) return nil diff --git a/lib/gitlab/gon_helper.rb b/lib/gitlab/gon_helper.rb index f751a3a12fd..2c218053163 100644 --- a/lib/gitlab/gon_helper.rb +++ b/lib/gitlab/gon_helper.rb @@ -12,7 +12,6 @@ module Gitlab if current_user gon.current_user_id = current_user.id - gon.api_token = current_user.private_token end end end diff --git a/spec/requests/api/api_helpers_spec.rb b/spec/requests/api/api_helpers_spec.rb index f22db61e744..227434f88c4 100644 --- a/spec/requests/api/api_helpers_spec.rb +++ b/spec/requests/api/api_helpers_spec.rb @@ -36,11 +36,36 @@ describe API::Helpers, api: true do params.delete(API::Helpers::SUDO_PARAM) end + def warden_authenticate_returns(value) + warden = double("warden", authenticate: value) + env['warden'] = warden + end + + def doorkeeper_guard_returns(value) + allow_any_instance_of(self.class).to receive(:doorkeeper_guard){ value } + end + def error!(message, status) raise Exception end describe ".current_user" do + subject { current_user } + + describe "when authenticating via Warden" do + before { doorkeeper_guard_returns false } + + context "fails" do + it { is_expected.to be_nil } + end + + context "succeeds" do + before { warden_authenticate_returns user } + + it { is_expected.to eq(user) } + end + end + describe "when authenticating using a user's private token" do it "should return nil for an invalid token" do env[API::Helpers::PRIVATE_TOKEN_HEADER] = 'invalid token' |