Commit f2932885 authored by Shinya Maeda's avatar Shinya Maeda

Security fix: redirection in google_api/authorizations_controller

parent 5ced761e
......@@ -9,8 +9,13 @@ module GoogleApi
session[GoogleApi::CloudPlatform::Client.session_key_for_expires_at] =
expires_at.to_s
if params[:state].present?
redirect_to params[:state]
key, _ = GoogleApi::CloudPlatform::Client
.session_key_for_second_redirect_uri(secure: params[:state])
second_redirect_uri = session[key]
if second_redirect_uri.present?
redirect_to second_redirect_uri
else
redirect_to root_path
end
......
......@@ -16,9 +16,13 @@ class Projects::ClustersController < Projects::ApplicationController
def login
begin
GoogleApi::CloudPlatform::Client.session_key_for_second_redirect_uri.tap do |key, secure|
session[key] = namespace_project_clusters_url.to_s
@authorize_url = GoogleApi::CloudPlatform::Client.new(
nil, callback_google_api_auth_url,
state: namespace_project_clusters_url.to_s).authorize_url
state: secure).authorize_url
end
rescue GoogleApi::Auth::ConfigMissingError
# no-op
end
......
......@@ -15,6 +15,11 @@ module GoogleApi
def session_key_for_expires_at
:cloud_platform_expires_at
end
def session_key_for_second_redirect_uri(secure: nil)
secure = SecureRandom.hex unless secure
return "cloud_platform_second_redirect_uri_#{secure}", secure
end
end
def scope
......
......@@ -3,12 +3,10 @@ require 'spec_helper'
describe GoogleApi::AuthorizationsController do
describe 'GET|POST #callback' do
let(:user) { create(:user) }
let(:project) { create(:project) }
let(:state) { project_clusters_url(project).to_s }
let(:token) { 'token' }
let(:expires_at) { 1.hour.since.strftime('%s') }
subject { get :callback, code: 'xxx', state: state }
subject { get :callback, code: 'xxx', state: @state }
before do
sign_in(user)
......@@ -17,7 +15,7 @@ describe GoogleApi::AuthorizationsController do
.to receive(:get_token).and_return([token, expires_at])
end
it 'sets token and expires_atin session' do
it 'sets token and expires_at in session' do
subject
expect(session[GoogleApi::CloudPlatform::Client.session_key_for_token])
......@@ -26,15 +24,24 @@ describe GoogleApi::AuthorizationsController do
.to eq(expires_at)
end
context 'when redirection url is stored in state' do
context 'when second redirection url key is stored in state' do
set(:project) { create(:project) }
let(:second_redirect_uri) { namespace_project_clusters_url(project.namespace, project).to_s } # TODO: revrt
before do
GoogleApi::CloudPlatform::Client
.session_key_for_second_redirect_uri.tap do |key, secure|
@state = secure
session[key] = second_redirect_uri
end
end
it 'redirects to the URL stored in state param' do
expect(subject).to redirect_to(state)
expect(subject).to redirect_to(second_redirect_uri)
end
end
context 'when redirection url is not stored in state' do
let(:state) { '' }
it 'redirects to root_path' do
expect(subject).to redirect_to(root_path)
end
......
......@@ -4,6 +4,29 @@ describe GoogleApi::CloudPlatform::Client do
let(:token) { 'token' }
let(:client) { described_class.new(token, nil) }
describe '.session_key_for_second_redirect_uri' do
subject { described_class.session_key_for_second_redirect_uri(secure: secure) }
context 'when pass a postfix' do
let(:secure) { SecureRandom.hex }
it 'creates a required session key' do
key, _ = described_class.session_key_for_second_redirect_uri(secure: secure)
expect(key).to eq("cloud_platform_second_redirect_uri_#{secure}")
end
end
context 'when pass a postfix' do
let(:secure) { nil }
it 'creates a new session key' do
key, secure = described_class.session_key_for_second_redirect_uri
expect(key).to include('cloud_platform_second_redirect_uri_')
expect(secure).not_to be_nil
end
end
end
describe '#validate_token' do
subject { client.validate_token(expires_at) }
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment