Skip to content

Commit 080a077

Browse files
committed
Refactor client storage to own domain type
Created storage.Client type that owns the complete client concept (OAuth fields plus metadata). Fosite's DefaultClient couldn't hold metadata, so we were losing CreatedAt timestamps from Firestore. Now storage.Client has CreatedAt and can extend with LastUsed, Description, Enabled in the future. Fosite becomes an implementation detail we adapt to via ToFositeClient(), not something that constrains our domain model. The /clients/{client_id} and /register endpoints now return actual creation timestamps instead of 0 or time.Now() at retrieval time.
1 parent 094e741 commit 080a077

File tree

8 files changed

+235
-193
lines changed

8 files changed

+235
-193
lines changed

CLAUDE.md

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -418,6 +418,29 @@ Remember: The mascot is an easter egg, not a distraction. Subtle movements creat
418418

419419
3. **Don't bring patterns from other systems** - Understand THIS system's design choices.
420420

421+
### Approaching Problems
422+
423+
**Never rush.** There is no time pressure. Approach every question with curiosity and investigation, not urgency.
424+
425+
When faced with a problem or decision:
426+
427+
1. **Seek the complete picture first** - Don't jump to solutions
428+
2. **Investigate thoroughly** - Read the code, understand the architecture, trace the data flow
429+
3. **Ask clarifying questions** - What's the actual use case? What's the value? What are the tradeoffs?
430+
4. **Think architecturally** - What's the right abstraction? Where do concerns belong?
431+
5. **Expand the solution space** - Don't settle on the first approach. Be creative. What if we ignored time and complexity? Would a larger refactoring lead to a fundamentally better design?
432+
6. **Consider future needs** - What might we want later? Does this approach scale to those needs?
433+
7. **Only then decide** - After exploring fully, choose the approach
434+
435+
The question is never "what's the quick fix?" The question is "what's the right design given what we now understand?"
436+
437+
**Example progression:**
438+
- ❌ "Here are three options, which one do you want?"
439+
- ✅ "Let me investigate how this works... [reads code]... I see we already store timestamps in Firestore but lose them during conversion. The core issue is fosite.DefaultClient can't hold metadata. What's the actual value of exposing this timestamp?"
440+
- ✅✅ "One approach is adding GetClientMetadata() alongside GetClient(). But let me think bigger - what if we stopped being constrained by fosite's storage interface? We could build our own Client type with metadata and use fosite as an implementation detail. Larger refactoring, but natural place for future metadata needs. What's your instinct on likely future requirements?"
441+
442+
Don't get anchored on the first solution. Explore the design space. Sometimes the right answer is a bigger change that solves the problem more fundamentally.
443+
421444
### When You're Stuck
422445

423446
- **ASK QUESTIONS** - Even "dumb" questions are better than wrong assumptions

internal/server/auth_handlers.go

Lines changed: 39 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@ package server
33
import (
44
"context"
55
"encoding/json"
6+
"errors"
67
"fmt"
78
"net/http"
89
"net/url"
@@ -101,7 +102,6 @@ func (h *AuthHandlers) ProtectedResourceMetadataHandler(w http.ResponseWriter, r
101102
}
102103
}
103104

