Configuration option to determine permissions with a SQL query

[simonw]

It would be neat if you could add a SQL query to the plugins configuration which helps determine if the current user should be allowed access.

That way users could maintain their own table of allowed GitHub users in one of the attached SQLite databases, hence being able to grant access to new individuals without editing the plugin configuration and restarting the server.

Something like this could work:

{
    "plugins": {
        "datasette-auth-github": {
            "client_id": {"$env": "GITHUB_CLIENT_ID"},
            "client_secret": {"$env": "GITHUB_CLIENT_SECRET"},
            "allow_sql": "select 1 from users where github_login = :username and staff = 1",
            "allow_sql_database": "userdb"
        }
    }
}

The allow_sql_database property specifies which attached database the query should be executed against. It would be optional: if you leave it off then the first (or ideally the only) attached database would be used.

[simonw]

Getting this to work with orgs and teams is a bit less obvious. We don't have a guaranteed efficient method of retrieving ALL of the orgs and teams that a user is a member of - instead, the existing code works by running an HTTP check for "is user X a member of team/org Y" for each of the configured options:

datasette-auth-github/datasette_auth_github/github_auth.py

Lines 181 to 210 in bf01f8f

	if self.allow_orgs is not None:
	# For each org, check if user is a member
	for org in force_list(self.allow_orgs):
	url = "https://api.github.com/orgs/{}/memberships/{}?access_token={}".format(
	org, auth["username"], access_token
	)
	response = await self.http_request(url)
	if response.status_code == 200 and response.json()["state"] == "active":
	return True
	if self.allow_teams is not None:
	for team in force_list(self.allow_teams):
	if team not in self.team_to_team_id:
	# Look up the team_id using the GitHub API
	org_slug, _, team_slug = team.partition("/")
	lookup_url = "https://api.github.com/orgs/{}/teams/{}?access_token={}".format(
	org_slug, team_slug, access_token
	)
	response = await self.http_request(lookup_url)
	if response.status_code == 200:
	self.team_to_team_id[team] = response.json()["id"]
	else:
	return False
	team_id = self.team_to_team_id[team]
	# Check if the user is a member of this team
	team_membership_url = "https://api.github.com/teams/{}/memberships/{}?access_token={}".format(
	team_id, auth["username"], access_token
	)
	response = await self.http_request(team_membership_url)
	if response.status_code == 200 and response.json()["state"] == "active":
	return True

So... maybe the SQL query should have an option of returning an instruction that says "check if the user is a member of these orgs/teams". Perhaps it could return results that look like this:

entity	value
org	myorg
team	otherorg/team1

This would mean "behave as if "allow_orgs" was ["myorg"] and "allow_teams" was ["otherorg/team1"]

[simonw]

In terms of available binding parameters for use in the SQL query, we could use anything in the auth cookie:

datasette-auth-github/datasette_auth_github/github_auth.py

Lines 254 to 259 in bf01f8f

	auth = {
	"id": str(profile["id"]),
	"name": profile["name"],
	"username": profile["login"],
	"email": profile["email"],
	}

I think we should only use id or username, because name could be set to anything and I'm not confident that GitHub don't allow users to set their email address to something that isn't verified as belonging to them.

[simonw]

I don't like :id as a named parameter though. I'm going to go with :user_id and :user_username as the two available parameters for the SQL query.

The sqlite3 module is fine with us sending a dictionary of parameters that aren't actually referenced in the query:

>>> sqlite3.connect(":memory:").execute("select 1", {"user": "foo"}).fetchall()                       
[(1,)]

[simonw]

I think there are two allowed return types for the SQL query.

It can return a single row with a single value of 1' which means "let this user in"

OR it can return one or more rows of two columns, where the first column is a entity type and the second column is an allowed value for that entity. This mirrors the existing allow_users / allow_teams / allow_orgs mechanism.

In fact, the returned query results should look like this for consistency with the plugin config language:

setting	value
allow_users	simonw
allow_users	cleopaws
allow_orgs	myorg
allow_teams	otherorg/team1

[simonw]

Spotted an implementation challenge: in order to execute a SQL query against an attached database, we need access to a datasette instance.

That's not generally available in the GitHubAuth class, because it's designed to be usable as ASGI middleware outside of Datasette.

It IS available in the plugin hook though (so that we can read the Datasette plugin configuration):

datasette-auth-github/datasette_auth_github/__init__.py

Lines 6 to 8 in b3aaa4c

	@hookimpl
	def asgi_wrapper(datasette):
	config = datasette.plugin_config("datasette-auth-github") or {}

So maybe the way to implement this is with a DatasetteGitHubAuth sub-class which is used only in the Datasette plugin context and which over-rides the existing async def user_is_allowed(self, auth, access_token): method.

[simonw]

This can happen in Datasette core instead: simonw/datasette#801