From 8e723f10dc3db49df9dc66ea5a90e3153eda54e8 Mon Sep 17 00:00:00 2001
From: Daniel Seiler <daniel.seiler@fachschaft.informatik.tu-darmstadt.de>
Date: Wed, 7 Aug 2019 15:31:10 +0200
Subject: [PATCH] Add error messages, fix LDAP error
---
app/Auth/Access/ExternalAuthService.php | 9 +++---
app/Auth/Access/LdapService.php | 1 -
app/Auth/Access/Saml2Service.php | 41 +++++++++++++++++--------
app/Config/services.php | 2 ++
app/Exceptions/SamlException.php | 2 +-
resources/lang/de/errors.php | 2 ++
resources/lang/de_informal/errors.php | 1 +
resources/lang/en/errors.php | 2 ++
resources/views/auth/login.blade.php | 2 +-
9 files changed, 42 insertions(+), 20 deletions(-)
diff --git a/app/Auth/Access/ExternalAuthService.php b/app/Auth/Access/ExternalAuthService.php
index b1c036018..77c7d1351 100644
--- a/app/Auth/Access/ExternalAuthService.php
+++ b/app/Auth/Access/ExternalAuthService.php
@@ -2,6 +2,7 @@
use BookStack\Auth\Role;
use BookStack\Auth\User;
+use Illuminate\Database\Eloquent\Builder;
class ExternalAuthService
{
@@ -57,19 +58,19 @@ class ExternalAuthService
/**
* Sync the groups to the user roles for the current user
* @param \BookStack\Auth\User $user
- * @param array $samlAttributes
+ * @param array $userGroups
*/
public function syncWithGroups(User $user, array $userGroups)
{
// Get the ids for the roles from the names
- $samlGroupsAsRoles = $this->matchGroupsToSystemsRoles($userSamlGroups);
+ $groupsAsRoles = $this->matchGroupsToSystemsRoles($userGroups);
// Sync groups
if ($this->config['remove_from_groups']) {
- $user->roles()->sync($samlGroupsAsRoles);
+ $user->roles()->sync($groupsAsRoles);
$this->userRepo->attachDefaultRole($user);
} else {
- $user->roles()->syncWithoutDetaching($samlGroupsAsRoles);
+ $user->roles()->syncWithoutDetaching($groupsAsRoles);
}
}
}
diff --git a/app/Auth/Access/LdapService.php b/app/Auth/Access/LdapService.php
index 3111ea9fa..b0700322f 100644
--- a/app/Auth/Access/LdapService.php
+++ b/app/Auth/Access/LdapService.php
@@ -5,7 +5,6 @@ use BookStack\Auth\User;
use BookStack\Auth\UserRepo;
use BookStack\Exceptions\LdapException;
use Illuminate\Contracts\Auth\Authenticatable;
-use Illuminate\Database\Eloquent\Builder;
/**
* Class LdapService
diff --git a/app/Auth/Access/Saml2Service.php b/app/Auth/Access/Saml2Service.php
index 95049efd2..056977a3d 100644
--- a/app/Auth/Access/Saml2Service.php
+++ b/app/Auth/Access/Saml2Service.php
@@ -5,8 +5,6 @@ use BookStack\Auth\User;
use BookStack\Auth\UserRepo;
use BookStack\Exceptions\SamlException;
use Illuminate\Contracts\Auth\Authenticatable;
-use Illuminate\Database\Eloquent\Builder;
-use Illuminate\Support\Facades\Log;
/**
@@ -117,6 +115,27 @@ class Saml2Service extends Access\ExternalAuthService
return $userGroups;
}
+ /**
+ * For an array of strings, return a default for an empty array,
+ * a string for an array with one element and the full array for
+ * more than one element.
+ *
+ * @param array $data
+ * @param $defaultValue
+ * @return string
+ */
+ protected function simplifyValue(array $data, $defaultValue) {
+ switch (count($data)) {
+ case 0:
+ $data = $defaultValue;
+ break;
+ case 1:
+ $data = $data[0];
+ break;
+ }
+ return $data;
+ }
+
/**
* Get a property from an SAML response.
* Handles properties potentially being an array.
@@ -128,16 +147,9 @@ class Saml2Service extends Access\ExternalAuthService
protected function getSamlResponseAttribute(array $samlAttributes, string $propertyKey, $defaultValue)
{
if (isset($samlAttributes[$propertyKey])) {
- $data = $samlAttributes[$propertyKey];
- if (is_array($data)) {
- if (count($data) == 0) {
- $data = $defaultValue;
- } else if (count($data) == 1) {
- $data = $data[0];
- }
- }
+ $data = $this->simplifyValue($samlAttributes[$propertyKey], $defaultValue);
} else {
- $data = $defaultValue;
+ $data = $defaultValue;
}
return $data;
@@ -190,6 +202,7 @@ class Saml2Service extends Access\ExternalAuthService
* they exist, optionally registering them automatically.
* @param string $samlID
* @param array $samlAttributes
+ * @throws SamlException
*/
public function processLoginCallback($samlID, $samlAttributes)
{
@@ -197,12 +210,14 @@ class Saml2Service extends Access\ExternalAuthService
$isLoggedIn = auth()->check();
if ($isLoggedIn) {
- logger()->error("Already logged in");
+ throw new SamlException(trans('errors.saml_already_logged_in'), '/login');
} else {
$user = $this->getOrRegisterUser($userDetails);
if ($user === null) {
- logger()->error("User does not exist");
+ throw new SamlException(trans('errors.saml_user_not_registered', ['name' => $userDetails['uid']]), '/login');
} else {
+ $groups = $this->getUserGroups($samlAttributes);
+ $this->syncWithGroups($user, $groups);
auth()->login($user);
}
}
diff --git a/app/Config/services.php b/app/Config/services.php
index 9cd647e6d..b3dc9f087 100644
--- a/app/Config/services.php
+++ b/app/Config/services.php
@@ -150,12 +150,14 @@ return [
],
'saml' => [
+ 'name' => env('SAML_NAME', 'SSO'),
'enabled' => env('SAML2_ENABLED', false),
'auto_register' => env('SAML_AUTO_REGISTER', false),
'email_attribute' => env('SAML_EMAIL_ATTRIBUTE', 'email'),
'display_name_attribute' => explode('|', env('SAML_DISPLAY_NAME_ATTRIBUTE', 'username')),
'user_name_attribute' => env('SAML_USER_NAME_ATTRIBUTE', null),
'group_attribute' => env('SAML_GROUP_ATTRIBUTE', 'group'),
+ 'remove_from_groups' => env('SAML_REMOVE_FROM_GROUPS',false),
'user_to_groups' => env('SAML_USER_TO_GROUPS', false),
'id_is_user_name' => env('SAML_ID_IS_USER_NAME', true),
]
diff --git a/app/Exceptions/SamlException.php b/app/Exceptions/SamlException.php
index f9668919c..13db23f27 100644
--- a/app/Exceptions/SamlException.php
+++ b/app/Exceptions/SamlException.php
@@ -1,6 +1,6 @@
<?php namespace BookStack\Exceptions;
-class SamlException extends PrettyException
+class SamlException extends NotifyException
{
}
diff --git a/resources/lang/de/errors.php b/resources/lang/de/errors.php
index 362641bc8..2cd9d8b30 100644
--- a/resources/lang/de/errors.php
+++ b/resources/lang/de/errors.php
@@ -15,6 +15,8 @@ return [
'ldap_fail_authed' => 'LDAP-Zugriff mit DN und Passwort ist fehlgeschlagen',
'ldap_extension_not_installed' => 'LDAP-PHP-Erweiterung ist nicht installiert.',
'ldap_cannot_connect' => 'Die Verbindung zum LDAP-Server ist fehlgeschlagen. Beim initialen Verbindungsaufbau trat ein Fehler auf.',
+ 'saml_already_logged_in' => 'Sie sind bereits angemeldet',
+ 'saml_user_not_registered' => 'Kein Benutzer mit ID :name registriert und die automatische Registrierung ist deaktiviert',
'social_no_action_defined' => 'Es ist keine Aktion definiert',
'social_login_bad_response' => "Fehler bei der :socialAccount-Anmeldung: \n:error",
'social_account_in_use' => 'Dieses :socialAccount-Konto wird bereits verwendet. Bitte melden Sie sich mit dem :socialAccount-Konto an.',
diff --git a/resources/lang/de_informal/errors.php b/resources/lang/de_informal/errors.php
index 924deee0d..420c35c8d 100644
--- a/resources/lang/de_informal/errors.php
+++ b/resources/lang/de_informal/errors.php
@@ -9,6 +9,7 @@ return [
// Auth
'email_already_confirmed' => 'Die E-Mail-Adresse ist bereits bestätigt. Bitte melde dich an.',
'email_confirmation_invalid' => 'Der Bestätigungslink ist nicht gültig oder wurde bereits verwendet. Bitte registriere dich erneut.',
+ 'saml_already_logged_in' => 'Du bist bereits angemeldet',
'social_account_in_use' => 'Dieses :socialAccount-Konto wird bereits verwendet. Bitte melde dich mit dem :socialAccount-Konto an.',
'social_account_email_in_use' => 'Die E-Mail-Adresse ":email" ist bereits registriert. Wenn Du bereits registriert bist, kannst Du Dein :socialAccount-Konto in Deinen Profil-Einstellungen verknüpfen.',
'social_account_not_used' => 'Dieses :socialAccount-Konto ist bisher keinem Benutzer zugeordnet. Du kannst das in Deinen Profil-Einstellungen tun.',
diff --git a/resources/lang/en/errors.php b/resources/lang/en/errors.php
index b91a0c3e1..40c0bbffb 100644
--- a/resources/lang/en/errors.php
+++ b/resources/lang/en/errors.php
@@ -17,6 +17,8 @@ return [
'ldap_fail_authed' => 'LDAP access failed using given dn & password details',
'ldap_extension_not_installed' => 'LDAP PHP extension not installed',
'ldap_cannot_connect' => 'Cannot connect to ldap server, Initial connection failed',
+ 'saml_already_logged_in' => 'Already logged in',
+ 'saml_user_not_registered' => 'The user :name is not registered and automatic registration is disabled',
'social_no_action_defined' => 'No action defined',
'social_login_bad_response' => "Error received during :socialAccount login: \n:error",
'social_account_in_use' => 'This :socialAccount account is already in use, Try logging in via the :socialAccount option.',
diff --git a/resources/views/auth/login.blade.php b/resources/views/auth/login.blade.php
index 72d8d00aa..8d89c1288 100644
--- a/resources/views/auth/login.blade.php
+++ b/resources/views/auth/login.blade.php
@@ -51,7 +51,7 @@
<div>
<a id="saml-login" class="button outline block svg" href="{{ url("/saml2/login") }}">
{{-- @icon('auth/github') --}}
- {{ trans('auth.log_in_with', ['socialDriver' => 'SAML']) }}
+ {{ trans('auth.log_in_with', ['socialDriver' => config('services.saml.name')]) }}
</a>
</div>
@endif