Fix #78 Add a APP_ACCOUNTS_EMAIL_UNIQUE environnement setting

This commit is contained in:
Timothée Jaussoin 2023-03-14 10:53:51 +00:00
parent 484fc0d923
commit eb0c97804e
13 changed files with 165 additions and 26 deletions

View file

@ -20,6 +20,7 @@ ACCOUNT_TRANSPORT_PROTOCOL_TEXT="TLS (recommended), TCP or UDP" # Simple text, t
ACCOUNT_REALM=null # Default realm for the accounts, fallback to the domain if not set, enforce null by default
# Account creation
ACCOUNT_EMAIL_UNIQUE=false # Emails are unique between all the accounts
ACCOUNT_CONSUME_EXTERNAL_ACCOUNT_ON_CREATE=false
ACCOUNT_BLACKLISTED_USERNAMES=

View file

@ -52,7 +52,7 @@ class AuthenticateController extends Controller
]);
$account = Account::where('username', $request->get('username'))
->first();
->first();
// Try alias
if (!$account) {
@ -104,9 +104,13 @@ class AuthenticateController extends Controller
/**
* Because several accounts can have the same email
*/
$account = Account::where('email', $request->get('email'))
->where('username', $request->get('username'))
->first();
$account = Account::where('username', $request->get('username'));
if (config('app.account_email_unique') == false) {
$account = $account->where('email', $request->get('email'));
}
$account = $account->first();
// Try alias
if (!$account) {
@ -148,7 +152,7 @@ class AuthenticateController extends Controller
public function validateEmail(Request $request, string $code)
{
$request->merge(['code' => $code]);
$request->validate(['code' => 'required|size:'.self::$emailCodeSize]);
$request->validate(['code' => 'required|size:' . self::$emailCodeSize]);
$account = Account::where('confirmation_key', $code)->first();
@ -188,7 +192,7 @@ class AuthenticateController extends Controller
]);
$account = Account::where('username', $request->get('phone'))
->first();
->first();
// Try alias
if (!$account) {
@ -209,7 +213,7 @@ class AuthenticateController extends Controller
$account->save();
$ovhSMS = new OvhSMS;
$ovhSMS->send($request->get('phone'), 'Your '.config('app.name').' validation code is '.$account->confirmation_key);
$ovhSMS->send($request->get('phone'), 'Your ' . config('app.name') . ' validation code is ' . $account->confirmation_key);
// Ask the user to set a password
if (!$account->activated) {
@ -229,7 +233,7 @@ class AuthenticateController extends Controller
]);
$account = Account::where('id', $request->get('account_id'))
->firstOrFail();
->firstOrFail();
if ($account->confirmation_key != $request->get('code')) {
return redirect()->back()->withErrors([

View file

@ -42,10 +42,14 @@ class EmailController extends Controller
$request->user()->email
? [
'email_current' => ['required', Rule::in([$request->user()->email])],
'email' => 'required|different:email_current|confirmed|email',
'email' => config('app.account_email_unique')
? 'required|different:email_current|confirmed|email|unique:accounts,email'
: 'required|different:email_current|confirmed|email',
]
: [
'email' => 'required|confirmed|email',
'email' => config('app.account_email_unique')
? 'required|email|confirmed|unique:accounts,email'
: 'required|confirmed|email',
]
);

View file

@ -84,7 +84,9 @@ class RegisterController extends Controller
new BlacklistedUsername
],
'g-recaptcha-response' => 'required|captcha',
'email' => 'required|email|confirmed'
'email' => config('app.account_email_unique')
? 'required|email|confirmed|unique:accounts,email'
: 'required|email|confirmed',
]);
$account = new Account;
@ -100,8 +102,7 @@ class RegisterController extends Controller
$account->confirmation_key = Str::random($this->emailCodeSize);
$account->save();
if (!empty(config('app.newsletter_registration_address'))
&& $request->has('newsletter')) {
if (!empty(config('app.newsletter_registration_address')) && $request->has('newsletter')) {
Mail::to(config('app.newsletter_registration_address'))->send(new NewsletterRegistration($account));
}
@ -115,7 +116,7 @@ class RegisterController extends Controller
public function storePhone(Request $request)
{
$request->validate([
'terms' =>'accepted',
'terms' => 'accepted',
'privacy' => 'accepted',
'username' => [
new NoUppercase,
@ -135,6 +136,9 @@ class RegisterController extends Controller
'unique:accounts,username',
new WithoutSpaces, 'starts_with:+'
],
'email' => config('app.account_email_unique')
? 'nullable|email|unique:accounts,email'
: 'nullable|email',
'g-recaptcha-response' => 'required|captcha',
]);
@ -161,7 +165,7 @@ class RegisterController extends Controller
$account->save();
$ovhSMS = new OvhSMS;
$ovhSMS->send($request->get('phone'), 'Your '.config('app.name').' validation code is '.$account->confirmation_key);
$ovhSMS->send($request->get('phone'), 'Your ' . config('app.name') . ' validation code is ' . $account->confirmation_key);
Log::channel('events')->info('Web: Account created using an SMS confirmation', ['id' => $account->identifier]);

View file

@ -32,6 +32,11 @@ use App\ExternalAccount;
use App\Http\Requests\CreateAccountRequest;
use App\Http\Requests\UpdateAccountRequest;
use App\Http\Controllers\Account\AuthenticateController as WebAuthenticateController;
use App\Rules\BlacklistedUsername;
use App\Rules\IsNotPhoneNumber;
use App\Rules\NoUppercase;
use App\Rules\WithoutSpaces;
use Illuminate\Validation\Rule;
class AccountController extends Controller
{
@ -67,6 +72,31 @@ class AccountController extends Controller
public function store(CreateAccountRequest $request)
{
$request->validate([
'username' => [
'required',
new NoUppercase,
new IsNotPhoneNumber,
new BlacklistedUsername,
Rule::unique('accounts', 'username')->where(function ($query) use ($request) {
$query->where('domain', $this->resolveDomain($request));
}),
'filled',
],
'dtmf_protocol' => 'nullable|in:' . Account::dtmfProtocolsRule(),
'email' => [
'nullable',
'email',
config('app.account_email_unique') ? Rule::unique('accounts', 'email') : null
],
'phone' => [
'nullable',
'unique:aliases,alias',
'unique:accounts,username',
new WithoutSpaces, 'starts_with:+'
]
]);
$account = new Account;
$account->username = $request->get('username');
$account->email = $request->get('email');
@ -96,6 +126,31 @@ class AccountController extends Controller
public function update(UpdateAccountRequest $request, $id)
{
$request->validate([
'username' => [
'required',
new NoUppercase,
new IsNotPhoneNumber,
new BlacklistedUsername,
Rule::unique('accounts', 'username')->where(function ($query) use ($request) {
$query->where('domain', $this->resolveDomain($request));
})->ignore($id),
'filled',
],
'dtmf_protocol' => 'nullable|in:' . Account::dtmfProtocolsRule(),
'email' => [
'nullable',
'email',
config('app.account_email_unique') ? Rule::unique('accounts', 'email')->ignore($id) : null
],
'phone' => [
'nullable',
'unique:aliases,alias',
'unique:accounts,username',
new WithoutSpaces, 'starts_with:+'
]
]);
$account = Account::findOrFail($id);
$account->username = $request->get('username');
$account->email = $request->get('email');

View file

@ -102,7 +102,9 @@ class AccountController extends Controller
'algorithm' => 'required|in:SHA-256,MD5',
'password' => 'required|filled',
'domain' => 'min:3',
'email' => 'required_without:phone|email',
'email' => config('app.account_email_unique')
? 'required_without:phone|email|unique:accounts,email'
: 'required_without:phone|email',
'phone' => [
'required_without:email',
'prohibits:username',
@ -241,6 +243,9 @@ class AccountController extends Controller
}),
'size:' . WebAuthenticateController::$emailCodeSize
],
'email' => config('app.account_email_unique')
? 'nullable|email|unique:accounts,email'
: 'nullable|email',
// For retro-compatibility
'token' => [
'required_without:account_creation_token',

View file

@ -119,7 +119,6 @@ class AccountController extends Controller
],
'algorithm' => 'required|in:SHA-256,MD5',
'password' => 'required|filled',
'email' => 'email',
'admin' => 'boolean|nullable',
'activated' => 'boolean|nullable',
'dtmf_protocol' => 'nullable|in:' . Account::dtmfProtocolsRule(),
@ -127,6 +126,9 @@ class AccountController extends Controller
'date_format:Y-m-d H:i:s',
'nullable',
],
'email' => config('app.account_email_unique')
? 'nullable|email|unique:accounts,email'
: 'nullable|email',
'phone' => [
'unique:aliases,alias',
'unique:accounts,username',

View file

@ -27,8 +27,14 @@ class EmailController extends Controller
{
public function requestUpdate(Request $request)
{
$rules = ['required', 'email', Rule::notIn([$request->user()->email])];
if (config('app.account_email_unique')) {
array_push($rules, Rule::unique('accounts', 'email'));
}
$request->validate([
'email' => ['required', 'email', Rule::notIn([$request->user()->email])],
'email' => $rules,
]);
$request->user()->requestEmailUpdate($request->get('email'));
}

View file

@ -28,6 +28,7 @@ return [
'proxy_registrar_address' => env('ACCOUNT_PROXY_REGISTRAR_ADDRESS', 'sip.domain.com'),
'transport_protocol_text' => env('ACCOUNT_TRANSPORT_PROTOCOL_TEXT', 'TLS (recommended), TCP or UDP'),
'account_email_unique' => env('ACCOUNT_EMAIL_UNIQUE', false),
'consume_external_account_on_create' => env('ACCOUNT_CONSUME_EXTERNAL_ACCOUNT_ON_CREATE', false),
'blacklisted_usernames' => env('ACCOUNT_BLACKLISTED_USERNAMES', ''),

View file

@ -7,10 +7,12 @@
<div class="card mt-3">
<div class="card-body">
{!! Form::open(['route' => 'account.authenticate.email']) !!}
<div class="form-group">
{!! Form::label('email', 'Email') !!}
{!! Form::email('email', old('email'), ['class' => 'form-control', 'placeholder' => 'bob@example.com', 'required']) !!}
</div>
@if (config('app.account_email_unique') == false)
<div class="form-group">
{!! Form::label('email', 'Email') !!}
{!! Form::email('email', old('email'), ['class' => 'form-control', 'placeholder' => 'bob@example.com', 'required']) !!}
</div>
@endif
<div class="form-group">
{!! Form::label('username', 'SIP Username') !!}
<div class=" input-group mb-3">

View file

@ -132,7 +132,7 @@ JSON parameters:
* `password` required minimum 6 characters
* `algorithm` required, values can be `SHA-256` or `MD5`
* `domain` if not set the value is enforced to the default registration domain set in the global configuration
* `email` optional if `phone` set, an email, set an email to the account
* `email` optional if `phone` set, an email, set an email to the account, must be unique if `ACCOUNT_EMAIL_UNIQUE` is set to `true`
* `phone` required if `username` not set, optional if `email` set, a phone number, set a phone number to the account
### `POST /accounts/with-account-creation-token`
@ -234,7 +234,7 @@ Delete the account.
Change the account email. An email will be sent to the new email address to confirm the operation.
JSON parameters:
* `email` the new email address
* `email` the new email address, must be unique if `ACCOUNT_EMAIL_UNIQUE` is set to `true`
### `POST /accounts/me/password`
<span class="badge badge-info">User</span>
@ -258,7 +258,7 @@ JSON parameters:
* `domain` **not configurable by default**. Only configurable if `APP_ADMINS_MANAGE_MULTI_DOMAINS` is set to `true` in the global configuration. Otherwise `APP_SIP_DOMAIN` is used.
* `activated` optional, a boolean, set to `false` by default
* `display_name` optional, string
* `email` optional, string, must be an email
* `email` optional, must be an email, must be unique if `ACCOUNT_EMAIL_UNIQUE` is set to `true`
* `admin` optional, a boolean, set to `false` by default, create an admin account
* `phone` optional, a phone number, set a phone number to the account
* `dtmf_protocol` optional, values must be `sipinfo` or `rfc2833`

View file

@ -1,6 +1,11 @@
@if (config('app.web_panel'))
<p class="text-center pt-3">
Set or recover your password using your <a href="{{ route('account.login_email') }}">Email address</a>
@if (config('app.account_email_unique'))
Set or recover your account
@else
Set or recover your password
@endif
using your <a href="{{ route('account.login_email') }}">Email address</a>
@if (config('app.phone_authentication'))
or your <a href="{{ route('account.login_phone') }}">Phone number</a>
@endif

View file

@ -471,6 +471,33 @@ class AccountApiTest extends TestCase
]);
}
public function testUniqueEmailAdmin()
{
$email = 'collision@email.com';
$existing = Password::factory()->create();
$existing->account->activated = false;
$existing->account->email = $email;
$existing->account->save();
config()->set('app.account_email_unique', true);
$admin = Admin::factory()->create();
$admin->account->generateApiKey();
$admin->account->save();
$this->keyAuthenticated($admin->account)
->json($this->method, $this->route, [
'username' => 'hop',
'email' => $email,
'domain' => 'server.com',
'algorithm' => 'SHA-256',
'password' => '123456',
])
->assertStatus(422)
->assertJsonValidationErrors(['email']);
}
/**
* /!\ Dangerous endpoints
*/
@ -594,6 +621,18 @@ class AccountApiTest extends TestCase
->assertStatus(422)
->assertJsonValidationErrors(['username']);
// Email is now unique
config()->set('app.account_email_unique', true);
$this->json($this->method, $this->route.'/public', [
'username' => 'johndoe',
'algorithm' => 'SHA-256',
'password' => '2',
'email' => 'john@doe.tld',
])
->assertStatus(422)
->assertJsonValidationErrors(['email']);
$this->assertDatabaseHas('accounts', [
'username' => $username,
'domain' => config('app.sip_domain')
@ -704,6 +743,7 @@ class AccountApiTest extends TestCase
public function testChangeEmail()
{
$password = Password::factory()->create();
$otherAccount = Password::factory()->create();
$password->account->generateApiKey();
$newEmail = 'new_email@test.com';
@ -737,6 +777,16 @@ class AccountApiTest extends TestCase
'new_email' => $newEmail
]
]);
// Email already exists
config()->set('app.account_email_unique', true);
$this->keyAuthenticated($password->account)
->json($this->method, $this->route.'/me/email/request', [
'email' => $otherAccount->account->email
])
->assertStatus(422)
->assertJsonValidationErrors(['email']);
}
public function testChangePassword()