[ARVADOS] created: 1.3.0-185-g4bb449eb5
Git user
git at public.curoverse.com
Fri Jan 25 19:08:28 EST 2019
at 4bb449eb541e7bc22dfb09c31451d2258f189495 (commit)
commit 4bb449eb541e7bc22dfb09c31451d2258f189495
Author: Lucas Di Pentima <ldipentima at veritasgenetics.com>
Date: Fri Jan 25 21:04:58 2019 -0300
14718: Issue salted token when passing 'remote' param to login.
When logging in with a 'remote' parameter, it's encoded into the return_to
param passed to the SSO server so that it's received back after the user
is authenticated.
When the omniauth callback receives a remote parameter inside the return_to
url, it returns the salted version of the client token.
Arvados-DCO-1.1-Signed-off-by: Lucas Di Pentima <ldipentima at veritasgenetics.com>
diff --git a/services/api/app/controllers/user_sessions_controller.rb b/services/api/app/controllers/user_sessions_controller.rb
index 020dfa53b..cb150bd94 100644
--- a/services/api/app/controllers/user_sessions_controller.rb
+++ b/services/api/app/controllers/user_sessions_controller.rb
@@ -95,7 +95,11 @@ class UserSessionsController < ApplicationController
@redirect_to = root_path
if params.has_key?(:return_to)
- return send_api_token_to(params[:return_to], user)
+ rt = params[:return_to]
+ # Extracts query params as {param1 => [value1], param2 => [value2], ...}
+ p = rt.index('?').nil? ? {} : CGI::parse(rt[rt.index('?')+1..-1])
+ remote = p["remote"] && p["remote"][0]
+ return send_api_token_to(params[:return_to], user, remote)
end
redirect_to @redirect_to
end
@@ -119,8 +123,6 @@ class UserSessionsController < ApplicationController
# to save the return_to parameter (if it exists; see the application
# controller). /auth/joshid bypasses the application controller.
def login
- auth_provider = if params[:auth_provider] then "auth_provider=#{CGI.escape(params[:auth_provider])}" else "" end
-
if current_user and params[:return_to]
# Already logged in; just need to send a token to the requesting
# API client.
@@ -128,15 +130,25 @@ class UserSessionsController < ApplicationController
# FIXME: if current_user has never authorized this app before,
# ask for confirmation here!
- send_api_token_to(params[:return_to], current_user)
- elsif params[:return_to]
- redirect_to "/auth/joshid?return_to=#{CGI.escape(params[:return_to])}&#{auth_provider}"
- else
- redirect_to "/auth/joshid?#{auth_provider}"
+ return send_api_token_to(params[:return_to], current_user, params[:remote])
+ end
+ p = []
+ p << "auth_provider=#{CGI.escape(params[:auth_provider])}" if params[:auth_provider]
+ if params[:return_to]
+ remote_param = ''
+ if params[:remote]
+ # Encode remote param inside return_to, so that we'll get it on the
+ # callback after login
+ remote_param += if params[:return_to].include? '?' then '&' else '?' end
+ remote_param += "remote=#{params[:remote]}"
+ end
+ p << "return_to=#{CGI.escape(params[:return_to]+remote_param)}"
end
+
+ redirect_to "/auth/joshid?#{p.join('&')}"
end
- def send_api_token_to(callback_url, user)
+ def send_api_token_to(callback_url, user, remote=nil)
# Give the API client a token for making API calls on behalf of
# the authenticated user
@@ -147,19 +159,24 @@ class UserSessionsController < ApplicationController
find_or_create_by(url_prefix: api_client_url_prefix)
end
- api_client_auth = ApiClientAuthorization.
+ @api_client_auth = ApiClientAuthorization.
new(user: user,
api_client: @api_client,
created_by_ip_address: remote_ip,
scopes: ["all"])
- api_client_auth.save!
+ @api_client_auth.save!
if callback_url.index('?')
callback_url += '&'
else
callback_url += '?'
end
- callback_url += 'api_token=' + api_client_auth.token
+ if remote.nil?
+ token = @api_client_auth.token
+ else
+ token = @api_client_auth.salted_token(remote: remote)
+ end
+ callback_url += 'api_token=' + token
redirect_to callback_url
end
diff --git a/services/api/app/models/api_client_authorization.rb b/services/api/app/models/api_client_authorization.rb
index 53ae6af46..39253e103 100644
--- a/services/api/app/models/api_client_authorization.rb
+++ b/services/api/app/models/api_client_authorization.rb
@@ -236,6 +236,13 @@ class ApiClientAuthorization < ArvadosModel
'v2/' + uuid + '/' + api_token
end
+ def salted_token(remote:)
+ if remote.nil?
+ token
+ end
+ 'v2/' + uuid + '/' + OpenSSL::HMAC.hexdigest('sha1', api_token, remote)
+ end
+
protected
def permission_to_create
diff --git a/services/api/test/functional/user_sessions_controller_test.rb b/services/api/test/functional/user_sessions_controller_test.rb
index f7021cf9d..43872e1b8 100644
--- a/services/api/test/functional/user_sessions_controller_test.rb
+++ b/services/api/test/functional/user_sessions_controller_test.rb
@@ -17,4 +17,14 @@ class UserSessionsControllerTest < ActionController::TestCase
assert_not_nil assigns(:api_client)
end
+ test "login with remote param returns a salted token" do
+ authorize_with :inactive
+ api_client_page = 'http://client.example.com/home'
+ remote_prefix = 'zbbbb'
+ get :login, return_to: api_client_page, remote: remote_prefix
+ assert_response :redirect
+ api_client_auth = assigns(:api_client_auth)
+ assert_not_nil api_client_auth
+ assert_includes(@response.redirect_url, 'api_token='+api_client_auth.salted_token(remote: remote_prefix))
+ end
end
diff --git a/services/api/test/integration/user_sessions_test.rb b/services/api/test/integration/user_sessions_test.rb
index 0497c6a7d..f221e58dd 100644
--- a/services/api/test/integration/user_sessions_test.rb
+++ b/services/api/test/integration/user_sessions_test.rb
@@ -5,11 +5,13 @@
require 'test_helper'
class UserSessionsApiTest < ActionDispatch::IntegrationTest
- def client_url
- 'https://wb.example.com'
+ def client_url(query_string: nil)
+ url = 'https://wb.example.com'
+ url += '?'+ query_string unless query_string.nil?
+ url
end
- def mock_auth_with(email: nil, username: nil, identity_url: nil)
+ def mock_auth_with(email: nil, username: nil, identity_url: nil, remote: nil)
mock = {
'provider' => 'josh_id',
'uid' => 'https://edward.example.com',
@@ -23,8 +25,9 @@ class UserSessionsApiTest < ActionDispatch::IntegrationTest
mock['info']['email'] = email unless email.nil?
mock['info']['username'] = username unless username.nil?
mock['info']['identity_url'] = identity_url unless identity_url.nil?
+ return_to = remote.nil? ? client_url : client_url(query_string: 'remote='+remote)
post('/auth/josh_id/callback',
- {return_to: client_url},
+ {return_to: return_to},
{'omniauth.auth' => mock})
assert_response :redirect, 'Did not redirect to client with token'
end
@@ -69,6 +72,13 @@ class UserSessionsApiTest < ActionDispatch::IntegrationTest
@response.redirect_url)
end
+ test 'issue salted token from omniauth callback with remote param' do
+ mock_auth_with(email: 'edward at example.com', remote: 'zbbbb')
+ api_client_auth = assigns(:api_client_auth)
+ assert_not_nil api_client_auth
+ assert_includes(@response.redirect_url, 'api_token=' + api_client_auth.salted_token(remote: 'zbbbb'))
+ end
+
# Test various combinations of auto_setup configuration and email
# address provided during a new user's first session setup.
[{result: :nope, email: nil, cfg: {auto: true, repo: true, vm: true}},
-----------------------------------------------------------------------
hooks/post-receive
--
More information about the arvados-commits
mailing list