Skip to content

[PR #1] LOLA: implementing authorization redirect includes activitypub_actor - #255

Merged
aaronjae22 merged 4 commits into
mainfrom
aaronaej/auth-redirect-includes-activitypub-actor
Jun 15, 2026
Merged

[PR #1] LOLA: implementing authorization redirect includes activitypub_actor#255
aaronjae22 merged 4 commits into
mainfrom
aaronaej/auth-redirect-includes-activitypub-actor

Conversation

@aaronjae22

@aaronjae22 aaronjae22 commented May 28, 2026

Copy link
Copy Markdown
Collaborator

Closes #253

This PR implements LOLA §5.3:

### 5.3 Source server response
If authorization is approved, the source server redirects the user back to the destination server with an authorization code with parameters:

code - the authorization code
state - the same random string passed by destination server
activitypub_actor - the ActivityPub Actor ID associated with the one account that access is granted to.

It is possible for the user to provide one Actor ID to the destination server, and then for the destination server to receive a different Actor ID in this response. The destination server MAY confirm this with the user but MUST use the Actor ID provided with the authorization code rather than the original one the user communicated.

Before this PR the source emitted only code and state. Now it will become code + state + activitypub_actor.

LOLA §5.3 indicates that the source server's authorization-approval redirect carry the three previously mentioned query parameters. code and state are produced by django-oauth-toolkit (DOT) as part of its standard OAuth 2.0 authorization-code.

The implementation already has the important backend security property, it does actor-bound tokens but it does not yet surface that actor identity back in the authorization redirect. This feature does not "invent" an actor identity, it just exposing, earlier, the same actor we use later when issuing the token.

A few things worth to mention:

  • The emitted activitypub_actor is an absolute actor URL, consistent with the project's actor URLs.
  • This feature also allow consistency between two moments in the process:
    • callback time: the destination needs to know which actor just granted access
    • token time: the source server bind the eventual access token to a specific actor
    • Currently, the token-binding side is already implemented in validators.py. What was missing was the earlier authorization redirect exposing the same actor identity to the destination
  • It fails closed. if the user has no source Actor, this implementation don't append a malformed/empty activitypub_actor. It just simple logs it and return DOT's redirect unchanged so the flow doesn't silently emit a half-shaped LOLA redirect. In practice, every user has a source Actor via the post-save signal, this just guards the edge case.
  • I only append for portability-scoped authorizations. If the requested scopes don't include activitypub_account_portability, I leave the redirect alone. Non-LOLA OAuth flows are unaffected.
  • An explicit path("oauth/authorize/", PortabilityAuthorizationView.as_view(), name="authorize") is registered before the path("oauth/", include("oauth2_provider.urls", namespace="oauth2_provider")). Django dispatch is first-match-wins, so /oauth/authorize/ resolves to the subclass while every other DOT endpoint (/oauth/token/, /oauth/revoke_token/, etc.) keeps resolving.
  • The subclass overrides form_valid (POST approval) and get() (which covers DOT's skip_authorization and approval_prompt=auto branches). These are the places where DOT 3.0.1 calls create_authorization_response() to build a success redirect, and they all are in the same class, so one helper covers all three with a single test per branch.
  • Two guards in _append_actor_to_redirect keep the parameter out of the wrong places: it only appends when the redirect's query carries code (so denial / error redirects like error=access_denied are left alone), and on the GET path it only appends when the response is an actual 3xx; the 200 consent-form render passes through untouched.
  • _add_query_param and _is_redirect are @staticmethod since I just wanted to pack these methods under one class for organization.
  • I did some updates related to a planning activitypub_bound_actor_id feature. I had the idea of having a "shared attribute" mechanism which will allow stampingrequest.activitypub_bound_actor_id = actor.pk in the view and reading it back in ActivityPubOAuth2Validator._save_bearer_token.
    • What actually kept the redirect actor and the token-bound actor in agreement was that both PortabilityAuthorizationView._resolve_source_actor() and ActivityPubOAuth2Validator._resolve_bound_actor() independently execute the same Actor.objects.get(user=user, role=ROLE_SOURCE) query, returning the same row because Actor.clean() enforces one source Actor per user.
    • I already know how to implement this feature but I thought it's out of scope for this task.
    • I would persist the chosen actor onto the Grant during save_authorization_code and reusing it during token issuance. Today's one-source-actor-per-user invariant makes the deterministic-lookup mechanism sufficient.
  • Corresponding tests included.

A bug that I need to take care of later on but worth mention now

First I decided to create a function build_absolute_actor_url and not use the existing build_actor_id.

def build_actor_id(actor_id, request):
    base_url = f"{request.scheme}://{request.get_host()}"
    return f"{base_url}/api/actors/{actor_id}"
def build_absolute_actor_url(actor_id, request):
    return request.build_absolute_uri(
        reverse("actor-detail", kwargs={"pk": actor_id})
    )
urlpatterns = [
    # Actor Endpoint: Retrieve actor details
    path("actors/<int:pk>/", actor_detail, name="actor-detail"),

The problem was that build_absolute_actor_url returns the reverse() form, which includes the trailing slash, while the JSON-LD id field everywhere else in the codebase emits the no-slash build-actor-id form.

Since LOLA §5.3 requires the destination to compare/use activitypub_actor as the Actor ID, if the two strings aren't identical, a strict comparison on the destination side fails even though the same Actor is involved.

So I chose the no-slash form which is the canonical for this project so far. It's already what every Actor publishes as id. Migrating everything to the reverse() slash form is a real refactor that touches JSON-LD output, existing tests, etc and is out of scope for this task.

So we have two scenarios over here:

Step Scenario A: activitypub_actor = no-slash (delegate to build_actor_id) Scenario B: activitypub_actor = with-slash (reverse() form)
1. Destination GETs activitypub_actor URL hits /api/actors/1 → 301 → /api/actors/1/ → 200 hits /api/actors/1/ → 200
2. JSON-LD response {"id": "http://host/api/actors/1"} (no slash — build_actor_id emits this) {"id": "http://host/api/actors/1"} (same — build_actor_id is unchanged)
3. Compare activitypub_actor against the document's id http://host/api/actors/1 == http://host/api/actors/1 → match http://host/api/actors/1/ != http://host/api/actors/1 → mismatch

Scenario A — send /api/actors/1 (no slash):

  1. Django's URL resolver looks for an exact match against /api/actors/1. No registered pattern matches because the only pattern is /api/actors/1/.
  2. Django's CommonMiddleware then uses APPEND_SLASH=True , which is the default. It tries appending a slash → /api/actors/1/. That matches a route, so the middleware returns an HTTP 301 redirect pointing the client at /api/actors/1/.
  3. The destination's HTTP client follows the 301 (default behavior of requests, curl, browsers, etc.) and re-sends a GET to /api/actors/1/.
  4. That request now matches the route, dispatches to the view, returns 200 with the JSON-LD.

The APPEND_SLASH only redirects for GET requests. For POST, etc, and unmatched no-slash URL would just return 404.

@aaronjae22 aaronjae22 self-assigned this Jun 2, 2026
@aaronjae22
aaronjae22 requested review from alexbainter and lisad June 2, 2026 04:51
@aaronjae22
aaronjae22 marked this pull request as ready for review June 2, 2026 04:52
@aaronjae22 aaronjae22 changed the title LOLA/implementating-authorization-redirect-includes-activitypub_actor LOLA: implementing authorization redirect includes activitypub_actor Jun 2, 2026
@aaronjae22 aaronjae22 changed the title LOLA: implementing authorization redirect includes activitypub_actor [PR #1] LOLA: implementing authorization redirect includes activitypub_actor Jun 4, 2026
@aaronjae22
aaronjae22 merged commit c642d55 into main Jun 15, 2026
3 checks passed
@aaronjae22
aaronjae22 deleted the aaronaej/auth-redirect-includes-activitypub-actor branch June 15, 2026 16:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Authorization Redirect Includes activitypub_actor

2 participants