Skip to content

Commit d259151

Browse files
OIDC: option to send the login page straight to the provider (#19)
Closes #17. Consumes the LOGIN_PAGE_REDIRECT seam from fogproject#1175. For an install where every account lives at the identity provider, FOG's username and password box is a dead end -- it cannot accept those credentials, so the only thing to do on that page is click past it. `opAutoRedirect`, per provider, sends an anonymous visitor straight to the provider instead. It is also the most dangerous setting in this plugin, and most of what is below is containment rather than feature. ## Off by default, in every place a default can be stated An appended ALTER (existing installs), createSql() (fresh ones), and the create page. An install that upgraded into this switched on would find its login form replaced by a redirect nobody asked for, and the only URL that still shows the form is one nobody has been told about. ## Two flagged providers refuse and render the form The login page cannot redirect to both. Silently picking one -- lowest id, first row, whatever -- hides a misconfiguration on the single page an admin is least able to debug, while sending everybody to a provider half of them may not have an account at. Refusing is a working login for everyone and visibly not what was asked for. The complaint names both providers in the error log and NOT on the page. This code runs for a visitor who has not signed in, and "this server has two misconfigured identity providers" is not something to tell one. ## Two loops that had to be closed first Both used to land on management/index.php, which is precisely the page this feature redirects: - A refused sign-in. `_fail()` now returns to management/login.php, so a provider that is down produces one error message rather than an infinite redirect -- and the flash message is attached to a page that stays put instead of one that immediately hops again. - Logging out with automatic redirect on but single logout off. Core's default landing spot would bounce the person who just signed out back to a provider whose SSO session is still alive and sign them silently back in. "Log out" that leaves you logged in is worse than no logout at all, so the USER_LOGGING_OUT listener falls back to management/login.php -- only as a fallback, so a real provider-logout URL still wins. ## The escape hatch Core's, and pinned there: index.php offers LOGIN_PAGE_REDIRECT only when FOG_LOCAL_LOGIN is undefined, so management/login.php never reaches this plugin's listener at all. Nothing here can change that, which is the whole reason it lives in core -- a plugin that is misconfigured, half-installed or throwing cannot take the page down with it, because it is not asked anything. The management page prints that URL next to the checkbox. Not decoration: an admin who ticks this box without knowing login.php exists has one expired certificate between themselves and being locked out of their own server, and at that point the URL is not something they could guess. ## Verification tests/oidc-auto-redirect.test.php, 20 checks, 21/21 mutations caught -- including the default flipping to on, a step inserted rather than appended, several flagged providers being silently resolved to one, the ambiguity reaching the page instead of the log, a relative start URL (which core's seam drops, so the setting would silently do nothing), both loop regressions, and the fallback overriding a real provider logout. One of those mutations survived the first draft and is worth recording: the escape-hatch assertion searched the whole page for the URL, and the page prints it twice -- so removing it from the autoRedirect label, which is the only place an admin reads it, still passed. Now scoped to that label. sh tests/run-all.sh -> 6 passed, 0 failed. Not exercised end to end against the lab Keycloak yet. Claude-Session: https://claude.ai/code/session_017aBSWrDArXHTpKWkkN27LR Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
1 parent 385cc5e commit d259151

8 files changed

Lines changed: 644 additions & 2 deletions

oidc/class/oidc.class.php

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -94,6 +94,10 @@ class OIDC extends FOGController
9494
// application behind that provider. See the note on the column in
9595
// OIDCManager::createSql().
9696
'singleLogout' => 'opSingleLogout',
97+
// Send the login page straight to this provider instead of showing
98+
// FOG's own form (#17). Off by default; see the note on the column
99+
// in OIDCManager::createSql() for why this one in particular.
100+
'autoRedirect' => 'opAutoRedirect',
97101
'icon' => 'opIcon'
98102
];
99103
/**
@@ -301,6 +305,24 @@ public static function postLogoutUri()
301305
{
302306
return self::absoluteUrl('management/login.php');
303307
}
308+
/**
309+
* The URL that begins a sign-in with one provider.
310+
*
311+
* Absolute, because core's LOGIN_PAGE_REDIRECT seam refuses anything
312+
* that is not an absolute http(s) URL -- what a hook left in a variable
313+
* is not a good enough answer for a Location header. The login-page
314+
* button uses a relative form of the same path and does not need this.
315+
*
316+
* @param int $id the provider id
317+
*
318+
* @return string
319+
*/
320+
public static function startUrl($id)
321+
{
322+
return self::absoluteUrl(
323+
sprintf('ext/oidc/start?provider=%d', (int)$id)
324+
);
325+
}
304326
/**
305327
* An absolute https URL for a path inside this FOG install.
306328
*

oidc/class/oidcflow.class.php

Lines changed: 83 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -304,6 +304,79 @@ private static function _rememberLogout($provider, array $config, array $token)
304304
'idToken' => (string)$token['id_token']
305305
];
306306
}
307+
/**
308+
* The one provider the login page must redirect to, or 0 for none.
309+
*
310+
* Shared by the login-page listener and the logout listener, because
311+
* both have to know the same thing: whether landing on
312+
* management/index.php would bounce the visitor straight to a provider.
313+
*
314+
* TWO providers flagged is refused rather than resolved. The login page
315+
* cannot redirect to both, and silently picking one -- lowest id, first
316+
* row, whatever -- hides a misconfiguration on the single page an admin
317+
* is least able to debug, while sending everybody to a provider half of
318+
* them may not have an account at. Refusing renders FOG's own form,
319+
* which is a working login for everyone and visibly not what was asked
320+
* for.
321+
*
322+
* The complaint goes to the error log and NOT to the page. This runs
323+
* for an anonymous visitor, and "this server has two misconfigured
324+
* identity providers" is not something to tell one.
325+
*
326+
* @return int the provider id, or 0
327+
*/
328+
public static function forcedProvider()
329+
{
330+
$ids = (array)Route::getIds(
331+
'oidc',
332+
['enabled' => [1], 'autoRedirect' => [1]]
333+
);
334+
if (count($ids) < 1) {
335+
return 0;
336+
}
337+
if (count($ids) > 1) {
338+
error_log(
339+
sprintf(
340+
'FOG OIDC: providers %s all have automatic redirect'
341+
. ' enabled; the login page cannot redirect to more than'
342+
. ' one, so it is showing the local form instead',
343+
implode(', ', array_map('intval', $ids))
344+
)
345+
);
346+
return 0;
347+
}
348+
return (int)reset($ids);
349+
}
350+
/**
351+
* Where the login page should send an anonymous visitor, or ''.
352+
*
353+
* Consumed by core's LOGIN_PAGE_REDIRECT seam (fogproject#1175), which
354+
* fires only for a visitor who is NOT signed in and only on the form
355+
* render -- so this can neither bounce a working session nor interrupt
356+
* the callback coming back from the provider.
357+
*
358+
* The row is re-read and re-checked rather than trusted from the id,
359+
* for the same reason _enabledProvider() re-checks at the start of every
360+
* flow: a provider disabled a moment ago must not still be receiving
361+
* people.
362+
*
363+
* @return string
364+
*/
365+
public static function loginRedirectUrl()
366+
{
367+
$id = self::forcedProvider();
368+
if ($id < 1) {
369+
return '';
370+
}
371+
$provider = self::getClass('OIDC', $id);
372+
if (!$provider->isValid()
373+
|| '1' !== (string)$provider->get('enabled')
374+
|| '1' !== (string)$provider->get('autoRedirect')
375+
) {
376+
return '';
377+
}
378+
return OIDC::startUrl($id);
379+
}
307380
/**
308381
* The provider logout URL for this session, or '' for none.
309382
*
@@ -1224,6 +1297,15 @@ private static function _fail($message)
12241297
{
12251298
self::_session();
12261299
self::setMessage($message, _('Sign-in failed'), 'error');
1227-
self::_redirect(OIDC::webrootBase() . 'management/index.php');
1300+
/*
1301+
* login.php, not index.php. On an install with automatic redirect
1302+
* on (#17), index.php sends the visitor straight back to the
1303+
* provider that just refused them -- which is an infinite redirect
1304+
* for a provider that is down, and an unreadable flash message even
1305+
* when it is not, because nothing renders between the two hops.
1306+
* login.php always renders FOG's own form (fogproject#1175), so the
1307+
* explanation is attached to a page that stays put.
1308+
*/
1309+
self::_redirect(OIDC::webrootBase() . 'management/login.php');
12281310
}
12291311
}

