How to perform complex API authorization in fewer SQL queries?

Viewed 466

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!

4 Answers
Related