fix: review comments and remove non needed tests

This commit is contained in:
Stavros
2026-08-14 17:06:53 +03:00
parent da7e0e39ba
commit f665d55bbf
3 changed files with 11 additions and 316 deletions
-136
View File
@@ -1,136 +0,0 @@
package repository_test
import (
"context"
"database/sql"
"os"
"path/filepath"
"testing"
"github.com/golang-migrate/migrate/v4"
pgxmigrate "github.com/golang-migrate/migrate/v4/database/pgx/v5"
"github.com/golang-migrate/migrate/v4/database/sqlite3"
"github.com/golang-migrate/migrate/v4/source/iofs"
_ "github.com/jackc/pgx/v5/stdlib"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
_ "modernc.org/sqlite"
"github.com/tinyauthapp/tinyauth/internal/assets"
"github.com/tinyauthapp/tinyauth/internal/repository"
"github.com/tinyauthapp/tinyauth/internal/repository/postgres"
"github.com/tinyauthapp/tinyauth/internal/repository/sqlite"
)
func setupSQLiteStore(t *testing.T) repository.Store {
t.Helper()
db, err := sql.Open("sqlite", filepath.Join(t.TempDir(), "test.db"))
require.NoError(t, err)
migrations, err := iofs.New(assets.Migrations, "migrations/sqlite")
require.NoError(t, err)
target, err := sqlite3.WithInstance(db, &sqlite3.Config{})
require.NoError(t, err)
migrator, err := migrate.NewWithInstance("iofs", migrations, "sqlite3", target)
require.NoError(t, err)
err = migrator.Up()
require.NoError(t, err)
t.Cleanup(func() { db.Close() })
return sqlite.NewStore(sqlite.New(db))
}
func setupPostgresStore(t *testing.T) repository.Store {
t.Helper()
url := os.Getenv("INTEGRATION_POSTGRES_URL")
if url == "" {
t.Skip("INTEGRATION_POSTGRES_URL not set, skipping postgres integration test")
}
db, err := sql.Open("pgx", url)
require.NoError(t, err)
migrations, err := iofs.New(assets.Migrations, "migrations/postgres")
require.NoError(t, err)
target, err := pgxmigrate.WithInstance(db, &pgxmigrate.Config{})
require.NoError(t, err)
migrator, err := migrate.NewWithInstance("iofs", migrations, "pgx", target)
require.NoError(t, err)
err = migrator.Up()
require.NoError(t, err)
t.Cleanup(func() { db.Close() })
return postgres.NewStore(postgres.New(db))
}
func TestConsentIntegration(t *testing.T) {
t.Run("sqlite", func(t *testing.T) {
runConsentScenarios(t, setupSQLiteStore(t))
})
t.Run("postgres", func(t *testing.T) {
runConsentScenarios(t, setupPostgresStore(t))
})
}
func runConsentScenarios(t *testing.T, store repository.Store) {
t.Helper()
ctx := context.Background()
// User consents to client A with openid profile, then client B with openid email
_, err := store.UpsertOIDCConsent(ctx, repository.UpsertOIDCConsentParams{
Username: "alice", ClientID: "client-a", Scope: "openid profile", CreatedAt: 100,
})
require.NoError(t, err)
_, err = store.UpsertOIDCConsent(ctx, repository.UpsertOIDCConsentParams{
Username: "alice", ClientID: "client-b", Scope: "openid email", CreatedAt: 101,
})
require.NoError(t, err)
consents, err := store.ListOIDCConsents(ctx)
require.NoError(t, err)
assert.Len(t, consents, 2)
gotA, err := store.GetOIDCConsentByUsernameAndClientID(ctx, repository.GetOIDCConsentByUsernameAndClientIDParams{Username: "alice", ClientID: "client-a"})
require.NoError(t, err)
assert.Equal(t, "openid profile", gotA.Scope)
gotB, err := store.GetOIDCConsentByUsernameAndClientID(ctx, repository.GetOIDCConsentByUsernameAndClientIDParams{Username: "alice", ClientID: "client-b"})
require.NoError(t, err)
assert.Equal(t, "openid email", gotB.Scope)
// Same user+client upsert keeps exactly one row
_, err = store.UpsertOIDCConsent(ctx, repository.UpsertOIDCConsentParams{
Username: "alice", ClientID: "client-a", Scope: "openid profile email", CreatedAt: 102,
})
require.NoError(t, err)
consents, err = store.ListOIDCConsents(ctx)
require.NoError(t, err)
assert.Len(t, consents, 2)
gotA, err = store.GetOIDCConsentByUsernameAndClientID(ctx, repository.GetOIDCConsentByUsernameAndClientIDParams{Username: "alice", ClientID: "client-a"})
require.NoError(t, err)
assert.Equal(t, "openid profile email", gotA.Scope)
// Removing a client deletes its consent rows
require.NoError(t, store.DeleteOIDCConsentByClientID(ctx, "client-a"))
consents, err = store.ListOIDCConsents(ctx)
require.NoError(t, err)
assert.Len(t, consents, 1)
assert.Equal(t, "client-b", consents[0].ClientID)
_, err = store.GetOIDCConsentByUsernameAndClientID(ctx, repository.GetOIDCConsentByUsernameAndClientIDParams{Username: "alice", ClientID: "client-a"})
assert.ErrorIs(t, err, repository.ErrNotFound)
}
-180
View File
@@ -1,180 +0,0 @@
package service
import (
"context"
"strings"
"testing"
"github.com/steveiliop56/ding"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
"github.com/tinyauthapp/tinyauth/internal/model"
"github.com/tinyauthapp/tinyauth/internal/repository"
"github.com/tinyauthapp/tinyauth/internal/repository/memory"
"github.com/tinyauthapp/tinyauth/internal/test"
"github.com/tinyauthapp/tinyauth/internal/utils/logger"
)
func newTestOIDCService(t *testing.T, cfg *model.Config, runtime *model.RuntimeConfig, store repository.Store) *OIDCService {
t.Helper()
log := logger.NewLogger().WithTestConfig()
log.Init()
ctx := context.Background()
dg := ding.New(ctx)
svc, err := NewOIDCService(OIDCServiceInput{
Log: log,
Config: cfg,
Runtime: runtime,
Queries: store,
Ding: dg,
})
require.NoError(t, err)
require.NotNil(t, svc)
return svc
}
func TestOIDCConsentPerClient(t *testing.T) {
cfg, runtime := test.CreateTestConfigs(t)
store := memory.New()
svc := newTestOIDCService(t, &cfg, &runtime, store)
// Consent for client A
_, err := svc.UpsertOIDCConsent(context.Background(), "testuser", "openid profile", "some-client-id")
require.NoError(t, err)
// Consent for client B
_, err = svc.UpsertOIDCConsent(context.Background(), "testuser", "openid email", "other-client-id")
require.NoError(t, err)
// Two separate rows, one per client
consents, err := store.ListOIDCConsents(context.Background())
require.NoError(t, err)
assert.Len(t, consents, 2)
// Each lookup returns the consent for that specific client
gotA, err := svc.GetOIDCConsent(context.Background(), "testuser", "some-client-id")
require.NoError(t, err)
require.NotNil(t, gotA)
assert.Equal(t, "openid profile", gotA.Scope)
gotB, err := svc.GetOIDCConsent(context.Background(), "testuser", "other-client-id")
require.NoError(t, err)
require.NotNil(t, gotB)
assert.Equal(t, "openid email", gotB.Scope)
}
func TestOIDCConsentUnionKeepsOneRow(t *testing.T) {
cfg, runtime := test.CreateTestConfigs(t)
store := memory.New()
svc := newTestOIDCService(t, &cfg, &runtime, store)
_, err := svc.UpsertOIDCConsent(context.Background(), "testuser", "openid profile", "some-client-id")
require.NoError(t, err)
// Second authorize with an additional scope
_, err = svc.UpsertOIDCConsent(context.Background(), "testuser", "openid email", "some-client-id")
require.NoError(t, err)
// Still exactly one row
consents, err := store.ListOIDCConsents(context.Background())
require.NoError(t, err)
assert.Len(t, consents, 1)
// Stored scope is the union of both requests
got, err := svc.GetOIDCConsent(context.Background(), "testuser", "some-client-id")
require.NoError(t, err)
require.NotNil(t, got)
assert.Equal(t, []string{"email", "openid", "profile"}, splitScopes(got.Scope))
}
func TestOIDCConsentSubsetDoesNotShrink(t *testing.T) {
cfg, runtime := test.CreateTestConfigs(t)
store := memory.New()
svc := newTestOIDCService(t, &cfg, &runtime, store)
_, err := svc.UpsertOIDCConsent(context.Background(), "testuser", "openid profile email", "some-client-id")
require.NoError(t, err)
// Authorize with a subset of already-granted scopes
_, err = svc.UpsertOIDCConsent(context.Background(), "testuser", "openid profile", "some-client-id")
require.NoError(t, err)
// Stored scope is unchanged
got, err := svc.GetOIDCConsent(context.Background(), "testuser", "some-client-id")
require.NoError(t, err)
require.NotNil(t, got)
assert.Equal(t, []string{"email", "openid", "profile"}, splitScopes(got.Scope))
}
func TestOIDCConsentNotFound(t *testing.T) {
cfg, runtime := test.CreateTestConfigs(t)
store := memory.New()
svc := newTestOIDCService(t, &cfg, &runtime, store)
got, err := svc.GetOIDCConsent(context.Background(), "testuser", "some-client-id")
require.NoError(t, err)
assert.Nil(t, got)
}
func TestOIDCConsentReconcileRemovesDeletedClients(t *testing.T) {
cfg, runtime := test.CreateTestConfigs(t)
cfg.OIDC.Clients["client-a"] = model.OIDCClientConfig{
ClientID: "client-a",
ClientSecret: "secret-a",
TrustedRedirectURIs: []string{"https://a.example.com/callback"},
}
cfg.OIDC.Clients["client-b"] = model.OIDCClientConfig{
ClientID: "client-b",
ClientSecret: "secret-b",
TrustedRedirectURIs: []string{"https://b.example.com/callback"},
}
// First startup: both clients configured
store := memory.New()
svc := newTestOIDCService(t, &cfg, &runtime, store)
_, err := svc.UpsertOIDCConsent(context.Background(), "testuser", "openid profile", "client-a")
require.NoError(t, err)
_, err = svc.UpsertOIDCConsent(context.Background(), "testuser", "openid email", "client-b")
require.NoError(t, err)
// Stale consent for a client that was never configured
_, err = svc.UpsertOIDCConsent(context.Background(), "testuser", "openid", "stale-client")
require.NoError(t, err)
// Second startup: client-b removed from configuration
delete(cfg.OIDC.Clients, "client-b")
svc2 := newTestOIDCService(t, &cfg, &runtime, store)
// Consents for the removed and unknown clients are gone
_, err = store.GetOIDCConsentByUsernameAndClientID(context.Background(), repository.GetOIDCConsentByUsernameAndClientIDParams{
Username: "testuser", ClientID: "client-a",
})
require.NoError(t, err)
_, err = store.GetOIDCConsentByUsernameAndClientID(context.Background(), repository.GetOIDCConsentByUsernameAndClientIDParams{
Username: "testuser", ClientID: "client-b",
})
assert.ErrorIs(t, err, repository.ErrNotFound)
_, err = store.GetOIDCConsentByUsernameAndClientID(context.Background(), repository.GetOIDCConsentByUsernameAndClientIDParams{
Username: "testuser", ClientID: "stale-client",
})
assert.ErrorIs(t, err, repository.ErrNotFound)
require.NotNil(t, svc2)
}
func splitScopes(scope string) []string {
return strings.Fields(scope)
}
+11
View File
@@ -16,6 +16,7 @@ import (
"os"
"path/filepath"
"strings"
"sync"
"time"
"slices"
@@ -163,6 +164,10 @@ type OIDCService struct {
usedCode *cache.CacheStore[UsedCodeEntry]
authorize *cache.CacheStore[AuthorizeRequest]
}
mus struct {
consent *sync.RWMutex
}
}
type OIDCServiceInput struct {
@@ -977,6 +982,9 @@ func (service *OIDCService) GetPrompt(prompt string) []OIDCPrompt {
}
func (service *OIDCService) GetOIDCConsent(ctx context.Context, username, clientId string) (*repository.OidcConsent, error) {
service.mus.consent.RLock()
defer service.mus.consent.RUnlock()
entry, err := service.queries.GetOIDCConsentByUsernameAndClientID(ctx, repository.GetOIDCConsentByUsernameAndClientIDParams{
Username: username,
ClientID: clientId,
@@ -993,6 +1001,9 @@ func (service *OIDCService) GetOIDCConsent(ctx context.Context, username, client
}
func (service *OIDCService) UpsertOIDCConsent(ctx context.Context, username, scope, clientId string) (repository.OidcConsent, error) {
service.mus.consent.Lock()
defer service.mus.consent.Unlock()
existing, err := service.GetOIDCConsent(ctx, username, clientId)
if err != nil {