oidc/class/oidcmanager.class.php

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -53,6 +53,7 @@ public function createSql()
5353
'opJITProvision',
5454
'opAllowAPI',
5555
'opSingleLogout',
56+
'opAutoRedirect',
5657
'opIcon'
5758
],
5859
[
@@ -71,6 +72,7 @@ public function createSql()
7172
"ENUM('0', '1')",
7273
"ENUM('0', '1')",
7374
"ENUM('0', '1')",
75+
"ENUM('0', '1')",
7476
'VARCHAR(255)'
7577
],
7678
[
@@ -89,6 +91,7 @@ public function createSql()
8991
false,
9092
false,
9193
false,
94+
false,
9295
false
9396
],
9497
[
@@ -133,6 +136,14 @@ public function createSql()
133136
// session because somebody left FOG is a surprise that
134137
// reaches applications FOG has nothing to do with.
135138
"'0'",
139+
// Sending everyone straight to this provider ships off, and
140+
// it is the most dangerous switch in this plugin: an
141+
// unconditional redirect to a provider that is unreachable,
142+
// whose certificate expired, or whose issuer was mistyped
143+
// takes the login form away from every administrator at
144+
// once. management/login.php (fogproject#1175) is the way
145+
// back, and the management page names it next to the box.
146+
"'0'",
136147
"'fa fa-id-badge'"
137148
],
138149
[
@@ -157,6 +168,7 @@ public function createSql()
157168
false,
158169
false,
159170
false,
171+
false,
160172
false
161173
],
162174
'InnoDB',
@@ -216,6 +228,14 @@ function () {
216228
// the column -- runs this harmlessly too.
217229
"ALTER TABLE `OIDCProviders` ADD COLUMN `opSingleLogout` "
218230
. "ENUM('0', '1') NOT NULL DEFAULT '0'",
231+
// 7 - send the login page straight to this provider (#17).
232+
// Appended for the same reason as step 6, and defaulting off for
233+
// a sharper one: an install that upgraded into this switched ON
234+
// would find its login form replaced by a redirect nobody asked
235+
// for, and the only URL that still shows the form is one nobody
236+
// has been told about yet.
237+
"ALTER TABLE `OIDCProviders` ADD COLUMN `opAutoRedirect` "
238+
. "ENUM('0', '1') NOT NULL DEFAULT '0'",
219239
];
220240
}
221241
/**
Lines changed: 95 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,95 @@
1+
<?php
2+
/**
3+
* Sends the login page straight to the identity provider.
4+
*
5+
* PHP version 7.4+
6+
*
7+
* @category OIDCLoginRedirect
8+
* @package FOGProject
9+
* @author Tom Elliott <tommygunsster@gmail.com>
10+
* @license http://opensource.org/licenses/gpl-3.0 GPLv3
11+
* @link https://fogproject.org
12+
*/
13+
/**
14+
* Sends the login page straight to the identity provider.
15+
*
16+
* @category OIDCLoginRedirect
17+
* @package FOGProject
18+
* @author Tom Elliott <tommygunsster@gmail.com>
19+
* @license http://opensource.org/licenses/gpl-3.0 GPLv3
20+
* @link https://fogproject.org
21+
*/
22+
class OIDCLoginRedirect extends Hook
23+
{
24+
/**
25+
* The name of this hook.
26+
*
27+
* @var string
28+
*/
29+
public $name = 'OIDCLoginRedirect';
30+
/**
31+
* The description.
32+
*
33+
* @var string
34+
*/
35+
public $description = 'Send the login page to the identity provider.';
36+
/**
37+
* For posterity.
38+
*
39+
* @var bool
40+
*/
41+
public $active = true;
42+
/**
43+
* The node to work with.
44+
*
45+
* @var string
46+
*/
47+
public $node = 'oidc';
48+
/**
49+
* Initialize object.
50+
*
51+
* @return void
52+
*/
53+
public function __construct()
54+
{
55+
parent::__construct();
56+
$this->registerInstalled([
57+
['LOGIN_PAGE_REDIRECT', 'loginRedirect']
58+
]);
59+
}
60+
/**
61+
* Where an anonymous visitor goes instead of FOG's login form.
62+
*
63+
* For an install where everyone signs in through one provider, landing
64+
* on a username and password box is a dead end: the accounts are at the
65+
* provider and the box cannot accept them. This is the setting that
66+
* removes the extra click.
67+
*
68+
* It is also the most dangerous setting in this plugin, and the design
69+
* of the seam is what contains it. Core only offers LOGIN_PAGE_REDIRECT
70+
* when FOG_LOCAL_LOGIN is undefined, so on management/login.php this
71+
* method is never reached -- not consulted and overruled, never asked.
72+
* That is what makes the escape hatch survive a provider whose
73+
* certificate expired, whose issuer was mistyped, or which is simply
74+
* switched off; and it also means a bug in this method cannot take that
75+
* page down, because the page does not run it.
76+
*
77+
* https://<fog>/fog/management/login.php
78+
*
79+
* A provider that refuses a sign-in sends the browser to that same page
80+
* rather than back to index.php (OIDCFlow::_fail()), so a provider that
81+
* is down produces one error message rather than a redirect loop.
82+
*
83+
* @param mixed $arguments where to send the browser instead
84+
*
85+
* @return void
86+
*/
87+
public function loginRedirect($arguments)
88+
{
89+
$url = OIDCFlow::loginRedirectUrl();
90+
if ('' === $url) {
91+
return;
92+
}
93+
$arguments['redirect'] = $url;
94+
}
95+
}

