oppi.li
commits
re: https://github.com/bluesky-social/atproto/pull/3966
Ran `make lexgen` and just kept stuff related to notification
preferences.
- `net/http` server middleware for "atproto admin auth" (which is just
Basic auth with user "admin), supporting multiple passwords for
operational flexibility (eg, new relay could use this)
- inter-service auth validation, including `net/http` middleware. fairly
complete and correct, though might have some performance issues
(purging/retrying identity resolution; and parsing keys on every
request)
The service auth code is loosely inspired by code in Discover (which is
not open source), which has been running in prod a long time now. For
example the test cases. But that code caches parsed keys much more
aggressively (with an in-process LRU).
There are some TODOs in here about XRPC error responses (aka, reply with
JSON). I'm not sure if we want to manage that here in middleware library
code, or leave it to service error handler to fill in.
The logic around LXM validation/enforcement could also use review, both
of the code and against the atproto XRPC specs.
Note that this package intentionally does not use "XRPC" in
method/variable names, as we are slowly deprecating that terminology.
This is a more-protocol-complete replacement for `xrpc.Client`. I've
iterated on the overall design a couple times and feel pretty confident
about this implementation. Features:
- `APIClient` for use with XRPC HTTP requests
- pluggable `AuthMethod` abstraction, with basic implementations for
admin auth and password auth sessions (other auth methods, like OAuth
and inter-service auth, will be separate packages)
- reusable `APIError` and `APIRequest` types
- handle corner-cases like retryable request bodies when auth method
needs to re-send requests
- test coverage
Design notes for reviewers:
- should this package be named `atproto/atclient` instead of
`atproto/client`? "client" is such a generic term
- plan is to have `atproto/auth` with both server and client
implementations of service auth; and then `atproto/auth/oauth` with
OAuth stuff
- observability: open to adding `slog` logging if desired. I'd be
resistant to adding things like otel traces or prometheus metrics; i'd
like to keep this base type minimal in terms of dependencies. could hook
those in via `http.Client`, or a wrapper type.
- some initialization methods return just a pointer to a struct (no
error). those could be returning the struct directly; not sure what our
house style should be for that
A couple small known limitations:
- one not-frequently-used protocol feature is to return the "last
indexed" repo rev in AppView responses, to help with read-after-write
and debugging. the `Get()` and `Post()` helpers don't have an easy way
to access that; could be saved on the `APIClient` struct itself?
- with password auth, there isn't an easy to way directly call
`Logout()`. it isn't so bad to type-unpack the `APIClient.Auth` when
needed? but alternatively the `DoWithAuth` method could detect any
`deleteSession` call and use the refresh token for that (instead of
access token); then calling code would just call that method. feels a
bit "magic" though.
- some headers, like `User-Agent`, don't get passed through and included
on some special calls, like token refresh. not sure how big a deal this
is
Description:
Corrected minor spelling errors in the documentation for improved
clarity and professionalism.
Runs `make lexgen` for
https://github.com/bluesky-social/atproto/pull/3901 and only keeps what
was needed for notification preferences.
Assuming 5k active PDS instances, this spreads things out over 5
seconds. had been seeing burst of errors when restarting relay, which I
suspect is related to too many sockets (and DNS lookups) at the same
time.
Assuming 5k active PDS instances, this spreads things out over 5
seconds. had been seeing burst of errors when restarting relay, which I
suspect is related to too many sockets (and DNS lookups) at the same
time.
- `net/http` server middleware for "atproto admin auth" (which is just
Basic auth with user "admin), supporting multiple passwords for
operational flexibility (eg, new relay could use this)
- inter-service auth validation, including `net/http` middleware. fairly
complete and correct, though might have some performance issues
(purging/retrying identity resolution; and parsing keys on every
request)
The service auth code is loosely inspired by code in Discover (which is
not open source), which has been running in prod a long time now. For
example the test cases. But that code caches parsed keys much more
aggressively (with an in-process LRU).
There are some TODOs in here about XRPC error responses (aka, reply with
JSON). I'm not sure if we want to manage that here in middleware library
code, or leave it to service error handler to fill in.
The logic around LXM validation/enforcement could also use review, both
of the code and against the atproto XRPC specs.
Note that this package intentionally does not use "XRPC" in
method/variable names, as we are slowly deprecating that terminology.
This is a more-protocol-complete replacement for `xrpc.Client`. I've
iterated on the overall design a couple times and feel pretty confident
about this implementation. Features:
- `APIClient` for use with XRPC HTTP requests
- pluggable `AuthMethod` abstraction, with basic implementations for
admin auth and password auth sessions (other auth methods, like OAuth
and inter-service auth, will be separate packages)
- reusable `APIError` and `APIRequest` types
- handle corner-cases like retryable request bodies when auth method
needs to re-send requests
- test coverage
Design notes for reviewers:
- should this package be named `atproto/atclient` instead of
`atproto/client`? "client" is such a generic term
- plan is to have `atproto/auth` with both server and client
implementations of service auth; and then `atproto/auth/oauth` with
OAuth stuff
- observability: open to adding `slog` logging if desired. I'd be
resistant to adding things like otel traces or prometheus metrics; i'd
like to keep this base type minimal in terms of dependencies. could hook
those in via `http.Client`, or a wrapper type.
- some initialization methods return just a pointer to a struct (no
error). those could be returning the struct directly; not sure what our
house style should be for that
A couple small known limitations:
- one not-frequently-used protocol feature is to return the "last
indexed" repo rev in AppView responses, to help with read-after-write
and debugging. the `Get()` and `Post()` helpers don't have an easy way
to access that; could be saved on the `APIClient` struct itself?
- with password auth, there isn't an easy to way directly call
`Logout()`. it isn't so bad to type-unpack the `APIClient.Auth` when
needed? but alternatively the `DoWithAuth` method could detect any
`deleteSession` call and use the refresh token for that (instead of
access token); then calling code would just call that method. feels a
bit "magic" though.
- some headers, like `User-Agent`, don't get passed through and included
on some special calls, like token refresh. not sure how big a deal this
is