From 9ff35195f0a05b9202e86e56f163cb4cf132e852 Mon Sep 17 00:00:00 2001
From: Bubka <858858+Bubka@users.noreply.github.com>
Date: Sat, 9 Dec 2023 17:22:24 +0100
Subject: [PATCH] Complete SSO (user model, error cases, tests, views) & Add
github provider
---
.env.example | 8 +-
app/Api/v1/Resources/UserResource.php | 12 +-
.../Controllers/Auth/PasswordController.php | 11 +-
.../Controllers/Auth/SocialiteController.php | 66 +++--
app/Http/Controllers/Auth/UserController.php | 6 +
app/Http/Controllers/SinglePageController.php | 8 +-
app/Listeners/RegisterOpenId.php | 29 ++
app/Models/User.php | 2 +-
app/Providers/EventServiceProvider.php | 2 +-
app/Providers/Socialite/OpenId.php | 6 +-
app/Providers/Socialite/RegisterOpenId.php | 13 -
config/2fauth.php | 2 +-
config/app.php | 4 +-
config/services.php | 21 ++
config/services/openid.php | 11 -
...12_06_131842_add_oauth_columns_to_user.php | 34 +++
resources/js/icons.js | 4 +-
resources/js/router/middlewares/authGuard.js | 1 +
.../js/router/middlewares/noEmptyError.js | 2 +-
resources/js/stores/user.js | 1 +
resources/js/views/Error.vue | 8 +-
resources/js/views/auth/Login.vue | 38 ++-
resources/js/views/settings/Account.vue | 11 +-
resources/js/views/settings/Options.vue | 2 +
resources/lang/en/auth.php | 5 +-
resources/lang/en/errors.php | 5 +
resources/lang/en/settings.php | 7 +-
.../Http/Auth/SocialiteControllerTest.php | 248 ++++++++++++++++++
28 files changed, 488 insertions(+), 79 deletions(-)
create mode 100644 app/Listeners/RegisterOpenId.php
delete mode 100644 app/Providers/Socialite/RegisterOpenId.php
delete mode 100644 config/services/openid.php
create mode 100644 database/migrations/2023_12_06_131842_add_oauth_columns_to_user.php
create mode 100644 tests/Feature/Http/Auth/SocialiteControllerTest.php
diff --git a/.env.example b/.env.example
index efdc0755..eff1e90a 100644
--- a/.env.example
+++ b/.env.example
@@ -221,7 +221,10 @@ WEBAUTHN_ID=null
WEBAUTHN_USER_VERIFICATION=preferred
-### OpenID settings ###
+
+#### SSO settings (for Socialite) ####
+
+# Uncomment lines for the OAuth providers you need.
# OPENID_AUTHORIZE_URL=
# OPENID_TOKEN_URL=
@@ -229,6 +232,9 @@ WEBAUTHN_USER_VERIFICATION=preferred
# OPENID_CLIENT_ID=
# OPENID_CLIENT_SECRET=
+# GITHUB_CLIENT_ID=
+# GITHUB_CLIENT_SECRET=
+
# Use this setting to declare trusted proxied.
# Supported:
diff --git a/app/Api/v1/Resources/UserResource.php b/app/Api/v1/Resources/UserResource.php
index 76887591..bf8a0c10 100644
--- a/app/Api/v1/Resources/UserResource.php
+++ b/app/Api/v1/Resources/UserResource.php
@@ -8,6 +8,7 @@ use Illuminate\Http\Resources\Json\JsonResource;
* @property mixed $id
* @property string $name
* @property string $email
+ * @property string $oauth_provider
* @property \Illuminate\Support\Collection $preferences
* @property string $is_admin
*/
@@ -22,11 +23,12 @@ class UserResource extends JsonResource
public function toArray($request)
{
return [
- 'id' => $this->id,
- 'name' => $this->name,
- 'email' => $this->email,
- 'preferences' => $this->preferences,
- 'is_admin' => $this->is_admin,
+ 'id' => $this->id,
+ 'name' => $this->name,
+ 'email' => $this->email,
+ 'oauth_provider' => $this->oauth_provider,
+ 'preferences' => $this->preferences,
+ 'is_admin' => $this->is_admin,
];
}
}
diff --git a/app/Http/Controllers/Auth/PasswordController.php b/app/Http/Controllers/Auth/PasswordController.php
index 4d64d7af..495b4487 100644
--- a/app/Http/Controllers/Auth/PasswordController.php
+++ b/app/Http/Controllers/Auth/PasswordController.php
@@ -17,8 +17,15 @@ class PasswordController extends Controller
*/
public function update(UserPatchPwdRequest $request)
{
+ $user = $request->user();
$validated = $request->validated();
+ if (config('auth.defaults.guard') === 'reverse-proxy-guard' || $user->oauth_provider) {
+ Log::notice('Password update rejected: reverse-proxy-guard enabled or account from external sso provider');
+
+ return response()->json(['message' => __('errors.account_managed_by_external_provider')], 400);
+ }
+
if (! Hash::check($validated['currentPassword'], Auth::user()->password)) {
Log::notice('Password update failed: wrong password provided');
@@ -26,10 +33,10 @@ class PasswordController extends Controller
}
if (! config('2fauth.config.isDemoApp')) {
- $request->user()->update([
+ $user->update([
'password' => bcrypt($validated['password']),
]);
- Log::info(sprintf('Password of user ID #%s updated', $request->user()->id));
+ Log::info(sprintf('Password of user ID #%s updated', $user->id));
}
return response()->json(['message' => __('auth.forms.password_successfully_changed')]);
diff --git a/app/Http/Controllers/Auth/SocialiteController.php b/app/Http/Controllers/Auth/SocialiteController.php
index 1600f978..a6705c2a 100644
--- a/app/Http/Controllers/Auth/SocialiteController.php
+++ b/app/Http/Controllers/Auth/SocialiteController.php
@@ -13,32 +13,58 @@ use Laravel\Socialite\Facades\Socialite;
class SocialiteController extends Controller
{
- public function redirect(Request $request, $driver)
+ /**
+ * Redirect to the provider's authentication url
+ *
+ * @return \Symfony\Component\HttpFoundation\RedirectResponse|\Illuminate\Http\RedirectResponse
+ */
+ public function redirect(Request $request, string $driver)
{
- return Socialite::driver($driver)->redirect();
- }
-
- public function callback(Request $request, $driver)
- {
- $socialiteUser = Socialite::driver($driver)->user();
-
- /** @var User $user */
- $user = User::firstOrNew([
- 'email' => $socialiteUser->getEmail(),
- ], [
- 'name' => $socialiteUser->getName(),
- 'password' => bcrypt(Str::random()),
- ]);
-
- if (!$user->exists && Settings::get('disableRegistrationSso')) {
- return response(401);
+ if (! config('services.' . $driver . '.client_id') || ! config('services.' . $driver . '.client_secret')) {
+ return redirect('/error?err=sso_bad_provider_setup');
}
+ return Settings::get('enableSso')
+ ? Socialite::driver($driver)->redirect()
+ : redirect('/error?err=sso_disabled');
+ }
+
+ /**
+ * Register (if needed) the user and authenticate him
+ *
+ * @return \Illuminate\Routing\Redirector|\Illuminate\Http\RedirectResponse
+ */
+ public function callback(Request $request, string $driver)
+ {
+ try {
+ $socialiteUser = Socialite::driver($driver)->user();
+ } catch (\Exception $e) {
+ return redirect('/error?err=sso_failed');
+ }
+
+ /** @var User|null $user */
+ $user = User::firstOrNew([
+ 'oauth_id' => $socialiteUser->getId(),
+ 'oauth_provider' => $driver,
+ ]);
+
+ if (! $user->exists) {
+ if (User::count() === 0) {
+ $user->is_admin = true;
+ }
+ else if (Settings::get('disableRegistration')) {
+ return redirect('/error?err=no_register');
+ }
+ $user->password = bcrypt(Str::random());
+ }
+
+ $user->email = $socialiteUser->getEmail() ?? $socialiteUser->getId() . '@' . $driver;
+ $user->name = $socialiteUser->getNickname() ?? $socialiteUser->getName() ?? $driver . ' #' . $socialiteUser->getId();
$user->last_seen_at = Carbon::now()->format('Y-m-d H:i:s');
$user->save();
- Auth::guard()->login($user, true);
+ Auth::guard()->login($user);
- return redirect('/accounts?authenticated');
+ return redirect('/accounts');
}
}
diff --git a/app/Http/Controllers/Auth/UserController.php b/app/Http/Controllers/Auth/UserController.php
index 7a18edb7..cf3f82a9 100644
--- a/app/Http/Controllers/Auth/UserController.php
+++ b/app/Http/Controllers/Auth/UserController.php
@@ -24,6 +24,12 @@ class UserController extends Controller
$user = $request->user();
$validated = $request->validated();
+ if (config('auth.defaults.guard') === 'reverse-proxy-guard' || $user->oauth_provider) {
+ Log::notice('Account update rejected: reverse-proxy-guard enabled or account from external sso provider');
+
+ return response()->json(['message' => __('errors.account_managed_by_external_provider')], 400);
+ }
+
if (! Hash::check($request->password, Auth::user()->password)) {
Log::notice('Account update failed: wrong password provided');
diff --git a/app/Http/Controllers/SinglePageController.php b/app/Http/Controllers/SinglePageController.php
index 5cd510a7..09fe79d1 100644
--- a/app/Http/Controllers/SinglePageController.php
+++ b/app/Http/Controllers/SinglePageController.php
@@ -27,7 +27,8 @@ class SinglePageController extends Controller
$isTestingApp = config('2fauth.config.isTestingApp') ? 'true' : 'false';
$lang = App::getLocale();
$locales = collect(config('2fauth.locales'))->toJson(); /** @phpstan-ignore-line */
- $openidAuth = config('services.openid.client_secret') ? true : false;
+ $openidAuth = config('services.openid.client_secret') ? true : false;
+ $githubAuth = config('services.github.client_secret') ? true : false;
// if (Auth::user()->preferences)
@@ -36,7 +37,10 @@ class SinglePageController extends Controller
'appConfig' => collect([
'proxyAuth' => $proxyAuth,
'proxyLogoutUrl' => $proxyLogoutUrl,
- 'openidAuth' => $openidAuth,
+ 'sso' => [
+ 'openid' => $openidAuth,
+ 'github' => $githubAuth,
+ ],
'subdirectory' => $subdir,
])->toJson(),
'defaultPreferences' => $defaultPreferences,
diff --git a/app/Listeners/RegisterOpenId.php b/app/Listeners/RegisterOpenId.php
new file mode 100644
index 00000000..88aca265
--- /dev/null
+++ b/app/Listeners/RegisterOpenId.php
@@ -0,0 +1,29 @@
+extendSocialite('openid', OpenId::class);
+ }
+}
diff --git a/app/Models/User.php b/app/Models/User.php
index 4c2db3f0..d5e0c359 100644
--- a/app/Models/User.php
+++ b/app/Models/User.php
@@ -49,7 +49,7 @@ class User extends Authenticatable implements WebAuthnAuthenticatable
* @var string[]
*/
protected $fillable = [
- 'name', 'email', 'password',
+ 'name', 'email', 'password', 'oauth_id', 'oauth_provider'
];
/**
diff --git a/app/Providers/EventServiceProvider.php b/app/Providers/EventServiceProvider.php
index 53cc3d48..d599233d 100644
--- a/app/Providers/EventServiceProvider.php
+++ b/app/Providers/EventServiceProvider.php
@@ -10,7 +10,7 @@ use App\Listeners\CleanIconStorage;
use App\Listeners\DissociateTwofaccountFromGroup;
use App\Listeners\ReleaseRadar;
use App\Listeners\ResetUsersPreference;
-use App\Providers\Socialite\RegisterOpenId;
+use App\Listeners\RegisterOpenId;
use Illuminate\Auth\Events\Registered;
use Illuminate\Auth\Listeners\SendEmailVerificationNotification;
use Illuminate\Foundation\Support\Providers\EventServiceProvider as ServiceProvider;
diff --git a/app/Providers/Socialite/OpenId.php b/app/Providers/Socialite/OpenId.php
index 23ba5c4e..29f32050 100644
--- a/app/Providers/Socialite/OpenId.php
+++ b/app/Providers/Socialite/OpenId.php
@@ -56,9 +56,11 @@ class OpenId extends AbstractProvider
}
/**
- * {@inheritdoc}
+ * Get a refresh token
+ *
+ * @return \Psr\Http\Message\ResponseInterface
*/
- protected function refreshToken($refreshToken)
+ protected function refreshToken(string $refreshToken)
{
return $this->getHttpClient()->post($this->getTokenUrl(), [
RequestOptions::FORM_PARAMS => [
diff --git a/app/Providers/Socialite/RegisterOpenId.php b/app/Providers/Socialite/RegisterOpenId.php
deleted file mode 100644
index 3dc9d545..00000000
--- a/app/Providers/Socialite/RegisterOpenId.php
+++ /dev/null
@@ -1,13 +0,0 @@
-extendSocialite('openid', OpenId::class);
- }
-}
diff --git a/config/2fauth.php b/config/2fauth.php
index ec4f814c..b75ae834 100644
--- a/config/2fauth.php
+++ b/config/2fauth.php
@@ -71,7 +71,7 @@ return [
'lastRadarScan' => 0,
'latestRelease' => false,
'disableRegistration' => false,
- 'disableRegistrationSso' => false,
+ 'enableSso' => true,
],
/*
diff --git a/config/app.php b/config/app.php
index 4fc95f5f..88fb6364 100644
--- a/config/app.php
+++ b/config/app.php
@@ -159,7 +159,7 @@ return [
/*
* Package Service Providers...
*/
-
+ \SocialiteProviders\Manager\ServiceProvider::class,
/*
* Application Service Providers...
*/
@@ -170,7 +170,7 @@ return [
App\Providers\RouteServiceProvider::class,
App\Providers\TwoFAuthServiceProvider::class,
App\Providers\MigrationServiceProvider::class,
- ])->toArray(),
+ ])->toArray(),
/*
|--------------------------------------------------------------------------
diff --git a/config/services.php b/config/services.php
index 0ace530e..4a113592 100644
--- a/config/services.php
+++ b/config/services.php
@@ -21,6 +21,27 @@ return [
'scheme' => 'https',
],
+ 'openid' => [
+ 'token_url' => env('OPENID_TOKEN_URL'),
+ 'authorize_url' => env('OPENID_AUTHORIZE_URL'),
+ 'userinfo_url' => env('OPENID_USERINFO_URL'),
+ 'client_id' => env('OPENID_CLIENT_ID'),
+ 'client_secret' => env('OPENID_CLIENT_SECRET'),
+ 'redirect' => '/socialite/callback/openid',
+ ],
+
+ 'github' => [
+ 'client_id' => env('GITHUB_CLIENT_ID'),
+ 'client_secret' => env('GITHUB_CLIENT_SECRET'),
+ 'redirect' => '/socialite/callback/github',
+ ],
+
+ // 'google' => [
+ // 'client_id' => env('GOOGLE_CLIENT_ID'),
+ // 'client_secret' => env('GOOGLE_CLIENT_SECRET'),
+ // 'redirect' => '/socialite/callback/google ',
+ // ],
+
'postmark' => [
'token' => env('POSTMARK_TOKEN'),
],
diff --git a/config/services/openid.php b/config/services/openid.php
deleted file mode 100644
index 7f753fdd..00000000
--- a/config/services/openid.php
+++ /dev/null
@@ -1,11 +0,0 @@
- env('OPENID_TOKEN_URL'),
- 'authorize_url' => env('OPENID_AUTHORIZE_URL'),
- 'userinfo_url' => env('OPENID_USERINFO_URL'),
-
- 'client_id' => env('OPENID_CLIENT_ID'),
- 'client_secret' => env('OPENID_CLIENT_SECRET'),
- 'redirect' => '/socialite/callback/openid',
-];
diff --git a/database/migrations/2023_12_06_131842_add_oauth_columns_to_user.php b/database/migrations/2023_12_06_131842_add_oauth_columns_to_user.php
new file mode 100644
index 00000000..adda820f
--- /dev/null
+++ b/database/migrations/2023_12_06_131842_add_oauth_columns_to_user.php
@@ -0,0 +1,34 @@
+string('oauth_provider', 100)
+ ->after('id')
+ ->nullable();
+ $table->string('oauth_id', 200)
+ ->after('id')
+ ->nullable();
+ });
+ }
+
+ /**
+ * Reverse the migrations.
+ */
+ public function down(): void
+ {
+ Schema::table('users', function (Blueprint $table) {
+ $table->dropColumn('oauth_id');
+ $table->dropColumn('oauth_provider');
+ });
+ }
+};
diff --git a/resources/js/icons.js b/resources/js/icons.js
index 059ec729..8f0aaebb 100644
--- a/resources/js/icons.js
+++ b/resources/js/icons.js
@@ -53,7 +53,8 @@ import {
} from '@fortawesome/free-regular-svg-icons'
import {
- faGithubAlt
+ faGithubAlt,
+ faOpenid
} from '@fortawesome/free-brands-svg-icons'
library.add(
@@ -103,6 +104,7 @@ library.add(
faVideoSlash,
faStar,
faChevronRight,
+ faOpenid,
);
export default FontAwesomeIcon
\ No newline at end of file
diff --git a/resources/js/router/middlewares/authGuard.js b/resources/js/router/middlewares/authGuard.js
index 88081380..3e2a2025 100644
--- a/resources/js/router/middlewares/authGuard.js
+++ b/resources/js/router/middlewares/authGuard.js
@@ -11,6 +11,7 @@ export default async function authGuard({ to, next, nextMiddleware, stores }) {
await user.loginAs({
name: currentUser.name,
email: currentUser.email,
+ oauth_provider: currentUser.oauth_provider,
preferences: currentUser.preferences,
isAdmin: currentUser.is_admin,
})
diff --git a/resources/js/router/middlewares/noEmptyError.js b/resources/js/router/middlewares/noEmptyError.js
index b9d37c1a..7458ea25 100644
--- a/resources/js/router/middlewares/noEmptyError.js
+++ b/resources/js/router/middlewares/noEmptyError.js
@@ -1,7 +1,7 @@
export default function noEmptyError({ to, next, nextMiddleware, stores }) {
const { notify } = stores
- if (notify.err == null) {
+ if (notify.err == null && ! to.query.err) {
// return to home if no err object is set to prevent an empty error message
next({ name: 'accounts' });
}
diff --git a/resources/js/stores/user.js b/resources/js/stores/user.js
index 10348bc0..444ae24a 100644
--- a/resources/js/stores/user.js
+++ b/resources/js/stores/user.js
@@ -13,6 +13,7 @@ export const useUserStore = defineStore({
return {
name: undefined,
email: undefined,
+ oauth_provider: undefined,
preferences: window.defaultPreferences,
isAdmin: false,
}
diff --git a/resources/js/views/Error.vue b/resources/js/views/Error.vue
index 70c7ccb2..25f8b35e 100644
--- a/resources/js/views/Error.vue
+++ b/resources/js/views/Error.vue
@@ -21,11 +21,17 @@
}
})
+ onMounted(() => {
+ if (route.query.err) {
+ errorHandler.message = trans('errors.' + route.query.err)
+ }
+ })
+
/**
* Exits the error view
*/
function exit() {
- window.history.length > 1 && route.name !== '404' && route.name !== 'notFound'
+ window.history.length > 1 && route.name !== '404' && route.name !== 'notFound' && !route.query.err
? router.go(-1)
: router.push({ name: 'accounts' })
}
diff --git a/resources/js/views/auth/Login.vue b/resources/js/views/auth/Login.vue
index 58d154de..558e7397 100644
--- a/resources/js/views/auth/Login.vue
+++ b/resources/js/views/auth/Login.vue
@@ -35,6 +35,7 @@
await user.loginAs({
name: response.data.name,
email: response.data.email,
+ oauth_provider: response.data.oauth_provider,
preferences: response.data.preferences,
isAdmin: response.data.is_admin,
})
@@ -63,6 +64,7 @@
await user.loginAs({
name: response.data.name,
email: response.data.email,
+ oauth_provider: response.data.oauth_provider,
preferences: response.data.preferences,
isAdmin: response.data.is_admin,
})
@@ -115,17 +117,25 @@
{{ $t('auth.login_and_password') }}
- {{ $t('auth.sign_in_using') }}
-
- OpenID
-
-
{{ $t('auth.forms.dont_have_account_yet') }}
{{ $t('auth.register') }}
+
+
+ {{ $t('auth.or_continue_with') }}
+
+
+
@@ -148,17 +158,25 @@
{{ $t('auth.webauthn.security_device') }}
- {{ $t('auth.sign_in_using') }}
-
- OpenID
-
-
{{ $t('auth.forms.dont_have_account_yet') }}
{{ $t('auth.register') }}
+
+
+ {{ $t('auth.or_continue_with') }}
+
+
+
diff --git a/resources/js/views/settings/Account.vue b/resources/js/views/settings/Account.vue
index cd707e4c..93061771 100644
--- a/resources/js/views/settings/Account.vue
+++ b/resources/js/views/settings/Account.vue
@@ -107,10 +107,13 @@
{{ $t('settings.you_are_administrator') }}
+
+ {{ $t('settings.account_linked_to_sso_x_provider', { provider: user.oauth_provider }) }}
+