Server: Only disable API access when an account is disabled

packages/server/src/middleware/ownerHandler.test.ts

@@ -1,5 +1,4 @@
-import { ErrorForbidden } from '../utils/errors';
-import { createUserAndSession, beforeAllDb, afterAllTests, beforeEachDb, koaAppContext, koaNext, models, expectHttpError } from '../utils/testing/testUtils';
+import { createUserAndSession, beforeAllDb, afterAllTests, beforeEachDb, koaAppContext, koaNext } from '../utils/testing/testUtils';
 import ownerHandler from './ownerHandler';
 
 describe('ownerHandler', function() {
@@ -45,18 +44,4 @@ describe('ownerHandler', function() {
 		expect(!!context.joplin.owner).toBe(false);
 	});
 
-	test('should not login if the user has been disabled', async function() {
-		const { user, session } = await createUserAndSession(1);
-
-		await models().user().save({ id: user.id, enabled: 0 });
-
-		const context = await koaAppContext({
-			sessionId: session.id,
-		});
-
-		context.joplin.owner = null;
-
-		await expectHttpError(async () => ownerHandler(context, koaNext), ErrorForbidden.httpCode);
-	});
-
 });

packages/server/src/middleware/ownerHandler.ts

@@ -1,15 +1,9 @@
 import { AppContext, KoaNext } from '../utils/types';
 import { contextSessionId } from '../utils/requestUtils';
-import { ErrorForbidden } from '../utils/errors';
-import { cookieSet } from '../utils/cookies';
 
 export default async function(ctx: AppContext, next: KoaNext): Promise<void> {
 	const sessionId = contextSessionId(ctx, false);
 	const owner = sessionId ? await ctx.joplin.models.session().sessionUser(sessionId) : null;
-	if (owner && !owner.enabled) {
-		cookieSet(ctx, 'sessionId', ''); // Clear cookie, otherwise the user cannot login at all anymore
-		throw new ErrorForbidden('This user account is disabled. Please contact support.');
-	}
 	ctx.joplin.owner = owner;
 	return next();
 }

packages/server/src/models/BaseModel.ts

@@ -211,6 +211,13 @@ export default abstract class BaseModel<T> {
 		return rows as T[];
 	}
 
