Skip to content

Commit

Permalink
Merge pull request #5164 from skateman/api-cookie
Browse files Browse the repository at this point in the history
Use cookie authentication against the API, drop the token renewal
  • Loading branch information
himdel authored Jan 22, 2019
2 parents 7816d3d + f161abd commit a3bbe07
Show file tree
Hide file tree
Showing 14 changed files with 44 additions and 161 deletions.
3 changes: 0 additions & 3 deletions app/assets/javascripts/miq_api.js
Original file line number Diff line number Diff line change
Expand Up @@ -19,9 +19,6 @@
patch: angularify(vanillaJsAPI.patch),
options: angularify(vanillaJsAPI.options),
wait_for_task: angularify(vanillaJsAPI.wait_for_task),
login: angularify(vanillaJsAPI.login),
logout: vanillaJsAPI.logout,
autorenew: vanillaJsAPI.autorenew,
};
}];

Expand Down
67 changes: 19 additions & 48 deletions app/assets/javascripts/miq_application.js
Original file line number Diff line number Diff line change
Expand Up @@ -699,54 +699,35 @@ function storeUserFeatures() {
// Send login authentication via ajax
function miqAjaxAuth(url) {
miqEnableLoginFields(false);
miqSparkleOn(); // miqJqueryRequest starts sparkle either way, but API login doesn't

var credentials = {
login: $('#user_name').val(),
password: $('#user_password').val(),
serialized: miqSerializeForm('login_div'),
};
return miqJqueryRequest(url || '/dashboard/authenticate', {
beforeSend: true,
data: miqSerializeForm('login_div'),
}).then(null, function(err) {
// HTTP failures only, authentication failures come as 200, with their own javascript
var message = __('Incorrect username or password');
if (err && err.status && err.status !== 200 && err.statusText) {
message = __('Login failed:') + ' ' + err.statusText;
}

return vanillaJsAPI.login(credentials.login, credentials.password)
.then(function() {
return vanillaJsAPI.ws_init();
})
.then(function() {
return storeUserFeatures();
})
.then(function() {
// API login ok, now do the normal one
miqJqueryRequest(url || '/dashboard/authenticate', {
beforeSend: true,
data: credentials.serialized,
});

// TODO vanillaJsAPI.autorenew is called on (non-login) page load - when?
})
.then(null, function(err) {
var message = __('Incorrect username or password');
if (err && err.status && err.status !== 200 && err.statusText) {
message = __('Login failed:') + ' ' + err.statusText;
}
clearFlash();
add_flash(message, 'error', { id: 'auth_failed' });

clearFlash();
add_flash(message, 'error', { id: 'auth_failed' });
miqAjaxAuthFail();
miqSparkleOff();
});
}

miqClearLoginFields();
miqEnableLoginFields(true);
miqSparkleOff();
});
function miqAjaxAuthFail() {
miqClearLoginFields();
miqEnableLoginFields(true);
}

// Send SSO login authentication via ajax
function miqAjaxAuthSso(url) {
miqEnableLoginFields(false);
miqSparkleOn();

// Note: /dashboard/kerberos_authenticate creates an API token
// based on the authenticated external user
// and stores it in localStorage.miq_token

miqJqueryRequest(url || '/dashboard/kerberos_authenticate', {
beforeSend: true,
});
Expand All @@ -757,19 +738,9 @@ function miqAjaxExtAuth(url) {
miqEnableLoginFields(false);
miqSparkleOn();

// Note: /dashboard/external_authenticate creates an API token
// based on the authenticated external user
// and stores it in localStorage.miq_token

var credentials = {
login: $('#user_name').val(),
password: $('#user_password').val(),
serialized: miqSerializeForm('login_div'),
};

miqJqueryRequest(url || '/dashboard/external_authenticate', {
beforeSend: true,
data: credentials.serialized,
data: miqSerializeForm('login_div'),
});
}

Expand Down
23 changes: 7 additions & 16 deletions app/controllers/dashboard_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -506,16 +506,16 @@ def saml_login

# Handle external-auth signon from login screen
def external_authenticate
authenticate_external_user_generate_api_token
authenticate_external_user
end

# Handle single-signon from login screen
def kerberos_authenticate
authenticate_external_user_generate_api_token
authenticate_external_user
end

# Handle user credentials from login screen
def authenticate(require_api_token = false)
def authenticate
@layout = "dashboard"

unless params[:task_id] # First time thru, check for buttons pressed
Expand Down Expand Up @@ -565,10 +565,8 @@ def authenticate(require_api_token = false)
when :wait_for_task
# noop, page content already set by initiate_wait_for_task
when :pass
miq_api_token = require_api_token ? generate_ui_api_token(user[:name]) : nil
render :update do |page|
page << javascript_prologue
page << "localStorage.miq_token = '#{j_str(miq_api_token)}';" if miq_api_token
page.redirect_to(validation.url)
end
when :fail
Expand All @@ -578,17 +576,12 @@ def authenticate(require_api_token = false)
page << javascript_prologue
page.replace("flash_msg_div", :partial => "layouts/flash_msg")
page << javascript_show("flash_div")
page << "miqAjaxAuthFail();"
page << "miqSparkle(false);"
page << "miqEnableLoginFields(true);"
end
end
end

def generate_ui_api_token(userid)
@api_user_token_service ||= Api::UserTokenService.new
@api_user_token_service.generate_token(userid, "ui")
end

def timeline
@breadcrumbs = []
@layout = "timeline"
Expand Down Expand Up @@ -745,13 +738,12 @@ def session_init(db_user)

private

# Authenticate external user and generate API token
def authenticate_external_user_generate_api_token
def authenticate_external_user
if @user_name.blank? && request.headers["X-Remote-User"].present?
@user_name = params[:user_name] = request.headers["X-Remote-User"].split("@").first
end

authenticate(true)
authenticate
end

def tl_toggle_button_enablement(button_id, enablement, typ)
Expand Down Expand Up @@ -863,8 +855,7 @@ def identity_provider_login(identity_type)
when :pass
render :template => "dashboard/#{identity_type}",
:layout => false,
:locals => {:api_auth_token => generate_ui_api_token(@user_name),
:validation_url => validation.url}
:locals => {:validation_url => validation.url}
return
when :fail
session[:user_validation_error] = validation.flash_msg || "User validation failed"
Expand Down
46 changes: 0 additions & 46 deletions app/javascript/http_api/api.js
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
import { miqFetch } from './fetch';
import { base64encode } from './compat';

const { miqDeferred } = window;

Expand All @@ -10,9 +9,6 @@ const { miqDeferred } = window;
* API.put - (the same)
* API.patch - (the same)
* API.options - (the same)
* API.login(login, password) - performs initial authentication, saves token on success, returns Promise
* API.logout() - clears login info, no return
* API.autorenew() - registers a 60second interval to query /api, returns a function to clear the interval
*
* the API token is persisted into localStorage
*/
Expand Down Expand Up @@ -46,52 +42,10 @@ function withData(method) {
}, data);
}


