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

Feat: implement OAuth/OIDC sign on with Authlib #414

Merged
merged 33 commits into from
Apr 15, 2022
Merged
Show file tree
Hide file tree
Changes from 30 commits
Commits
Show all changes
33 commits
Select commit Hold shift + click to select a range
dc4a927
feat: add minimal login path
afeld Apr 7, 2022
5f30b74
fix: avoid name conflict on auth apps
afeld Apr 7, 2022
2424110
fix: remove use of default_app_config
afeld Apr 7, 2022
c43197d
feat: implement minimal authentication with Django auth
afeld Apr 7, 2022
be456f1
feat: add an endpoint for checking login state
afeld Apr 7, 2022
7c08132
fix: remove auth from session helper
afeld Apr 7, 2022
9c17907
feat(dependencies): add Authlib
thekaveman Apr 8, 2022
3e39842
feat(settings): configure OIDC Authlib client
thekaveman Apr 9, 2022
893230e
Revert "fix: remove auth from session helper"
thekaveman Apr 8, 2022
56db601
feat(auth): authorize redirect login and token verify
thekaveman Apr 10, 2022
396a874
feat(eligibility): update origin to start page
thekaveman Apr 10, 2022
71f01d8
feat(eligibility): point login button at route
thekaveman Apr 10, 2022
75ee969
fix(settings): session cookie samesite to Lax
thekaveman Apr 10, 2022
7af5d42
refactor(oauth): name app to reflect focus
thekaveman Apr 10, 2022
9d36dc6
refactor(oauth): rename route to match oauth language
thekaveman Apr 10, 2022
264b65f
fix(eligibility): button label comes from AuthProvider model
thekaveman Apr 11, 2022
5ff71c9
fix(urls): update authorize view name
thekaveman Apr 11, 2022
84298be
fix(oauth): ensure absolute redirect_uri
thekaveman Apr 11, 2022
f408e50
fix(oauth): client must be registered
thekaveman Apr 12, 2022
fc413a6
refactor(oauth): move client name to setting
thekaveman Apr 12, 2022
3fe158f
refactor(urls): use new client name setting
thekaveman Apr 12, 2022
8b089af
refactor(oauth): simplify client and session management
thekaveman Apr 12, 2022
856e60c
fix(oauth): only register non-empty names
thekaveman Apr 12, 2022
842540b
docs(settings): add note about Lax session cookie
thekaveman Apr 12, 2022
f3d2b06
docs(oauth): add note clarifying token authorization step
thekaveman Apr 12, 2022
0da50ab
feat(eligibility): skip login button if session auth is True
thekaveman Apr 12, 2022
9394eb5
fix(eligibility): raise exception if missing OAuth client config
thekaveman Apr 13, 2022
68b6039
refactor(oauth): create registry where it is used
thekaveman Apr 13, 2022
beb84a3
fix(oauth): invalid token redirect back to start
thekaveman Apr 14, 2022
2aca3c8
refactor(oauth): use named route constants
thekaveman Apr 14, 2022
06eecda
Merge branch 'dev' into feat/oauth
thekaveman Apr 14, 2022
cf6764b
fix(tests): sample oauth environment for tests to pass
thekaveman Apr 15, 2022
0cbe73b
fix(env): better defaults in sample
thekaveman Apr 15, 2022
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions .devcontainer/.env.sample
Original file line number Diff line number Diff line change
Expand Up @@ -34,5 +34,10 @@ DJANGO_RATE_LIMIT=0
DJANGO_RATE_LIMIT_METHODS=GET,POST,PUT,DELETE
DJANGO_RATE_LIMIT_PERIOD=0

DJANGO_OAUTH_AUTHORITY=https://example.com
DJANGO_OAUTH_CLIENT_ID=client-id-123
DJANGO_OAUTH_CLIENT_NAME=client_name
DJANGO_OAUTH_SCOPE=scope

# tests config
CYPRESS_baseUrl=http://client:8000
3 changes: 0 additions & 3 deletions benefits/core/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,3 @@ class CoreAppConfig(AppConfig):
name = "benefits.core"
label = "core"
verbose_name = "Core"