+	public async count(): Promise<number> {
+		const r = await this
+			.db(this.tableName)
+			.count('*', { as: 'item_count' });
+		return r[0].item_count;
+	}
+
 	public fromApiInput(object: T): T {
 		const blackList = ['updated_time', 'created_time', 'owner_id'];
 		const whiteList = Object.keys(databaseSchema[this.tableName]);

packages/server/src/models/UserModel.ts

@@ -118,7 +118,6 @@ export default class UserModel extends BaseModel<User> {
 	public async login(email: string, password: string): Promise<User> {
 		const user = await this.loadByEmail(email);
 		if (!user) return null;
-		if (!user.enabled) throw new ErrorForbidden('This account is disabled. Please contact support if you need to re-activate it.');
 		if (!auth.checkPassword(password, user.password)) return null;
 		return user;
 	}

packages/server/src/routes/api/items.test.ts

@@ -1,4 +1,4 @@
-import { beforeAllDb, afterAllTests, beforeEachDb, createUserAndSession, models, createItem, makeTempFileWithContent, makeNoteSerializedBody, createItemTree, expectHttpError, createNote, expectNoHttpError } from '../../utils/testing/testUtils';
+import { beforeAllDb, afterAllTests, beforeEachDb, createUserAndSession, models, createItem, makeTempFileWithContent, makeNoteSerializedBody, createItemTree, expectHttpError, createNote, expectNoHttpError, getItem } from '../../utils/testing/testUtils';
 import { NoteEntity } from '@joplin/lib/services/database/types';
 import { ModelType } from '@joplin/lib/BaseModel';
 import { deleteApi, getApi, putApi } from '../../utils/testing/apiUtils';
@@ -272,6 +272,19 @@ describe('api_items', function() {
 		expect(item.jop_share_id).toBe(share.id);
 	});
 
+	test('should not upload or download items if the account is disabled', async function() {
+		const { session, user } = await createUserAndSession(1);
+
+		// Should work
+		await createItem(session.id, 'root:/test1.txt:', 'test1');
+		expect(await getItem(session.id, 'root:/test1.txt:')).toBe('test1');
+
+		// Should no longer work
+		await models().user().save({ id: user.id, enabled: 0 });
+		await expectHttpError(async () => createItem(session.id, 'root:/test2.txt:', 'test2'), ErrorForbidden.httpCode);
+		await expectHttpError(async () => getItem(session.id, 'root:/test1.txt:'), ErrorForbidden.httpCode);
+	});
+
 	test('should check permissions - only share participants can associate an item with a share', async function() {
 		const { session: session1 } = await createUserAndSession(1);
 		const { session: session2 } = await createUserAndSession(2);

packages/server/src/utils/routeUtils.ts

@@ -1,5 +1,5 @@
 import config, { baseUrl } from '../config';
-import { Item, ItemAddressingType, Uuid } from '../services/database/types';
+import { Item, ItemAddressingType, User, Uuid } from '../services/database/types';
 import { ErrorBadRequest, ErrorForbidden, ErrorNotFound } from './errors';
 import Router from './Router';
 import { AppContext, HttpMethod, RouteType } from './types';
@@ -182,6 +182,12 @@ export function routeResponseFormat(context: AppContext): RouteResponseFormat {
 	return path.indexOf('api') === 0 || path.indexOf('/api') === 0 ? RouteResponseFormat.Json : RouteResponseFormat.Html;
 }
 
+function disabledAccountCheck(route: MatchedRoute, user: User) {
+	if (!user || user.enabled) return;
+
+	if (route.subPath.schema.startsWith('api/')) throw new ErrorForbidden(`This account is disabled. Please login to ${config().baseUrl} for more information.`);
+}
+
 export async function execRequest(routes: Routers, ctx: AppContext) {
 	const match = findMatchingRoute(ctx.path, routes);
 	if (!match) throw new ErrorNotFound();
@@ -197,6 +203,7 @@ export async function execRequest(routes: Routers, ctx: AppContext) {
 	if (!isPublicRoute && !ctx.joplin.owner) throw new ErrorForbidden();
 
 	await csrfCheck(ctx, isPublicRoute);
+	disabledAccountCheck(match, ctx.joplin.owner);
 
 	return endPoint.handler(match.subPath, ctx);
 }

packages/server/src/utils/testing/testUtils.ts

@@ -15,7 +15,7 @@ import * as fs from 'fs-extra';
 import * as jsdom from 'jsdom';
 import setupAppContext from '../setupAppContext';
 import { ApiError } from '../errors';
-import { putApi } from './apiUtils';
+import { getApi, putApi } from './apiUtils';
 import { FolderEntity, NoteEntity, ResourceEntity } from '@joplin/lib/services/database/types';
 import { ModelType } from '@joplin/lib/BaseModel';
 import { initializeJoplinUtils } from '../joplinUtils';
@@ -327,6 +327,11 @@ export async function createItemTree3(userId: Uuid, parentFolderId: string, shar
 	}
 }
 
+export async function getItem(sessionId: string, path: string): Promise<string> {
+	const item: Buffer = await getApi(sessionId, `items/${path}/content`);
+	return item.toString();
+}
+
 export async function createItem(sessionId: string, path: string, content: string | Buffer): Promise<Item> {
 	const tempFilePath = await makeTempFileWithContent(content);
 	const item: Item = await putApi(sessionId, `items/${path}/content`, null, { filePath: tempFilePath });

readme/spec/server_user_status.md

@@ -0,0 +1,20 @@
+# Joplin Server user status
+
+## User flags
+
+User flags are used to indicate problem conditions with a particular account. They are usually automatically set by various services, for example when an account go over the limit, or when a payment fails. Likewise they are removed automatically when the condition changes.
+
+The list of flags is defined in `UserFlagType`.
+
+## User status
+
+A user can have various status that affects the possible actions they can do. **User statuses are derived from user flags**.
+
+| Status | Values | Description |
+| --- | --- | --- | 
+| can_upload | 0 or 1 | Whether the user can upload items, such as notes or tags, to the server.
+| enabled | 0 or 1 | A disabled user cannot upload or download data from the server API anymore. However, they can still login to the website, make change to their profile, etc.
+
+Perhaps a third status: "blocked" could be created. It would be like `enabled = 0`, except they won't be able to login to the website either.
+
+These status should only be set as a results of user flags. In other words, the application should not directly set `enabled` to 0 or 1 but instead set a user flag that would indicate the issue. A script will then process the user flags and set the status as a result.