API.login = (login, password) => {
API.logout();

return API.get('/api/auth?requester_type=ui', {
headers: {
Authorization: `Basic ${base64encode([login, password].join(':'))}`,
},
skipErrors: [401, 503],
skipLoginRedirect: true,
})
.then((response) => {
localStorage.miq_token = response.auth_token;
});
};

API.ws_destroy = () => {
document.cookie = 'ws_token=; path=/ws/notifications; Max-Age=0;';
};

API.logout = () => {
// delete user allowed features data from local storage
delete localStorage.userFeatures;
if (localStorage.miq_token) {
API.delete('/api/auth', {
skipErrors: [401],
skipLoginRedirect: true,
});
}

API.ws_destroy();
delete localStorage.miq_token;
};

API.autorenew = () => {
const id = setInterval(() => {
API.get('/api')
.then(null, (...args) => {
console.warn('API autorenew fail', args);
clearInterval(id);
});
}, 60 * 1000);

return () => clearInterval(id);
};

API.ws_init = () => API.get('/api/auth?requester_type=ws').then((response) => {
API.ws_destroy();
document.cookie = `ws_token=${response.auth_token}; path=/ws/notifications`;
Expand Down
9 changes: 0 additions & 9 deletions app/javascript/http_api/compat.js

This file was deleted.

12 changes: 4 additions & 8 deletions app/javascript/http_api/fetch.js
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ function processOptions(options) {
delete o.type;
delete o.url;
delete o.transformResponse;
delete o.csrf;

o.headers = o.headers || {};

Expand All @@ -30,14 +31,9 @@ function processOptions(options) {
delete o.skipTokenRenewal;
}

if (localStorage.miq_token) {
o.headers['X-Auth-Token'] = localStorage.miq_token;
}

if (o.csrf) {
const elem = document.querySelector("meta[name=csrf-token]");
o.headers['X-CSRF-Token'] = elem ? elem.getAttribute('content') : '';
delete o.csrf;
const csrfElem = document.querySelector("meta[name=csrf-token]");
if (csrfElem) {
o.headers['X-CSRF-Token'] = csrfElem.getAttribute('content');
}

if (Object.keys(o.headers).length) {
Expand Down
3 changes: 1 addition & 2 deletions app/views/dashboard/oidc_login.html.haml
Original file line number Diff line number Diff line change
Expand Up @@ -5,10 +5,9 @@
= javascript_include_tag 'application'

%body
- if api_auth_token && validation_url
- if validation_url
:javascript
miqFlashClearSaved();
localStorage.miq_token = '#{j_str api_auth_token}';
window.location = '#{j_str validation_url}';
- else
:javascript
Expand Down
3 changes: 1 addition & 2 deletions app/views/dashboard/saml_login.html.haml
Original file line number Diff line number Diff line change
Expand Up @@ -6,10 +6,9 @@
= javascript_include_tag 'application'

%body
- if api_auth_token && validation_url
- if validation_url
:javascript
miqFlashClearSaved();
localStorage.miq_token = '#{j_str api_auth_token}';
window.location = '#{j_str validation_url}';
- else
:javascript
Expand Down
1 change: 0 additions & 1 deletion app/views/layouts/application.html.haml
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,6 @@
ManageIQ.charts.provider = "#{Charting.backend}";
ManageIQ.browser = "#{j browser_info(:name_ui)}";
ManageIQ.controller = "#{j controller_name}";
vanillaJsAPI.autorenew();

- if ::Settings.server.asynchronous_notifications
:javascript
Expand Down
2 changes: 1 addition & 1 deletion app/views/layouts/login.html.haml
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@
= yield

:javascript
vanillaJsAPI.logout();
delete localStorage['patternfly-navigation-secondary'];
delete localStorage['patternfly-navigation-tertiary'];
miqFlashClearSaved();
API.ws_destroy();
2 changes: 0 additions & 2 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,6 @@
"angular-ui-sortable": "~0.16.1",
"angular.validators": "~4.4.3",
"array-includes": "~3.0.3",
"base64-js": "~1.2.3",
"bootstrap-filestyle": "~1.2.1",
"bootstrap-switch": "~3.3.4",
"classnames": "~2.2.6",
Expand Down Expand Up @@ -78,7 +77,6 @@
"spice-html5-bower": "~1.7.3",
"spin.js": "~4.0.0",
"sprintf-js": "~1.1.1",
"text-encoder-lite": "git://github.com/coolaj86/TextEncoderLite.git#e1e031b",
"ui-select": "0.19.8",
"whatwg-fetch": "~2.0.4",
"xml_display": "~0.1.1"
Expand Down
15 changes: 10 additions & 5 deletions spec/controllers/dashboard_controller_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -117,21 +117,17 @@
%i(saml oidc).each do |protocol|
it "#{protocol.upcase} protected page should render the #{protocol}_login page with the proper validation_url and api token" do
user = FactoryBot.create(:user, :userid => "johndoe", :role => "test")
auth_token = "aabbccddeeff"
validation_url = "/user_validation_url"

request.env["HTTP_X_REMOTE_USER"] = user.userid
skip_data_checks(validation_url)

allow(User).to receive(:authenticate).and_return(user)
allow_any_instance_of(Api::UserTokenService).to receive(:generate_token)
.with(user.userid, "ui")
.and_return(auth_token)

expect(controller).to receive(:render)
.with(:template => "dashboard/#{protocol}_login",
:layout => false,
:locals => {:api_auth_token => auth_token, :validation_url => validation_url})
:locals => {:validation_url => validation_url})
.exactly(1).times

controller.send("#{protocol}_login")
Expand Down Expand Up @@ -505,6 +501,15 @@
end
end

describe '#authenticate_external_user' do
it 'sets the user name based on the header' do
allow(controller).to receive(:authenticate)
request.headers['X-Remote-User'] = 'foo@bar'
controller.send(:authenticate_external_user)
expect(assigns(:user_name)).to eq('foo')
end
end

def skip_data_checks(url = '/')
allow_any_instance_of(UserValidationService).to receive(:server_ready?).and_return(true)
allow(controller).to receive(:start_url_for_user).and_return(url)
Expand Down
17 changes: 0 additions & 17 deletions spec/javascripts/miq_api_spec.js

This file was deleted.

Loading

0 comments on commit a3bbe07

Please sign in to comment.