Skip to content

Commit 547fe81

Browse files
Merge pull request #16961 from rabbitmq/rabbitmq-server-16957
By @thisisnsh: Restore the pre-login page after an idp_initiated OAuth login
2 parents e97e108 + 8d7333e commit 547fe81

3 files changed

Lines changed: 157 additions & 14 deletions

File tree

deps/rabbitmq_management/priv/www/js/main.js

Lines changed: 24 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -15,11 +15,15 @@ function format_error_response(response, reason) {
1515
});
1616
}
1717

18-
$(document).ready(function() {
18+
$(document).ready(function() {
1919
var url_string = window.location.href;
2020
var url = new URL(url_string);
2121
var error = url.searchParams.get('error');
2222
if (error) {
23+
// A failed IDP-initiated login returns here with an error. Drop the
24+
// pending state markers to avoid a confusing redirect the next successful login.
25+
clear_pref("oauth-idp-pending");
26+
clear_pref("oauth-return-to");
2327
if (oauth.enabled) {
2428
renderWarningMessageInLoginStatus(oauth, fmt_escape_html(error));
2529
}
@@ -48,8 +52,6 @@ function removeDuplicates(array){
4852

4953

5054
function startWithOAuthLogin (oauth) {
51-
store_pref("oauth-return-to", window.location.hash);
52-
5355
if (!oauth.logged_in) {
5456
hasAnyResourceServerReady(oauth, (oauth, warnings) => { render_login_oauth(oauth, warnings); start_app_login(); })
5557
} else {
@@ -107,7 +109,7 @@ function start_app_login () {
107109
});
108110
}
109111
})
110-
112+
111113
if (oauth.enabled) {
112114
if (has_auth_credentials()) {
113115
check_login();
@@ -192,14 +194,22 @@ function start_app() {
192194
var url = this.location.toString();
193195
var hash = this.location.hash;
194196
var pathname = this.location.pathname;
197+
198+
var return_to = '#/';
199+
if (get_pref("oauth-idp-pending")) {
200+
clear_pref("oauth-idp-pending");
201+
return_to = get_pref("oauth-return-to") || '#/';
202+
clear_pref("oauth-return-to");
203+
}
204+
195205
if (url.indexOf('#') == -1) {
196-
this.location = url + '#/';
206+
this.location = url + return_to;
197207
} else if (hash.indexOf('#token_type') != - 1 && pathname == '/') {
198208
// This is equivalent to previous `if` clause when uaa authorisation is used.
199209
// Tokens are passed in the url hash, so the url always contains a #.
200210
// We need to check the current path is `/` and token is present,
201211
// so we can redirect to `/#/`
202-
this.location = url.replace(/#token_type.+/gi, '#/');
212+
this.location = url.replace(/#token_type.+/gi, return_to);
203213
}
204214

205215
app = new Sammy.Application(dispatcher);
@@ -361,7 +371,7 @@ function go_to_home() {
361371
// location.href = rabbit_path_prefix() + "/"
362372
location.href = "/"
363373
}
364-
374+
365375
function set_timer_interval(interval) {
366376
timer_interval = interval;
367377
reset_timer();
@@ -1447,7 +1457,7 @@ function with_req(method, path, body, fun, on404fun) {
14471457
if (check_bad_response(req, !on404fun, on404fun)) {
14481458
last_successful_connect = new Date();
14491459
fun(req);
1450-
}
1460+
}
14511461
}
14521462
};
14531463
outstanding_reqs.push(req);
@@ -1538,7 +1548,7 @@ function initiate_logout(oauth, error = "") {
15381548
}
15391549
/**
15401550
* Handle bad http response
1541-
* @param {*} req
1551+
* @param {*} req
15421552
* @param {*} full_page_404 In case of 404, reload entire html page with the error message
15431553
* @param {*} on404fun In case of 404, call this function or else show a popup error message
15441554
* @returns true if there was no bad response
@@ -1566,10 +1576,10 @@ function check_bad_response(req, full_page_404, on404fun) {
15661576
if (typeof(reason) != 'string') {
15671577
reason = JSON.stringify(reason);
15681578
}
1569-
if ( error == 'bad_request' ||
1570-
error == 'not_found' ||
1571-
reason == 'Not Found' ||
1572-
error == 'not_authorised' ||
1579+
if ( error == 'bad_request' ||
1580+
error == 'not_found' ||
1581+
reason == 'Not Found' ||
1582+
error == 'not_authorised' ||
15731583
error == 'not_authorized') {
15741584
if ((req.status == 401 || req.status == 403) && oauth.enabled) {
15751585
initiate_logout(oauth, reason);
@@ -1886,7 +1896,7 @@ function is_internal(queue) {
18861896
return queue.internal;
18871897
}
18881898

1889-
function get_queue_type (queue) {
1899+
function get_queue_type (queue) {
18901900
switch(queue.type) {
18911901
case "classic":
18921902
case "quorum":

deps/rabbitmq_management/priv/www/js/oidc-oauth/helper.js

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -256,6 +256,8 @@ export function oauth_initiateLogin(resource_server_id) {
256256
if (!resource_server) return;
257257
set_auth_resource(resource_server_id)
258258

259+
store_pref("oauth-return-to", window.location.hash);
260+
259261
oauth.sp_initiated = resource_server.sp_initiated
260262
oauth.authority = resource_server.oauth_provider_url
261263

@@ -268,6 +270,7 @@ export function oauth_initiateLogin(resource_server_id) {
268270
_management_logger.error(err)
269271
})
270272
} else {
273+
store_pref("oauth-idp-pending", "true")
271274
location.href = resource_server.oauth_provider_url
272275
}
273276
}
Lines changed: 130 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,130 @@
1+
const { By, Key, until, Builder } = require('selenium-webdriver')
2+
const assert = require('assert')
3+
const { buildDriver, goToHome, goToExchanges, goToQueues, captureScreensFor, teardown } = require('../../utils')
4+
5+
const SSOHomePage = require('../../pageobjects/SSOHomePage')
6+
const FakePortalPage = require('../../pageobjects/FakePortalPage')
7+
const ExchangesPage = require('../../pageobjects/ExchangesPage')
8+
const QueuesAndStreamsPage = require('../../pageobjects/QueuesAndStreamsPage')
9+
const OverviewPage = require('../../pageobjects/OverviewPage')
10+
11+
// Exercises the oauth-return-to behaviour for the idp_initiated flow: after
12+
// logging in, the user should land back on the page they were on when they
13+
// clicked "log in", and a plain visit to "/" must never be hijacked to a saved
14+
// page. Each describe uses a fresh browser so it starts logged out.
15+
16+
describe('A user who logs in from the home page', function () {
17+
let driver
18+
let homePage
19+
let fakePortal
20+
let overview
21+
let captureScreen
22+
23+
before(async function () {
24+
driver = buildDriver()
25+
homePage = new SSOHomePage(driver)
26+
fakePortal = new FakePortalPage(driver)
27+
overview = new OverviewPage(driver)
28+
await goToHome(driver)
29+
captureScreen = captureScreensFor(driver, __filename)
30+
})
31+
32+
it('should land on the home page', async function () {
33+
await homePage.clickToLogin()
34+
if (!await fakePortal.isLoaded()) {
35+
throw new Error('Failed to load fakePortal')
36+
}
37+
await fakePortal.login()
38+
if (!await overview.isLoaded()) {
39+
throw new Error('Home page did not load after login')
40+
}
41+
const url = await driver.driver.getCurrentUrl()
42+
assert.ok(url.endsWith('#/'),
43+
'Expected to land on the home page (#/) but got ' + url)
44+
})
45+
46+
after(async function () {
47+
await teardown(driver, this, captureScreen)
48+
})
49+
})
50+
51+
describe('A user who accesses a protected page before logging in', function () {
52+
let driver
53+
let homePage
54+
let fakePortal
55+
let exchanges
56+
let overview
57+
let captureScreen
58+
59+
before(async function () {
60+
driver = buildDriver()
61+
homePage = new SSOHomePage(driver)
62+
fakePortal = new FakePortalPage(driver)
63+
exchanges = new ExchangesPage(driver)
64+
overview = new OverviewPage(driver)
65+
await goToExchanges(driver)
66+
captureScreen = captureScreensFor(driver, __filename)
67+
})
68+
69+
it('should be redirected back to that page after logging in', async function () {
70+
await homePage.clickToLogin()
71+
if (!await fakePortal.isLoaded()) {
72+
throw new Error('Failed to load fakePortal')
73+
}
74+
await fakePortal.login()
75+
if (!await exchanges.isLoaded()) {
76+
throw new Error('Was not redirected back to the exchanges page after login')
77+
}
78+
await exchanges.getPagingSectionHeaderText()
79+
})
80+
81+
it('should go to the home page when later visiting "/" directly', async function () {
82+
await goToHome(driver)
83+
if (!await overview.isLoaded()) {
84+
throw new Error('Home page did not load')
85+
}
86+
const url = await driver.driver.getCurrentUrl()
87+
assert.ok(url.endsWith('#/'),
88+
'Expected to land on the home page (#/) but got ' + url)
89+
})
90+
91+
after(async function () {
92+
await teardown(driver, this, captureScreen)
93+
})
94+
})
95+
96+
describe('A user who visits several pages before logging in', function () {
97+
let driver
98+
let homePage
99+
let fakePortal
100+
let queuesAndStreams
101+
let captureScreen
102+
103+
before(async function () {
104+
driver = buildDriver()
105+
homePage = new SSOHomePage(driver)
106+
fakePortal = new FakePortalPage(driver)
107+
queuesAndStreams = new QueuesAndStreamsPage(driver)
108+
// Access exchanges first, then move to queues while still logged out. Queues
109+
// is the page the user ends up on, so that is the page that must be restored.
110+
await goToExchanges(driver)
111+
await goToQueues(driver)
112+
captureScreen = captureScreensFor(driver, __filename)
113+
})
114+
115+
it('should be redirected to the last page it visited', async function () {
116+
await homePage.clickToLogin()
117+
if (!await fakePortal.isLoaded()) {
118+
throw new Error('Failed to load fakePortal')
119+
}
120+
await fakePortal.login()
121+
if (!await queuesAndStreams.isLoaded()) {
122+
throw new Error('Was not redirected back to the queues page after login')
123+
}
124+
await queuesAndStreams.getPagingSectionHeaderText()
125+
})
126+
127+
after(async function () {
128+
await teardown(driver, this, captureScreen)
129+
})
130+
})

0 commit comments

Comments
 (0)