Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -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");
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
4 changes: 2 additions & 2 deletions apps/hash-api/src/graph/knowledge/system-types/org.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down Expand Up @@ -319,7 +319,7 @@ export const getOrgByShortname: ImpureGraphFunction<
systemPropertyTypes.shortname.propertyTypeBaseUrl,
],
},
{ parameter: params.shortname },
{ parameter: params.shortname.trim().toLowerCase() },
],
},
],
Expand Down
4 changes: 2 additions & 2 deletions apps/hash-api/src/graph/knowledge/system-types/user.ts
Original file line number Diff line number Diff line change
Expand Up @@ -264,7 +264,7 @@ export const getUser: ImpureGraphFunction<
systemPropertyTypes.shortname.propertyTypeBaseUrl,
],
},
{ parameter: knownShortname },
{ parameter: knownShortname?.trim().toLowerCase() },
],
};
}
Expand Down Expand Up @@ -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",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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 $$
Expand Down
Original file line number Diff line number Diff line change
@@ -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));
9 changes: 5 additions & 4 deletions libs/@local/graph/postgres-store/src/store/postgres/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3942,11 +3942,12 @@ impl<C: AsClient> AccountStore for PostgresStore<C> {
"
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],
)
Expand All @@ -3959,10 +3960,10 @@ impl<C: AsClient> AccountStore for PostgresStore<C> {
.await
.change_context(WebRetrievalError)?
.map(|row| {
let role_ids = row.get::<_, Vec<WebRoleId>>(1);
let role_ids = row.get::<_, Vec<WebRoleId>>(2);
Web {
id: row.get(0),
shortname: Some(shortname.to_owned()),
shortname: row.get(1),
roles: role_ids.into_iter().collect(),
}
}))
Expand Down
24 changes: 21 additions & 3 deletions libs/@local/graph/sdk/typescript/src/entity.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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();
}
}
}

Expand All @@ -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) {
Expand Down
Original file line number Diff line number Diff line change
@@ -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";
Expand Down Expand Up @@ -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 };

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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 };

Expand Down
Loading