Skip to content

Componentize OAuth Logic#9

Merged
germ-mark merged 50 commits into
mainfrom
mark/oauth4web-shadow
Mar 16, 2026
Merged

Componentize OAuth Logic#9
germ-mark merged 50 commits into
mainfrom
mark/oauth4web-shadow

Conversation

@germ-mark
Copy link
Copy Markdown
Contributor

@germ-mark germ-mark commented Mar 5, 2026

This is the next (last)? macro PR in building this out from OAuthenticator.

Context

So far the code sequence has largely followed OAuthenticator. In previous steps, the project was reorganized to separate out OAuth specific types and logic into the oauth4swift package, and break them free of the entangling with atproto concepts (did, handle resolution) that we introduced.

Prior to this PR, oauth4swift had very little request handling logic, the logic portions of oauth4swift were focused on state management (e.g. lazily calling refresh when making a protected resource call, and serializing those authorization refresh requests). This reflects the legacy of OAuthenticator, where the bulk of the logic is implemented per service, and passed as abstract handlers into the core Authenticator object (https://github.com/ChimeHQ/OAuthenticator/tree/main/Sources/OAuthenticator/Services). This became evident here when a lot of OAuth logic was sitting in AtprotoOAuth and not in the OAuth library. This PR aims to fix that.

OAuthenticator's primary component was the state management in Authenticator, which carries on in oauth4Swift. The motivation for embarking on this reorganization was to collect the service-independent logic as primitive components in the way that oauth4web did.

This pr starts doing so by paralleling the method naming of oauth4web, and shadowing the (mostly) pure functions it defines.

Divergence from oauth4web

  • Url construction differs slightly. We favor passing typed structs around such as URLRequest instead of headers, bodies, and parameter dictionaries, so we construct our url requests in a somewhat different order than in oauth4web
  • While oauth4web aims to be stateless (e.g. it doesn't handle refresh logic, which we do), DPoP handling undermines that statelessness. Making a dpop request requires getting, setting, and retrying with nonce, which oauth4web does. Rather than injecting an optional DPoP object as OAuthenticator and oauth4web do, I want to encapsulate the read, set, and retry logic of the mutable nonce state within the isolation of where those nonces are stored. Hence we have optional conformance to a DPoPSigner protocol, upon which we conditionally insert a proof.
  • There is an OAuthComponents namespace for pure functions, but it's mostly for response handling. Making requests will conditionally need to access mutable state, which means most of them need to be (protocol) methods which can at runtime determine if the object they're attached to is a DPoPSigner

ToDo

  • Clean up unused types, commented out code
  • Clean up the logic around optionally injecting Dpop proof and retry
  • (followup pr) add testing now that the OAuth API is more stable

@germ-mark germ-mark changed the title Componentize OAuth Logic WIP: Componentize OAuth Logic Mar 5, 2026
Copy link
Copy Markdown
Contributor

@ThisIsMissEm ThisIsMissEm left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Quite a lengthy review, I've added some explainers where necessary and raised a few questions.

Comment thread DemoApp/AtprotoOAuthDemoApp/CachedAuthenticated/LoginVM.swift Outdated
Comment thread LocalPackages/GermConvenience/Sources/GermConvenience/HTTPDataResponse.swift Outdated
Comment thread LocalPackages/GermConvenience/Sources/GermConvenience/HTTPDataResponse.swift Outdated
Comment thread LocalPackages/oauth4swift/Sources/OAuth/DPoP/DPoPResponse.swift Outdated
Comment thread LocalPackages/oauth4swift/Sources/OAuth/DPoP/DPoPResponse.swift Outdated
Comment thread LocalPackages/oauth4swift/Sources/OAuth/TokenEndpointRequest.swift Outdated
Comment on lines +77 to +78
//NOTE: oauth4web prepends this to the incoming path,
let requestUrl = issuer.appending(path: "/.well-known/oauth-authorization-server")
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is to support OIDC and deployments like keycloak, because .well-known is a well known mess, funnily enough the spec says:

Well-known URIs are rooted in the top of the path's hierarchy; they are not well-known by definition in other parts of the path. For example, "/.well-known/example" is a well-known URI, whereas "/foo/.well-known/example" is not.

And OIDC's like "nah bro, I know better":

[...] JSON document available at the path formed by concatenating the string /.well-known/openid-configuration to the Issuer.

Which gives you weird stuff like https://login.microsoftonline.com/common/v2.0/.well-known/openid-configuration or https://keycloak.example/realms/<name>/.well-known/openid-configuration

OAuth Authorization Server Metadata Discovery in trying to be compatible with OIDC carries through this "appending" nonsense which isn't actually "well known" according to the well known spec: https://datatracker.ietf.org/doc/html/rfc8414#section-3

That's why it's appending to the issuer.

Comment thread LocalPackages/oauth4swift/Sources/OAuth/TokenEndpointRequest.swift Outdated
Comment thread Sources/AtprotoOAuth/Session/SessionImpl.swift
Comment thread Sources/AtprotoOAuth/AuthorizerImpl.swift Outdated
germ-mark and others added 24 commits March 6, 2026 16:13
Co-authored-by: Emelia Smith <ThisIsMissEm@users.noreply.github.com>
@germ-mark
Copy link
Copy Markdown
Contributor Author

I think the structural pieces are basically in place. adding a few review questions

Comment thread Sources/AtprotoOAuth/AtprotoOauth.swift Outdated
Comment thread LocalPackages/oauth4swift/Sources/OAuth/Authorize.swift Outdated
Comment thread LocalPackages/oauth4swift/Sources/OAuth/Authorize.swift
Comment thread LocalPackages/oauth4swift/Sources/OAuth/Authorize.swift Outdated
Comment thread Sources/AtprotoOAuth/AtprotoOauth.swift
Comment thread Sources/AtprotoOAuth/AtprotoOauth.swift Outdated
Comment thread Sources/AtprotoOAuth/AtprotoOauth.swift
@germ-mark germ-mark marked this pull request as ready for review March 11, 2026 02:43
@germ-mark germ-mark changed the title WIP: Componentize OAuth Logic Componentize OAuth Logic Mar 14, 2026
@germ-mark germ-mark merged commit 5b63d48 into main Mar 16, 2026
4 checks passed
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.

2 participants