Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Integrate with new authentication system in Datasette 0.44 #62

Closed
simonw opened this issue Jun 8, 2020 · 17 comments
Closed

Integrate with new authentication system in Datasette 0.44 #62

simonw opened this issue Jun 8, 2020 · 17 comments
Labels
enhancement New feature or request
Milestone

Comments

@simonw
Copy link
Owner

simonw commented Jun 8, 2020

simonw/datasette#806

@simonw
Copy link
Owner Author

simonw commented Jun 9, 2020

I'm going to try to do this in a way that preserves the ability for the plugin to still work as a standalone ASGI middleware. This could be hard.

@simonw simonw added this to the 1.0 milestone Jun 9, 2020
@simonw
Copy link
Owner Author

simonw commented Jun 10, 2020

I don't actually need to implement actor_from_request here - I can sign the user in with GitHub and then set the ds_actor cookie: https://datasette.readthedocs.io/en/latest/authentication.html#the-ds-actor-cookie

@simonw
Copy link
Owner Author

simonw commented Jun 10, 2020

I'm going to leave the allow_orgs etc stuff in here (as opposed to replacing it with the new Datasette permissions stuff). It's useful for the ASGI middleware variant plus it can act as an initial limit on which users get an auth cookie set.

@simonw
Copy link
Owner Author

simonw commented Jun 10, 2020

This plugin currently only sets an authentication cookie that lasts for cookie_ttl seconds - defaulting to one hour. That's because this is intended as a SSO system where the user's credentials are re-checked frequently, to protect against them being removed from a GitHub organization but maintaining a valid cookie for Datasette.

The Datasette ds_actor cookie doesn't have a mechanism for setting a signed expiry yet. I could add one - I'd need to add it to the datasette.sign() method. I would need to use https://itsdangerous.palletsprojects.com/en/1.1.x/url_safe/ itsdangerous.url_safe.URLSafeTimedSerializer

@simonw
Copy link
Owner Author

simonw commented Jun 10, 2020

Maybe this would be easier if I renamed this existing project asgi-auth-github and created a brand new datasette-auth-github that doesn't attempt to solve both problems?

@simonw
Copy link
Owner Author

simonw commented Jun 10, 2020

Also tricky: the "stay logged out" cookie. This prevents the automatic redirect-based SSO mechanism from kicking in if the user has explicitly logged out:

output_cookies[self.logout_cookie_name] = "stay-logged-out"
output_cookies[self.logout_cookie_name]["path"] = "/"
headers.append(["set-cookie", output_cookies.output(header="").lstrip()])
await send_html(send, "", 302, headers)

@simonw
Copy link
Owner Author

simonw commented Jun 10, 2020

I'm going to fork the existing codebase as asgi-auth-github and ship datasette-auth-github 1.0 as supporting Datasette only.

@simonw
Copy link
Owner Author

simonw commented Jun 10, 2020

Without the ASGI stuff this gets a whole lot easier. I think it may even be reduceable to just two parts:

@simonw
Copy link
Owner Author

simonw commented Jun 10, 2020

I can further simplify the code by dropping the allow_orgs / allow_teams stuff in favour of a mechanism that checks for membership of specific teams/orgs and then bakes those into the actor object. Then they can be checked using allow blocks: https://datasette.readthedocs.io/en/latest/authentication.html#defining-permissions-with-allow-blocks

@simonw
Copy link
Owner Author

simonw commented Jun 10, 2020

The teams/orgs stuff means that setting an expiry on the cookie is really important. I'm going to upgrade Datasette's signing infrastructure to be able to set expiry, and I'll teach Datasette that the ds_actor cookie has a timestamp on it.

@simonw
Copy link
Owner Author

simonw commented Jun 11, 2020

I'm going to drop the "require_auth": false option in favour of the new permissions mechanism.

I should still document (and have a test for) how to do this. Should be the following:

{
    "allow": {"id": "*"}
}

@simonw
Copy link
Owner Author

simonw commented Jun 11, 2020

I need to decide what to do about the auto-login mechanism.

The current plugin requires authentication for all users, and automatically bounces through GitHub if authentication fails.

In the new plugin I want to default to auth not required, since I'm building on Datasette's new permissions layer.

So I think that means auto-login can still happen, but should only kick in on captured 403 errors.

@simonw
Copy link
Owner Author

simonw commented Jun 11, 2020

Made a start on the new README: https://github.com/simonw/datasette-auth-github/blob/f3eb2bcec1f80c57d851c69aa97a65d55db7061b/README.md

New ideas from that README update:

You can also restrict access to users who are members of a specific GitHub organization.

You'll need to configure the plugin to check if the user is a member of that organization when they first sign in. You can do that using the "load_orgs" plugin configuration option.

Then you can use "allow": {"gh_orgs": [...]} to specify which organizations are allowed access.

{
    "plugins": {
        "datasette-auth-github": {
            "...": "...",
            "load_orgs": ["your-organization"]
        }
    },
    "allow": {
        "gh_org": "your-organization"
    }
}

If your organization is arranged into teams you can restrict access to a specific team like this:

{
    "plugins": {
        "datasette-auth-github": {
            "...": "...",
            "load_teams": [
                "your-organization/staff",
                "your-organization/engineering",
            ]
        }
    },
    "allow": {
        "gh_team": "your-organization/engineering"
    }
}

@simonw
Copy link
Owner Author

simonw commented Jun 11, 2020

So I need to re-implement this code for the new load_teams and load_orgs mechanism:

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/{}".format(
org, auth["username"]
)
response = await self.http_request(
url, headers={"Authorization": "token {}".format(access_token)}
)
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/{}".format(
org_slug, team_slug
)
response = await self.http_request(
lookup_url,
headers={"Authorization": "token {}".format(access_token)},
)
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/{}".format(
team_id, auth["username"]
)
response = await self.http_request(
team_membership_url,
headers={"Authorization": "token {}".format(access_token)},
)
if response.status_code == 200 and response.json()["state"] == "active":
return True

@simonw
Copy link
Owner Author

simonw commented Jan 22, 2021

This is blocking people from using this plugin in collaboration with https://github.com/simonw/datasette-auth-tokens - which is bad! I should really get this fixed.

@simonw
Copy link
Owner Author

simonw commented Jan 22, 2021

I'm going to drop the auto-login mechanism entirely, it's not a good fit for the new auth system.

simonw added a commit that referenced this issue Jan 22, 2021
simonw added a commit that referenced this issue Jan 22, 2021
@simonw
Copy link
Owner Author

simonw commented Jan 24, 2021

Just needs some extra documentation now: #70

@simonw simonw closed this as completed Jan 24, 2021
simonw added a commit that referenced this issue Jan 24, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

1 participant