diff --git a/go.mod b/go.mod --- a/go.mod +++ b/go.mod @@ -4,16 +4,13 @@ require ( github.com/bluesky-social/indigo v0.0.0-20251223190123-598fbf0e146e - //github.com/gen2brain/beeep v0.11.1 - github.com/golang-jwt/jwt/v5 v5.2.2 // indirect + github.com/godbus/dbus/v5 v5.1.0 github.com/ipfs/go-cid v0.5.0 github.com/pkg/browser v0.0.0-20240102092130-5ac0b6a4141c github.com/spf13/cobra v1.9.1 github.com/zalando/go-keyring v0.2.6 -//golang.design/x/clipboard v0.7.1 + golang.org/x/term v0.38.0 ) - -require github.com/godbus/dbus/v5 v5.1.0 require ( al.essio.dev/pkg/shellescape v1.6.0 // indirect @@ -22,6 +19,7 @@ github.com/danieljoos/wincred v1.2.2 // indirect github.com/davecgh/go-spew v1.1.2-0.20180830191138-d8f796af33cc // indirect github.com/earthboundkid/versioninfo/v2 v2.24.1 // indirect + github.com/golang-jwt/jwt/v5 v5.2.2 // indirect github.com/google/go-querystring v1.1.0 // indirect github.com/hashicorp/golang-lru/v2 v2.0.7 // indirect github.com/inconshreveable/mousetrap v1.1.0 // indirect @@ -45,7 +43,7 @@ gitlab.com/yawning/secp256k1-voi v0.0.0-20230925100816-f2616030848b // indirect gitlab.com/yawning/tuplehash v0.0.0-20230713102510-df83abbf9a02 // indirect golang.org/x/crypto v0.39.0 // indirect - golang.org/x/sys v0.33.0 // indirect + golang.org/x/sys v0.39.0 // indirect golang.org/x/time v0.8.0 // indirect golang.org/x/xerrors v0.0.0-20240903120638-7835f813f4da // indirect google.golang.org/protobuf v1.36.6 // indirect diff --git a/go.sum b/go.sum --- a/go.sum +++ b/go.sum @@ -82,8 +82,10 @@ golang.org/x/crypto v0.39.0 h1:SHs+kF4LP+f+p14esP5jAoDpHU8Gu/v9lFRK6IT5imM= golang.org/x/crypto v0.39.0/go.mod h1:L+Xg3Wf6HoL4Bn4238Z6ft6KfEpN0tJGo53AAPC632U= golang.org/x/sys v0.1.0/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= -golang.org/x/sys v0.33.0 h1:q3i8TbbEz+JRD9ywIRlyRAQbM0qF7hu24q3teo2hbuw= -golang.org/x/sys v0.33.0/go.mod h1:BJP2sWEmIv4KK5OTEluFJCKSidICx8ciO85XgH3Ak8k= +golang.org/x/sys v0.39.0 h1:CvCKL8MeisomCi6qNZ+wbb0DN9E5AATixKsvNtMoMFk= +golang.org/x/sys v0.39.0/go.mod h1:OgkHotnGiDImocRcuBABYBEXf8A9a87e/uXjp9XT3ks= +golang.org/x/term v0.38.0 h1:PQ5pkm/rLO6HnxFR7N2lJHOZX6Kez5Y1gDSJla6jo7Q= +golang.org/x/term v0.38.0/go.mod h1:bSEAKrOT1W+VSu9TSCMtoGEOUcKxOKgl3LE5QEF/xVg= golang.org/x/time v0.8.0 h1:9i3RxcPv3PZnitoVGMPDKZSq1xW1gK1Xy3ArNOGZfEg= golang.org/x/time v0.8.0/go.mod h1:3BpzKBy/shNhVucY/MWOyx10tF3SFh9QdLuxbVysPQM= golang.org/x/xerrors v0.0.0-20191204190536-9bdfabe68543/go.mod h1:I/5z698sn9Ka8TeJc9MKroUUfqBBauWjQqLJ2OPfmY0= diff --git a/cmd/blup/main.go b/cmd/blup/main.go --- a/cmd/blup/main.go +++ b/cmd/blup/main.go @@ -3,6 +3,7 @@ import ( "context" "encoding/json" + "errors" "fmt" "io" "log/slog" @@ -14,12 +15,12 @@ "github.com/bluesky-social/indigo/api/atproto" "github.com/bluesky-social/indigo/atproto/atclient" - lex_util "github.com/bluesky-social/indigo/lex/util" + "github.com/bluesky-social/indigo/atproto/syntax" "github.com/spf13/cobra" "tangled.sh/evan.jarrett.net/blup/internal/auth" "tangled.sh/evan.jarrett.net/blup/internal/clipboard" "tangled.sh/evan.jarrett.net/blup/internal/screenshot" - "tangled.sh/evan.jarrett.net/blup/internal/util" + "tangled.sh/evan.jarrett.net/blup/internal/ui" ) const ( @@ -159,6 +160,14 @@ } func runCapture(cmd *cobra.Command, args []string) error { + // Pre-check auth in non-interactive mode BEFORE taking screenshot. + // This prevents the confusing GNOME "screenshot captured" notification + // when upload will fail anyway due to missing auth. + if !ui.IsInteractive() && !auth.HasSavedCredentials() { + ui.NotifyLoginRequired() + return auth.ErrNoLoginIdentifier + } + // Take screenshot using XDG Desktop Portal fmt.Println("Opening screenshot dialog...") imagePath, err := screenshot.CaptureScreenshot(true) @@ -176,6 +185,9 @@ // Copy to clipboard using wl-copy if err := copyToClipboard(url); err != nil { fmt.Fprintf(os.Stderr, "Warning: failed to copy to clipboard: %v\n", err) + if !ui.IsInteractive() { + ui.NotifyError("Failed to copy URL to clipboard") + } } fmt.Println(url) @@ -228,8 +240,24 @@ // Get authenticated session (will re-auth if needed using saved login identifier) sess, err := auth.RefreshTokens("") + if errors.Is(err, auth.ErrNoLoginIdentifier) { + if !ui.IsInteractive() { + // Non-interactive mode (keyboard shortcut) - show notification and fail + ui.NotifyLoginRequired() + return "", auth.ErrNoLoginIdentifier + } + // Interactive mode - prompt for login + fmt.Print("Enter your ATProto handle: ") + var handle string + fmt.Scanln(&handle) + handle = strings.TrimPrefix(handle, "@") + sess, err = auth.RefreshTokens(handle) + } if err != nil { - return "", fmt.Errorf("not authenticated, run '%s login' first: %w", Name, err) + if !ui.IsInteractive() { + ui.NotifyError(err.Error()) + } + return "", fmt.Errorf("authentication failed: %w", err) } // Get API client from session @@ -295,15 +323,15 @@ return "", fmt.Errorf("failed to create record: %w", err) } - // Extract blob CID for server URL - blob := record["blob"].(*lex_util.LexBlob) - blobCID := blob.Ref.String() - converted, err := util.ConvertCIDBase32ToBase62(blobCID) + // Extract rkey from the record URI for shorter URLs + // URI format: at://did:plc:xxx/blue.imgs.blup.image/{rkey} + uri, err := syntax.ParseATURI(recordOut.Uri) if err != nil { - return "", err + return "", fmt.Errorf("failed to parse record URI: %w", err) } + rkey := uri.RecordKey().String() - return fmt.Sprintf("%s/%s/%s", CDN, handle, converted), nil + return fmt.Sprintf("%s/%s/%s", CDN, handle, rkey), nil } func setupLogging() { diff --git a/internal/auth/keyring.go b/internal/auth/keyring.go new file mode 100644 --- /dev/null +++ b/internal/auth/keyring.go @@ -0,0 +1,31 @@ +package auth + +import ( + "github.com/zalando/go-keyring" +) + +// Keyring defines the interface for keyring operations. +// This allows mocking the system keyring for testing. +type Keyring interface { + Get(service, key string) (string, error) + Set(service, key, value string) error + Delete(service, key string) error +} + +// RealKeyring implements Keyring using the system keyring. +type RealKeyring struct{} + +func (r *RealKeyring) Get(service, key string) (string, error) { + return keyring.Get(service, key) +} + +func (r *RealKeyring) Set(service, key, value string) error { + return keyring.Set(service, key, value) +} + +func (r *RealKeyring) Delete(service, key string) error { + return keyring.Delete(service, key) +} + +// DefaultKeyring is the default keyring implementation. +var DefaultKeyring Keyring = &RealKeyring{} diff --git a/internal/auth/metadata_test.go b/internal/auth/metadata_test.go new file mode 100644 --- /dev/null +++ b/internal/auth/metadata_test.go @@ -0,0 +1,119 @@ +package auth + +import ( + "testing" +) + +func TestGetClientMetadata(t *testing.T) { + metadata := GetClientMetadata() + + // Verify required fields are populated + if metadata.ClientID == "" { + t.Error("ClientID should not be empty") + } + if metadata.ClientName == "" { + t.Error("ClientName should not be empty") + } + if metadata.ClientURI == "" { + t.Error("ClientURI should not be empty") + } + if len(metadata.RedirectURIs) == 0 { + t.Error("RedirectURIs should not be empty") + } + if len(metadata.GrantTypes) == 0 { + t.Error("GrantTypes should not be empty") + } + if metadata.Scope == "" { + t.Error("Scope should not be empty") + } + + // Verify expected values from client-metadata.json + if metadata.ClientID != "https://blup.imgs.blue/oauth-client-metadata.json" { + t.Errorf("ClientID = %q, want %q", metadata.ClientID, "https://blup.imgs.blue/oauth-client-metadata.json") + } + if metadata.ClientName != "Blup" { + t.Errorf("ClientName = %q, want %q", metadata.ClientName, "Blup") + } + if metadata.ClientURI != "https://blup.imgs.blue" { + t.Errorf("ClientURI = %q, want %q", metadata.ClientURI, "https://blup.imgs.blue") + } + if metadata.RedirectURIs[0] != "https://blup.imgs.blue/oauth/callback" { + t.Errorf("RedirectURIs[0] = %q, want %q", metadata.RedirectURIs[0], "https://blup.imgs.blue/oauth/callback") + } + if !metadata.DpopBoundAccessTokens { + t.Error("DpopBoundAccessTokens should be true") + } + if metadata.TokenEndpointAuthMethod != "none" { + t.Errorf("TokenEndpointAuthMethod = %q, want %q", metadata.TokenEndpointAuthMethod, "none") + } + if metadata.ApplicationType != "native" { + t.Errorf("ApplicationType = %q, want %q", metadata.ApplicationType, "native") + } +} + +func TestGetClientMetadataGrantTypes(t *testing.T) { + metadata := GetClientMetadata() + + expectedGrantTypes := []string{"authorization_code", "refresh_token"} + if len(metadata.GrantTypes) != len(expectedGrantTypes) { + t.Fatalf("GrantTypes length = %d, want %d", len(metadata.GrantTypes), len(expectedGrantTypes)) + } + + for i, gt := range expectedGrantTypes { + if metadata.GrantTypes[i] != gt { + t.Errorf("GrantTypes[%d] = %q, want %q", i, metadata.GrantTypes[i], gt) + } + } +} + +func TestGetClientMetadataResponseTypes(t *testing.T) { + metadata := GetClientMetadata() + + if len(metadata.ResponseTypes) != 1 { + t.Fatalf("ResponseTypes length = %d, want 1", len(metadata.ResponseTypes)) + } + if metadata.ResponseTypes[0] != "code" { + t.Errorf("ResponseTypes[0] = %q, want %q", metadata.ResponseTypes[0], "code") + } +} + +func TestGetClientConfig(t *testing.T) { + config := GetClientConfig() + + // Verify the config is built correctly + if config.ClientID == "" { + t.Error("ClientID should not be empty") + } + if config.CallbackURL == "" { + t.Error("CallbackURL should not be empty") + } + if len(config.Scopes) == 0 { + t.Error("Scopes should not be empty") + } + + // Verify expected values + if config.ClientID != "https://blup.imgs.blue/oauth-client-metadata.json" { + t.Errorf("ClientID = %q, want %q", config.ClientID, "https://blup.imgs.blue/oauth-client-metadata.json") + } + if config.CallbackURL != "https://blup.imgs.blue/oauth/callback" { + t.Errorf("CallbackURL = %q, want %q", config.CallbackURL, "https://blup.imgs.blue/oauth/callback") + } +} + +func TestGetClientConfigScopes(t *testing.T) { + config := GetClientConfig() + + // Scope in metadata is "atproto repo:blue.imgs.blup.image blob:image/*" + // Should be split into 3 scopes + expectedScopes := []string{"atproto", "repo:blue.imgs.blup.image", "blob:image/*"} + + if len(config.Scopes) != len(expectedScopes) { + t.Fatalf("Scopes length = %d, want %d; got %v", len(config.Scopes), len(expectedScopes), config.Scopes) + } + + for i, scope := range expectedScopes { + if config.Scopes[i] != scope { + t.Errorf("Scopes[%d] = %q, want %q", i, config.Scopes[i], scope) + } + } +} diff --git a/internal/auth/oauth.go b/internal/auth/oauth.go --- a/internal/auth/oauth.go +++ b/internal/auth/oauth.go @@ -4,6 +4,7 @@ "bufio" "context" "encoding/json" + "errors" "fmt" "log/slog" "net/http" @@ -17,6 +18,87 @@ "github.com/pkg/browser" ) +// ErrNoLoginIdentifier is returned when no active session exists and no login identifier +// is saved. Callers should prompt the user for their handle and retry. +var ErrNoLoginIdentifier = errors.New("no active session and no login identifier provided") + +// SSEAuthData holds the parsed auth completion data from SSE events. +type SSEAuthData struct { + Code string `json:"code"` + Iss string `json:"iss"` + State string `json:"state"` +} + +// parseSSEAuthData parses the JSON data from an SSE auth-complete event. +func parseSSEAuthData(data string) (*SSEAuthData, error) { + data = strings.TrimSpace(data) + if data == "" { + return nil, fmt.Errorf("empty auth data") + } + + var authData SSEAuthData + if err := json.Unmarshal([]byte(data), &authData); err != nil { + return nil, fmt.Errorf("failed to parse auth data: %w", err) + } + + if authData.Code == "" || authData.Iss == "" || authData.State == "" { + return nil, fmt.Errorf("missing required fields in auth data") + } + + return &authData, nil +} + +// NewClientApp creates a new OAuth client app and keyring store. +// Optionally accepts a KeyringAuthStore for testing; if not provided, creates a real one. +func NewClientApp(stores ...*KeyringAuthStore) (*oauth.ClientApp, *KeyringAuthStore) { + var store *KeyringAuthStore + if len(stores) > 0 && stores[0] != nil { + store = stores[0] + } else { + store = NewKeyringAuthStore() + } + clientConfig := GetClientConfig() + app := oauth.NewClientApp(&clientConfig, store) + return app, store +} + +// AuthenticateAndResume performs a full OAuth authentication flow and returns a resumed session. +func AuthenticateAndResume(ctx context.Context, loginIdentifier string) (*oauth.ClientSession, error) { + flow, err := NewOAuthFlow(loginIdentifier) + if err != nil { + return nil, err + } + sess, err := flow.Authenticate() + if err != nil { + return nil, err + } + app, _ := NewClientApp() + return app.ResumeSession(ctx, sess.AccountDID, sess.SessionID) +} + +// ResumeCurrentSession retrieves the current session from the store and resumes it. +// Returns the session, session data, and any error. +func ResumeCurrentSession(ctx context.Context) (*oauth.ClientSession, *oauth.ClientSessionData, error) { + app, store := NewClientApp() + sessData, err := store.GetCurrentSession(ctx) + if err != nil { + return nil, nil, err + } + sess, err := app.ResumeSession(ctx, sessData.AccountDID, sessData.SessionID) + if err != nil { + return nil, sessData, err + } + return sess, sessData, nil +} + +// HTTPDoer abstracts HTTP client operations for testing. +type HTTPDoer interface { + Do(req *http.Request) (*http.Response, error) +} + +// BrowserOpener is a function that opens a URL in the browser. +type BrowserOpener func(url string) error + type OAuthFlow struct { app *oauth.ClientApp loginIdentifier string @@ -24,10 +106,59 @@ authSuccess chan *oauth.ClientSessionData authError chan error savedState string + + // Injectable dependencies (nil means use defaults) + httpClient HTTPDoer + openBrowser BrowserOpener } -func NewOAuthFlow(loginIdentifier string) (*OAuthFlow, error) { - store := NewKeyringAuthStore() +// OAuthFlowOption configures an OAuthFlow. +type OAuthFlowOption func(*OAuthFlow) + +// WithHTTPClient sets a custom HTTP client for the OAuth flow. +func WithHTTPClient(client HTTPDoer) OAuthFlowOption { + return func(f *OAuthFlow) { + f.httpClient = client + } +} + +// WithBrowserOpener sets a custom browser opener for the OAuth flow. +func WithBrowserOpener(opener BrowserOpener) OAuthFlowOption { + return func(f *OAuthFlow) { + f.openBrowser = opener + } +} + +// WithStore sets a custom keyring store for the OAuth flow. +func WithStore(store *KeyringAuthStore) OAuthFlowOption { + return func(f *OAuthFlow) { + f.store = store + } +} + +func NewOAuthFlow(loginIdentifier string, opts ...OAuthFlowOption) (*OAuthFlow, error) { + flow := &OAuthFlow{ + loginIdentifier: loginIdentifier, + authSuccess: make(chan *oauth.ClientSessionData, 1), + authError: make(chan error, 1), + } + + // Apply options + for _, opt := range opts { + opt(flow) + } + + // Set defaults for nil dependencies + if flow.store == nil { + flow.store = NewKeyringAuthStore() + } + if flow.httpClient == nil { + flow.httpClient = &http.Client{Timeout: 5 * time.Minute} + } + if flow.openBrowser == nil { + flow.openBrowser = browser.OpenURL + } + clientConfig := GetClientConfig() // Debug: show what we're requesting @@ -39,15 +170,9 @@ "scope_raw", metadata.Scope, ) - app := oauth.NewClientApp(&clientConfig, store) + flow.app = oauth.NewClientApp(&clientConfig, flow.store) - return &OAuthFlow{ - app: app, - loginIdentifier: loginIdentifier, - store: store, - authSuccess: make(chan *oauth.ClientSessionData, 1), - authError: make(chan error, 1), - }, nil + return flow, nil } func (f *OAuthFlow) Authenticate() (*oauth.ClientSessionData, error) { @@ -75,7 +200,7 @@ // Open browser to authorization URL fmt.Printf("Opening browser for authentication...\n") - if err := browser.OpenURL(redirectURL); err != nil { + if err := f.openBrowser(redirectURL); err != nil { fmt.Printf("Failed to open browser automatically.\n") fmt.Printf("Please open this URL manually:\n%s\n", redirectURL) } @@ -119,8 +244,7 @@ req.Header.Set("Accept", "text/event-stream") req.Header.Set("Cache-Control", "no-cache") - client := &http.Client{Timeout: 5 * time.Minute} - resp, err := client.Do(req) + resp, err := f.httpClient.Do(req) if err != nil { slog.Debug("SSE connection error", "error", err) f.authError <- err @@ -149,14 +273,9 @@ dataLine, _ := reader.ReadString('\n') slog.Debug("SSE auth data", "data", dataLine) if after, ok := strings.CutPrefix(dataLine, "data: "); ok { - data := after - var authData struct { - Code string `json:"code"` - Iss string `json:"iss"` - State string `json:"state"` - } - if err := json.Unmarshal([]byte(data), &authData); err != nil { - f.authError <- fmt.Errorf("failed to parse auth data: %w", err) + authData, err := parseSSEAuthData(after) + if err != nil { + f.authError <- err return } slog.Debug("SSE auth complete", "iss", authData.Iss, "state", authData.State) @@ -201,24 +320,10 @@ // GetSession retrieves the current session, refreshing tokens if needed func GetSession() (*oauth.ClientSession, error) { - ctx := context.Background() - store := NewKeyringAuthStore() - - // Check for current session - sessData, err := store.GetCurrentSession(ctx) - if err != nil { - return nil, fmt.Errorf("no active session") - } - - clientConfig := GetClientConfig() - app := oauth.NewClientApp(&clientConfig, store) - - // Resume the session - sess, err := app.ResumeSession(ctx, sessData.AccountDID, sessData.SessionID) + sess, _, err := ResumeCurrentSession(context.Background()) if err != nil { return nil, fmt.Errorf("failed to resume session: %w", err) } - return sess, nil } @@ -227,70 +332,55 @@ // If empty, it will use the saved login identifier from keyring func RefreshTokens(loginIdentifier string) (*oauth.ClientSession, error) { ctx := context.Background() + + // Try to resume current session + sess, _, err := ResumeCurrentSession(ctx) + if err == nil { + return sess, nil + } + + // Session doesn't exist or failed to resume - need to authenticate + // Get login identifier from keyring if not provided + if loginIdentifier == "" { + _, store := NewClientApp() + loginIdentifier, err = store.GetLoginIdentifier() + if err != nil { + return nil, ErrNoLoginIdentifier + } + } + + return AuthenticateAndResume(ctx, loginIdentifier) +} + +// HasSavedCredentials returns true if there's a saved session or login identifier. +// This is a lightweight check that doesn't attempt authentication. +func HasSavedCredentials() bool { + ctx := context.Background() store := NewKeyringAuthStore() - // Check for current session - sessData, err := store.GetCurrentSession(ctx) - if err != nil { - // No session, need to authenticate - if loginIdentifier == "" { - return nil, fmt.Errorf("no active session and no login identifier provided") - } - flow, err := NewOAuthFlow(loginIdentifier) - if err != nil { - return nil, err - } - sess, err := flow.Authenticate() - if err != nil { - return nil, err - } - // Resume the session to return a ClientSession - clientConfig := GetClientConfig() - app := oauth.NewClientApp(&clientConfig, store) - return app.ResumeSession(ctx, sess.AccountDID, sess.SessionID) + // Check for existing session + if _, err := store.GetCurrentSession(ctx); err == nil { + return true } - clientConfig := GetClientConfig() - app := oauth.NewClientApp(&clientConfig, store) - - // Resume the session - this will auto-refresh tokens on 401 - sess, err := app.ResumeSession(ctx, sessData.AccountDID, sessData.SessionID) - if err != nil { - // Session invalid, need to re-authenticate - // Get login identifier from keyring if not provided - if loginIdentifier == "" { - loginIdentifier, err = store.GetLoginIdentifier() - if err != nil { - return nil, fmt.Errorf("session expired and no saved login identifier: %w", err) - } - } - flow, err := NewOAuthFlow(loginIdentifier) - if err != nil { - return nil, err - } - newSess, err := flow.Authenticate() - if err != nil { - return nil, err - } - return app.ResumeSession(ctx, newSess.AccountDID, newSess.SessionID) + // Check for saved login identifier (can be used to re-authenticate) + if _, err := store.GetLoginIdentifier(); err == nil { + return true } - return sess, nil + return false } // Logout revokes tokens and clears the session func Logout() error { ctx := context.Background() - store := NewKeyringAuthStore() + app, store := NewClientApp() sessData, err := store.GetCurrentSession(ctx) if err != nil { // No session to logout return nil } - - clientConfig := GetClientConfig() - app := oauth.NewClientApp(&clientConfig, store) // Logout revokes tokens and deletes session if err := app.Logout(ctx, sessData.AccountDID, sessData.SessionID); err != nil { diff --git a/internal/auth/oauth_test.go b/internal/auth/oauth_test.go new file mode 100644 --- /dev/null +++ b/internal/auth/oauth_test.go @@ -0,0 +1,246 @@ +package auth + +import ( + "net/http" + "testing" +) + +func TestNewOAuthFlow(t *testing.T) { + flow, err := NewOAuthFlow("test.bsky.social") + if err != nil { + t.Fatalf("NewOAuthFlow() error = %v", err) + } + + if flow == nil { + t.Fatal("NewOAuthFlow() returned nil") + } + + if flow.loginIdentifier != "test.bsky.social" { + t.Errorf("loginIdentifier = %q, want %q", flow.loginIdentifier, "test.bsky.social") + } + + if flow.app == nil { + t.Error("app should not be nil") + } + + if flow.store == nil { + t.Error("store should not be nil") + } + + if flow.authSuccess == nil { + t.Error("authSuccess channel should not be nil") + } + + if flow.authError == nil { + t.Error("authError channel should not be nil") + } + + if flow.httpClient == nil { + t.Error("httpClient should not be nil (default)") + } + + if flow.openBrowser == nil { + t.Error("openBrowser should not be nil (default)") + } +} + +func TestNewOAuthFlowEmptyIdentifier(t *testing.T) { + flow, err := NewOAuthFlow("") + if err != nil { + t.Fatalf("NewOAuthFlow() error = %v", err) + } + + if flow == nil { + t.Fatal("NewOAuthFlow() returned nil") + } + + if flow.loginIdentifier != "" { + t.Errorf("loginIdentifier = %q, want empty string", flow.loginIdentifier) + } +} + +func TestAuthenticateRequiresIdentifier(t *testing.T) { + flow, err := NewOAuthFlow("") + if err != nil { + t.Fatalf("NewOAuthFlow() error = %v", err) + } + + _, err = flow.Authenticate() + if err == nil { + t.Error("Authenticate() expected error with empty identifier, got nil") + } + + expectedErr := "login identifier is required" + if err.Error() != expectedErr { + t.Errorf("Authenticate() error = %q, want %q", err.Error(), expectedErr) + } +} + +// MockHTTPClient implements HTTPDoer for testing +type MockHTTPClient struct { + DoFunc func(req *http.Request) (*http.Response, error) +} + +func (m *MockHTTPClient) Do(req *http.Request) (*http.Response, error) { + return m.DoFunc(req) +} + +func TestNewOAuthFlowWithOptions(t *testing.T) { + mockStore := NewKeyringAuthStoreWithKeyring(NewMockKeyring()) + mockHTTP := &MockHTTPClient{} + browserCalled := false + mockBrowser := func(url string) error { + browserCalled = true + return nil + } + + flow, err := NewOAuthFlow("test.bsky.social", + WithStore(mockStore), + WithHTTPClient(mockHTTP), + WithBrowserOpener(mockBrowser), + ) + if err != nil { + t.Fatalf("NewOAuthFlow() error = %v", err) + } + + if flow.store != mockStore { + t.Error("store was not injected correctly") + } + + if flow.httpClient != mockHTTP { + t.Error("httpClient was not injected correctly") + } + + // Test browser opener was injected + flow.openBrowser("http://test.com") + if !browserCalled { + t.Error("openBrowser was not injected correctly") + } +} + +func TestNewClientAppWithStore(t *testing.T) { + mockKeyring := NewMockKeyring() + mockStore := NewKeyringAuthStoreWithKeyring(mockKeyring) + + app, store := NewClientApp(mockStore) + + if app == nil { + t.Error("NewClientApp() app should not be nil") + } + + if store != mockStore { + t.Error("NewClientApp() should return the injected store") + } +} + +func TestNewClientAppWithoutStore(t *testing.T) { + app, store := NewClientApp() + + if app == nil { + t.Error("NewClientApp() app should not be nil") + } + + if store == nil { + t.Error("NewClientApp() should create a default store") + } +} + +func TestParseSSEAuthData(t *testing.T) { + tests := []struct { + name string + input string + wantErr bool + code string + iss string + state string + }{ + { + name: "valid auth data", + input: `{"code":"auth_code_123","iss":"https://bsky.social","state":"state_abc"}`, + wantErr: false, + code: "auth_code_123", + iss: "https://bsky.social", + state: "state_abc", + }, + { + name: "valid with whitespace", + input: ` {"code":"code","iss":"iss","state":"state"} `, + wantErr: false, + code: "code", + iss: "iss", + state: "state", + }, + { + name: "empty string", + input: "", + wantErr: true, + }, + { + name: "whitespace only", + input: " ", + wantErr: true, + }, + { + name: "invalid JSON", + input: "not json", + wantErr: true, + }, + { + name: "missing code", + input: `{"iss":"https://bsky.social","state":"state_abc"}`, + wantErr: true, + }, + { + name: "missing iss", + input: `{"code":"auth_code_123","state":"state_abc"}`, + wantErr: true, + }, + { + name: "missing state", + input: `{"code":"auth_code_123","iss":"https://bsky.social"}`, + wantErr: true, + }, + { + name: "empty code", + input: `{"code":"","iss":"https://bsky.social","state":"state_abc"}`, + wantErr: true, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + result, err := parseSSEAuthData(tt.input) + + if tt.wantErr { + if err == nil { + t.Errorf("parseSSEAuthData() expected error, got nil") + } + return + } + + if err != nil { + t.Errorf("parseSSEAuthData() unexpected error: %v", err) + return + } + + if result.Code != tt.code { + t.Errorf("Code = %q, want %q", result.Code, tt.code) + } + if result.Iss != tt.iss { + t.Errorf("Iss = %q, want %q", result.Iss, tt.iss) + } + if result.State != tt.state { + t.Errorf("State = %q, want %q", result.State, tt.state) + } + }) + } +} + +func TestLogoutNoSession(t *testing.T) { + // Logout should succeed even when no session exists + // Note: This uses the real keyring, but should still work + // because it handles the "no session" case gracefully + err := Logout() + // We can't easily test this without mocking, but we can verify it doesn't panic + _ = err +} diff --git a/internal/auth/storage.go b/internal/auth/storage.go --- a/internal/auth/storage.go +++ b/internal/auth/storage.go @@ -7,7 +7,6 @@ "github.com/bluesky-social/indigo/atproto/auth/oauth" "github.com/bluesky-social/indigo/atproto/syntax" - "github.com/zalando/go-keyring" ) const ( @@ -20,10 +19,19 @@ ) // KeyringAuthStore implements oauth.ClientAuthStore using the system keyring -type KeyringAuthStore struct{} +type KeyringAuthStore struct { + keyring Keyring +} +// NewKeyringAuthStore creates a new KeyringAuthStore using the system keyring. func NewKeyringAuthStore() *KeyringAuthStore { - return &KeyringAuthStore{} + return &KeyringAuthStore{keyring: DefaultKeyring} +} + +// NewKeyringAuthStoreWithKeyring creates a KeyringAuthStore with a custom Keyring implementation. +// This is useful for testing with a mock keyring. +func NewKeyringAuthStoreWithKeyring(kr Keyring) *KeyringAuthStore { + return &KeyringAuthStore{keyring: kr} } // sessionKey creates the keyring key for a session @@ -33,7 +41,7 @@ // GetSession retrieves a session from the keyring func (s *KeyringAuthStore) GetSession(ctx context.Context, did syntax.DID, sessionID string) (*oauth.ClientSessionData, error) { - data, err := keyring.Get(keyringService, sessionKey(did, sessionID)) + data, err := s.keyring.Get(keyringService, sessionKey(did, sessionID)) if err != nil { return nil, err } @@ -53,12 +61,12 @@ return err } - return keyring.Set(keyringService, sessionKey(sess.AccountDID, sess.SessionID), string(data)) + return s.keyring.Set(keyringService, sessionKey(sess.AccountDID, sess.SessionID), string(data)) } // DeleteSession removes a session from the keyring func (s *KeyringAuthStore) DeleteSession(ctx context.Context, did syntax.DID, sessionID string) error { - return keyring.Delete(keyringService, sessionKey(did, sessionID)) + return s.keyring.Delete(keyringService, sessionKey(did, sessionID)) } // authRequestKey creates the keyring key for an auth request @@ -68,7 +76,7 @@ // GetAuthRequestInfo retrieves pending auth request info func (s *KeyringAuthStore) GetAuthRequestInfo(ctx context.Context, state string) (*oauth.AuthRequestData, error) { - data, err := keyring.Get(keyringService, authRequestKey(state)) + data, err := s.keyring.Get(keyringService, authRequestKey(state)) if err != nil { return nil, err } @@ -89,27 +97,27 @@ } // Save the auth request data - if err := keyring.Set(keyringService, authRequestKey(info.State), string(data)); err != nil { + if err := s.keyring.Set(keyringService, authRequestKey(info.State), string(data)); err != nil { return err } // Also save the state as the current pending auth (for SSE correlation) - return keyring.Set(keyringService, pendingAuthStateKey, info.State) + return s.keyring.Set(keyringService, pendingAuthStateKey, info.State) } // GetPendingAuthState returns the state of the current pending auth request func (s *KeyringAuthStore) GetPendingAuthState() (string, error) { - return keyring.Get(keyringService, pendingAuthStateKey) + return s.keyring.Get(keyringService, pendingAuthStateKey) } // ClearPendingAuthState removes the pending auth state func (s *KeyringAuthStore) ClearPendingAuthState() error { - return keyring.Delete(keyringService, pendingAuthStateKey) + return s.keyring.Delete(keyringService, pendingAuthStateKey) } // DeleteAuthRequestInfo removes pending auth request info func (s *KeyringAuthStore) DeleteAuthRequestInfo(ctx context.Context, state string) error { - return keyring.Delete(keyringService, authRequestKey(state)) + return s.keyring.Delete(keyringService, authRequestKey(state)) } // CurrentSessionRef stores reference to the current active session @@ -120,7 +128,7 @@ // GetCurrentSession retrieves the current active session for the CLI func (s *KeyringAuthStore) GetCurrentSession(ctx context.Context) (*oauth.ClientSessionData, error) { - refData, err := keyring.Get(keyringService, currentSessionKey) + refData, err := s.keyring.Get(keyringService, currentSessionKey) if err != nil { return nil, err } @@ -150,25 +158,25 @@ return err } - return keyring.Set(keyringService, currentSessionKey, string(data)) + return s.keyring.Set(keyringService, currentSessionKey, string(data)) } // ClearCurrentSession removes the current session reference func (s *KeyringAuthStore) ClearCurrentSession() error { - return keyring.Delete(keyringService, currentSessionKey) + return s.keyring.Delete(keyringService, currentSessionKey) } // GetLoginIdentifier retrieves the stored login identifier (handle or PDS URL) func (s *KeyringAuthStore) GetLoginIdentifier() (string, error) { - return keyring.Get(keyringService, loginIdentifierKey) + return s.keyring.Get(keyringService, loginIdentifierKey) } // SetLoginIdentifier stores the login identifier for re-authentication func (s *KeyringAuthStore) SetLoginIdentifier(id string) error { - return keyring.Set(keyringService, loginIdentifierKey, id) + return s.keyring.Set(keyringService, loginIdentifierKey, id) } // ClearLoginIdentifier removes the stored login identifier func (s *KeyringAuthStore) ClearLoginIdentifier() error { - return keyring.Delete(keyringService, loginIdentifierKey) + return s.keyring.Delete(keyringService, loginIdentifierKey) } diff --git a/internal/auth/storage_test.go b/internal/auth/storage_test.go new file mode 100644 --- /dev/null +++ b/internal/auth/storage_test.go @@ -0,0 +1,318 @@ +package auth + +import ( + "context" + "errors" + "testing" + + "github.com/bluesky-social/indigo/atproto/auth/oauth" + "github.com/bluesky-social/indigo/atproto/syntax" +) + +// MockKeyring is an in-memory implementation of Keyring for testing. +type MockKeyring struct { + data map[string]map[string]string // service -> key -> value +} + +func NewMockKeyring() *MockKeyring { + return &MockKeyring{ + data: make(map[string]map[string]string), + } +} + +var ErrNotFound = errors.New("secret not found in keyring") + +func (m *MockKeyring) Get(service, key string) (string, error) { + if svc, ok := m.data[service]; ok { + if val, ok := svc[key]; ok { + return val, nil + } + } + return "", ErrNotFound +} + +func (m *MockKeyring) Set(service, key, value string) error { + if _, ok := m.data[service]; !ok { + m.data[service] = make(map[string]string) + } + m.data[service][key] = value + return nil +} + +func (m *MockKeyring) Delete(service, key string) error { + if svc, ok := m.data[service]; ok { + delete(svc, key) + } + return nil +} + +func TestSessionKey(t *testing.T) { + did, _ := syntax.ParseDID("did:plc:test123") + key := sessionKey(did, "session-abc") + expected := "session:did:plc:test123:session-abc" + if key != expected { + t.Errorf("sessionKey() = %q, want %q", key, expected) + } +} + +func TestAuthRequestKey(t *testing.T) { + key := authRequestKey("state123") + expected := "auth-request:state123" + if key != expected { + t.Errorf("authRequestKey() = %q, want %q", key, expected) + } +} + +func TestSaveAndGetSession(t *testing.T) { + mock := NewMockKeyring() + store := NewKeyringAuthStoreWithKeyring(mock) + ctx := context.Background() + + did, _ := syntax.ParseDID("did:plc:testuser") + sess := oauth.ClientSessionData{ + AccountDID: did, + SessionID: "test-session-id", + } + + // Save session + if err := store.SaveSession(ctx, sess); err != nil { + t.Fatalf("SaveSession() error = %v", err) + } + + // Get session + retrieved, err := store.GetSession(ctx, did, "test-session-id") + if err != nil { + t.Fatalf("GetSession() error = %v", err) + } + + if retrieved.AccountDID.String() != did.String() { + t.Errorf("GetSession() DID = %q, want %q", retrieved.AccountDID.String(), did.String()) + } + if retrieved.SessionID != "test-session-id" { + t.Errorf("GetSession() SessionID = %q, want %q", retrieved.SessionID, "test-session-id") + } +} + +func TestDeleteSession(t *testing.T) { + mock := NewMockKeyring() + store := NewKeyringAuthStoreWithKeyring(mock) + ctx := context.Background() + + did, _ := syntax.ParseDID("did:plc:testuser") + sess := oauth.ClientSessionData{ + AccountDID: did, + SessionID: "test-session-id", + } + + // Save then delete + store.SaveSession(ctx, sess) + if err := store.DeleteSession(ctx, did, "test-session-id"); err != nil { + t.Fatalf("DeleteSession() error = %v", err) + } + + // Should not be found + _, err := store.GetSession(ctx, did, "test-session-id") + if err == nil { + t.Error("GetSession() expected error after delete, got nil") + } +} + +func TestSaveAndGetAuthRequestInfo(t *testing.T) { + mock := NewMockKeyring() + store := NewKeyringAuthStoreWithKeyring(mock) + ctx := context.Background() + + info := oauth.AuthRequestData{ + State: "test-state-123", + } + + // Save auth request + if err := store.SaveAuthRequestInfo(ctx, info); err != nil { + t.Fatalf("SaveAuthRequestInfo() error = %v", err) + } + + // Get auth request + retrieved, err := store.GetAuthRequestInfo(ctx, "test-state-123") + if err != nil { + t.Fatalf("GetAuthRequestInfo() error = %v", err) + } + + if retrieved.State != "test-state-123" { + t.Errorf("GetAuthRequestInfo() State = %q, want %q", retrieved.State, "test-state-123") + } + + // Pending auth state should also be set + state, err := store.GetPendingAuthState() + if err != nil { + t.Fatalf("GetPendingAuthState() error = %v", err) + } + if state != "test-state-123" { + t.Errorf("GetPendingAuthState() = %q, want %q", state, "test-state-123") + } +} + +func TestDeleteAuthRequestInfo(t *testing.T) { + mock := NewMockKeyring() + store := NewKeyringAuthStoreWithKeyring(mock) + ctx := context.Background() + + info := oauth.AuthRequestData{ + State: "test-state-456", + } + + store.SaveAuthRequestInfo(ctx, info) + if err := store.DeleteAuthRequestInfo(ctx, "test-state-456"); err != nil { + t.Fatalf("DeleteAuthRequestInfo() error = %v", err) + } + + _, err := store.GetAuthRequestInfo(ctx, "test-state-456") + if err == nil { + t.Error("GetAuthRequestInfo() expected error after delete, got nil") + } +} + +func TestClearPendingAuthState(t *testing.T) { + mock := NewMockKeyring() + store := NewKeyringAuthStoreWithKeyring(mock) + ctx := context.Background() + + info := oauth.AuthRequestData{ + State: "test-state-789", + } + store.SaveAuthRequestInfo(ctx, info) + + if err := store.ClearPendingAuthState(); err != nil { + t.Fatalf("ClearPendingAuthState() error = %v", err) + } + + _, err := store.GetPendingAuthState() + if err == nil { + t.Error("GetPendingAuthState() expected error after clear, got nil") + } +} + +func TestSetAndGetCurrentSession(t *testing.T) { + mock := NewMockKeyring() + store := NewKeyringAuthStoreWithKeyring(mock) + ctx := context.Background() + + did, _ := syntax.ParseDID("did:plc:currentuser") + sess := oauth.ClientSessionData{ + AccountDID: did, + SessionID: "current-session-id", + } + + // Save the actual session first + if err := store.SaveSession(ctx, sess); err != nil { + t.Fatalf("SaveSession() error = %v", err) + } + + // Set as current session + if err := store.SetCurrentSession(ctx, &sess); err != nil { + t.Fatalf("SetCurrentSession() error = %v", err) + } + + // Get current session + retrieved, err := store.GetCurrentSession(ctx) + if err != nil { + t.Fatalf("GetCurrentSession() error = %v", err) + } + + if retrieved.AccountDID.String() != did.String() { + t.Errorf("GetCurrentSession() DID = %q, want %q", retrieved.AccountDID.String(), did.String()) + } + if retrieved.SessionID != "current-session-id" { + t.Errorf("GetCurrentSession() SessionID = %q, want %q", retrieved.SessionID, "current-session-id") + } +} + +func TestClearCurrentSession(t *testing.T) { + mock := NewMockKeyring() + store := NewKeyringAuthStoreWithKeyring(mock) + ctx := context.Background() + + did, _ := syntax.ParseDID("did:plc:currentuser") + sess := oauth.ClientSessionData{ + AccountDID: did, + SessionID: "current-session-id", + } + + store.SaveSession(ctx, sess) + store.SetCurrentSession(ctx, &sess) + + if err := store.ClearCurrentSession(); err != nil { + t.Fatalf("ClearCurrentSession() error = %v", err) + } + + _, err := store.GetCurrentSession(ctx) + if err == nil { + t.Error("GetCurrentSession() expected error after clear, got nil") + } +} + +func TestSetAndGetLoginIdentifier(t *testing.T) { + mock := NewMockKeyring() + store := NewKeyringAuthStoreWithKeyring(mock) + + if err := store.SetLoginIdentifier("user.bsky.social"); err != nil { + t.Fatalf("SetLoginIdentifier() error = %v", err) + } + + id, err := store.GetLoginIdentifier() + if err != nil { + t.Fatalf("GetLoginIdentifier() error = %v", err) + } + + if id != "user.bsky.social" { + t.Errorf("GetLoginIdentifier() = %q, want %q", id, "user.bsky.social") + } +} + +func TestClearLoginIdentifier(t *testing.T) { + mock := NewMockKeyring() + store := NewKeyringAuthStoreWithKeyring(mock) + + store.SetLoginIdentifier("user.bsky.social") + + if err := store.ClearLoginIdentifier(); err != nil { + t.Fatalf("ClearLoginIdentifier() error = %v", err) + } + + _, err := store.GetLoginIdentifier() + if err == nil { + t.Error("GetLoginIdentifier() expected error after clear, got nil") + } +} + +func TestGetSessionNotFound(t *testing.T) { + mock := NewMockKeyring() + store := NewKeyringAuthStoreWithKeyring(mock) + ctx := context.Background() + + did, _ := syntax.ParseDID("did:plc:nonexistent") + _, err := store.GetSession(ctx, did, "no-such-session") + if err == nil { + t.Error("GetSession() expected error for non-existent session, got nil") + } +} + +func TestGetCurrentSessionNotFound(t *testing.T) { + mock := NewMockKeyring() + store := NewKeyringAuthStoreWithKeyring(mock) + ctx := context.Background() + + _, err := store.GetCurrentSession(ctx) + if err == nil { + t.Error("GetCurrentSession() expected error when no current session, got nil") + } +} + +func TestGetLoginIdentifierNotFound(t *testing.T) { + mock := NewMockKeyring() + store := NewKeyringAuthStoreWithKeyring(mock) + + _, err := store.GetLoginIdentifier() + if err == nil { + t.Error("GetLoginIdentifier() expected error when not set, got nil") + } +} diff --git a/internal/ui/notification.go b/internal/ui/notification.go new file mode 100644 --- /dev/null +++ b/internal/ui/notification.go @@ -0,0 +1,47 @@ +package ui + +import ( + "github.com/godbus/dbus/v5" +) + +const ( + notifyService = "org.freedesktop.Notifications" + notifyPath = "/org/freedesktop/Notifications" + notifyInterface = "org.freedesktop.Notifications" +) + +// Notify sends a desktop notification via DBus +func Notify(title, message, icon string) error { + conn, err := dbus.ConnectSessionBus() + if err != nil { + return err + } + defer conn.Close() + + obj := conn.Object(notifyService, notifyPath) + call := obj.Call(notifyInterface+".Notify", 0, + "blup", // app_name + uint32(0), // replaces_id + icon, // app_icon + title, // summary + message, // body + []string{}, // actions + map[string]dbus.Variant{}, // hints + int32(5000), // expire_timeout (ms) + ) + return call.Err +} + +// NotifyLoginRequired shows a notification telling user to run blup login +func NotifyLoginRequired() error { + return Notify( + "blup: Login Required", + "Run 'blup login' in a terminal to authenticate.", + "dialog-password", + ) +} + +// NotifyError shows an error notification +func NotifyError(message string) error { + return Notify("blup: Error", message, "dialog-error") +} diff --git a/internal/ui/ui.go b/internal/ui/ui.go new file mode 100644 --- /dev/null +++ b/internal/ui/ui.go @@ -0,0 +1,12 @@ +package ui + +import ( + "os" + + "golang.org/x/term" +) + +// IsInteractive returns true if stdin is connected to a terminal +func IsInteractive() bool { + return term.IsTerminal(int(os.Stdin.Fd())) +}