From 3764f258dd680d484b4e313dfb69eccfc578837e Mon Sep 17 00:00:00 2001 From: bryan newbold Date: Fri, 28 Aug 2026 15:56:37 -0700 Subject: [PATCH] refactor SpaceClient --- atspace/client.go | 113 ++++++++++++++++++++++------------------- atspace/client_test.go | 12 +---- atspace/session.go | 57 ++++++++------------- 3 files changed, 84 insertions(+), 98 deletions(-) diff --git a/atspace/client.go b/atspace/client.go index 1608926..9512448 100644 --- a/atspace/client.go +++ b/atspace/client.go @@ -10,17 +10,21 @@ import ( "tangled.org/bnewbold.net/cobalt/atspace/comatproto" ) -// Scoped to a single space. -// TODO: refactor +// Basic atproto permissioned data space client. Requires PDS account authentication, scoped to a single space, and can fetch/resolve any records and repos in the space. +// +// Use PDSClient (separate package) to manage space repos and records for the current account. +// +// This implementation currently does not support persisting or loading auth sessions (eg, space credentials and DPoP keys). Or session refreshes. Or concurrency. type SpaceClient struct { Dir identity.Directory Space SpaceRef PDSClient *atclient.APIClient SpaceHostClient *atclient.APIClient - RepoSession *ClientSession + AuthSession *SpaceSession } -func NewSpaceClient(ctx context.Context, space SpaceRef, pdsClient *atclient.APIClient, dir identity.Directory) (*SpaceClient, error) { +// Creates a new [SpaceClient] with an initial space credential fetched from the space host. +func NewSpaceClient(ctx context.Context, dir identity.Directory, pdsClient *atclient.APIClient, space SpaceRef) (*SpaceClient, error) { ident, err := dir.LookupDID(ctx, space.SpaceAuthority()) if err != nil { @@ -28,95 +32,100 @@ func NewSpaceClient(ctx context.Context, space SpaceRef, pdsClient *atclient.API } shost := ident.SpaceHostEndpoint() if shost == "" { - return nil, fmt.Errorf("space authority has not host URL") + return nil, fmt.Errorf("space authority has no space host endpoint registered") } shostc := atclient.NewAPIClient(shost) - // XXX: DPoP AuthMethod - return &SpaceClient{ + sc := SpaceClient{ Dir: dir, Space: space, PDSClient: pdsClient, SpaceHostClient: shostc, - }, nil -} + } -// TODO: this should get wrapped inside GetSpaceCredential (?) -func (sc *SpaceClient) GetDelegationToken(ctx context.Context) (string, error) { - resp, err := comatproto.SpaceGetDelegationToken(ctx, sc.PDSClient, sc.Space.String()) - if err != nil { - return "", fmt.Errorf("failed to fetch space delegation token from PDS: %w", err) + if err := sc.RefreshSession(ctx); err != nil { + return nil, err } - return resp.Token, nil + + return &sc, nil } -func (sc *SpaceClient) GetSpaceCredential(ctx context.Context, delegationToken string) (string, error) { - // XXX - sess, err := NewClientSession() +func (sc *SpaceClient) RefreshSession(ctx context.Context) error { + tok, err := sc.GetDelegationToken(ctx) if err != nil { - return "", err + return err } - sc.SpaceHostClient.Auth = sess - _, err = sess.GetSpaceCredential(ctx, sc.Space, sc.SpaceHostClient.Host, delegationToken) + sess, err := NewSpaceSession(sc.Space, sc.SpaceHostClient.Host) if err != nil { - return "", err + return err } + _, err = sess.RefreshSpaceCredential(ctx, tok) + if err != nil { + return err + } + sc.SpaceHostClient.Auth = sess + sc.AuthSession = sess - sc.RepoSession = sess - return sess.SpaceCredential, nil + return nil } -func (sc *SpaceClient) ListSpaceRecords(ctx context.Context, repo syntax.DID) (*comatproto.SpaceListRecords_Output, error) { - - if sc.RepoSession == nil { - return nil, fmt.Errorf("repo fetch auth not configured yet") +func (sc *SpaceClient) GetDelegationToken(ctx context.Context) (string, error) { + resp, err := comatproto.SpaceGetDelegationToken(ctx, sc.PDSClient, sc.Space.String()) + if err != nil { + return "", fmt.Errorf("failed to fetch space delegation token from PDS: %w", err) } + return resp.Token, nil +} - // XXX: check that space-uri matches client space-ref +func (sc *SpaceClient) ListSpaceRecords(ctx context.Context, repo syntax.DID) (*comatproto.SpaceListRecords_Output, error) { + // XXX: the helper here should be an iterator, not just a call to comatproto.SpaceListRecords - ident, err := sc.Dir.LookupDID(ctx, repo) + repoc, err := sc.RepoClient(ctx, suri.RepoAuthority()) if err != nil { return nil, err } - ahost := ident.PDSEndpoint() - // XXX: is this supposed to be the space host? or always PDS? - if ahost == "" { - return nil, fmt.Errorf("") - } - - repoc := atclient.NewAPIClient(ahost) - repoc.Auth = sc.RepoSession // SpaceListRecords(ctx context.Context, c lexutil.LexClient, collection string, cursor string, excludeValues *bool, limit *int64, repo string, reverse *bool, space string) (*SpaceListRecords_Output, error) { resp, err := comatproto.SpaceListRecords(ctx, repoc, "", "", nil, nil, repo.String(), nil, sc.Space.String()) if err != nil { return nil, err } - // XXX return resp, nil } -func (sc *SpaceClient) GetSpaceRecord(ctx context.Context, suri SpaceURI) (any, error) { +// Creates an authenticated [atclient.APIClient] to talk to the PDS host of the indicated account (DID) using space credential and DPoP. +func (sc *SpaceClient) RepoClient(ctx context.Context, repo syntax.DID) (*atclient.APIClient, error) { - if sc.RepoSession == nil { - return nil, fmt.Errorf("repo fetch auth not configured yet") + ident, err := sc.Dir.LookupDID(ctx, repo) + if err != nil { + return nil, err } - // XXX: check that space-uri matches client space-ref + // TODO: is this supposed to be the space host? or always PDS? + host := ident.PDSEndpoint() + if host == "" { + return nil, fmt.Errorf("repo account does not declare a PDS host: %s", repo) + } - ident, err := sc.Dir.LookupDID(ctx, suri.RepoAuthority()) - if err != nil { - return nil, err + c := atclient.NewAPIClient(host) + c.Auth = sc.AuthSession + return c, nil +} + +func (sc *SpaceClient) GetSpaceRecord(ctx context.Context, suri SpaceURI) (any, error) { + + if suri.SpaceRef() != sc.Space { + return nil, fmt.Errorf("space record URI did not match client space: %s", suri.SpaceRef()) } - ahost := ident.PDSEndpoint() - // XXX: is this supposed to be the space host? or always PDS? - if ahost == "" { - return nil, fmt.Errorf("") + if suri.RecordKey() == "" { + return nil, fmt.Errorf("not a full space record URI: %s", suri) } - repoc := atclient.NewAPIClient(ahost) - repoc.Auth = sc.RepoSession + repoc, err := sc.RepoClient(ctx, suri.RepoAuthority()) + if err != nil { + return nil, err + } resp, err := comatproto.SpaceGetRecord(ctx, repoc, suri.Collection().String(), suri.RepoAuthority().String(), suri.RecordKey().String(), sc.Space.String()) if err != nil { diff --git a/atspace/client_test.go b/atspace/client_test.go index 0b40284..efa57fe 100644 --- a/atspace/client_test.go +++ b/atspace/client_test.go @@ -13,7 +13,7 @@ import ( "github.com/stretchr/testify/require" ) -func TestSpaceClientBasic(t *testing.T) { +func TestSpaceClientLive(t *testing.T) { //assert := assert.New(t) require := require.New(t) ctx := t.Context() @@ -36,15 +36,7 @@ func TestSpaceClientBasic(t *testing.T) { pdsc, err := atclient.LoginWithPassword(ctx, dir, usr, passwd, "", nil) require.NoError(err) - sc, err := NewSpaceClient(ctx, sref, pdsc, dir) - require.NoError(err) - - // get delegation token from PDS - dtoken, err := sc.GetDelegationToken(ctx) - require.NoError(err) - - // get space credential token - _, err = sc.GetSpaceCredential(ctx, dtoken) + sc, err := NewSpaceClient(ctx, dir, pdsc, sref) require.NoError(err) resp, err := sc.ListSpaceRecords(ctx, suri.RepoAuthority()) diff --git a/atspace/session.go b/atspace/session.go index cec5daa..758bf7e 100644 --- a/atspace/session.go +++ b/atspace/session.go @@ -27,26 +27,31 @@ type dpopClaims struct { AccessTokenHash *string `json:"ath,omitempty"` } -type ClientSession struct { +type SpaceSession struct { + Space SpaceRef + SpaceHost string DPoPPrivateKey atcrypto.PrivateKey - SpaceCredential string + SpaceCredential string // optional (during initialization) } -func NewClientSession() (*ClientSession, error) { +func NewSpaceSession(space SpaceRef, spaceHost string) (*SpaceSession, error) { + // TODO: parse/verify spaceHost is a URL priv, err := atcrypto.GeneratePrivateKeyP256() if err != nil { return nil, err } - return &ClientSession{ + return &SpaceSession{ + Space: space, + SpaceHost: spaceHost, DPoPPrivateKey: priv, }, nil } // 'bind' is optional. If included, it is used to generate an 'ath' field. -func (sess *ClientSession) GenerateDPoP(method, reqURL, bind string) (string, error) { - // TODO - //sess.lk.RLock() - //defer sess.lk.RUnlock() +func (sess *SpaceSession) GenerateDPoP(method, reqURL, bind string) (string, error) { + if sess.DPoPPrivateKey == nil { + return "", fmt.Errorf("SpaceSession not properly initialized") + } claims := dpopClaims{ HTTPMethod: method, TargetURI: reqURL, @@ -66,7 +71,7 @@ func (sess *ClientSession) GenerateDPoP(method, reqURL, bind string) (string, er return "", err } - // TODO: store a copy of this JWK on the ClientSession as a private field, for efficiency + // TODO: store a copy of this JWK as a private field, for efficiency? pub, err := sess.DPoPPrivateKey.PublicKey() if err != nil { return "", err @@ -82,23 +87,23 @@ func (sess *ClientSession) GenerateDPoP(method, reqURL, bind string) (string, er return token.SignedString(sess.DPoPPrivateKey) } -func (sess *ClientSession) GetSpaceCredential(ctx context.Context, space SpaceRef, spaceHost, delegationToken string) (string, error) { +func (sess *SpaceSession) RefreshSpaceCredential(ctx context.Context, delegationToken string) (string, error) { // TODO: safer consruction/parsing? - durl := spaceHost + "/xrpc/com.atproto.space.getSpaceCredential" + durl := sess.SpaceHost + "/xrpc/com.atproto.space.getSpaceCredential" slog.Warn("space cred url", "url", durl) dpopJWT, err := sess.GenerateDPoP("POST", durl, "") if err != nil { return "", err } - // XXX: creates a throw-away client - c := atclient.NewAPIClient(spaceHost) + // TODO: creates a throw-away client; could retain it instead? configurable (eg User-Agent)? + c := atclient.NewAPIClient(sess.SpaceHost) c.Headers.Set("Authorization", fmt.Sprintf("Bearer %s", delegationToken)) c.Headers.Set("DPoP", dpopJWT) resp, err := comatproto.SpaceGetSpaceCredential(ctx, c, &comatproto.SpaceGetSpaceCredential_Input{ - Space: space.String(), + Space: sess.Space.String(), }) if err != nil { return "", fmt.Errorf("failed to fetch space credential from space host: %w", err) @@ -107,7 +112,8 @@ func (sess *ClientSession) GetSpaceCredential(ctx context.Context, space SpaceRe return resp.Credential, nil } -func (sess *ClientSession) DoWithAuth(c *http.Client, req *http.Request, endpoint syntax.NSID) (*http.Response, error) { +func (sess *SpaceSession) DoWithAuth(c *http.Client, req *http.Request, endpoint syntax.NSID) (*http.Response, error) { + // TODO: expiration/retry behavior durl := dpopURL(req.URL) dpopJWT, err := sess.GenerateDPoP(req.Method, durl, sess.SpaceCredential) @@ -118,28 +124,7 @@ func (sess *ClientSession) DoWithAuth(c *http.Client, req *http.Request, endpoin req.Header.Set("Authorization", fmt.Sprintf("DPoP %s", sess.SpaceCredential)) req.Header.Set("DPoP", dpopJWT) - // XXX - slog.Warn("space session request", "headers", req.Header) - return c.Do(req) - - /* TODO: expiration/retry behavior - resp, err = c.Do(req) - if err != nil { - return nil, err - } - - // on Success, or many types of error, just return HTTP response - // "Unauthorized" is HTTP status code 401 - if resp.StatusCode != http.StatusUnauthorized || resp.Header.Get("WWW-Authenticate") == "" { - return resp, nil - } - - authHdr := resp.Header.Get("WWW-Authenticate") - if isExpiredAccessTokenHeader(authHdr) { - ... - } - */ } // copy a request URL and strip query params and fragment, for DPoP -- 2.54.0