I'm trying to add an authorization layer to an API, and the current design I have results in more SQL queries than it feels like should be required, so I'm wondering how I can simplify this.
Context
Here's the database schema for this piece of the problem:
CREATE TABLE IF NOT EXISTS users (
id TEXT PRIMARY KEY,
email CITEXT NOT NULL UNIQUE,
password TEXT NOT NULL,
name TEXT NOT NULL,
created_at DATE NOT NULL DEFAULT CURRENT_TIMESTAMP
);
CREATE TABLE IF NOT EXISTS teams (
id TEXT PRIMARY KEY,
email CITEXT NOT NULL,
name TEXT NOT NULL,
created_at DATE NOT NULL DEFAULT CURRENT_TIMESTAMP
);
CREATE TABLE IF NOT EXISTS memberships (
id TEXT PRIMARY KEY,
"user" TEXT NOT NULL REFERENCES users(id) ON UPDATE CASCADE ON DELETE CASCADE,
team TEXT NOT NULL REFERENCES teams(id) ON UPDATE CASCADE ON DELETE CASCADE,
role TEXT NOT NULL,
created_at DATE NOT NULL DEFAULT CURRENT_TIMESTAMP,
UNIQUE("user", team)
);
And the API endpoint in question is GET /users/:user/teams
, which returns all of the teams a user is a member of. Here's what the controller for that route looks like:
(Note: all of this is Javascript, but it's been sort of pseudocode'd for clarity.)
async getTeams(currentId, userId) {
await exists(userId)
await canFindTeams(currentUser, userId)
let teams = await findTeams(userId)
let maskedTeams = await maskTeams(currentUser, teams)
return maskedTeams
}
Those four asynchronous functions are the core logical steps that need to happen for the authorization to be "complete". Here's what each of those functions roughly looks like:
async exists(userId) {
let user = await query(`
SELECT id
FROM users
WHERE id = $[userId]
`)
if (!user) throw new Error('user_not_found')
return user
}
exists
simply checks whether a user by that userId
even exists in the database, and throws the proper error code if not.
query
is just pseudocode for running a SQL query with escaped variables.
async canFindTeams(currentUser, userId) {
if (currentUser.id == userId) return
let isTeammate = await query(`
SELECT role
FROM memberships
WHERE "user" = $[currentUser.id]
AND team IN (
SELECT team
FROM memberships
WHERE "user" = $[userId]
)
`)
if (!isTeammate) throw new Error('team_find_unauthorized')
}
canFindTeams
ensures that either the current user is the one making the request, or that the current user is a teammate of the user in question. Anyone else should not be authorized to find the user in question. In my real implementation, it's actually done with roles
that have associated actions
, so that a teammate can teams.read
but can't teams.admin
unless they are an own. But I simplified that for this example.
async findTeams(userId) {
return await query(`
SELECT
teams.id,
teams.email,
teams.name,
teams.created_at
FROM teams
LEFT JOIN memberships ON teams.id = memberships.team
LEFT JOIN users ON users.id = memberships.user
WHERE users.id = $[userId]
ORDER BY
memberships.created_at DESC,
teams.id
`)
}
findTeams
will actually query the database for the teams objects.
async maskTeams(currentUser, teams) {
let memberships = await query(`
SELECT team
FROM memberships
WHERE "user" = $[currentUser.id]
`)
let teamIds = memberships.map(membership => membership.team)
let maskedTeams = teams.filter(team => teamIds.includes(team.id))
return maskedTeams
}
maskTeams
will return only the teams that a given user should see. This is needed because a user should be able to see all of their teams, but teammates should only be able to see their teams in common, so as to not leak information.
Problems
One of the requirements that led me to break it up like this is that I need a way to throw those specific error codes, so that the errors returned to API clients are helpful. For example, the exists
function runs before the canFindTeams
function so that not everything errors with a 403 Unauthorized
.
Another, that's not well communicated here in pseudocode, is that the currentUser
can actually be an app
(a third-party client), a team
(an access token that pertains to the team itself) or a user
(the common case). This requirement makes it difficult to implement the canFindTeams
or the maskTeams
function as single SQL statements, since the logic has to be forked three ways... In my implementation, both functions are actually switch statements around the logic for handling all three cases—that the requester is an app
, a team
and a user
.
But even given those constraints, this feels like a lot of extra code to write to ensure all of these authentication requirements. I'm worried about performance, code maintainability, and also about the fact that these queries aren't all in single transactions.
Questions
- Do the extra queries meaningfully affect performance?
- Can they be combined into fewer queries easily?
- Is there a better design for the authorization that simplifies this?
- Does not using transactions pose problems?
- Anything else you'd change?
Thanks!