oidc/hooks/oidclogout.hook.php

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -84,6 +84,22 @@ public function __construct()
8484
public function providerLogout($arguments)
8585
{
8686
$url = OIDCFlow::logoutUrl();
87+
if ('' === $url && OIDCFlow::forcedProvider() > 0) {
88+
/*
89+
* No provider logout to do, but this install sends its login
90+
* page straight to a provider (#17) -- so core's default
91+
* landing spot, management/index.php, would bounce the person
92+
* who just signed out back to a provider whose SSO session is
93+
* still alive, and sign them silently back in. "Log out" that
94+
* leaves you logged in is worse than no logout at all.
95+
*
96+
* management/login.php is the one page that cannot do that.
97+
* It does not end the provider session -- only single logout
98+
* does -- but it leaves somebody looking at a form instead of
99+
* back where they started.
100+
*/
101+
$url = OIDC::postLogoutUri();
102+
}
87103
if ('' === $url) {
88104
return;
89105
}

oidc/js/fog.oidc.export.js

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@
2222
{data: 'jitProvision', visible: false},
2323
{data: 'allowapi', visible: false},
2424
{data: 'singleLogout', visible: false},
25+
{data: 'autoRedirect', visible: false},
2526
{data: 'icon', visible: false}
2627
]);
2728
})(jQuery);