default_app_config = "benefits.core.CoreAppConfig"
11 changes: 0 additions & 11 deletions benefits/core/migrations/0001_initial.py
Original file line number Diff line number Diff line change
Expand Up @@ -29,17 +29,6 @@ class Migration(migrations.Migration):
name="AuthProvider",
fields=[
("id", models.AutoField(primary_key=True, serialize=False)),
("authority", models.TextField(help_text="Authorization server address")),
("client_id", models.TextField(help_text="The application ID registered with auth server")),
("redirect_uri", models.TextField(help_text="Post-login, the user is redirected here")),
("post_logout_redirect_uri", models.TextField(help_text="Post-logout, the user is redirected here")),
("response_type", models.TextField(help_text='Must be "code" to use the authorization-code flow')),
(
"scope",
models.TextField(
help_text='Space-delimited list. If you need refresh tokens, you must add the "offline access" scope'
),
),
thekaveman marked this conversation as resolved.
Show resolved Hide resolved
("sign_in_button_label", models.TextField()),
("sign_out_button_label", models.TextField()),
],
Expand Down
8 changes: 0 additions & 8 deletions benefits/core/models.py
Original file line number Diff line number Diff line change
Expand Up @@ -32,17 +32,9 @@ def jwk(self):
class AuthProvider(models.Model):
"""An entity that provides authentication for eligibility verifiers."""

# fmt: off
id = models.AutoField(primary_key=True)
authority = models.TextField(help_text="Authorization server address")
client_id = models.TextField(help_text="The application ID registered with auth server")
redirect_uri = models.TextField(help_text="Post-login, the user is redirected here")
post_logout_redirect_uri = models.TextField(help_text="Post-logout, the user is redirected here")
response_type = models.TextField(help_text="Must be \"code\" to use the authorization-code flow")
scope = models.TextField(help_text="Space-delimited list. If you need refresh tokens, you must add the \"offline access\" scope") # noqa: 503
sign_in_button_label = models.TextField()
sign_out_button_label = models.TextField()
# fmt: on


class EligibilityType(models.Model):
Expand Down
3 changes: 0 additions & 3 deletions benefits/eligibility/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,3 @@ class EligibilityAppConfig(AppConfig):
name = "benefits.eligibility"
label = "eligibility"
verbose_name = "Eligibility Verification"


default_app_config = "benefits.eligibility.EligibilityAppConfig"
10 changes: 7 additions & 3 deletions benefits/eligibility/views.py
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
from benefits.core import middleware, recaptcha, session, viewmodels
from benefits.core.models import EligibilityVerifier
from benefits.core.views import PageTemplateResponse
from benefits.settings import OAUTH_CLIENT_NAME
from . import analytics, api, forms


Expand Down Expand Up @@ -58,12 +59,15 @@ def index(request):
def start(request):
"""View handler for the eligibility verification getting started screen."""

session.update(request, eligibility_types=[])
session.update(request, eligibility_types=[], origin=reverse("eligibility:start"))
verifier = session.verifier(request)

if verifier.requires_authentication:
if verifier.requires_authentication and not session.auth(request):
if OAUTH_CLIENT_NAME is None:
raise Exception("EligibilityVerifier requires authentication, but OAUTH_CLIENT_NAME is None")

auth_provider = verifier.auth_provider
button = viewmodels.Button.external(text=_(auth_provider.sign_in_button_label), url="#", id="login")
button = viewmodels.Button.external(text=_(auth_provider.sign_in_button_label), url=reverse("oauth:login"), id="login")
angela-tran marked this conversation as resolved.
Show resolved Hide resolved
else:
button = viewmodels.Button.primary(text=_("eligibility.buttons.continue"), url=reverse("eligibility:confirm"))

Expand Down
3 changes: 0 additions & 3 deletions benefits/enrollment/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,3 @@ class EnrollmentAppConfig(AppConfig):
name = "benefits.enrollment"
label = "enrollment"
verbose_name = "Benefits Enrollment"


default_app_config = "benefits.enrollment.EnrollmentAppConfig"
10 changes: 10 additions & 0 deletions benefits/oauth/__init__.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
"""
The oauth application: Implements OAuth-based authentication
"""
from django.apps import AppConfig


class OAuthAppConfig(AppConfig):
name = "benefits.oauth"
label = "oauth"
verbose_name = "Benefits OAuth"
11 changes: 11 additions & 0 deletions benefits/oauth/urls.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
from django.urls import path

from . import views


app_name = "oauth"
urlpatterns = [
# /oauth
path("login", views.login, name="login"),
path("authorize", views.authorize, name="authorize"),
]
42 changes: 42 additions & 0 deletions benefits/oauth/views.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,42 @@
from django.shortcuts import redirect
from django.urls import reverse

from authlib.integrations.django_client import OAuth

from benefits.core import session
from benefits.settings import OAUTH_CLIENT_NAME


if OAUTH_CLIENT_NAME:
_oauth = OAuth()
_oauth.register(OAUTH_CLIENT_NAME)
oauth_client = _oauth.create_client(OAUTH_CLIENT_NAME)


