From ade40fdcd2a4ee879bd2aba939cffaff39c65228 Mon Sep 17 00:00:00 2001 From: Timothy Andrew Date: Tue, 19 Apr 2016 16:22:15 +0530 Subject: Authenticate non-API requests with personal access tokens. - Rename the `authenticate_user_from_token!` filter to `authenticate_user_from_private_token!` - Add a new `authenticate_user_from_personal_access_token!` filter - Add tests for both. --- app/controllers/application_controller.rb | 19 +++++++++++++++++-- 1 file changed, 17 insertions(+), 2 deletions(-) (limited to 'app/controllers/application_controller.rb') diff --git a/app/controllers/application_controller.rb b/app/controllers/application_controller.rb index 1c53b0b21a3..590f9383f7f 100644 --- a/app/controllers/application_controller.rb +++ b/app/controllers/application_controller.rb @@ -7,7 +7,8 @@ class ApplicationController < ActionController::Base include GitlabRoutingHelper include PageLayoutHelper - before_action :authenticate_user_from_token! + before_action :authenticate_user_from_private_token! + before_action :authenticate_user_from_personal_access_token! before_action :authenticate_user! before_action :validate_user_service_ticket! before_action :reject_blocked! @@ -65,7 +66,7 @@ class ApplicationController < ActionController::Base # From https://github.com/plataformatec/devise/wiki/How-To:-Simple-Token-Authentication-Example # https://gist.github.com/josevalim/fb706b1e933ef01e4fb6 - def authenticate_user_from_token! + def authenticate_user_from_private_token! user_token = if params[:authenticity_token].presence params[:authenticity_token].presence elsif params[:private_token].presence @@ -84,6 +85,20 @@ class ApplicationController < ActionController::Base end end + def authenticate_user_from_personal_access_token! + token_string = params[:personal_access_token].presence || request.headers['PERSONAL_ACCESS_TOKEN'].presence + personal_access_token = PersonalAccessToken.active.find_by_token(token_string) + user = personal_access_token && personal_access_token.user + + if user + # Notice we are passing store false, so the user is not + # actually stored in the session and a token is needed + # for every request. If you want the token to work as a + # sign in token, you can simply remove store: false. + sign_in user, store: false + end + end + def authenticate_user!(*args) if redirect_to_home_page_url? redirect_to current_application_settings.home_page_url and return -- cgit v1.2.1 From 051324e12a7384b15aaa68f53dd43ad0b3c67812 Mon Sep 17 00:00:00 2001 From: Timothy Andrew Date: Tue, 19 Apr 2016 16:24:33 +0530 Subject: Refactor `authenticate_user_from_private_token!` - No need to use `if`s when we have a `presence` check already. --- app/controllers/application_controller.rb | 8 +------- 1 file changed, 1 insertion(+), 7 deletions(-) (limited to 'app/controllers/application_controller.rb') diff --git a/app/controllers/application_controller.rb b/app/controllers/application_controller.rb index 590f9383f7f..2b2726c048c 100644 --- a/app/controllers/application_controller.rb +++ b/app/controllers/application_controller.rb @@ -67,13 +67,7 @@ class ApplicationController < ActionController::Base # From https://github.com/plataformatec/devise/wiki/How-To:-Simple-Token-Authentication-Example # https://gist.github.com/josevalim/fb706b1e933ef01e4fb6 def authenticate_user_from_private_token! - user_token = if params[:authenticity_token].presence - params[:authenticity_token].presence - elsif params[:private_token].presence - params[:private_token].presence - elsif request.headers['PRIVATE-TOKEN'].present? - request.headers['PRIVATE-TOKEN'] - end + user_token = params[:authenticity_token].presence || params[:private_token].presence || request.headers['PRIVATE-TOKEN'].presence user = user_token && User.find_by_authentication_token(user_token.to_s) if user -- cgit v1.2.1 From bafbf22c6ab613d25287d7119d7e30770c531fdb Mon Sep 17 00:00:00 2001 From: Timothy Andrew Date: Mon, 25 Apr 2016 14:30:59 +0530 Subject: Address @DouweM's feedback on !3749. - Use `TokenAuthenticatable` to generate the personal access token - Remove a check for `authenticity_token` in application controller; this should've been `authentication_token`, maybe, and doesn't make any sense now. - Have the datepicker appear inline --- app/controllers/application_controller.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'app/controllers/application_controller.rb') diff --git a/app/controllers/application_controller.rb b/app/controllers/application_controller.rb index 2b2726c048c..eb5ffc44c3b 100644 --- a/app/controllers/application_controller.rb +++ b/app/controllers/application_controller.rb @@ -67,7 +67,7 @@ class ApplicationController < ActionController::Base # From https://github.com/plataformatec/devise/wiki/How-To:-Simple-Token-Authentication-Example # https://gist.github.com/josevalim/fb706b1e933ef01e4fb6 def authenticate_user_from_private_token! - user_token = params[:authenticity_token].presence || params[:private_token].presence || request.headers['PRIVATE-TOKEN'].presence + user_token = params[:private_token].presence || request.headers['PRIVATE-TOKEN'].presence user = user_token && User.find_by_authentication_token(user_token.to_s) if user -- cgit v1.2.1 From d915e7d5cad99b8971e65d30accc8bc7a05fecbc Mon Sep 17 00:00:00 2001 From: Timothy Andrew Date: Wed, 11 May 2016 10:16:23 +0530 Subject: Reuse the private token param and header for personal access tokens. - https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/3749#note_11626427 - Personal access tokens are still a separate entity as far as the codebase is concerned - they just happen to use the same entry point as private tokens. - Update tests and documentation to reflect this change --- app/controllers/application_controller.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'app/controllers/application_controller.rb') diff --git a/app/controllers/application_controller.rb b/app/controllers/application_controller.rb index 72ba1a85cff..b26afb42e74 100644 --- a/app/controllers/application_controller.rb +++ b/app/controllers/application_controller.rb @@ -80,7 +80,7 @@ class ApplicationController < ActionController::Base end def authenticate_user_from_personal_access_token! - token_string = params[:personal_access_token].presence || request.headers['PERSONAL_ACCESS_TOKEN'].presence + token_string = params[:private_token].presence || request.headers['PRIVATE-TOKEN'].presence personal_access_token = PersonalAccessToken.active.find_by_token(token_string) user = personal_access_token && personal_access_token.user -- cgit v1.2.1 From 05b319b0b45288cbbe0bce15bf7bed7f58f6cf76 Mon Sep 17 00:00:00 2001 From: Timothy Andrew Date: Wed, 1 Jun 2016 14:04:38 +0530 Subject: Perform private token and personal access token authentication in the same `before_action`. - So that the check for valid personal access tokens happens only if private token auth fails. --- app/controllers/application_controller.rb | 38 +++++++++++++------------------ 1 file changed, 16 insertions(+), 22 deletions(-) (limited to 'app/controllers/application_controller.rb') diff --git a/app/controllers/application_controller.rb b/app/controllers/application_controller.rb index b26afb42e74..9dbaba00ff5 100644 --- a/app/controllers/application_controller.rb +++ b/app/controllers/application_controller.rb @@ -7,8 +7,7 @@ class ApplicationController < ActionController::Base include GitlabRoutingHelper include PageLayoutHelper - before_action :authenticate_user_from_private_token! - before_action :authenticate_user_from_personal_access_token! + before_action :authenticate_user_from_token! before_action :authenticate_user! before_action :validate_user_service_ticket! before_action :reject_blocked! @@ -64,26 +63,8 @@ class ApplicationController < ActionController::Base end end - # From https://github.com/plataformatec/devise/wiki/How-To:-Simple-Token-Authentication-Example - # https://gist.github.com/josevalim/fb706b1e933ef01e4fb6 - def authenticate_user_from_private_token! - user_token = params[:private_token].presence || request.headers['PRIVATE-TOKEN'].presence - user = user_token && User.find_by_authentication_token(user_token.to_s) - - if user - # Notice we are passing store false, so the user is not - # actually stored in the session and a token is needed - # for every request. If you want the token to work as a - # sign in token, you can simply remove store: false. - sign_in user, store: false - end - end - - def authenticate_user_from_personal_access_token! - token_string = params[:private_token].presence || request.headers['PRIVATE-TOKEN'].presence - personal_access_token = PersonalAccessToken.active.find_by_token(token_string) - user = personal_access_token && personal_access_token.user - + def authenticate_user_from_token! + user = get_user_from_private_token || get_user_from_personal_access_token if user # Notice we are passing store false, so the user is not # actually stored in the session and a token is needed @@ -383,4 +364,17 @@ class ApplicationController < ActionController::Base (controller_name == 'groups' && action_name == page_type) || (controller_name == 'dashboard' && action_name == page_type) end + + # From https://github.com/plataformatec/devise/wiki/How-To:-Simple-Token-Authentication-Example + # https://gist.github.com/josevalim/fb706b1e933ef01e4fb6 + def get_user_from_private_token + user_token = params[:private_token].presence || request.headers['PRIVATE-TOKEN'].presence + User.find_by_authentication_token(user_token.to_s) if user_token + end + + def get_user_from_personal_access_token + token_string = params[:private_token].presence || request.headers['PRIVATE-TOKEN'].presence + personal_access_token = PersonalAccessToken.active.find_by_token(token_string) + personal_access_token.user if personal_access_token + end end -- cgit v1.2.1 From 6d444331764fec1910d15b300d89b6246a1f83ea Mon Sep 17 00:00:00 2001 From: Timothy Andrew Date: Wed, 1 Jun 2016 14:09:17 +0530 Subject: Don't look for personal access tokens in the DB when the parameter/header is not passed. --- app/controllers/application_controller.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'app/controllers/application_controller.rb') diff --git a/app/controllers/application_controller.rb b/app/controllers/application_controller.rb index 9dbaba00ff5..17bd980b454 100644 --- a/app/controllers/application_controller.rb +++ b/app/controllers/application_controller.rb @@ -374,7 +374,7 @@ class ApplicationController < ActionController::Base def get_user_from_personal_access_token token_string = params[:private_token].presence || request.headers['PRIVATE-TOKEN'].presence - personal_access_token = PersonalAccessToken.active.find_by_token(token_string) + personal_access_token = PersonalAccessToken.active.find_by_token(token_string) if token_string personal_access_token.user if personal_access_token end end -- cgit v1.2.1 From 0dff6fd7148957fa94d2626e3912cd929ba150d3 Mon Sep 17 00:00:00 2001 From: Timothy Andrew Date: Fri, 3 Jun 2016 10:10:58 +0530 Subject: Fix rubocop spec. --- app/controllers/application_controller.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'app/controllers/application_controller.rb') diff --git a/app/controllers/application_controller.rb b/app/controllers/application_controller.rb index c8baea2579b..23a0e16ca43 100644 --- a/app/controllers/application_controller.rb +++ b/app/controllers/application_controller.rb @@ -368,7 +368,7 @@ class ApplicationController < ActionController::Base # From https://github.com/plataformatec/devise/wiki/How-To:-Simple-Token-Authentication-Example # https://gist.github.com/josevalim/fb706b1e933ef01e4fb6 def get_user_from_private_token - user_token = params[:private_token].presence || request.headers['PRIVATE-TOKEN'].presence + user_token = params[:private_token].presence || request.headers['PRIVATE-TOKEN'].presence User.find_by_authentication_token(user_token.to_s) if user_token end -- cgit v1.2.1 From 7ee0898a9ec4a03c9a55841b1cbea67add460c50 Mon Sep 17 00:00:00 2001 From: Timothy Andrew Date: Thu, 16 Jun 2016 08:24:13 +0530 Subject: Implement @DouweM's feedback. - Extract a duplicated `redirect_to` - Fix a typo: "token", not "certificate" - Have the "Expires at" datepicker be attached to a text field, not inline - Have both private tokens and personal access tokens verified in a single "authenticate_from_private_token" method, both in the application and API. Move relevant logic to `User#find_by_personal_access_token` - Remove unnecessary constants relating to API auth. We don't need a separate constant for personal access tokens since the param is the same as for private tokens. --- app/controllers/application_controller.rb | 22 ++++++---------------- 1 file changed, 6 insertions(+), 16 deletions(-) (limited to 'app/controllers/application_controller.rb') diff --git a/app/controllers/application_controller.rb b/app/controllers/application_controller.rb index a5b358f10f1..72d1b97bf56 100644 --- a/app/controllers/application_controller.rb +++ b/app/controllers/application_controller.rb @@ -8,7 +8,7 @@ class ApplicationController < ActionController::Base include PageLayoutHelper include WorkhorseHelper - before_action :authenticate_user_from_token! + before_action :authenticate_user_from_private_token! before_action :authenticate_user! before_action :validate_user_service_ticket! before_action :reject_blocked! @@ -64,8 +64,11 @@ class ApplicationController < ActionController::Base end end - def authenticate_user_from_token! - user = get_user_from_private_token || get_user_from_personal_access_token + # This filter handles both private tokens and personal access tokens + def authenticate_user_from_private_token! + token_string = params[:private_token].presence || request.headers['PRIVATE-TOKEN'].presence + user = User.find_by_authentication_token(token_string) || User.find_by_personal_access_token(token_string) + if user # Notice we are passing store false, so the user is not # actually stored in the session and a token is needed @@ -376,17 +379,4 @@ class ApplicationController < ActionController::Base (controller_name == 'groups' && action_name == page_type) || (controller_name == 'dashboard' && action_name == page_type) end - - # From https://github.com/plataformatec/devise/wiki/How-To:-Simple-Token-Authentication-Example - # https://gist.github.com/josevalim/fb706b1e933ef01e4fb6 - def get_user_from_private_token - user_token = params[:private_token].presence || request.headers['PRIVATE-TOKEN'].presence - User.find_by_authentication_token(user_token.to_s) if user_token - end - - def get_user_from_personal_access_token - token_string = params[:private_token].presence || request.headers['PRIVATE-TOKEN'].presence - personal_access_token = PersonalAccessToken.active.find_by_token(token_string) if token_string - personal_access_token.user if personal_access_token - end end -- cgit v1.2.1