Logo Questions Linux Laravel Mysql Ubuntu Git Menu
 

How to perform complex API authorization in fewer SQL queries?

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!

like image 915
Ian Storm Taylor Avatar asked Mar 10 '16 07:03

Ian Storm Taylor


People also ask

Which SQL clause used to reduce the complexity of the queries which are lengthy?

1. Eliminate OR Clauses When Possible. The easiest way to make queries less complex is by eliminating OR clauses whenever possible. Because OR is inclusive, SQL Server has to process each component of the clause separately, which really slows down operations.

Why is SQL better for complex queries?

As your queries are complex, SQL is the way to go. MongoDB is what's known as a NoSQL database. It is very fast, however it sacrifices robustness and has weak relational guarantees. Contrasting, SQL is resistant to outages and guarantees consistency between queries.


1 Answers

I made it a function and simplified the tables just to be easier to test. SQL Fiddle. I'm making assumptions since some of the rules are embedded in the javascript pseudo code which I do not quite understand.

create or replace function visible_teams (
    _user_id int, _current_user_id int
) returns table (
    current_user_role int,
    team_id int,
    team_email text,
    team_name text,
    team_created_at date
) as $$
    select
        m0.role,
        m0.team,
        t.email,
        t.name,
        t.created_at
    from
        memberships m0
        left join
        memberships m1 on m0.team = m1.team and m1.user = _user_id
        inner join
        teams t on t.id = m0.team
    where m0.user = _current_user_id

    union

    select null, null, null, null, null
    where not exists (select 1 from users where id = _user_id)

    order by role nulls first
    ;
$$ language sql;

Returns all current user's teams plus the the user common teams:

select * from visible_teams(3, 1);
 current_user_role | team_id | team_email | team_name | team_created_at 
-------------------+---------+------------+-----------+-----------------
                 1 |       1 | email_1    | team_1    | 2016-03-13
                 1 |       3 | email_3    | team_3    | 2016-03-13
                 2 |       2 | email_2    | team_2    | 2016-03-13
(3 rows)

When the user does not exist it returns the first line containing nulls plus all current user's teams:

select * from visible_teams(5, 1);
 current_user_role | team_id | team_email | team_name | team_created_at 
-------------------+---------+------------+-----------+-----------------
                   |         |            |           | 
                 1 |       1 | email_1    | team_1    | 2016-03-13
                 1 |       3 | email_3    | team_3    | 2016-03-13
                 2 |       2 | email_2    | team_2    | 2016-03-13
(4 rows)

When the current user does not exist then an empty set:

select * from visible_teams(1, 5);
 current_user_role | team_id | team_email | team_name | team_created_at 
-------------------+---------+------------+-----------+-----------------
(0 rows)
like image 78
Clodoaldo Neto Avatar answered Oct 16 '22 03:10

Clodoaldo Neto