diff --git a/cmd/main.go b/cmd/main.go index 002ddf1..118306f 100644 --- a/cmd/main.go +++ b/cmd/main.go @@ -1,6 +1,7 @@ package main import ( + "context" "encoding/json" "fmt" "log" @@ -8,18 +9,20 @@ import ( "strings" "time" - "github.com/teal-fm/piper/service/applemusic" - "github.com/teal-fm/piper/service/lastfm" - "github.com/teal-fm/piper/service/playingnow" - "github.com/spf13/viper" + "golang.org/x/oauth2" + spotifyOauth "golang.org/x/oauth2/spotify" + "github.com/teal-fm/piper/config" "github.com/teal-fm/piper/db" "github.com/teal-fm/piper/oauth" "github.com/teal-fm/piper/oauth/atproto" "github.com/teal-fm/piper/pages" apikeyService "github.com/teal-fm/piper/service/apikey" + "github.com/teal-fm/piper/service/applemusic" + "github.com/teal-fm/piper/service/lastfm" "github.com/teal-fm/piper/service/musicbrainz" + "github.com/teal-fm/piper/service/playingnow" "github.com/teal-fm/piper/service/spotify" "github.com/teal-fm/piper/session" ) @@ -180,12 +183,19 @@ func main() { // Register Spotify OAuth service only if Spotify is enabled and configured if spotifyService != nil { spotifyOAuth := oauth.NewOAuth2Service( - viper.GetString("spotify.client_id"), - viper.GetString("spotify.client_secret"), - viper.GetString("callback.spotify"), - viper.GetStringSlice("spotify.scopes"), - "spotify", + oauth2.Config{ + ClientID: viper.GetString("spotify.client_id"), + ClientSecret: viper.GetString("spotify.client_secret"), + RedirectURL: viper.GetString("callback.spotify"), + Scopes: viper.GetStringSlice("spotify.scopes"), + Endpoint: spotifyOauth.Endpoint, + }, spotifyService, + log.Default(), + func(ctx context.Context) int64 { + id, _ := session.GetUserID(ctx) + return id + }, ) oauthManager.RegisterService("spotify", spotifyOAuth) log.Println("Spotify OAuth service registered") diff --git a/cmd/routes.go b/cmd/routes.go index eda0246..2a58bad 100644 --- a/cmd/routes.go +++ b/cmd/routes.go @@ -17,8 +17,6 @@ func (app *application) routes() http.Handler { mux.HandleFunc("/", session.WithPossibleAuth(home(app.database, app.pages), app.sessionManager)) // OAuth Routes - mux.HandleFunc("/login/spotify", app.oauthManager.HandleLogin("spotify")) - mux.HandleFunc("/callback/spotify", session.WithPossibleAuth(app.oauthManager.HandleCallback("spotify"), app.sessionManager)) // Use possible auth mux.HandleFunc("/login/atproto", app.oauthManager.HandleLogin("atproto")) mux.HandleFunc("/callback/atproto", session.WithPossibleAuth(app.oauthManager.HandleCallback("atproto"), app.sessionManager)) // Use possible auth @@ -26,6 +24,10 @@ func (app *application) routes() http.Handler { mux.HandleFunc("/current-track", session.WithAuth(app.spotifyService.HandleCurrentTrack, app.sessionManager)) mux.HandleFunc("/history", session.WithAuth(app.spotifyService.HandleTrackHistory, app.sessionManager)) mux.HandleFunc("/api-keys", session.WithAuth(app.apiKeyService.HandleAPIKeyManagement(app.database, app.pages), app.sessionManager)) + + mux.HandleFunc("/login/spotify", session.WithAuth(app.oauthManager.HandleLogin("spotify"), app.sessionManager)) + mux.HandleFunc("/callback/spotify", session.WithAuth(app.oauthManager.HandleCallback("spotify"), app.sessionManager)) + mux.HandleFunc("/link-lastfm", session.WithAuth(handleLinkLastfmForm(app.database, app.pages), app.sessionManager)) // GET form mux.HandleFunc("/link-lastfm/submit", session.WithAuth(handleLinkLastfmSubmit(app.database), app.sessionManager)) // POST submit - Changed route slightly mux.HandleFunc("/link-applemusic", session.WithAuth(handleAppleMusicLink(app.pages, app.appleMusicService), app.sessionManager)) diff --git a/oauth/oauth2.go b/oauth/oauth2.go index 8eb214d..ae3b8a1 100644 --- a/oauth/oauth2.go +++ b/oauth/oauth2.go @@ -9,156 +9,239 @@ import ( "fmt" "log" "net/http" - "strings" + "sync" + "time" - "github.com/teal-fm/piper/session" "golang.org/x/oauth2" - "golang.org/x/oauth2/spotify" ) +type userIDGetter func(context.Context) int64 + +// Service implements the PKCE authorization-code flow with single-use CSRF +// states. It is safe for concurrent use. type Service struct { config oauth2.Config - state string - codeVerifier string - codeChallenge string tokenReceiver TokenReceiver + store *memoryStateStore + logger *log.Logger + getUserID userIDGetter +} + +type stateEntry struct { + verifier string + expiresAt time.Time + userID int64 +} + +type memoryStateStore struct { + mu sync.Mutex + entries map[string]stateEntry } -func GenerateRandomState() string { - b := make([]byte, 16) - //This probably should panic - rand.Read(b) - return base64.URLEncoding.EncodeToString(b) +func newMemoryStateStore() *memoryStateStore { + return &memoryStateStore{ + entries: make(map[string]stateEntry), + } } -func NewOAuth2Service(clientID, clientSecret, redirectURI string, scopes []string, provider string, tokenReceiver TokenReceiver) *Service { - var endpoint oauth2.Endpoint +func (s *memoryStateStore) Set(state, verifier string, userID int64, ttl time.Duration) { + s.mu.Lock() + defer s.mu.Unlock() - switch strings.ToLower(provider) { - case "spotify": - endpoint = spotify.Endpoint - default: - // placeholder - log.Printf("Warning: OAuth2 provider '%s' not explicitly configured. Using placeholder endpoints.", provider) - endpoint = oauth2.Endpoint{ - AuthURL: "https://example.com/auth", - TokenURL: "https://example.com/token", + now := time.Now() + for k, v := range s.entries { + if now.After(v.expiresAt) { + delete(s.entries, k) } } - codeVerifier := GenerateCodeVerifier() - codeChallenge := GenerateCodeChallenge(codeVerifier) + s.entries[state] = stateEntry{verifier: verifier, expiresAt: now.Add(ttl), userID: userID} +} - return &Service{ - config: oauth2.Config{ - ClientID: clientID, - ClientSecret: clientSecret, - RedirectURL: redirectURI, - Scopes: scopes, - Endpoint: endpoint, - }, - state: GenerateRandomState(), - codeVerifier: codeVerifier, - codeChallenge: codeChallenge, - tokenReceiver: tokenReceiver, +func (s *memoryStateStore) GetAndDelete(state string) (string, int64, bool) { + s.mu.Lock() + defer s.mu.Unlock() + + e, ok := s.entries[state] + if !ok { + return "", 0, false + } + + delete(s.entries, state) + if time.Now().After(e.expiresAt) { + return "", 0, false } + + return e.verifier, e.userID, true } -// GenerateCodeVerifier generate a random code verifier, for PKCE -func GenerateCodeVerifier() string { - b := make([]byte, 64) - //This probably should panic - rand.Read(b) - return base64.RawURLEncoding.EncodeToString(b) +const randAlphabet = "ABCDEFGHIJKLMNOPQRSTUVWXYZ234567" + +func randText(n int) string { + b := make([]byte, n) + if _, err := rand.Read(b); err != nil { + panic(fmt.Sprintf("rand.Read: %v", err)) + } + + for i := range b { + b[i] = randAlphabet[int(b[i])%len(randAlphabet)] + } + + return string(b) +} + +func NewOAuth2Service(cfg oauth2.Config, tokenReceiver TokenReceiver, logger *log.Logger, getUserID userIDGetter) *Service { + return &Service{ + config: cfg, + tokenReceiver: tokenReceiver, + store: newMemoryStateStore(), + logger: logger, + getUserID: getUserID, + } } -// GenerateCodeChallenge generate a code challenge for verification later -func GenerateCodeChallenge(verifier string) string { +func generateCodeChallenge(verifier string) string { h := sha256.New() h.Write([]byte(verifier)) + return base64.RawURLEncoding.EncodeToString(h.Sum(nil)) } +// HandleLogin starts the authorization code flow with PKCE. +// It generates a single-use state and a S256 code challenge, stores the +// verifier with the initiating user ID and a 10-minute expiry, and redirects +// to the provider authorization URL. The request must carry an authenticated +// Piper session; otherwise HandleLogin responds with 401. func (o *Service) HandleLogin(w http.ResponseWriter, r *http.Request) { + initiatorID := o.getUserID(r.Context()) + if initiatorID == 0 { + http.Error(w, "authentication required", http.StatusUnauthorized) + return + } + + state := randText(26) + verifier := randText(64) + challenge := generateCodeChallenge(verifier) + + o.store.Set(state, verifier, initiatorID, 10*time.Minute) + opts := []oauth2.AuthCodeOption{ - oauth2.SetAuthURLParam("code_challenge", o.codeChallenge), + oauth2.SetAuthURLParam("code_challenge", challenge), oauth2.SetAuthURLParam("code_challenge_method", "S256"), } - authURL := o.config.AuthCodeURL(o.state, opts...) + + authURL := o.config.AuthCodeURL(state, opts...) http.Redirect(w, r, authURL, http.StatusSeeOther) } func (o *Service) HandleLogout(w http.ResponseWriter, r *http.Request) { - //TODO not implemented yet. not sure what the api call is for this package + // TODO not implemented yet. not sure what the api call is for this package http.Redirect(w, r, "/", http.StatusSeeOther) } +type completeLoginParams struct { + State string + Code string + ProviderError string + ProviderDesc string + UserID int64 +} + +// HandleCallback handles the provider redirect. It requires query parameters +// state and code (or error/error_description on provider denial). +// +// On success it returns the authenticated user ID. On failure it writes a +// generic error (never reflecting provider strings) with 400 for client +// errors and 500 for server errors and returns a sentinel for errors.Is. func (o *Service) HandleCallback(w http.ResponseWriter, r *http.Request) (int64, error) { - state := r.URL.Query().Get("state") - if state != o.state { - log.Printf("OAuth2 Callback Error: State mismatch. Expected '%s', got '%s'", o.state, state) - http.Error(w, "State mismatch", http.StatusBadRequest) - return 0, errors.New("state mismatch") + params := completeLoginParams{ + State: r.URL.Query().Get("state"), + Code: r.URL.Query().Get("code"), + ProviderError: r.URL.Query().Get("error"), + ProviderDesc: r.URL.Query().Get("error_description"), + UserID: o.getUserID(r.Context()), } - code := r.URL.Query().Get("code") - if code == "" { - errMsg := r.URL.Query().Get("error") - errDesc := r.URL.Query().Get("error_description") - log.Printf("OAuth2 Callback Error: No code provided. Error: '%s', Description: '%s'", errMsg, errDesc) - http.Error(w, fmt.Sprintf("Authorization failed: %s (%s)", errMsg, errDesc), http.StatusBadRequest) - return 0, errors.New("no code provided") + userID, err := o.completeLogin(r.Context(), params) + if err != nil { + status := httpStatusForOAuthError(err) + http.Error(w, err.Error(), status) + return 0, err } - if o.tokenReceiver == nil { - log.Printf("OAuth2 Callback Error: TokenReceiver is not configured for this service.") - http.Error(w, "Internal server configuration error", http.StatusInternalServerError) - return 0, errors.New("token receiver not configured") + return userID, nil +} + +var ( + errStateMismatch = errors.New("state mismatch") + errNoCode = errors.New("no code provided") + errNoReceiver = errors.New("token receiver not configured") + errExchangeFailed = errors.New("failed to exchange code for token") + errStoreFailed = errors.New("failed to store access token") +) + +func httpStatusForOAuthError(err error) int { + if errors.Is(err, errNoReceiver) || errors.Is(err, errExchangeFailed) || errors.Is(err, errStoreFailed) { + return http.StatusInternalServerError } - opts := []oauth2.AuthCodeOption{ - oauth2.SetAuthURLParam("code_verifier", o.codeVerifier), + return http.StatusBadRequest +} + +// State is consumed before any other check so a missing code still +// invalidates the entry and provider errors are only logged for a valid +// state, preventing CSRF bypass via forged error. +func (o *Service) completeLogin(ctx context.Context, p completeLoginParams) (int64, error) { + verifier, storedUserID, ok := o.store.GetAndDelete(p.State) + if !ok { + o.logger.Printf("OAuth2 Callback Error: State mismatch or expired. Got '%s'", p.State) + return 0, errStateMismatch } - log.Println(code) + if storedUserID != p.UserID { + o.logger.Printf("OAuth2 Callback Error: State user mismatch: expected %d, got %d", storedUserID, p.UserID) + return 0, errStateMismatch + } - token, err := o.config.Exchange(context.Background(), code, opts...) - if err != nil { - log.Printf("OAuth2 Callback Error: Failed to exchange code for token: %v", err) - http.Error(w, fmt.Sprintf("Error exchanging code for token: %v", err), http.StatusInternalServerError) - return 0, errors.New("failed to exchange code for token") + if p.Code == "" { + if p.ProviderError != "" || p.ProviderDesc != "" { + o.logger.Printf("OAuth2 Callback Error: provider returned error for state '%s': error=%q desc=%q", p.State, p.ProviderError, p.ProviderDesc) + } else { + o.logger.Printf("OAuth2 Callback Error: No code provided for state '%s'", p.State) + } + return 0, errNoCode } - userId, hasSession := session.GetUserID(r.Context()) - // store token and get uid - userID, err := o.tokenReceiver.SetAccessToken(token.AccessToken, token.RefreshToken, userId, hasSession) - if err != nil { - log.Printf("OAuth2 Callback Info: TokenReceiver did not return a valid user ID for token: %s...", token.AccessToken[:customMin(10, len(token.AccessToken))]) + if o.tokenReceiver == nil { + o.logger.Printf("OAuth2 Callback Error: TokenReceiver is not configured") + return 0, errNoReceiver } - log.Printf("OAuth2 Callback Success: Exchanged code for token, UserID: %d", userID) - return userID, nil + return o.exchangeAndStore(ctx, p.Code, verifier, p.UserID) } -func (o *Service) GetToken(code string) (*oauth2.Token, error) { +func (o *Service) exchangeAndStore(ctx context.Context, code, verifier string, uid int64) (int64, error) { opts := []oauth2.AuthCodeOption{ - oauth2.SetAuthURLParam("code_verifier", o.codeVerifier), + oauth2.SetAuthURLParam("code_verifier", verifier), } - return o.config.Exchange(context.Background(), code, opts...) -} - -func (o *Service) GetClient(token *oauth2.Token) *http.Client { - return o.config.Client(context.Background(), token) -} -func (o *Service) RefreshToken(token *oauth2.Token) (*oauth2.Token, error) { - source := o.config.TokenSource(context.Background(), token) - return oauth2.ReuseTokenSource(token, source).Token() -} + token, err := o.config.Exchange(ctx, code, opts...) + if err != nil { + o.logger.Printf("OAuth2 Callback Error: Failed to exchange code for token: %v", err) + return 0, errExchangeFailed + } -func customMin(a, b int) int { - if a < b { - return a + userID, err := o.tokenReceiver.SetAccessToken(token.AccessToken, token.RefreshToken, uid) + if err != nil { + o.logger.Printf( + "OAuth2 Callback Info: TokenReceiver failed for token %q...: %v", + token.AccessToken[:min(len(token.AccessToken), 10)], + err, + ) + return 0, errStoreFailed } - return b + + o.logger.Printf("OAuth2 Callback Success: Exchanged code for token, UserID: %d", userID) + + return userID, nil } diff --git a/oauth/oauth2_test.go b/oauth/oauth2_test.go new file mode 100644 index 0000000..ddc1bc0 --- /dev/null +++ b/oauth/oauth2_test.go @@ -0,0 +1,505 @@ +package oauth + +import ( + "context" + "errors" + "io" + "log" + "net/http" + "net/http/httptest" + "net/url" + "strings" + "testing" + + "golang.org/x/oauth2" + + "github.com/teal-fm/piper/session" +) + +type mockReceiver struct { + returnID int64 + returnErr error + called bool + lastUID int64 +} + +func (m *mockReceiver) SetAccessToken(token, refresh string, uid int64) (int64, error) { + m.called = true + m.lastUID = uid + + if m.returnErr != nil { + return 0, m.returnErr + } + + if m.returnID != 0 { + return m.returnID, nil + } + + return 42, nil +} + +func sessionUser(ctx context.Context) int64 { + id, _ := session.GetUserID(ctx) + return id +} + +func withUserCtx(uid int64) context.Context { + return session.WithUserID(context.Background(), uid) +} + +func stateFromLogin(svc *Service) string { + return stateFromLoginAs(svc, 1) +} + +func stateFromLoginAs(svc *Service, uid int64) string { + ctx := withUserCtx(uid) + req := httptest.NewRequestWithContext(ctx, http.MethodGet, "/login", nil) + rr := httptest.NewRecorder() + svc.HandleLogin(rr, req) + loc := rr.Result().Header.Get("Location") + u, _ := url.Parse(loc) + return u.Query().Get("state") +} + +func forgedState(svc *Service) string { + _ = stateFromLogin(svc) + return "forged-state" +} + +func consumedState(svc *Service) string { + s := stateFromLogin(svc) + _, _ = svc.completeLogin(withUserCtx(1), completeLoginParams{State: s, Code: "c", UserID: 1}) + return s +} + +func queryMissingCode(svc *Service) string { + s := stateFromLogin(svc) + return "state=" + url.QueryEscape(s) + "&error=access_denied" +} + +func queryValidCode(svc *Service) string { + s := stateFromLogin(svc) + return "state=" + url.QueryEscape(s) + "&code=c" +} + +func queryMismatch(_ *Service) string { + return "state=not-issued&code=c" +} + +func okTokenHandler(w http.ResponseWriter, _ *http.Request) { + w.Header().Set("Content-Type", "application/json") + _, _ = w.Write([]byte(`{"access_token":"at","token_type":"bearer","refresh_token":"rt"}`)) +} + +func errorTokenHandler(w http.ResponseWriter, _ *http.Request) { + w.WriteHeader(500) + _, _ = w.Write([]byte(`{"error":"server_error"}`)) +} + +func wantErr(substr string) func(t *testing.T, gotID int64, err error, r *mockReceiver) { + return func(t *testing.T, _ int64, err error, _ *mockReceiver) { + t.Helper() + if err == nil || !strings.Contains(err.Error(), substr) { + t.Fatalf("err = %v, want containing %q", err, substr) + } + } +} + +func wantID(want int64) func(t *testing.T, gotID int64, err error, r *mockReceiver) { + return func(t *testing.T, gotID int64, err error, _ *mockReceiver) { + t.Helper() + if err != nil { + t.Fatalf("unexpected err %v", err) + } + if gotID != want { + t.Fatalf("id = %d, want %d", gotID, want) + } + } +} + +func wantIDWithSession(wantID, wantUID int64) func(t *testing.T, gotID int64, err error, r *mockReceiver) { + return func(t *testing.T, gotID int64, err error, r *mockReceiver) { + t.Helper() + if err != nil { + t.Fatalf("unexpected err %v", err) + } + if gotID != wantID { + t.Fatalf("id = %d, want %d", gotID, wantID) + } + if r.lastUID != wantUID { + t.Fatalf("receiver = %+v, want uid %d", r, wantUID) + } + } +} + +func wantStoreFailed(t *testing.T, gotID int64, err error, _ *mockReceiver) { + t.Helper() + if err == nil || !strings.Contains(err.Error(), "failed to store") { + t.Fatalf("err = %v, want containing %q", err, "failed to store") + } + if !errors.Is(err, errStoreFailed) { + t.Fatalf("err = %v, want errStoreFailed", err) + } + if got := httpStatusForOAuthError(err); got != http.StatusInternalServerError { + t.Fatalf("http status = %d, want %d", got, http.StatusInternalServerError) + } +} + +func TestGenerateCodeChallenge(t *testing.T) { + tests := []struct { + name string + verifier string + want string + }{ + { + name: "rfc7636", + verifier: "dBjftJeZ4CVP-mB92K27uhbUJU1p1r_wW1gFWFOEjXk", + want: "E9Melhoa2OwvFrEMTJguCHaoeK1t8URWbuGJSstw-cM", + }, + { + name: "empty", + verifier: "", + want: "47DEQpj8HBSa-_TImW-5JCeuQeRkm5NMpJWZG3hSuFU", + }, + { + name: "hello", + verifier: "hello", + want: "LPJNul-wow4m6DsqxbninhsWHlwfp0JecwQzYpOLmCQ", + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + if got := generateCodeChallenge(tt.verifier); got != tt.want { + t.Fatalf("got %q, want %q", got, tt.want) + } + }) + } +} + +func TestHandleLogin_Redirect(t *testing.T) { + svc := NewOAuth2Service( + oauth2.Config{ + ClientID: "id", + Endpoint: oauth2.Endpoint{ + AuthURL: "http://example.com/auth", + TokenURL: "http://example.com/token", + }, + }, + nil, + log.New(io.Discard, "", 0), + sessionUser, + ) + + req := httptest.NewRequestWithContext(withUserCtx(1), http.MethodGet, "http://example.com/login", nil) + rr := httptest.NewRecorder() + svc.HandleLogin(rr, req) + + resp := rr.Result() + if resp.StatusCode != http.StatusSeeOther { + t.Fatalf("status = %d, want %d", resp.StatusCode, http.StatusSeeOther) + } + + loc := resp.Header.Get("Location") + u, err := url.Parse(loc) + if err != nil { + t.Fatalf("invalid Location: %v", err) + } + q := u.Query() + + if q.Get("state") == "" { + t.Fatalf("missing state") + } + if q.Get("code_challenge") == "" { + t.Fatalf("missing code_challenge") + } + if q.Get("code_challenge_method") != "S256" { + t.Fatalf("method = %q, want S256", q.Get("code_challenge_method")) + } +} + +func TestHandleLogin_ChallengeBindsToStoredVerifier(t *testing.T) { + var capturedVerifier string + ts := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + _ = r.ParseForm() + capturedVerifier = r.Form.Get("code_verifier") + w.Header().Set("Content-Type", "application/json") + _, _ = w.Write([]byte(`{"access_token":"tok","token_type":"bearer"}`)) + })) + defer ts.Close() + + cfg := oauth2.Config{ + ClientID: "id", + ClientSecret: "s", + RedirectURL: "http://localhost/cb", + Endpoint: oauth2.Endpoint{ + AuthURL: "http://example.com/auth", + TokenURL: ts.URL, + }, + } + svc := NewOAuth2Service(cfg, &mockReceiver{returnID: 1}, log.New(io.Discard, "", 0), sessionUser) + + req := httptest.NewRequestWithContext(withUserCtx(1), http.MethodGet, "/login", nil) + rr := httptest.NewRecorder() + svc.HandleLogin(rr, req) + loc := rr.Result().Header.Get("Location") + u, _ := url.Parse(loc) + state := u.Query().Get("state") + challenge := u.Query().Get("code_challenge") + + reqCB := httptest.NewRequestWithContext(withUserCtx(1), http.MethodGet, "/callback?state="+url.QueryEscape(state)+"&code=c", nil) + rrCB := httptest.NewRecorder() + if _, err := svc.HandleCallback(rrCB, reqCB); err != nil { + t.Fatalf("HandleCallback: %v", err) + } + + if capturedVerifier == "" { + t.Fatalf("no code_verifier sent to token endpoint") + } + if got := generateCodeChallenge(capturedVerifier); got != challenge { + t.Fatalf("challenge %q != S256(verifier %q) %q", challenge, capturedVerifier, got) + } +} + +func TestCompleteLogin(t *testing.T) { + tests := []struct { + name string + receiver *mockReceiver + tokenFunc http.HandlerFunc + setup func(svc *Service) string + code string + uid int64 + initUID int64 + check func(t *testing.T, gotID int64, err error, receiver *mockReceiver) + }{ + { + name: "state mismatch", + receiver: &mockReceiver{}, + tokenFunc: okTokenHandler, + setup: forgedState, + code: "c", + uid: 0, + initUID: 0, + check: wantErr("state mismatch"), + }, + { + name: "no code", + receiver: &mockReceiver{}, + tokenFunc: okTokenHandler, + setup: stateFromLogin, + code: "", + uid: 1, + initUID: 1, + check: wantErr("no code provided"), + }, + { + name: "no receiver", + receiver: nil, + tokenFunc: okTokenHandler, + setup: stateFromLogin, + code: "c", + uid: 1, + initUID: 1, + check: wantErr("token receiver not configured"), + }, + { + name: "exchange failure", + receiver: &mockReceiver{}, + tokenFunc: errorTokenHandler, + setup: stateFromLogin, + code: "c", + uid: 1, + initUID: 1, + check: wantErr("failed to exchange"), + }, + { + name: "success", + receiver: &mockReceiver{returnID: 777}, + tokenFunc: okTokenHandler, + setup: stateFromLogin, + code: "c", + uid: 1, + initUID: 1, + check: wantID(777), + }, + { + name: "success with session", + receiver: &mockReceiver{returnID: 99}, + tokenFunc: okTokenHandler, + setup: func(svc *Service) string { return stateFromLoginAs(svc, 555) }, + code: "c", + uid: 555, + initUID: 555, + check: wantIDWithSession(99, 555), + }, + { + name: "replay single-use", + receiver: &mockReceiver{returnID: 1}, + tokenFunc: okTokenHandler, + setup: consumedState, + code: "c", + uid: 0, + initUID: 0, + check: wantErr("state mismatch"), + }, + { + name: "cross-user state transfer blocked", + receiver: &mockReceiver{returnID: 99}, + tokenFunc: okTokenHandler, + setup: func(svc *Service) string { return stateFromLoginAs(svc, 111) }, + code: "c", + uid: 222, + initUID: 111, + check: wantErr("state mismatch"), + }, + { + name: "store failure maps to 500", + receiver: &mockReceiver{returnErr: errors.New("db down")}, + tokenFunc: okTokenHandler, + setup: stateFromLogin, + code: "c", + uid: 1, + initUID: 1, + check: wantStoreFailed, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + ts := httptest.NewServer(tt.tokenFunc) + defer ts.Close() + + var recv TokenReceiver + if tt.receiver != nil { + recv = tt.receiver + } + svc := NewOAuth2Service( + oauth2.Config{ + ClientID: "id", + ClientSecret: "s", + RedirectURL: "http://localhost/cb", + Endpoint: oauth2.Endpoint{ + AuthURL: "http://example.com/auth", + TokenURL: ts.URL, + }, + }, + recv, + log.New(io.Discard, "", 0), + sessionUser, + ) + state := tt.setup(svc) + + gotID, err := svc.completeLogin(withUserCtx(tt.uid), completeLoginParams{State: state, Code: tt.code, UserID: tt.uid}) + tt.check(t, gotID, err, tt.receiver) + }) + } +} + +func TestHandleCallback_HTTPMapping(t *testing.T) { + tests := []struct { + name string + setup func(svc *Service) string + receiver TokenReceiver + wantStatus int + uid int64 + }{ + { + name: "missing code maps to 400", + setup: queryMissingCode, + receiver: &mockReceiver{}, + wantStatus: http.StatusBadRequest, + uid: 1, + }, + { + name: "nil receiver maps to 500", + setup: queryValidCode, + receiver: nil, + wantStatus: http.StatusInternalServerError, + uid: 1, + }, + { + name: "state mismatch maps to 400", + setup: queryMismatch, + receiver: &mockReceiver{}, + wantStatus: http.StatusBadRequest, + uid: 1, + }, + { + name: "store failure maps to 500", + setup: queryValidCode, + receiver: &mockReceiver{returnErr: errors.New("db down")}, + wantStatus: http.StatusInternalServerError, + uid: 1, + }, + { + name: "user mismatch maps to 400", + setup: func(svc *Service) string { return stateFromLoginAs(svc, 111) }, + receiver: &mockReceiver{}, + wantStatus: http.StatusBadRequest, + uid: 222, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + ts := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { + w.Header().Set("Content-Type", "application/json") + _, _ = w.Write([]byte(`{"access_token":"at","token_type":"bearer"}`)) + })) + defer ts.Close() + + svc := NewOAuth2Service( + oauth2.Config{ + ClientID: "id", + Endpoint: oauth2.Endpoint{ + AuthURL: "http://a", + TokenURL: ts.URL, + }, + }, + tt.receiver, + log.New(io.Discard, "", 0), + sessionUser, + ) + + query := tt.setup(svc) + req := httptest.NewRequestWithContext(withUserCtx(tt.uid), http.MethodGet, "/callback?"+query, nil) + rr := httptest.NewRecorder() + _, _ = svc.HandleCallback(rr, req) + + resp := rr.Result() + if resp.StatusCode != tt.wantStatus { + t.Fatalf("status = %d, want %d", resp.StatusCode, tt.wantStatus) + } + }) + } +} + +func TestHandleLogin_RequiresAuth(t *testing.T) { + svc := NewOAuth2Service( + oauth2.Config{ + ClientID: "id", + Endpoint: oauth2.Endpoint{ + AuthURL: "http://example.com/auth", + TokenURL: "http://example.com/token", + }, + }, + nil, + log.New(io.Discard, "", 0), + sessionUser, + ) + + // Unauthenticated should be 401 + req := httptest.NewRequest(http.MethodGet, "/login", nil) + rr := httptest.NewRecorder() + svc.HandleLogin(rr, req) + if rr.Code != http.StatusUnauthorized { + t.Fatalf("unauth status = %d, want %d", rr.Code, http.StatusUnauthorized) + } + + // Authenticated should redirect + req2 := httptest.NewRequestWithContext(withUserCtx(123), http.MethodGet, "/login", nil) + rr2 := httptest.NewRecorder() + svc.HandleLogin(rr2, req2) + if rr2.Code != http.StatusSeeOther { + t.Fatalf("auth status = %d, want %d", rr2.Code, http.StatusSeeOther) + } +} diff --git a/oauth/service.go b/oauth/service.go index fff06aa..2dc930a 100644 --- a/oauth/service.go +++ b/oauth/service.go @@ -18,5 +18,5 @@ type AuthService interface { type TokenReceiver interface { // SetAccessToken stores the access token in the db // if there is a session, will associate the token with the session - SetAccessToken(token string, refreshToken string, currentId int64, hasSession bool) (int64, error) + SetAccessToken(token string, refreshToken string, userID int64) (int64, error) } diff --git a/service/spotify/spotify.go b/service/spotify/spotify.go index 90f355e..f980805 100644 --- a/service/spotify/spotify.go +++ b/service/spotify/spotify.go @@ -1,6 +1,7 @@ package spotify import ( + "context" "crypto/sha256" "encoding/base64" "encoding/json" @@ -15,12 +16,7 @@ import ( "sync" "time" - "context" // Added for context.Context - - // Added for atproto.RepoCreateRecord_Input - // Added for lexutil.LexiconTypeDecoder - // Added for xrpc.Client - "github.com/spf13/viper" // Added for teal.FeedPlay + "github.com/spf13/viper" "github.com/teal-fm/piper/db" "github.com/teal-fm/piper/models" atprotoauth "github.com/teal-fm/piper/oauth/atproto" @@ -106,8 +102,8 @@ func (s *Service) SubmitTrackToPDS(did string, mostRecentAtProtoSessionID string return atprotoservice.SubmitPlayToPDS(ctx, did, mostRecentAtProtoSessionID, track, s.atprotoAuthService) } -func (s *Service) SetAccessToken(token string, refreshToken string, userId int64, hasSession bool) (int64, error) { - userID, err := s.identifyAndStoreUser(token, refreshToken, userId, hasSession) +func (s *Service) SetAccessToken(token string, refreshToken string, userId int64) (int64, error) { + userID, err := s.identifyAndStoreUser(token, refreshToken, userId) if err != nil { s.logger.Printf("Error identifying and storing user: %v", err) return 0, err @@ -115,14 +111,14 @@ func (s *Service) SetAccessToken(token string, refreshToken string, userId int64 return userID, nil } -func (s *Service) identifyAndStoreUser(token string, refreshToken string, userId int64, hasSession bool) (int64, error) { +func (s *Service) identifyAndStoreUser(token string, refreshToken string, userId int64) (int64, error) { userProfile, err := s.fetchSpotifyProfile(token) if err != nil { s.logger.Printf("Error fetching Spotify profile: %v", err) return 0, err } - s.logger.Printf("uid: %d hasSession: %t", userId, hasSession) + s.logger.Printf("uid: %d", userId) user, err := s.DB.GetUserBySpotifyID(userProfile.ID) if err != nil { @@ -135,7 +131,7 @@ func (s *Service) identifyAndStoreUser(token string, refreshToken string, userId // We don't intend users to log in via spotify! if user == nil { - if !hasSession { + if userId == 0 { s.logger.Printf("User does not seem to exist") return 0, fmt.Errorf("user does not seem to exist") } diff --git a/session/session.go b/session/session.go index 4defa9f..8b619bd 100644 --- a/session/session.go +++ b/session/session.go @@ -242,13 +242,13 @@ func WithAuth(handler http.HandlerFunc, sm *Manager) http.HandlerFunc { // if not found, check cookies for session value cookie, err := r.Cookie("session") if err != nil { - http.Redirect(w, r, "/login/spotify", http.StatusSeeOther) + http.Redirect(w, r, "/", http.StatusSeeOther) return } session, exists := sm.GetSession(cookie.Value) if !exists { - http.Redirect(w, r, "/login/spotify", http.StatusSeeOther) + http.Redirect(w, r, "/", http.StatusSeeOther) return }