diff options
author | Rémy Coutable <remy@gitlab.com> | 2016-09-19 13:04:04 +0000 |
---|---|---|
committer | Rémy Coutable <remy@rymai.me> | 2016-09-19 15:50:49 +0200 |
commit | 8ddfeec8b29e0c053ee5e1c16e660382d7abe44f (patch) | |
tree | 7b269ba25e3b5714185d0515f260531efa784749 | |
parent | d7ee1e7eee6f125a73527e675c0e0ab368e58b53 (diff) | |
download | gitlab-ce-8ddfeec8b29e0c053ee5e1c16e660382d7abe44f.tar.gz |
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 f9034abfca4..0299c805f90 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.10.10 + - Allow the Rails cookie to be used for API authentication. + v 8.10.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 89b0ac697ed..19c59df8a9d 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 d1e6c54c521..81d0e3e6451 100644 --- a/doc/api/README.md +++ b/doc/api/README.md @@ -47,11 +47,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 { @@ -90,6 +91,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 d6e4eb2afd7..28225281fee 100644 --- a/lib/api/helpers.rb +++ b/lib/api/helpers.rb @@ -16,13 +16,30 @@ module API nil 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.new(@current_user).allowed? return nil diff --git a/lib/gitlab/gon_helper.rb b/lib/gitlab/gon_helper.rb index c5a11148d33..2c21804fe7a 100644 --- a/lib/gitlab/gon_helper.rb +++ b/lib/gitlab/gon_helper.rb @@ -11,7 +11,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 831889afb6c..90a26d56a90 100644 --- a/spec/requests/api/api_helpers_spec.rb +++ b/spec/requests/api/api_helpers_spec.rb @@ -35,11 +35,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' |