diff --git a/app/__test__/auth/strategies/PasswordStrategy.spec.ts b/app/__test__/auth/strategies/PasswordStrategy.spec.ts new file mode 100644 index 0000000..7b290a4 --- /dev/null +++ b/app/__test__/auth/strategies/PasswordStrategy.spec.ts @@ -0,0 +1,13 @@ +import { PasswordStrategy } from "auth/authenticate/strategies/PasswordStrategy"; +import { describe, expect, it } from "bun:test"; + +describe("PasswordStrategy", () => { + it("should enforce provided minimum length", async () => { + const strategy = new PasswordStrategy({ minLength: 8, hashing: "plain" }); + + expect(strategy.verify("password")({} as any)).rejects.toThrow(); + expect( + strategy.verify("password1234")({ strategy_value: "password1234" } as any), + ).resolves.toBeUndefined(); + }); +}); diff --git a/app/__test__/integration/auth.integration.test.ts b/app/__test__/integration/auth.integration.test.ts index 477951f..b63b2b5 100644 --- a/app/__test__/integration/auth.integration.test.ts +++ b/app/__test__/integration/auth.integration.test.ts @@ -1,9 +1,15 @@ import { afterAll, beforeAll, describe, expect, it } from "bun:test"; import { App, createApp, type AuthResponse } from "../../src"; import { auth } from "../../src/modules/middlewares"; -import { randomString, secureRandomString, withDisabledConsole } from "../../src/core/utils"; +import { + mergeObject, + randomString, + secureRandomString, + withDisabledConsole, +} from "../../src/core/utils"; import { disableConsoleLog, enableConsoleLog } from "core/utils/test"; import { getDummyConnection } from "../helper"; +import type { AppAuthSchema } from "auth/auth-schema"; beforeAll(disableConsoleLog); afterAll(enableConsoleLog); @@ -62,12 +68,12 @@ const configs = { }, }; -function createAuthApp() { +function createAuthApp(config?: Partial) { const { dummyConnection } = getDummyConnection(); const app = createApp({ connection: dummyConnection, config: { - auth: configs.auth, + auth: mergeObject(configs.auth, config ?? {}), }, }); @@ -132,6 +138,16 @@ const fns = (app: App, mode?: Mode) = return { res, data }; }, + register: async (user: any): Promise<{ res: Response; data: AuthResponse }> => { + const res = (await app.server.request("/api/auth/password/register", { + method: "POST", + headers: headers(), + body: body(user), + })) as Response; + const data = mode === "cookie" ? getCookie(res, "auth") : await res.json(); + + return { res, data }; + }, me: async (token?: string): Promise> => { const res = (await app.server.request("/api/auth/me", { method: "GET", @@ -245,4 +261,61 @@ describe("integration auth", () => { expect(await $fns.me()).toEqual({ user: null as any }); } }); + + it("should register users with default role", async () => { + const app = createAuthApp({ default_role_register: "guest" }); + await app.build(); + const $fns = fns(app); + + // takes default role + expect( + await app + .createUser({ + email: "test@bknd.io", + password: "12345678", + }) + .then((r) => r.role), + ).toBe("guest"); + + // throws error if role doesn't exist + expect( + app.createUser({ + email: "test@bknd.io", + password: "12345678", + role: "doesnt exist", + }), + ).rejects.toThrow(); + + // takes role if provided + expect( + await app + .createUser({ + email: "test2@bknd.io", + password: "12345678", + role: "admin", + }) + .then((r) => r.role), + ).toBe("admin"); + + // registering with role is not allowed + expect( + await $fns + .register({ + email: "test3@bknd.io", + password: "12345678", + role: "admin", + }) + .then((r) => r.res.ok), + ).toBe(false); + + // takes default role + expect( + await $fns + .register({ + email: "test3@bknd.io", + password: "12345678", + }) + .then((r) => r.data.user.role), + ).toBe("guest"); + }); }); diff --git a/app/__test__/modules/AppAuth.spec.ts b/app/__test__/modules/AppAuth.spec.ts index 14cc5c5..59b0b1c 100644 --- a/app/__test__/modules/AppAuth.spec.ts +++ b/app/__test__/modules/AppAuth.spec.ts @@ -223,4 +223,32 @@ describe("AppAuth", () => { } } }); + + test("default role for registration must be a valid role", async () => { + const app = createApp({ + config: { + auth: { + enabled: true, + jwt: { + secret: "123456", + }, + allow_register: true, + roles: { + guest: { + is_default: true, + }, + }, + }, + }, + }); + + await app.build(); + + const auth = app.module.auth; + // doesn't allow invalid role + expect(auth.schema().patch("default_role_register", "admin")).rejects.toThrow(); + // allows valid role + await auth.schema().patch("default_role_register", "guest"); + expect(auth.toJSON().default_role_register).toBe("guest"); + }); }); diff --git a/app/src/auth/AppAuth.ts b/app/src/auth/AppAuth.ts index 4b23919..4d3b319 100644 --- a/app/src/auth/AppAuth.ts +++ b/app/src/auth/AppAuth.ts @@ -46,6 +46,22 @@ export class AppAuth extends Module { to.strategies!.password!.enabled = true; } + if (to.default_role_register && to.default_role_register?.length > 0) { + const valid_to_role = Object.keys(to.roles ?? {}).includes(to.default_role_register); + + if (!valid_to_role) { + const msg = `Default role for registration not found: ${to.default_role_register}`; + // if changing to a new value + if (from.default_role_register !== to.default_role_register) { + throw new Error(msg); + } + + // resetting gracefully, since role doesn't exist anymore + $console.warn(`${msg}, resetting to undefined`); + to.default_role_register = undefined; + } + } + return to; } @@ -82,6 +98,7 @@ export class AppAuth extends Module { this._authenticator = new Authenticator(strategies, new AppUserPool(this), { jwt: this.config.jwt, cookie: this.config.cookie, + default_role_register: this.config.default_role_register, }); this.registerEntities(); @@ -171,10 +188,20 @@ export class AppAuth extends Module { } catch (e) {} } - async createUser({ email, password, ...additional }: CreateUserPayload): Promise { + async createUser({ + email, + password, + role, + ...additional + }: CreateUserPayload): Promise { if (!this.enabled) { throw new Error("Cannot create user, auth not enabled"); } + if (role) { + if (!Object.keys(this.config.roles ?? {}).includes(role)) { + throw new Error(`Role "${role}" not found`); + } + } const strategy = "password" as const; const pw = this.authenticator.strategy(strategy) as PasswordStrategy; @@ -183,6 +210,7 @@ export class AppAuth extends Module { mutator.__unstable_toggleSystemEntityCreation(false); const { data: created } = await mutator.insertOne({ ...(additional as any), + role: role || this.config.default_role_register || undefined, email, strategy, strategy_value, diff --git a/app/src/auth/api/AuthController.ts b/app/src/auth/api/AuthController.ts index 99f1000..af846ce 100644 --- a/app/src/auth/api/AuthController.ts +++ b/app/src/auth/api/AuthController.ts @@ -13,6 +13,7 @@ import { InvalidSchemaError, transformObject, mcpTool, + $console, } from "bknd/utils"; import type { PasswordStrategy } from "auth/authenticate/strategies"; @@ -210,7 +211,7 @@ export class AuthController extends Controller { const idType = s.anyOf([s.number({ title: "Integer" }), s.string({ title: "UUID" })]); const getUser = async (params: { id?: string | number; email?: string }) => { - let user: DB["users"] | undefined = undefined; + let user: DB["users"] | undefined; if (params.id) { const { data } = await this.userRepo.findId(params.id); user = data; @@ -225,26 +226,33 @@ export class AuthController extends Controller { }; const roles = Object.keys(this.auth.config.roles ?? {}); - mcp.tool( - "auth_user_create", - { - description: "Create a new user", - inputSchema: s.object({ - email: s.string({ format: "email" }), - password: s.string({ minLength: 8 }), - role: s - .string({ - enum: roles.length > 0 ? roles : undefined, - }) - .optional(), - }), - }, - async (params, c) => { - await c.context.ctx().helper.granted(c, AuthPermissions.createUser); + try { + const actions = this.auth.authenticator.strategy("password").getActions(); + if (actions.create) { + const schema = actions.create.schema; + mcp.tool( + "auth_user_create", + { + description: "Create a new user", + inputSchema: s.object({ + ...schema.properties, + role: s + .string({ + enum: roles.length > 0 ? roles : undefined, + }) + .optional(), + }), + }, + async (params, c) => { + await c.context.ctx().helper.granted(c, AuthPermissions.createUser); - return c.json(await this.auth.createUser(params)); - }, - ); + return c.json(await this.auth.createUser(params)); + }, + ); + } + } catch (e) { + $console.warn("error creating auth_user_create tool", e); + } mcp.tool( "auth_user_token", diff --git a/app/src/auth/auth-schema.ts b/app/src/auth/auth-schema.ts index e479ea1..405fe66 100644 --- a/app/src/auth/auth-schema.ts +++ b/app/src/auth/auth-schema.ts @@ -51,6 +51,7 @@ export const authConfigSchema = $object( basepath: s.string({ default: "/api/auth" }), entity_name: s.string({ default: "users" }), allow_register: s.boolean({ default: true }).optional(), + default_role_register: s.string().optional(), jwt: jwtConfig, cookie: cookieConfig, strategies: $record( diff --git a/app/src/auth/authenticate/Authenticator.ts b/app/src/auth/authenticate/Authenticator.ts index 9a2b8b1..03f13e9 100644 --- a/app/src/auth/authenticate/Authenticator.ts +++ b/app/src/auth/authenticate/Authenticator.ts @@ -74,6 +74,7 @@ export const jwtConfig = s.strictObject( export const authenticatorConfig = s.object({ jwt: jwtConfig, cookie: cookieConfig, + default_role_register: s.string().optional(), }); type AuthConfig = s.Static; @@ -164,9 +165,13 @@ export class Authenticator< if (!("strategy_value" in profile)) { throw new InvalidConditionsException("Profile must have a strategy value"); } + if ("role" in profile) { + throw new InvalidConditionsException("Role cannot be provided during registration"); + } const user = await this.userPool.create(strategy.getName(), { ...profile, + role: this.config.default_role_register, strategy_value: profile.strategy_value, }); diff --git a/app/src/auth/authenticate/strategies/PasswordStrategy.ts b/app/src/auth/authenticate/strategies/PasswordStrategy.ts index 0c6066b..7f2c964 100644 --- a/app/src/auth/authenticate/strategies/PasswordStrategy.ts +++ b/app/src/auth/authenticate/strategies/PasswordStrategy.ts @@ -10,6 +10,7 @@ const schema = s .object({ hashing: s.string({ enum: ["plain", "sha256", "bcrypt"], default: "sha256" }), rounds: s.number({ minimum: 1, maximum: 10 }).optional(), + minLength: s.number({ default: 8, minimum: 1 }).optional(), }) .strict(); @@ -37,7 +38,7 @@ export class PasswordStrategy extends AuthStrategy { format: "email", }), password: s.string({ - minLength: 8, // @todo: this should be configurable + minLength: this.config.minLength, }), }); } @@ -65,12 +66,21 @@ export class PasswordStrategy extends AuthStrategy { return await bcryptCompare(compare, actual); } - return false; + return actual === compare; } verify(password: string) { return async (user: User) => { - const compare = await this.compare(user?.strategy_value!, password); + if (!user || !user.strategy_value) { + throw new InvalidCredentialsException(); + } + + if (!this.getPayloadSchema().properties.password.validate(password).valid) { + $console.debug("PasswordStrategy: Invalid password", password); + throw new InvalidCredentialsException(); + } + + const compare = await this.compare(user.strategy_value, password); if (compare !== true) { throw new InvalidCredentialsException(); } diff --git a/app/src/ui/elements/auth/AuthForm.tsx b/app/src/ui/elements/auth/AuthForm.tsx index d412b8e..edf837e 100644 --- a/app/src/ui/elements/auth/AuthForm.tsx +++ b/app/src/ui/elements/auth/AuthForm.tsx @@ -103,7 +103,7 @@ export function AuthForm({ - +