diff --git a/apps/hash-api/src/graph/knowledge/primitive/entity/before-update-entity-hooks/user-before-update-entity-hook-callback.ts b/apps/hash-api/src/graph/knowledge/primitive/entity/before-update-entity-hooks/user-before-update-entity-hook-callback.ts index 586cdda0c83..052466fc221 100644 --- a/apps/hash-api/src/graph/knowledge/primitive/entity/before-update-entity-hooks/user-before-update-entity-hook-callback.ts +++ b/apps/hash-api/src/graph/knowledge/primitive/entity/before-update-entity-hooks/user-before-update-entity-hook-callback.ts @@ -163,7 +163,10 @@ export const userBeforeEntityUpdateHookCallback: BeforeUpdateEntityHookCallback ); if (updatedShortname) { - if (currentShortname && currentShortname !== updatedShortname) { + if ( + currentShortname && + currentShortname.toLowerCase() !== updatedShortname.toLowerCase() + ) { throw Error.badUserInput("Cannot change shortname"); } diff --git a/apps/hash-api/src/graph/knowledge/system-types/account.fields.ts b/apps/hash-api/src/graph/knowledge/system-types/account.fields.ts index 51a9d0b0a10..987fcff6943 100644 --- a/apps/hash-api/src/graph/knowledge/system-types/account.fields.ts +++ b/apps/hash-api/src/graph/knowledge/system-types/account.fields.ts @@ -107,7 +107,7 @@ export const shortnameIsRestricted: PureGraphFunction< { shortname: string }, boolean > = ({ shortname }): boolean => { - return RESTRICTED_SHORTNAMES.includes(shortname); + return RESTRICTED_SHORTNAMES.includes(shortname.toLowerCase()); }; // TODO: Depending on the approached chosen outlined in `get*ByShortname` functions, this function may be changed diff --git a/apps/hash-api/src/graph/knowledge/system-types/org.ts b/apps/hash-api/src/graph/knowledge/system-types/org.ts index 86ea139b682..ccc7952ebc3 100644 --- a/apps/hash-api/src/graph/knowledge/system-types/org.ts +++ b/apps/hash-api/src/graph/knowledge/system-types/org.ts @@ -190,7 +190,7 @@ export const createOrg: ImpureGraphFunction< const properties: OrganizationPropertiesWithMetadata = { value: { "https://hash.ai/@h/types/property-type/shortname/": { - value: shortname, + value: shortname.trim().toLowerCase(), metadata: { dataTypeId: "https://blockprotocol.org/@blockprotocol/types/data-type/text/v/1", @@ -319,7 +319,7 @@ export const getOrgByShortname: ImpureGraphFunction< systemPropertyTypes.shortname.propertyTypeBaseUrl, ], }, - { parameter: params.shortname }, + { parameter: params.shortname.trim().toLowerCase() }, ], }, ], diff --git a/apps/hash-api/src/graph/knowledge/system-types/user.ts b/apps/hash-api/src/graph/knowledge/system-types/user.ts index 9fb7b5e39bc..b2e905793e3 100644 --- a/apps/hash-api/src/graph/knowledge/system-types/user.ts +++ b/apps/hash-api/src/graph/knowledge/system-types/user.ts @@ -264,7 +264,7 @@ export const getUser: ImpureGraphFunction< systemPropertyTypes.shortname.propertyTypeBaseUrl, ], }, - { parameter: knownShortname }, + { parameter: knownShortname?.trim().toLowerCase() }, ], }; } @@ -430,7 +430,7 @@ export const createUser: ImpureGraphFunction< ...(shortname !== undefined ? { "https://hash.ai/@h/types/property-type/shortname/": { - value: shortname, + value: shortname.trim().toLowerCase(), metadata: { dataTypeId: "https://blockprotocol.org/@blockprotocol/types/data-type/text/v/1", diff --git a/libs/@local/graph/migrations/graph-migrations/v002__principals/down.sql b/libs/@local/graph/migrations/graph-migrations/v002__principals/down.sql index 61b3de91ed0..b9e05eba9c3 100644 --- a/libs/@local/graph/migrations/graph-migrations/v002__principals/down.sql +++ b/libs/@local/graph/migrations/graph-migrations/v002__principals/down.sql @@ -34,7 +34,9 @@ DROP TABLE team; DROP TRIGGER web_prevent_delete_trigger ON web; DROP TRIGGER web_register_trigger ON web; +DROP TRIGGER web_normalize_shortname_trigger ON web; DROP FUNCTION register_web(); +DROP FUNCTION normalize_web_shortname(); DROP TABLE web; -- Drop actor group triggers diff --git a/libs/@local/graph/migrations/graph-migrations/v002__principals/up.sql b/libs/@local/graph/migrations/graph-migrations/v002__principals/up.sql index 7b581480ec3..434dea86b6e 100644 --- a/libs/@local/graph/migrations/graph-migrations/v002__principals/up.sql +++ b/libs/@local/graph/migrations/graph-migrations/v002__principals/up.sql @@ -260,6 +260,21 @@ CREATE TABLE web ( CREATE UNIQUE INDEX idx_web_shortname ON web (shortname) WHERE shortname IS NOT NULL; +-- Trigger to normalize shortnames to lowercase on insert/update +CREATE FUNCTION normalize_web_shortname() +RETURNS TRIGGER AS $$ +BEGIN + IF NEW.shortname IS NOT NULL THEN + NEW.shortname := lower(trim(NEW.shortname)); + END IF; + RETURN NEW; +END; +$$ LANGUAGE plpgsql; + +CREATE TRIGGER web_normalize_shortname_trigger +BEFORE INSERT OR UPDATE OF shortname ON web +FOR EACH ROW EXECUTE FUNCTION normalize_web_shortname(); + -- Web registration trigger - creates actor group record when web is created CREATE FUNCTION register_web() RETURNS TRIGGER AS $$ diff --git a/libs/@local/graph/postgres-store/postgres_migrations/V49__case_insensitive_shortname.sql b/libs/@local/graph/postgres-store/postgres_migrations/V49__case_insensitive_shortname.sql new file mode 100644 index 00000000000..756d6fd2be8 --- /dev/null +++ b/libs/@local/graph/postgres-store/postgres_migrations/V49__case_insensitive_shortname.sql @@ -0,0 +1,17 @@ +-- Trigger to normalize shortnames to lowercase and trimmed on insert/update +CREATE FUNCTION normalize_web_shortname() +RETURNS TRIGGER AS $$ +BEGIN + IF NEW.shortname IS NOT NULL THEN + NEW.shortname := lower(trim(NEW.shortname)); + END IF; + RETURN NEW; +END; +$$ LANGUAGE plpgsql; + +CREATE TRIGGER web_normalize_shortname_trigger +BEFORE INSERT OR UPDATE OF shortname ON web +FOR EACH ROW EXECUTE FUNCTION normalize_web_shortname(); + +-- Normalize existing shortnames +UPDATE web SET shortname = lower(trim(shortname)) WHERE shortname IS NOT NULL AND shortname <> lower(trim(shortname)); diff --git a/libs/@local/graph/postgres-store/src/store/postgres/mod.rs b/libs/@local/graph/postgres-store/src/store/postgres/mod.rs index 93c0d7f2d39..5fd8a492611 100644 --- a/libs/@local/graph/postgres-store/src/store/postgres/mod.rs +++ b/libs/@local/graph/postgres-store/src/store/postgres/mod.rs @@ -3942,11 +3942,12 @@ impl AccountStore for PostgresStore { " SELECT web.id, + web.shortname, array_remove(array_agg(role.id), NULL) FROM web LEFT OUTER JOIN role ON web.id = role.actor_group_id - WHERE web.shortname = $1 - GROUP BY web.id + WHERE web.shortname = LOWER(TRIM($1)) + GROUP BY web.id, web.shortname ", &[&shortname], ) @@ -3959,10 +3960,10 @@ impl AccountStore for PostgresStore { .await .change_context(WebRetrievalError)? .map(|row| { - let role_ids = row.get::<_, Vec>(1); + let role_ids = row.get::<_, Vec>(2); Web { id: row.get(0), - shortname: Some(shortname.to_owned()), + shortname: row.get(1), roles: role_ids.into_iter().collect(), } })) diff --git a/libs/@local/graph/sdk/typescript/src/entity.ts b/libs/@local/graph/sdk/typescript/src/entity.ts index 0d318f26b0e..b0c474c160f 100644 --- a/libs/@local/graph/sdk/typescript/src/entity.ts +++ b/libs/@local/graph/sdk/typescript/src/entity.ts @@ -1228,6 +1228,18 @@ export class HashEntity< } } } + + if (targetBaseUrl === shortnamePropertyBaseUrl) { + if (patch.op === "remove") { + throw new Error("Cannot remove the shortname of a user"); + } + + if (typeof patch.property.value !== "string") { + throw new Error("Shortname must be a string"); + } + + patch.property.value = patch.property.value.trim().toLowerCase(); + } } } @@ -1249,11 +1261,17 @@ export class HashEntity< throw new Error("Cannot remove the organization shortname"); } - if ( - patch.property.value !== this.properties[shortnamePropertyBaseUrl] - ) { + if (typeof patch.property.value !== "string") { + throw new Error("Shortname must be a string"); + } + + const normalizedPatch = patch.property.value.trim().toLowerCase(); + const stored = this.properties[shortnamePropertyBaseUrl]; + if (normalizedPatch !== stored) { throw new Error("Cannot change the shortname of an organization"); } + + patch.property.value = normalizedPatch; } if (patch.path[0] === organizationNamePropertyBaseUrl) { diff --git a/tests/hash-backend-integration/src/tests/graph/knowledge/system-types/org.test.ts b/tests/hash-backend-integration/src/tests/graph/knowledge/system-types/org.test.ts index 055b32b82e0..a9db127480d 100644 --- a/tests/hash-backend-integration/src/tests/graph/knowledge/system-types/org.test.ts +++ b/tests/hash-backend-integration/src/tests/graph/knowledge/system-types/org.test.ts @@ -1,6 +1,7 @@ import { ensureSystemGraphIsInitialized } from "@apps/hash-api/src/graph/ensure-system-graph-is-initialized"; import type { Org } from "@apps/hash-api/src/graph/knowledge/system-types/org"; import { + createOrg, getOrgByShortname, updateOrgName, } from "@apps/hash-api/src/graph/knowledge/system-types/org"; @@ -68,6 +69,37 @@ describe("Org", () => { expect(fetchedOrg).toEqual(createdOrg); }); + it("can get an org by its shortname with different casing", async () => { + const authentication = { actorId: systemAccountId }; + + const fetchedOrg = await getOrgByShortname(graphContext, authentication, { + shortname: shortname.toUpperCase(), + }); + + expect(fetchedOrg).toEqual(createdOrg); + }); + + it("cannot create an org with a shortname differing only in case", async () => { + const authentication = { actorId: systemAccountId }; + + await expect( + createOrg(graphContext, authentication, { + name: "Case test org", + shortname: shortname.toUpperCase(), + }), + ).rejects.toThrowError("already exists"); + }); + + it("can get an org by its shortname with leading/trailing whitespace", async () => { + const authentication = { actorId: systemAccountId }; + + const fetchedOrg = await getOrgByShortname(graphContext, authentication, { + shortname: ` ${shortname} `, + }); + + expect(fetchedOrg).toEqual(createdOrg); + }); + it("can read the org roles", async () => { const authentication = { actorId: systemAccountId }; diff --git a/tests/hash-backend-integration/src/tests/graph/knowledge/system-types/user.test.ts b/tests/hash-backend-integration/src/tests/graph/knowledge/system-types/user.test.ts index 013304ad820..7cb067c3b64 100644 --- a/tests/hash-backend-integration/src/tests/graph/knowledge/system-types/user.test.ts +++ b/tests/hash-backend-integration/src/tests/graph/knowledge/system-types/user.test.ts @@ -143,6 +143,48 @@ describe("User model class", () => { expect(fetchedUser).toEqual(createdUser); }); + it("can get a user by its shortname with different casing", async () => { + const authentication = { actorId: createdUser.accountId }; + + const fetchedUser = await getUser(graphContext, authentication, { + shortname: shortname.toUpperCase(), + }); + + expect(fetchedUser).not.toBeNull(); + expect(fetchedUser).toEqual(createdUser); + }); + + it("cannot create a user with a shortname differing only in case", async () => { + const authentication = { actorId: systemAccountId }; + + const identity = await createKratosIdentity({ + traits: { + emails: ["case-test-user@example.com"], + }, + verifyEmails: true, + }); + + await expect( + createUser(graphContext, authentication, { + emails: ["case-test-user@example.com"], + kratosIdentityId: identity.id, + shortname: shortname.toUpperCase(), + displayName: "Case Test", + }), + ).rejects.toThrowError("already exists"); + }); + + it("can get a user by its shortname with leading/trailing whitespace", async () => { + const authentication = { actorId: createdUser.accountId }; + + const fetchedUser = await getUser(graphContext, authentication, { + shortname: ` ${shortname} `, + }); + + expect(fetchedUser).not.toBeNull(); + expect(fetchedUser).toEqual(createdUser); + }); + it("can get a user by its kratos identity id", async () => { const authentication = { actorId: createdUser.accountId };