feat: restrict non credential provider interactions (#871)

* wip: add provider field to sqlite user table

* feat: disable invites when credentials provider is not used

* wip: add migration for provider field in user table with sqlite

* wip: remove fields that can not be modified by non credential users

* wip: make username, mail and avatar disabled instead of hidden

* wip: external users membership of group cannot be managed manually

* feat: add alerts to inform about disabled fields and managing group members

* wip: add mysql migration for provider on user table

* chore: fix format issues

* chore: address pull request feedback

* fix: build issue

* fix: deepsource issues

* fix: tests not working

* feat: restrict login to specific auth providers

* chore: address pull request feedback

* fix: deepsource issue
This commit is contained in:
Meier Lukas
2024-07-27 11:38:51 +02:00
committed by GitHub
parent eba4052522
commit 6f7327b774
36 changed files with 2989 additions and 116 deletions

View File

@@ -57,6 +57,7 @@ export const groupRouter = createTRPCRouter({
name: true,
email: true,
image: true,
provider: true,
},
},
},

View File

@@ -6,9 +6,11 @@ import { invites } from "@homarr/db/schema/sqlite";
import { z } from "@homarr/validation";
import { createTRPCRouter, protectedProcedure } from "../trpc";
import { throwIfCredentialsDisabled } from "./invite/checks";
export const inviteRouter = createTRPCRouter({
getAll: protectedProcedure.query(async ({ ctx }) => {
throwIfCredentialsDisabled();
const dbInvites = await ctx.db.query.invites.findMany({
orderBy: asc(invites.expirationDate),
columns: {
@@ -32,6 +34,7 @@ export const inviteRouter = createTRPCRouter({
}),
)
.mutation(async ({ ctx, input }) => {
throwIfCredentialsDisabled();
const id = createId();
const token = randomBytes(20).toString("hex");
@@ -54,6 +57,7 @@ export const inviteRouter = createTRPCRouter({
}),
)
.mutation(async ({ ctx, input }) => {
throwIfCredentialsDisabled();
const dbInvite = await ctx.db.query.invites.findFirst({
where: eq(invites.id, input.id),
});

View File

@@ -0,0 +1,12 @@
import { TRPCError } from "@trpc/server";
import { env } from "@homarr/auth/env.mjs";
export const throwIfCredentialsDisabled = () => {
if (!env.AUTH_PROVIDERS.includes("credentials")) {
throw new TRPCError({
code: "FORBIDDEN",
message: "Credentials provider is disabled",
});
}
};

View File

@@ -170,8 +170,8 @@ describe("byId should return group by id including members and permissions", ()
expect(result.members.length).toBe(1);
const userKeys = Object.keys(result.members[0] ?? {});
expect(userKeys.length).toBe(4);
expect(["id", "name", "email", "image"].some((key) => userKeys.includes(key)));
expect(userKeys.length).toBe(5);
expect(["id", "name", "email", "image", "provider"].some((key) => userKeys.includes(key)));
expect(result.permissions.length).toBe(1);
expect(result.permissions[0]).toBe("admin");
});

View File

@@ -22,6 +22,15 @@ vi.mock("@homarr/auth", async () => {
return { ...mod, auth: () => ({}) as Session };
});
// Mock the env module to return the credentials provider
vi.mock("@homarr/auth/env.mjs", () => {
return {
env: {
AUTH_PROVIDERS: ["credentials"],
},
};
});
describe("all should return all existing invites without sensitive informations", () => {
test("invites should not contain sensitive informations", async () => {
// Arrange

View File

@@ -13,6 +13,15 @@ vi.mock("@homarr/auth", async () => {
return { ...mod, auth: () => ({}) as Session };
});
// Mock the env module to return the credentials provider
vi.mock("@homarr/auth/env.mjs", () => {
return {
env: {
AUTH_PROVIDERS: ["credentials"],
},
};
});
describe("initUser should initialize the first user", () => {
it("should throw an error if a user already exists", async () => {
const db = createDb();
@@ -230,6 +239,7 @@ describe("editProfile shoud update user", () => {
password: null,
image: null,
homeBoardId: null,
provider: "credentials",
});
});
@@ -270,6 +280,7 @@ describe("editProfile shoud update user", () => {
password: null,
image: null,
homeBoardId: null,
provider: "credentials",
});
});
});
@@ -294,6 +305,7 @@ describe("delete should delete user", () => {
password: null,
salt: null,
homeBoardId: null,
provider: "ldap" as const,
},
{
id: userToDelete,
@@ -314,6 +326,7 @@ describe("delete should delete user", () => {
password: null,
salt: null,
homeBoardId: null,
provider: "oidc" as const,
},
];

View File

@@ -4,12 +4,17 @@ import { createSaltAsync, hashPasswordAsync } from "@homarr/auth";
import type { Database } from "@homarr/db";
import { and, createId, eq, schema } from "@homarr/db";
import { groupMembers, groupPermissions, groups, invites, users } from "@homarr/db/schema/sqlite";
import type { SupportedAuthProvider } from "@homarr/definitions";
import { logger } from "@homarr/log";
import { validation, z } from "@homarr/validation";
import { createTRPCRouter, protectedProcedure, publicProcedure } from "../trpc";
import { throwIfCredentialsDisabled } from "./invite/checks";
export const userRouter = createTRPCRouter({
initUser: publicProcedure.input(validation.user.init).mutation(async ({ ctx, input }) => {
throwIfCredentialsDisabled();
const firstUser = await ctx.db.query.users.findFirst({
columns: {
id: true,
@@ -40,6 +45,7 @@ export const userRouter = createTRPCRouter({
});
}),
register: publicProcedure.input(validation.user.registrationApi).mutation(async ({ ctx, input }) => {
throwIfCredentialsDisabled();
const inviteWhere = and(eq(invites.id, input.inviteId), eq(invites.token, input.token));
const dbInvite = await ctx.db.query.invites.findFirst({
columns: {
@@ -56,7 +62,7 @@ export const userRouter = createTRPCRouter({
});
}
await checkUsernameAlreadyTakenAndThrowAsync(ctx.db, input.username);
await checkUsernameAlreadyTakenAndThrowAsync(ctx.db, "credentials", input.username);
await createUserAsync(ctx.db, input);
@@ -64,7 +70,8 @@ export const userRouter = createTRPCRouter({
await ctx.db.delete(invites).where(inviteWhere);
}),
create: publicProcedure.input(validation.user.create).mutation(async ({ ctx, input }) => {
await checkUsernameAlreadyTakenAndThrowAsync(ctx.db, input.username);
throwIfCredentialsDisabled();
await checkUsernameAlreadyTakenAndThrowAsync(ctx.db, "credentials", input.username);
await createUserAsync(ctx.db, input);
}),
@@ -93,6 +100,7 @@ export const userRouter = createTRPCRouter({
columns: {
id: true,
image: true,
provider: true,
},
where: eq(users.id, input.userId),
});
@@ -104,6 +112,13 @@ export const userRouter = createTRPCRouter({
});
}
if (user.provider !== "credentials") {
throw new TRPCError({
code: "FORBIDDEN",
message: "Profile image can not be changed for users with external providers",
});
}
await ctx.db
.update(users)
.set({
@@ -112,13 +127,14 @@ export const userRouter = createTRPCRouter({
.where(eq(users.id, input.userId));
}),
getAll: publicProcedure.query(async ({ ctx }) => {
return ctx.db.query.users.findMany({
return await ctx.db.query.users.findMany({
columns: {
id: true,
name: true,
email: true,
emailVerified: true,
image: true,
provider: true,
},
});
}),
@@ -139,6 +155,7 @@ export const userRouter = createTRPCRouter({
email: true,
emailVerified: true,
image: true,
provider: true,
},
where: eq(users.id, input.userId),
});
@@ -154,7 +171,7 @@ export const userRouter = createTRPCRouter({
}),
editProfile: publicProcedure.input(validation.user.editProfile).mutation(async ({ input, ctx }) => {
const user = await ctx.db.query.users.findFirst({
columns: { email: true },
columns: { email: true, provider: true },
where: eq(users.id, input.id),
});
@@ -165,7 +182,14 @@ export const userRouter = createTRPCRouter({
});
}
await checkUsernameAlreadyTakenAndThrowAsync(ctx.db, input.name, input.id);
if (user.provider !== "credentials") {
throw new TRPCError({
code: "FORBIDDEN",
message: "Username and email can not be changed for users with external providers",
});
}
await checkUsernameAlreadyTakenAndThrowAsync(ctx.db, "credentials", input.name, input.id);
const emailDirty = input.email && user.email !== input.email;
await ctx.db
@@ -190,26 +214,38 @@ export const userRouter = createTRPCRouter({
});
}
const dbUser = await ctx.db.query.users.findFirst({
columns: {
id: true,
password: true,
salt: true,
provider: true,
},
where: eq(users.id, input.userId),
});
if (!dbUser) {
throw new TRPCError({
code: "NOT_FOUND",
message: "User not found",
});
}
if (dbUser.provider !== "credentials") {
throw new TRPCError({
code: "FORBIDDEN",
message: "Password can not be changed for users with external providers",
});
}
// Admins can change the password of other users without providing the previous password
const isPreviousPasswordRequired = ctx.session.user.id === input.userId;
logger.info(
`User ${user.id} is changing password for user ${input.userId}, previous password is required: ${isPreviousPasswordRequired}`,
);
if (isPreviousPasswordRequired) {
const dbUser = await ctx.db.query.users.findFirst({
columns: {
id: true,
password: true,
salt: true,
},
where: eq(users.id, input.userId),
});
if (!dbUser) {
throw new TRPCError({
code: "NOT_FOUND",
message: "User not found",
});
}
const previousPasswordHash = await hashPasswordAsync(input.previousPassword, dbUser.salt ?? "");
const isValid = previousPasswordHash === dbUser.password;
@@ -249,9 +285,14 @@ const createUserAsync = async (db: Database, input: z.infer<typeof validation.us
return userId;
};
const checkUsernameAlreadyTakenAndThrowAsync = async (db: Database, username: string, ignoreId?: string) => {
const checkUsernameAlreadyTakenAndThrowAsync = async (
db: Database,
provider: SupportedAuthProvider,
username: string,
ignoreId?: string,
) => {
const user = await db.query.users.findFirst({
where: eq(users.name, username.toLowerCase()),
where: and(eq(users.name, username.toLowerCase()), eq(users.provider, provider)),
});
if (!user) return;