From 0847dde610b05ce540e5012d8164f8849a112228 Mon Sep 17 00:00:00 2001 From: Elias Projahn Date: Wed, 13 May 2020 13:42:07 +0200 Subject: [PATCH] server: Better account handling The account related routes were moved to /account. For performance reasons, we use a weaker password hash algorithm. The possibility to modify or delete an account was added. --- server/lib/src/auth.dart | 171 +++++++++++++++++++++++++++-------- server/lib/src/crypt.dart | 6 +- server/lib/src/database.dart | 4 + server/lib/src/server.dart | 8 +- 4 files changed, 144 insertions(+), 45 deletions(-) diff --git a/server/lib/src/auth.dart b/server/lib/src/auth.dart index d46a499..ef93164 100644 --- a/server/lib/src/auth.dart +++ b/server/lib/src/auth.dart @@ -8,43 +8,31 @@ import 'compute.dart'; import 'crypt.dart'; import 'database.dart'; -/// Information on the rights of the user making the request. +/// Information on the user making the request. extension AuthorizationInfo on Request { + /// The username of the logged in user. + /// + /// If this is a non null value, the user was authenticated. + String get username => this.attachments['username']; + set username(String value) => this.attachments['username'] = value; + /// Whether the user may create new resources. - set mayUpload(bool value) => this.attachments['mayUpload'] = value; + /// + /// This can only be true if the user was authenticated. bool get mayUpload => this.attachments['mayUpload'] ?? false; + set mayUpload(bool value) => this.attachments['mayUpload'] = value; /// Whether the user may edit existing resources. - set mayEdit(bool value) => this.attachments['mayEdit'] = value; + /// + /// This can only be true if the user was authenticated. bool get mayEdit => this.attachments['mayEdit'] ?? false; + set mayEdit(bool value) => this.attachments['mayEdit'] = value; /// Whether the user may delete resources. - set mayDelete(bool value) => this.attachments['mayDelete'] = value; + /// + /// This can only be true if the user was authenticated. bool get mayDelete => this.attachments['mayDelete'] ?? false; -} - -/// A user as presented within a request. -class RequestUser { - /// The unique user name. - final String name; - - /// An optional email address. - final String email; - - /// The password in clear text. - final String password; - - RequestUser({ - this.name, - this.email, - this.password, - }); - - factory RequestUser.fromJson(Map json) => RequestUser( - name: json['name'], - email: json['email'], - password: json['password'], - ); + set mayDelete(bool value) => this.attachments['mayDelete'] = value; } /// Endpoint controller for user registration. @@ -59,10 +47,13 @@ class RegisterController extends Controller { Future handle(Request request) async { if (request.method == 'POST') { final json = await request.body.decode>(); - final requestUser = RequestUser.fromJson(json); + + final String username = json['username']; + final String email = json['email']; + final String password = json['password']; // Check if we already have a user with that name. - final existingUser = await db.getUser(requestUser.name); + final existingUser = await db.getUser(username); if (existingUser != null) { // Returning something different than 200 here has the security // implication that an attacker can check for existing user names. At @@ -72,11 +63,11 @@ class RegisterController extends Controller { return Response.conflict(); } else { // This will take a long time, so we run it in a new isolate. - final result = await compute(Crypt.hashPassword, requestUser.password); + final result = await compute(Crypt.hashPassword, password); db.updateUser(User( - name: requestUser.name, - email: requestUser.email, + name: username, + email: email, salt: result.salt, hash: result.hash, mayUpload: true, @@ -109,23 +100,25 @@ class LoginController extends Controller { Future handle(Request request) async { if (request.method == 'POST') { final json = await request.body.decode>(); - final requestUser = RequestUser.fromJson(json); - final realUser = await db.getUser(requestUser.name); - if (realUser != null) { + final String username = json['username']; + final String password = json['password']; + + final user = await db.getUser(username); + if (user != null) { // We check the password in a new isolate, because this can take a long // time. if (await compute( Crypt.checkPassword, CheckPasswordRequest( - password: requestUser.password, - salt: realUser.salt, - hash: realUser.hash, + password: password, + salt: user.salt, + hash: user.hash, ), )) { final builder = JWTBuilder() ..expiresAt = DateTime.now().add(Duration(minutes: 30)) - ..setClaim('user', requestUser.name); + ..setClaim('user', username); final token = builder.getSignedToken(_signer).toString(); @@ -140,6 +133,103 @@ class LoginController extends Controller { } } +/// An endpoint controller for retrieving and changing account details. +class AccountDetailsController extends Controller { + final ServerDatabase db; + + AccountDetailsController(this.db); + + @override + Future handle(Request request) async { + if (request.method == 'GET') { + if (request.username != null) { + final user = await db.getUser(request.username); + return Response.ok({ + 'email': user.email, + }); + } else { + return Response.forbidden(); + } + } else if (request.method == 'POST') { + final json = await request.body.decode>(); + + final String username = json['username']; + final String password = json['password']; + final String newEmail = json['newEmail']; + final String newPassword = json['newPassword']; + + final user = await db.getUser(username); + + // Check whether the user exists and the password was right. + if (user != null && + await compute( + Crypt.checkPassword, + CheckPasswordRequest( + password: password, + salt: user.salt, + hash: user.hash, + ), + )) { + final hashResult = await compute(Crypt.hashPassword, newPassword); + + db.updateUser(User( + name: username, + email: newEmail, + salt: hashResult.salt, + hash: hashResult.hash, + mayUpload: user.mayUpload, + mayEdit: user.mayEdit, + mayDelete: user.mayDelete, + )); + + return Response.ok(null); + } else { + return Response.forbidden(); + } + } else { + return Response(HttpStatus.methodNotAllowed, null, null); + } + } +} + +/// An endpoint controller for deleting an account. +class AccountDeleteController extends Controller { + final ServerDatabase db; + + AccountDeleteController(this.db); + + @override + Future handle(Request request) async { + if (request.method == 'POST') { + final json = await request.body.decode>(); + + final String username = json['username']; + final String password = json['password']; + + final user = await db.getUser(username); + + // Check whether the user exists and the password was right. + if (user != null && + await compute( + Crypt.checkPassword, + CheckPasswordRequest( + password: password, + salt: user.salt, + hash: user.hash, + ), + )) { + await db.deleteUser(username); + + return Response.ok(null); + } else { + return Response.forbidden(); + } + } else { + return Response(HttpStatus.methodNotAllowed, null, null); + } + } +} + /// Middleware for checking authorization. /// /// This will set the fields defined in [AuthorizationInfo] on this request @@ -172,6 +262,7 @@ class AuthorizationController extends Controller { if (JWTValidator().validate(jwt, signer: _signer).isEmpty) { final user = await db.getUser(jwt.claims['user']); if (user != null) { + request.username = user.name; request.mayUpload = user.mayUpload; request.mayEdit = user.mayEdit; request.mayDelete = user.mayDelete; diff --git a/server/lib/src/crypt.dart b/server/lib/src/crypt.dart index dc46639..37dd63c 100644 --- a/server/lib/src/crypt.dart +++ b/server/lib/src/crypt.dart @@ -38,17 +38,17 @@ class CheckPasswordRequest { /// Methods for handling passwords. class Crypt { - static final _crypt = PassCrypt(); + static final _crypt = PassCrypt('SHA-512/HMAC/PBKDF2'); static final _rand = Random.secure(); /// Compute a hash for a password. - /// + /// /// The result will contain the hash and a randomly generated salt. static HashPasswordResult hashPassword(String password) { final bytes = List.generate(32, (i) => _rand.nextInt(256)); final salt = base64UrlEncode(bytes); final hash = _crypt.hashPass(salt, password); - + return HashPasswordResult( hash: hash, salt: salt, diff --git a/server/lib/src/database.dart b/server/lib/src/database.dart index 8490430..88b08dd 100644 --- a/server/lib/src/database.dart +++ b/server/lib/src/database.dart @@ -16,4 +16,8 @@ class ServerDatabase extends _$ServerDatabase { Future updateUser(User user) async { await into(users).insert(user, mode: InsertMode.insertOrReplace); } + + Future deleteUser(String name) async { + await (delete(users)..where((u) => u.name.equals(name))).go(); + } } diff --git a/server/lib/src/server.dart b/server/lib/src/server.dart index b07ce22..68acc39 100644 --- a/server/lib/src/server.dart +++ b/server/lib/src/server.dart @@ -41,8 +41,12 @@ class MusicusServer extends ApplicationChannel { @override Controller get entryPoint => Router() - ..route('/login').link(() => LoginController(serverDb, secret)) - ..route('/register').link(() => RegisterController(serverDb)) + ..route('/account/register').link(() => RegisterController(serverDb)) + ..route('/account/details') + .link(() => AuthorizationController(serverDb, secret)) + .link(() => AccountDetailsController(serverDb)) + ..route('/account/delete').link(() => AccountDeleteController(serverDb)) + ..route('/account/login').link(() => LoginController(serverDb, secret)) ..route('/persons/[:id]') .link(() => AuthorizationController(serverDb, secret)) .link(() => PersonsController(db))