oidc/pages/oidcmanagement.page.php

Lines changed: 34 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -275,7 +275,8 @@ function (&$serverFault) {
275275
->set('enabled', '0')
276276
->set('jitProvision', '0')
277277
->set('allowapi', '0')
278-
->set('singleLogout', '0');
278+
->set('singleLogout', '0')
279+
->set('autoRedirect', '0');
279280
if (!$OIDC->save()) {
280281
$serverFault = true;
281282
throw new \Exception(_('Add provider failed!'));
@@ -498,6 +499,34 @@ public function oidcGeneral()
498499
// the provider's error page instead of back at FOG. That looks
499500
// like this plugin is broken, and the fix is a value an admin
500501
// has to copy from somewhere.
502+
self::makeLabel(
503+
$this->_labelClass,
504+
'autoRedirect',
505+
_('Redirect Login To This Provider')
506+
. '<br/>('
507+
. sprintf(
508+
// The escape hatch is named right here, on purpose. An
509+
// admin who ticks this without knowing about login.php
510+
// has one bad certificate between themselves and being
511+
// locked out of their own server -- and the URL is not
512+
// something they could guess at that point.
513+
_('the local login form stays available at %s'),
514+
'<code>' . Initiator::e(OIDC::postLogoutUri()) . '</code>'
515+
)
516+
. ')'
517+
) => self::makeInput(
518+
'',
519+
'autoRedirect',
520+
'',
521+
'checkbox',
522+
'autoRedirect',
523+
'',
524+
false,
525+
false,
526+
-1,
527+
-1,
528+
$checked('autoRedirect')
529+
),
501530
self::makeLabel(
502531
$this->_labelClass,
503532
'postLogoutUri',
@@ -602,6 +631,10 @@ public function oidcGeneralPost()
602631
->set(
603632
'singleLogout',
604633
isset($_POST['singleLogout']) ? '1' : '0'
634+
)
635+
->set(
636+
'autoRedirect',
637+
isset($_POST['autoRedirect']) ? '1' : '0'
605638
);
606639

607640
// The secret is only written when the admin actually typed one. An

0 commit comments

Comments
 (0)