ROUTE_AUTH = "oauth:authorize"
ROUTE_START = "eligibility:start"
ROUTE_CONFIRM = "eligibility:confirm"


def login(request):
if not oauth_client:
raise Exception("No OAuth client")

route = reverse(ROUTE_AUTH)
redirect_uri = request.build_absolute_uri(route)

return oauth_client.authorize_redirect(request, redirect_uri)


def authorize(request):
if not oauth_client:
raise Exception("No OAuth client")

token = oauth_client.authorize_access_token(request)
thekaveman marked this conversation as resolved.
Show resolved Hide resolved

if token is None:
return redirect(ROUTE_START)
else:
# we are intentionally not storing anything about the user, including their token
session.update(request, auth=True)
thekaveman marked this conversation as resolved.
Show resolved Hide resolved
return redirect(ROUTE_CONFIRM)
24 changes: 23 additions & 1 deletion benefits/settings.py
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ def _filter_empty(ls):
"benefits.core",
"benefits.enrollment",
"benefits.eligibility",
"benefits.oauth",
]

if ADMIN:
Expand Down Expand Up @@ -70,9 +71,17 @@ def _filter_empty(ls):
CSRF_COOKIE_HTTPONLY = True
CSRF_TRUSTED_ORIGINS = _filter_empty(os.environ["DJANGO_TRUSTED_ORIGINS"].split(","))

SESSION_COOKIE_SAMESITE = "Strict"
# With `Strict`, the user loses their Django session between leaving our app to
# sign in with OAuth, and coming back into our app from the OAuth redirect.
# This is because `Strict` disallows our cookie being sent from an external
# domain and so the session cookie is lost.
#
# `Lax` allows the cookie to travel with the user and be sent back to us by the
# OAuth server, as long as the request is "safe" i.e. GET
SESSION_COOKIE_SAMESITE = "Lax"
thekaveman marked this conversation as resolved.
Show resolved Hide resolved
SESSION_ENGINE = "django.contrib.sessions.backends.signed_cookies"
SESSION_EXPIRE_AT_BROWSER_CLOSE = True
SESSION_COOKIE_NAME = "_benefitssessionid"

if not DEBUG:
CSRF_COOKIE_SECURE = True
Expand Down Expand Up @@ -148,6 +157,19 @@ def _filter_empty(ls):
]
)

# OAuth configuration

OAUTH_CLIENT_NAME = os.environ.get("DJANGO_OAUTH_CLIENT_NAME")

if OAUTH_CLIENT_NAME:
AUTHLIB_OAUTH_CLIENTS = {
OAUTH_CLIENT_NAME: {
"client_id": os.environ.get("DJANGO_OAUTH_CLIENT_ID"),
"server_metadata_url": f"{os.environ.get('DJANGO_OAUTH_AUTHORITY')}/.well-known/openid-configuration",
"client_kwargs": {"code_challenge_method": "S256", "scope": os.environ.get("DJANGO_OAUTH_SCOPE")},
}
}

# Internationalization

LANGUAGE_CODE = "en"
Expand Down
10 changes: 8 additions & 2 deletions benefits/urls.py
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@

from django.urls import include, path

from benefits.settings import ADMIN
from benefits.settings import ADMIN, OAUTH_CLIENT_NAME


logger = logging.getLogger(__name__)
Expand All @@ -28,7 +28,13 @@
if ADMIN:
from django.contrib import admin

logger.debug("Register admin/ urls")
logger.debug("Register admin urls")
urlpatterns.append(path("admin/", admin.site.urls))
else:
logger.debug("Skip url registrations for admin")

if OAUTH_CLIENT_NAME:
logger.info("Register oauth urls")
urlpatterns.append(path("oauth/", include("benefits.oauth.urls")))
else:
logger.debug("Skip url registrations for oauth")
6 changes: 0 additions & 6 deletions fixtures/02_authprovider.json
Original file line number Diff line number Diff line change
Expand Up @@ -3,12 +3,6 @@
"model": "core.authprovider",
"pk": 1,
"fields": {
"authority": "https://example.com",
"client_id": "b2c086d3-2e78-4e1f-8dde-f4b162743048",
"redirect_uri": "https://example.com/redirect",
"post_logout_redirect_uri": "https://example.com/logout",
"response_type": "code",
"scope": "openid",
"sign_in_button_label": "eligibility.buttons.signin",
"sign_out_button_label": "eligibility.buttons.signout"
}
Expand Down
1 change: 1 addition & 0 deletions requirements.txt
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
Authlib==1.0.1
cryptography==36.0.2
Django==3.2.12
django-csp==3.7
Expand Down