From 08fb8abb411749f258171e377af7e1ae6faf72de Mon Sep 17 00:00:00 2001 From: Evan Jarrett Date: Tue, 21 Oct 2025 09:54:23 -0500 Subject: [PATCH] invalidate sessions when scopes change --- cmd/appview/serve.go | 10 + pkg/appview/db/oauth_store.go | 83 +++++ pkg/appview/db/oauth_store_test.go | 371 ++++++++++++++++++++ pkg/appview/db/readonly.go | 2 +- pkg/appview/templates/pages/repository.html | 8 +- pkg/auth/oauth/client.go | 27 ++ pkg/auth/oauth/client_test.go | 65 ++++ pkg/auth/oauth/refresher.go | 17 +- 8 files changed, 580 insertions(+), 3 deletions(-) create mode 100644 pkg/appview/db/oauth_store_test.go create mode 100644 pkg/auth/oauth/client_test.go diff --git a/cmd/appview/serve.go b/cmd/appview/serve.go index c87dc68..de36110 100644 --- a/cmd/appview/serve.go +++ b/cmd/appview/serve.go @@ -148,6 +148,16 @@ func serveRegistry(cmd *cobra.Command, args []string) error { } fmt.Println("Using full OAuth scopes (including blob: scope)") + // Invalidate sessions with mismatched scopes on startup + // This ensures all users have the latest required scopes after deployment + desiredScopes := oauth.GetDefaultScopes(defaultHoldDID) + invalidatedCount, err := oauthStore.InvalidateSessionsWithMismatchedScopes(context.Background(), desiredScopes) + if err != nil { + fmt.Printf("Warning: Failed to invalidate sessions with mismatched scopes: %v\n", err) + } else if invalidatedCount > 0 { + fmt.Printf("Invalidated %d OAuth session(s) due to scope changes\n", invalidatedCount) + } + // Create oauth token refresher refresher := oauth.NewRefresher(oauthApp) diff --git a/pkg/appview/db/oauth_store.go b/pkg/appview/db/oauth_store.go index 90de6ba..9fd3412 100644 --- a/pkg/appview/db/oauth_store.go +++ b/pkg/appview/db/oauth_store.go @@ -234,6 +234,89 @@ func (s *OAuthStore) CleanupExpiredAuthRequests(ctx context.Context) error { return nil } +// InvalidateSessionsWithMismatchedScopes removes all sessions whose scopes don't match the desired scopes +// This is called on AppView startup to ensure all sessions have current scopes +// Returns the count of invalidated sessions +func (s *OAuthStore) InvalidateSessionsWithMismatchedScopes(ctx context.Context, desiredScopes []string) (int, error) { + // Query all sessions + rows, err := s.db.QueryContext(ctx, ` + SELECT session_key, account_did, session_id, session_data + FROM oauth_sessions + `) + if err != nil { + return 0, fmt.Errorf("failed to query sessions: %w", err) + } + defer rows.Close() + + var sessionsToDelete []string + for rows.Next() { + var sessionKey, accountDID, sessionID, sessionDataJSON string + if err := rows.Scan(&sessionKey, &accountDID, &sessionID, &sessionDataJSON); err != nil { + fmt.Printf("WARNING [oauth/store]: Failed to scan session row: %v\n", err) + continue + } + + // Parse session data + var sessionData oauth.ClientSessionData + if err := json.Unmarshal([]byte(sessionDataJSON), &sessionData); err != nil { + fmt.Printf("WARNING [oauth/store]: Failed to parse session data for %s: %v\n", sessionKey, err) + // Delete malformed sessions + sessionsToDelete = append(sessionsToDelete, sessionKey) + continue + } + + // Check if scopes match (need to import oauth package for ScopesMatch) + // Since we're in db package, we can't import oauth (circular dependency) + // So we'll implement a simple scope comparison here + if !scopesMatch(sessionData.Scopes, desiredScopes) { + sessionsToDelete = append(sessionsToDelete, sessionKey) + } + } + + if err := rows.Err(); err != nil { + return 0, fmt.Errorf("error iterating sessions: %w", err) + } + + // Delete sessions with mismatched scopes + if len(sessionsToDelete) > 0 { + for _, key := range sessionsToDelete { + _, err := s.db.ExecContext(ctx, ` + DELETE FROM oauth_sessions WHERE session_key = ? + `, key) + if err != nil { + fmt.Printf("WARNING [oauth/store]: Failed to delete session %s: %v\n", key, err) + } + } + fmt.Printf("Invalidated %d OAuth session(s) with mismatched scopes\n", len(sessionsToDelete)) + } + + return len(sessionsToDelete), nil +} + +// scopesMatch checks if two scope lists are equivalent (order-independent) +// Local implementation to avoid circular dependency with oauth package +func scopesMatch(stored, desired []string) bool { + if len(stored) == 0 && len(desired) == 0 { + return true + } + if len(stored) != len(desired) { + return false + } + + desiredMap := make(map[string]bool, len(desired)) + for _, scope := range desired { + desiredMap[scope] = true + } + + for _, scope := range stored { + if !desiredMap[scope] { + return false + } + } + + return true +} + // makeSessionKey creates a composite key for session storage func makeSessionKey(did, sessionID string) string { return fmt.Sprintf("%s:%s", did, sessionID) diff --git a/pkg/appview/db/oauth_store_test.go b/pkg/appview/db/oauth_store_test.go new file mode 100644 index 0000000..fd40bbe --- /dev/null +++ b/pkg/appview/db/oauth_store_test.go @@ -0,0 +1,371 @@ +package db + +import ( + "context" + "testing" + "time" + + "github.com/bluesky-social/indigo/atproto/auth/oauth" + "github.com/bluesky-social/indigo/atproto/syntax" +) + +func TestInvalidateSessionsWithMismatchedScopes(t *testing.T) { + // Create in-memory test database + db, err := InitDB(":memory:") + if err != nil { + t.Fatalf("Failed to init database: %v", err) + } + defer db.Close() + + store := NewOAuthStore(db) + ctx := context.Background() + + // Test 1: Empty database - should return 0 + count, err := store.InvalidateSessionsWithMismatchedScopes(ctx, []string{"atproto", "blob:image/png"}) + if err != nil { + t.Fatalf("Expected no error with empty DB, got: %v", err) + } + if count != 0 { + t.Errorf("Expected 0 invalidated sessions in empty DB, got %d", count) + } + + // Helper to create session data + createSession := func(did, sessionID string, scopes []string) oauth.ClientSessionData { + parsedDID, _ := syntax.ParseDID(did) + return oauth.ClientSessionData{ + AccountDID: parsedDID, + SessionID: sessionID, + HostURL: "https://bsky.social", + AuthServerURL: "https://bsky.social", + AuthServerTokenEndpoint: "https://bsky.social/oauth/token", + Scopes: scopes, + AccessToken: "test_access_token", + RefreshToken: "test_refresh_token", + DPoPAuthServerNonce: "test_nonce", + DPoPHostNonce: "test_host_nonce", + DPoPPrivateKeyMultibase: "test_key", + } + } + + // Test 2: Session with matching scopes - should not be invalidated + matchingSession := createSession("did:plc:test1", "session1", []string{"atproto", "blob:image/png"}) + if err := store.SaveSession(ctx, matchingSession); err != nil { + t.Fatalf("Failed to save matching session: %v", err) + } + + count, err = store.InvalidateSessionsWithMismatchedScopes(ctx, []string{"atproto", "blob:image/png"}) + if err != nil { + t.Fatalf("Expected no error, got: %v", err) + } + if count != 0 { + t.Errorf("Expected 0 invalidated sessions (all match), got %d", count) + } + + // Verify session still exists + retrieved, err := store.GetSession(ctx, matchingSession.AccountDID, matchingSession.SessionID) + if err != nil { + t.Errorf("Expected session to still exist, got error: %v", err) + } + if retrieved == nil { + t.Error("Expected session to still exist, got nil") + } + + // Test 3: Session with mismatched scopes (missing scope) - should be invalidated + mismatchedSession := createSession("did:plc:test2", "session2", []string{"atproto"}) // Missing blob scope + if err := store.SaveSession(ctx, mismatchedSession); err != nil { + t.Fatalf("Failed to save mismatched session: %v", err) + } + + count, err = store.InvalidateSessionsWithMismatchedScopes(ctx, []string{"atproto", "blob:image/png"}) + if err != nil { + t.Fatalf("Expected no error, got: %v", err) + } + if count != 1 { + t.Errorf("Expected 1 invalidated session, got %d", count) + } + + // Verify mismatched session was deleted + retrieved, err = store.GetSession(ctx, mismatchedSession.AccountDID, mismatchedSession.SessionID) + if err == nil { + t.Error("Expected session to be deleted (should error), but got no error") + } + + // Test 4: Session with extra scopes - should be invalidated + extraScopeSession := createSession("did:plc:test3", "session3", []string{"atproto", "blob:image/png", "extra:scope"}) + if err := store.SaveSession(ctx, extraScopeSession); err != nil { + t.Fatalf("Failed to save extra scope session: %v", err) + } + + count, err = store.InvalidateSessionsWithMismatchedScopes(ctx, []string{"atproto", "blob:image/png"}) + if err != nil { + t.Fatalf("Expected no error, got: %v", err) + } + if count != 1 { + t.Errorf("Expected 1 invalidated session (extra scope), got %d", count) + } + + // Test 5: Multiple sessions with mixed matches - only mismatch should be invalidated + matching1 := createSession("did:plc:test4", "session4", []string{"atproto", "blob:image/png"}) + matching2 := createSession("did:plc:test5", "session5", []string{"blob:image/png", "atproto"}) // Different order + mismatched1 := createSession("did:plc:test6", "session6", []string{"atproto"}) + mismatched2 := createSession("did:plc:test7", "session7", []string{"wrong", "scopes"}) + + for _, sess := range []oauth.ClientSessionData{matching1, matching2, mismatched1, mismatched2} { + if err := store.SaveSession(ctx, sess); err != nil { + t.Fatalf("Failed to save session: %v", err) + } + } + + count, err = store.InvalidateSessionsWithMismatchedScopes(ctx, []string{"atproto", "blob:image/png"}) + if err != nil { + t.Fatalf("Expected no error, got: %v", err) + } + if count != 2 { + t.Errorf("Expected 2 invalidated sessions, got %d", count) + } + + // Verify matching sessions still exist + for _, sess := range []oauth.ClientSessionData{matching1, matching2} { + retrieved, err := store.GetSession(ctx, sess.AccountDID, sess.SessionID) + if err != nil { + t.Errorf("Expected matching session %s to exist, got error: %v", sess.SessionID, err) + } + if retrieved == nil { + t.Errorf("Expected matching session %s to exist, got nil", sess.SessionID) + } + } + + // Test 6: Malformed session data - should be deleted + parsedDID, _ := syntax.ParseDID("did:plc:test8") + _, err = db.ExecContext(ctx, ` + INSERT INTO oauth_sessions (session_key, account_did, session_id, session_data, created_at, updated_at) + VALUES (?, ?, ?, ?, datetime('now'), datetime('now')) + `, makeSessionKey("did:plc:test8", "malformed"), "did:plc:test8", "malformed", "invalid json data") + if err != nil { + t.Fatalf("Failed to insert malformed session: %v", err) + } + + count, err = store.InvalidateSessionsWithMismatchedScopes(ctx, []string{"atproto", "blob:image/png"}) + if err != nil { + t.Fatalf("Expected no error handling malformed data, got: %v", err) + } + if count != 1 { + t.Errorf("Expected 1 invalidated session (malformed), got %d", count) + } + + // Verify malformed session was deleted + retrieved, err = store.GetSession(ctx, parsedDID, "malformed") + if err == nil { + t.Error("Expected malformed session to be deleted, but got no error") + } +} + +func TestScopesMatch(t *testing.T) { + // Test the local scopesMatch function to ensure it matches the oauth.ScopesMatch behavior + tests := []struct { + name string + stored []string + desired []string + expected bool + }{ + { + name: "exact match", + stored: []string{"atproto", "blob:image/png"}, + desired: []string{"atproto", "blob:image/png"}, + expected: true, + }, + { + name: "different order", + stored: []string{"blob:image/png", "atproto"}, + desired: []string{"atproto", "blob:image/png"}, + expected: true, + }, + { + name: "missing scope", + stored: []string{"atproto"}, + desired: []string{"atproto", "blob:image/png"}, + expected: false, + }, + { + name: "extra scope", + stored: []string{"atproto", "blob:image/png", "extra"}, + desired: []string{"atproto", "blob:image/png"}, + expected: false, + }, + { + name: "both empty", + stored: []string{}, + desired: []string{}, + expected: true, + }, + { + name: "nil vs empty", + stored: nil, + desired: []string{}, + expected: true, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + result := scopesMatch(tt.stored, tt.desired) + if result != tt.expected { + t.Errorf("scopesMatch(%v, %v) = %v, want %v", + tt.stored, tt.desired, result, tt.expected) + } + }) + } +} + +func TestOAuthStoreSessionLifecycle(t *testing.T) { + // Basic test to ensure SaveSession, GetSession, DeleteSession work correctly + db, err := InitDB(":memory:") + if err != nil { + t.Fatalf("Failed to init database: %v", err) + } + defer db.Close() + + store := NewOAuthStore(db) + ctx := context.Background() + + // Create test session + did, _ := syntax.ParseDID("did:plc:testuser") + sessionData := oauth.ClientSessionData{ + AccountDID: did, + SessionID: "test_session_id", + HostURL: "https://bsky.social", + AuthServerURL: "https://bsky.social", + AuthServerTokenEndpoint: "https://bsky.social/oauth/token", + Scopes: []string{"atproto", "blob:image/png"}, + AccessToken: "test_access_token", + RefreshToken: "test_refresh_token", + DPoPAuthServerNonce: "test_nonce", + DPoPHostNonce: "test_host_nonce", + DPoPPrivateKeyMultibase: "test_key", + } + + // Test SaveSession + if err := store.SaveSession(ctx, sessionData); err != nil { + t.Fatalf("Failed to save session: %v", err) + } + + // Test GetSession + retrieved, err := store.GetSession(ctx, did, "test_session_id") + if err != nil { + t.Fatalf("Failed to get session: %v", err) + } + if retrieved == nil { + t.Fatal("Retrieved session is nil") + } + if retrieved.SessionID != sessionData.SessionID { + t.Errorf("Expected session ID %s, got %s", sessionData.SessionID, retrieved.SessionID) + } + if len(retrieved.Scopes) != len(sessionData.Scopes) { + t.Errorf("Expected %d scopes, got %d", len(sessionData.Scopes), len(retrieved.Scopes)) + } + + // Test UpdateSession (upsert) + sessionData.AccessToken = "new_access_token" + if err := store.SaveSession(ctx, sessionData); err != nil { + t.Fatalf("Failed to update session: %v", err) + } + + retrieved, err = store.GetSession(ctx, did, "test_session_id") + if err != nil { + t.Fatalf("Failed to get updated session: %v", err) + } + if retrieved.AccessToken != "new_access_token" { + t.Errorf("Expected updated access token, got %s", retrieved.AccessToken) + } + + // Test DeleteSession + if err := store.DeleteSession(ctx, did, "test_session_id"); err != nil { + t.Fatalf("Failed to delete session: %v", err) + } + + // Verify deletion + retrieved, err = store.GetSession(ctx, did, "test_session_id") + if err == nil { + t.Error("Expected error after deletion, got nil") + } +} + +func TestCleanupOldSessions(t *testing.T) { + db, err := InitDB(":memory:") + if err != nil { + t.Fatalf("Failed to init database: %v", err) + } + defer db.Close() + + store := NewOAuthStore(db) + ctx := context.Background() + + // Insert old session (31 days ago) + did1, _ := syntax.ParseDID("did:plc:old") + oldSessionData := oauth.ClientSessionData{ + AccountDID: did1, + SessionID: "old_session", + HostURL: "https://bsky.social", + AuthServerURL: "https://bsky.social", + AuthServerTokenEndpoint: "https://bsky.social/oauth/token", + Scopes: []string{"atproto"}, + AccessToken: "old_token", + RefreshToken: "old_refresh", + DPoPAuthServerNonce: "old_nonce", + DPoPHostNonce: "old_host_nonce", + DPoPPrivateKeyMultibase: "old_key", + } + + // Save and manually update timestamp to be old + if err := store.SaveSession(ctx, oldSessionData); err != nil { + t.Fatalf("Failed to save old session: %v", err) + } + + // Update timestamp to 31 days ago + oldTime := time.Now().Add(-31 * 24 * time.Hour) + _, err = db.ExecContext(ctx, ` + UPDATE oauth_sessions + SET updated_at = ? + WHERE session_key = ? + `, oldTime, makeSessionKey(did1.String(), "old_session")) + if err != nil { + t.Fatalf("Failed to update session timestamp: %v", err) + } + + // Insert recent session (1 day ago) + did2, _ := syntax.ParseDID("did:plc:recent") + recentSessionData := oauth.ClientSessionData{ + AccountDID: did2, + SessionID: "recent_session", + HostURL: "https://bsky.social", + AuthServerURL: "https://bsky.social", + AuthServerTokenEndpoint: "https://bsky.social/oauth/token", + Scopes: []string{"atproto"}, + AccessToken: "recent_token", + RefreshToken: "recent_refresh", + DPoPAuthServerNonce: "recent_nonce", + DPoPHostNonce: "recent_host_nonce", + DPoPPrivateKeyMultibase: "recent_key", + } + + if err := store.SaveSession(ctx, recentSessionData); err != nil { + t.Fatalf("Failed to save recent session: %v", err) + } + + // Run cleanup (remove sessions older than 30 days) + if err := store.CleanupOldSessions(ctx, 30*24*time.Hour); err != nil { + t.Fatalf("Failed to cleanup old sessions: %v", err) + } + + // Verify old session was deleted + _, err = store.GetSession(ctx, did1, "old_session") + if err == nil { + t.Error("Expected old session to be deleted") + } + + // Verify recent session still exists + _, err = store.GetSession(ctx, did2, "recent_session") + if err != nil { + t.Errorf("Expected recent session to exist, got error: %v", err) + } +} diff --git a/pkg/appview/db/readonly.go b/pkg/appview/db/readonly.go index 172c140..786cc96 100644 --- a/pkg/appview/db/readonly.go +++ b/pkg/appview/db/readonly.go @@ -92,7 +92,7 @@ func InitializeDatabase(uiEnabled bool, dbPath string) (*sql.DB, *sql.DB, *Sessi // Start cleanup goroutines for all SQLite stores go func() { - ticker := time.NewTicker(5 * time.Minute) + ticker := time.NewTicker(1 * time.Hour) defer ticker.Stop() for range ticker.C { ctx := context.Background() diff --git a/pkg/appview/templates/pages/repository.html b/pkg/appview/templates/pages/repository.html index dd58db6..ab06ff2 100644 --- a/pkg/appview/templates/pages/repository.html +++ b/pkg/appview/templates/pages/repository.html @@ -196,7 +196,13 @@ {{ end }} {{ if .IsManifestList }} - {{ .PlatformCount }} platforms + {{ if .Platforms }} +
+ {{ range .Platforms }} + {{ .OS }}/{{ .Architecture }}{{ if .Variant }}/{{ .Variant }}{{ end }} + {{ end }} +
+ {{ end }} {{ end }} diff --git a/pkg/auth/oauth/client.go b/pkg/auth/oauth/client.go index 1c80c54..49b50a6 100644 --- a/pkg/auth/oauth/client.go +++ b/pkg/auth/oauth/client.go @@ -139,3 +139,30 @@ func GetDefaultScopes(did string) []string { fmt.Sprintf("repo:%s", atproto.SailorProfileCollection), } } + +// ScopesMatch checks if two scope lists are equivalent (order-independent) +// Returns true if both lists contain the same scopes, regardless of order +func ScopesMatch(stored, desired []string) bool { + // Handle nil/empty cases + if len(stored) == 0 && len(desired) == 0 { + return true + } + if len(stored) != len(desired) { + return false + } + + // Build map of desired scopes for O(1) lookup + desiredMap := make(map[string]bool, len(desired)) + for _, scope := range desired { + desiredMap[scope] = true + } + + // Check if all stored scopes exist in desired + for _, scope := range stored { + if !desiredMap[scope] { + return false + } + } + + return true +} diff --git a/pkg/auth/oauth/client_test.go b/pkg/auth/oauth/client_test.go new file mode 100644 index 0000000..8668ff6 --- /dev/null +++ b/pkg/auth/oauth/client_test.go @@ -0,0 +1,65 @@ +package oauth + +import "testing" + +func TestScopesMatch(t *testing.T) { + tests := []struct { + name string + stored []string + desired []string + expected bool + }{ + { + name: "exact match", + stored: []string{"atproto", "blob:image/png"}, + desired: []string{"atproto", "blob:image/png"}, + expected: true, + }, + { + name: "different order", + stored: []string{"blob:image/png", "atproto"}, + desired: []string{"atproto", "blob:image/png"}, + expected: true, + }, + { + name: "missing scope in stored", + stored: []string{"atproto"}, + desired: []string{"atproto", "blob:image/png"}, + expected: false, + }, + { + name: "extra scope in stored", + stored: []string{"atproto", "blob:image/png", "extra"}, + desired: []string{"atproto", "blob:image/png"}, + expected: false, + }, + { + name: "both empty", + stored: []string{}, + desired: []string{}, + expected: true, + }, + { + name: "nil vs empty", + stored: nil, + desired: []string{}, + expected: true, + }, + { + name: "completely different", + stored: []string{"foo", "bar"}, + desired: []string{"baz", "qux"}, + expected: false, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + result := ScopesMatch(tt.stored, tt.desired) + if result != tt.expected { + t.Errorf("ScopesMatch(%v, %v) = %v, want %v", + tt.stored, tt.desired, result, tt.expected) + } + }) + } +} diff --git a/pkg/auth/oauth/refresher.go b/pkg/auth/oauth/refresher.go index ec281f8..99e754d 100644 --- a/pkg/auth/oauth/refresher.go +++ b/pkg/auth/oauth/refresher.go @@ -106,11 +106,26 @@ func (r *Refresher) resumeSession(ctx context.Context, did string) (*oauth.Clien return nil, fmt.Errorf("store must implement GetLatestSessionForDID (SQLite store required)") } - _, sessionID, err := getter.GetLatestSessionForDID(ctx, did) + sessionData, sessionID, err := getter.GetLatestSessionForDID(ctx, did) if err != nil { return nil, fmt.Errorf("no session found for DID: %s", did) } + // Validate that session scopes match current desired scopes + desiredScopes := r.app.GetConfig().Scopes + if !ScopesMatch(sessionData.Scopes, desiredScopes) { + fmt.Printf("DEBUG [oauth/refresher]: Scope mismatch for DID %s - deleting session\n", did) + fmt.Printf(" Stored scopes: %v\n", sessionData.Scopes) + fmt.Printf(" Desired scopes: %v\n", desiredScopes) + + // Delete the session from database since scopes have changed + if err := r.app.clientApp.Store.DeleteSession(ctx, accountDID, sessionID); err != nil { + fmt.Printf("WARNING [oauth/refresher]: Failed to delete session with mismatched scopes: %v\n", err) + } + + return nil, fmt.Errorf("OAuth scopes changed, re-authentication required") + } + // Resume session session, err := r.app.ResumeSession(ctx, accountDID, sessionID) if err != nil {