104-
// ClientMetadataHandler serves OAuth 2.0 Client Metadata for dynamic discovery
105105
func (h *AuthHandlers) ClientMetadataHandler(w http.ResponseWriter, r *http.Request) {
106106
clientID := r.PathValue("client_id")
107107
if clientID == "" {
@@ -111,26 +111,30 @@ func (h *AuthHandlers) ClientMetadataHandler(w http.ResponseWriter, r *http.Requ
111111

112112
log.Logf("Client metadata handler called for client: %s", clientID)
113113

114-
client, err := h.storage.GetClient(r.Context(), clientID)
114+
client, err := h.storage.GetClientWithMetadata(r.Context(), clientID)
115115
if err != nil {
116116
log.LogError("Failed to get client %s: %v", clientID, err)
117-
jsonwriter.WriteNotFound(w, "Client not found")
117+
if errors.Is(err, fosite.ErrNotFound) {
118+
jsonwriter.WriteNotFound(w, "Client not found")
119+
} else {
120+
jsonwriter.WriteInternalServerError(w, "Failed to retrieve client")
121+
}
118122
return
119123
}
120124

121125
tokenEndpointAuthMethod := "none"
122-
if len(client.GetHashedSecret()) > 0 {
126+
if len(client.Secret) > 0 {
123127
tokenEndpointAuthMethod = "client_secret_post"
124128
}
125129

126130
metadata := oauth.BuildClientMetadata(
127-
client.GetID(),
128-
client.GetRedirectURIs(),
129-
client.GetGrantTypes(),
130-
client.GetResponseTypes(),
131-
client.GetScopes(),
131+
client.ID,
132+
client.RedirectURIs,
133+
client.GrantTypes,
134+
client.ResponseTypes,
135+
client.Scopes,
132136
tokenEndpointAuthMethod,
133-
0,
137+
client.CreatedAt,
134138
)
135139

136140
w.Header().Set("Content-Type", "application/json")
@@ -417,19 +421,17 @@ func (h *AuthHandlers) TokenHandler(w http.ResponseWriter, r *http.Request) {
417421
h.oauthProvider.WriteAccessResponse(ctx, w, accessRequest, response)
418422
}
419423

420-
// buildClientRegistrationResponse creates the registration response for a client
421-
func (h *AuthHandlers) buildClientRegistrationResponse(client *fosite.DefaultClient, tokenEndpointAuthMethod string, clientSecret string) map[string]any {
424+
func (h *AuthHandlers) buildClientRegistrationResponse(client *storage.Client, tokenEndpointAuthMethod string, clientSecret string) map[string]any {
422425
response := map[string]any{
423-
"client_id": client.GetID(),
424-
"client_id_issued_at": time.Now().Unix(),
425-
"redirect_uris": client.GetRedirectURIs(),
426-
"grant_types": client.GetGrantTypes(),
427-
"response_types": client.GetResponseTypes(),
428-
"scope": strings.Join(client.GetScopes(), " "), // Space-separated string
426+
"client_id": client.ID,
427+
"client_id_issued_at": client.CreatedAt,
428+
"redirect_uris": client.RedirectURIs,
429+
"grant_types": client.GrantTypes,
430+
"response_types": client.ResponseTypes,
431+
"scope": strings.Join(client.Scopes, " "),
429432
"token_endpoint_auth_method": tokenEndpointAuthMethod,
430433
}
431434

432-
// Include client_secret only for confidential clients
433435
if clientSecret != "" {
434436
response["client_secret"] = clientSecret
435437
}
@@ -461,27 +463,34 @@ func (h *AuthHandlers) RegisterHandler(w http.ResponseWriter, r *http.Request) {
461463
return
462464
}
463465

464-
// Check if client requests client_secret_post authentication
465466
tokenEndpointAuthMethod := "none"
466-
var client *fosite.DefaultClient
467+
var client *storage.Client
467468
var plaintextSecret string
468469
clientID := crypto.GenerateSecureToken()
469470

470471
if authMethod, ok := metadata["token_endpoint_auth_method"].(string); ok && authMethod == "client_secret_post" {
471-
// Create confidential client with a secret
472472
plaintextSecret = crypto.GenerateSecureToken()
473473
hashedSecret, err := crypto.HashClientSecret(plaintextSecret)
474474
if err != nil {
475475
log.LogError("Failed to hash client secret: %v", err)
476476
jsonwriter.WriteInternalServerError(w, "Failed to create client")
477477
return
478478
}
479-
client = h.storage.CreateConfidentialClient(clientID, hashedSecret, redirectURIs, scopes, h.authConfig.Issuer)
479+
client, err = h.storage.CreateConfidentialClient(clientID, hashedSecret, redirectURIs, scopes, h.authConfig.Issuer)
480+
if err != nil {
481+
log.LogError("Failed to create confidential client: %v", err)
482+
jsonwriter.WriteInternalServerError(w, "Failed to create client")
483+
return
484+
}
480485
tokenEndpointAuthMethod = "client_secret_post"
481486
log.Logf("Creating confidential client %s with client_secret_post authentication", clientID)
482487
} else {
483-
// Create public client (no secret)
484-
client = h.storage.CreateClient(clientID, redirectURIs, scopes, h.authConfig.Issuer)
488+
client, err = h.storage.CreateClient(clientID, redirectURIs, scopes, h.authConfig.Issuer)
489+
if err != nil {
490+
log.LogError("Failed to create client: %v", err)
491+
jsonwriter.WriteInternalServerError(w, "Failed to create client")
492+
return
493+
}
485494
log.Logf("Creating public client %s with no authentication", clientID)
486495
}
487496

@@ -640,7 +649,11 @@ func (h *AuthHandlers) CompleteOAuthHandler(w http.ResponseWriter, r *http.Reque
640649
client, err := h.storage.GetClient(ctx, upstreamOAuthState.ClientID)
641650
if err != nil {
642651
log.LogError("Failed to get client: %v", err)
643-
jsonwriter.WriteInternalServerError(w, "Client not found")
652+
if errors.Is(err, fosite.ErrNotFound) {
653+
jsonwriter.WriteNotFound(w, "Client not found")
654+
} else {
655+
jsonwriter.WriteInternalServerError(w, "Failed to retrieve client")
656+
}
644657
return
645658
}
646659

internal/storage/client.go

Lines changed: 43 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,43 @@
1+
package storage
2+
3+
import "github.com/ory/fosite"
4+
5+
type Client struct {
6+
ID string
7+
Secret []byte
8+
RedirectURIs []string
9+
Scopes []string
10+
GrantTypes []string
11+
ResponseTypes []string
12+
Audience []string
13+
Public bool
14+
15+
CreatedAt int64
16+
}
17+
18+
func (c *Client) ToFositeClient() *fosite.DefaultClient {
19+
return &fosite.DefaultClient{
20+
ID: c.ID,
21+
Secret: c.Secret,
22+
RedirectURIs: c.RedirectURIs,
23+
Scopes: c.Scopes,
24+
GrantTypes: c.GrantTypes,
25+
ResponseTypes: c.ResponseTypes,
26+
Audience: c.Audience,
27+
Public: c.Public,
28+
}
29+
}
30+
31+
func FromFositeClient(fc *fosite.DefaultClient, createdAt int64) *Client {
32+
return &Client{
33+
ID: fc.ID,
34+
Secret: fc.Secret,
35+
RedirectURIs: fc.RedirectURIs,
36+
Scopes: fc.Scopes,
37+
GrantTypes: fc.GrantTypes,
38+
ResponseTypes: fc.ResponseTypes,
39+
Audience: fc.Audience,
40+
Public: fc.Public,
41+
CreatedAt: createdAt,
42+
}
43+
}

0 commit comments

Comments
 (0)