Auth, user management and storage improvements

auth/oidc/store/cleanup.go

@@ -7,6 +7,14 @@ import (
 	"github.com/in4it/go-devops-platform/auth/oidc"
 )
 
+// PENDING_STATE_TTL is how long a state without token (a login that was
+// started, but not completed yet) is kept
+const PENDING_STATE_TTL = 10 * time.Minute
+
+// MAX_PENDING_STATES is the maximum number of states without token. When a new
+// state is saved and there are more, the oldest ones are dropped.
+var MAX_PENDING_STATES = 1000
+
 func (store *Store) CleanupOAuth2DataForAllEntries() int {
 	deleted := 0
 	for _, oauthData := range store.GetOAuth2DataCopy() {
@@ -24,16 +32,17 @@ func (store *Store) CleanupOAuth2Data(oauthData oidc.OAuthData) int {
 	for k := range store.OAuth2Data {
 		if oauthData.CreatedAt.After(store.OAuth2Data[k].CreatedAt) {
 			// cleanup old oauth2 data that might be duplicates (same subject & oidc provider, but older tokens)
-			if store.OAuth2Data[k].ID != oauthData.ID && store.OAuth2Data[k].OIDCProviderID == oauthData.OIDCProviderID && store.OAuth2Data[k].Subject == oauthData.Subject {
+			// (only for entries with a subject: pending states without token have none)
+			if oauthData.Subject != "" && store.OAuth2Data[k].ID != oauthData.ID && store.OAuth2Data[k].OIDCProviderID == oauthData.OIDCProviderID && store.OAuth2Data[k].Subject == oauthData.Subject {
 				keysToDelete1 = append(keysToDelete1, k)
 			}
 			// cleanup oauthdata with the same email address
-			if store.OAuth2Data[k].ID != oauthData.ID && store.OAuth2Data[k].UserInfo.Email == oauthData.UserInfo.Email {
+			if oauthData.UserInfo.Email != "" && store.OAuth2Data[k].ID != oauthData.ID && store.OAuth2Data[k].UserInfo.Email == oauthData.UserInfo.Email {
 				keysToDelete3 = append(keysToDelete3, k)
 			}
 		}
 		// cleanup old oauth2 data that doesn't have a token and is stale
-		if store.OAuth2Data[k].Token.AccessToken == "" && store.OAuth2Data[k].CreatedAt.Add(10*time.Minute).After(time.Now()) {
+		if store.OAuth2Data[k].Token.AccessToken == "" && store.OAuth2Data[k].CreatedAt.Add(PENDING_STATE_TTL).Before(time.Now()) {
 			keysToDelete2 = append(keysToDelete2, k)
 		}
 	}
@@ -48,3 +57,33 @@ func (store *Store) CleanupOAuth2Data(oauthData oidc.OAuthData) int {
 	}
 	return len(keysToDelete)
 }
+
+// prunePendingStates removes stale states without a token, and the oldest
+// states without a token when there are more than MAX_PENDING_STATES - 1, to
+// make room for a new one. Must be called with store.Mu held.
+func (store *Store) prunePendingStates(now time.Time) {
+	type pendingState struct {
+		key       string
+		createdAt time.Time
+	}
+	pending := []pendingState{}
+	for k, oauthData := range store.OAuth2Data {
+		if oauthData.Token.AccessToken != "" {
+			continue
+		}
+		if oauthData.CreatedAt.Add(PENDING_STATE_TTL).Before(now) {
+			delete(store.OAuth2Data, k)
+			continue
+		}
+		pending = append(pending, pendingState{key: k, createdAt: oauthData.CreatedAt})
+	}
+	if len(pending) < MAX_PENDING_STATES {
+		return
+	}
+	slices.SortFunc(pending, func(a, b pendingState) int {
+		return a.createdAt.Compare(b.createdAt)
+	})
+	for _, p := range pending[:len(pending)-MAX_PENDING_STATES+1] {
+		delete(store.OAuth2Data, p.key)
+	}
+}

auth/oidc/store/cleanup_test.go

@@ -0,0 +1,76 @@
+package oidcstore
+
+import (
+	"fmt"
+	"testing"
+	"time"
+
+	"github.com/in4it/go-devops-platform/auth/oidc"
+	memorystorage "github.com/in4it/go-devops-platform/storage/memory"
+)
+
+// TestCleanupPendingStates: states without token are removed once they're
+// older than PENDING_STATE_TTL, younger ones (logins in progress) are kept.
+func TestCleanupPendingStates(t *testing.T) {
+	store, err := NewStore(&memorystorage.MockMemoryStorage{})
+	if err != nil {
+		t.Fatalf("new store error: %s", err)
+	}
+	store.OAuth2Data["young"] = oidc.OAuthData{ID: "young", OIDCProviderID: "prov-1", CreatedAt: time.Now().Add(-time.Minute)}
+	store.OAuth2Data["young2"] = oidc.OAuthData{ID: "young2", OIDCProviderID: "prov-1", CreatedAt: time.Now().Add(-2 * time.Minute)}
+	store.OAuth2Data["old"] = oidc.OAuthData{ID: "old", CreatedAt: time.Now().Add(-PENDING_STATE_TTL - time.Minute)}
+	store.OAuth2Data["withtoken"] = oidc.OAuthData{ID: "withtoken", CreatedAt: time.Now().Add(-24 * time.Hour), Token: oidc.Token{AccessToken: "abc"}, Subject: "sub-1", OIDCProviderID: "prov-1", UserInfo: oidc.UserInfo{Email: "john@example.com"}}
+
+	deleted := store.CleanupOAuth2DataForAllEntries()
+	if deleted != 1 {
+		t.Fatalf("expected 1 deleted entry, got %d", deleted)
+	}
+	if _, ok := store.GetOAuth2DataByKey("young"); !ok {
+		t.Fatalf("pending login in progress was deleted")
+	}
+	if _, ok := store.GetOAuth2DataByKey("young2"); !ok {
+		t.Fatalf("other pending login in progress was deleted")
+	}
+	if _, ok := store.GetOAuth2DataByKey("old"); ok {
+		t.Fatalf("stale pending state was not deleted")
+	}
+	if _, ok := store.GetOAuth2DataByKey("withtoken"); !ok {
+		t.Fatalf("entry with token was deleted")
+	}
+}
+
+// TestPendingStatesCapped: unauthenticated users starting logins can't grow
+// the store without bound.
+func TestPendingStatesCapped(t *testing.T) {
+	maxPendingStates := MAX_PENDING_STATES
+	MAX_PENDING_STATES = 10
+	defer func() { MAX_PENDING_STATES = maxPendingStates }()
+
+	store, err := NewStore(&memorystorage.MockMemoryStorage{})
+	if err != nil {
+		t.Fatalf("new store error: %s", err)
+	}
+	if err := store.SaveOAuth2Data(oidc.OAuthData{ID: "loggedin", CreatedAt: time.Now().Add(-time.Hour), Token: oidc.Token{AccessToken: "abc"}}, "loggedin"); err != nil {
+		t.Fatalf("save error: %s", err)
+	}
+	start := time.Now().Add(-5 * time.Minute)
+	for i := 0; i < 25; i++ {
+		err := store.SaveOAuth2Data(oidc.OAuthData{ID: fmt.Sprintf("id-%d", i), CreatedAt: start.Add(time.Duration(i) * time.Second)}, fmt.Sprintf("state-%d", i))
+		if err != nil {
+			t.Fatalf("save error: %s", err)
+		}
+	}
+	all := store.GetOAuth2DataCopy()
+	if len(all) != MAX_PENDING_STATES+1 {
+		t.Fatalf("expected %d entries, got %d", MAX_PENDING_STATES+1, len(all))
+	}
+	if _, ok := all["loggedin"]; !ok {
+		t.Fatalf("entry with token was dropped")
+	}
+	if _, ok := all["state-24"]; !ok {
+		t.Fatalf("newest pending state was dropped")
+	}
+	if _, ok := all["state-0"]; ok {
+		t.Fatalf("oldest pending state was kept")
+	}
+}

auth/oidc/store/renewal/worker.go

@@ -13,6 +13,11 @@ const DEFAULT_RENEWAL_TIME_MINUTES = 60 // every hour we want to refresh the tok
 const RENEWAL_RETRIES = 3               // 3 retries before we suspend a user
 const RENEWAL_BACKOFF_SECONDS = 10      // time between requests to the oidc providers
 
+// backoffSleep waits between requests to the oidc providers
+var backoffSleep = func() {
+	time.Sleep(RENEWAL_BACKOFF_SECONDS * time.Second)
+}
+
 func (r *Renewal) Worker() {
 	// do renewal
 	if r.enabled {
@@ -23,26 +28,59 @@ func (r *Renewal) Worker() {
 			time.Sleep(WAKEUP_TIME_SECONDS * time.Second)
 			continue
 		}
-		deletedEntries := r.oidcStore.CleanupOAuth2DataForAllEntries()
-		if deletedEntries > 0 {
-			err := r.oidcStore.SaveOIDCStore()
-			if err != nil {
-				log.Printf("Renewal Worker: [warning] couldn't save oidc store after cleanup: %s", err)
-			}
+		r.renewAll()
+		time.Sleep(WAKEUP_TIME_SECONDS * time.Second)
+	}
+}
+
+func (r *Renewal) renewAll() {
+	deletedEntries := r.oidcStore.CleanupOAuth2DataForAllEntries()
+	if deletedEntries > 0 {
+		err := r.oidcStore.SaveOIDCStore()
+		if err != nil {
+			log.Printf("Renewal Worker: [warning] couldn't save oidc store after cleanup: %s", err)
 		}
-		for key, oauth2Data := range r.oidcStore.GetOAuth2DataCopy() {
-			logging.DebugLog(fmt.Errorf("running canRenew of %s", oauth2Data.ID))
-			// can we renew? Do we have expiration date and it is expired?
-			canRenew, oidcProvider, discovery, err := canRenew(r.renewalTime, oauth2Data, r.oidcStore, r.oidcProviders)
-			if err != nil {
-				log.Printf("Renewal Worker: [warning] needsRenewal: %s", err)
-			}
-			if canRenew {
-				logging.DebugLog(fmt.Errorf("we can renew %s", oauth2Data.ID))
-				r.renew(discovery, key, oauth2Data, oidcProvider) // error logging within function
+	}
+	for key, oauth2Data := range r.oidcStore.GetOAuth2DataCopy() {
+		logging.DebugLog(fmt.Errorf("running canRenew of %s", oauth2Data.ID))
+		// can we renew? Do we have expiration date and it is expired?
+		canRenew, oidcProvider, discovery, err := canRenew(r.renewalTime, oauth2Data, r.oidcStore, r.oidcProviders)
+		if err != nil {
+			log.Printf("Renewal Worker: [warning] needsRenewal: %s", err)
+		}
+		if canRenew {
+			logging.DebugLog(fmt.Errorf("we can renew %s", oauth2Data.ID))
+			userDisabled := r.renew(discovery, key, oauth2Data, oidcProvider) // error logging within function
+			if userDisabled {
+				r.disableUser(oauth2Data.ID)
 			}
-			time.Sleep(RENEWAL_BACKOFF_SECONDS * time.Second)
+			backoffSleep() // only wait after a request to the oidc provider
 		}
-		time.Sleep(WAKEUP_TIME_SECONDS * time.Second)
+	}
+}
+
+// disableUser disables the connections of the user with the given oidc id
+// after token renewal failed
+func (r *Renewal) disableUser(oidcID string) {
+	if r.userStore == nil {
+		return
+	}
+	user, err := r.userStore.GetUserByOIDCIDs([]string{oidcID})
+	if err != nil {
+		logging.ErrorLog(fmt.Errorf("renewal Worker: no user found with oidc id %s", oidcID))
+		return
+	}
+	logging.DebugLog(fmt.Errorf("disable user with oidc id %s", oidcID))
+	if r.userStore.UserHooks.DisableFunc != nil {
+		err = r.userStore.UserHooks.DisableFunc(r.storage, user)
+		if err != nil {
+			logging.ErrorLog(fmt.Errorf("renewal Worker: could not disable connections for userID %s: %s", user.ID, err))
+			return
+		}
+	}
+	user.ConnectionsDisabledOnAuthFailure = true
+	err = r.userStore.UpdateUser(user)
+	if err != nil {
+		logging.ErrorLog(fmt.Errorf("renewal Worker: could not update connectionsDisabledOnAuthFailure for userID %s: %s", user.ID, err))
 	}
 }

auth/oidc/store/renewal/worker_test.go

@@ -0,0 +1,127 @@
+package oidcrenewal
+
+import (
+	"encoding/base64"
+	"encoding/json"
+	"net/http"
+	"net/http/httptest"
+	"sync"
+	"testing"
+	"time"
+
+	"github.com/in4it/go-devops-platform/auth/oidc"
+	oidcstore "github.com/in4it/go-devops-platform/auth/oidc/store"
+	"github.com/in4it/go-devops-platform/storage"
+	memorystorage "github.com/in4it/go-devops-platform/storage/memory"
+	"github.com/in4it/go-devops-platform/users"
+)
+
+func fakeAccessToken(t *testing.T, exp time.Time) string {
+	t.Helper()
+	payload, err := json.Marshal(jwtExp{Expiration: exp.Unix()})
+	if err != nil {
+		t.Fatalf("marshal error: %s", err)
+	}
+	return "eyJhbGciOiJub25lIn0." + base64.RawURLEncoding.EncodeToString(payload) + ".sig"
+}
+
+func newTestRenewal(t *testing.T, tokenEndpointStatus int) (*Renewal, *httptest.Server) {
+	t.Helper()
+	var ts *httptest.Server
+	ts = httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		switch r.URL.Path {
+		case "/discovery.json":
+			out, _ := json.Marshal(oidc.Discovery{Issuer: "test-issuer", TokenEndpoint: ts.URL + "/token"})
+			w.Write(out)
+		case "/token":
+			w.WriteHeader(tokenEndpointStatus)
+			w.Write([]byte(`{"error": "invalid_grant"}`))
+		default:
+			w.WriteHeader(http.StatusNotFound)
+		}
+	}))
+	t.Cleanup(ts.Close)
+
+	storage := &memorystorage.MockMemoryStorage{}
+	store, err := oidcstore.NewStore(storage)
+	if err != nil {
+		t.Fatalf("new store error: %s", err)
+	}
+	userStore, err := users.NewUserStore(storage, 100)
+	if err != nil {
+		t.Fatalf("new user store error: %s", err)
+	}
+	return &Renewal{
+		oidcStore:     store,
+		enabled:       true,
+		oidcProviders: []oidc.OIDCProvider{{ID: "prov-1", ClientID: "client", ClientSecret: "secret", DiscoveryURI: ts.URL + "/discovery.json"}},
+		userStore:     userStore,
+		renewalTime:   time.Hour,
+		storage:       storage,
+	}, ts
+}
+
+// TestWorkerDisablesUserOnRenewalFailure: when token renewal fails for the last
+// time, the background worker disables the user's connections.
+func TestWorkerDisablesUserOnRenewalFailure(t *testing.T) {
+	backoffSleep = func() {}
+	r, _ := newTestRenewal(t, http.StatusBadRequest)
+
+	var mu sync.Mutex
+	disabled := []string{}
+	r.userStore.UserHooks.DisableFunc = func(_ storage.Iface, user users.User) error {
+		mu.Lock()
+		defer mu.Unlock()
+		disabled = append(disabled, user.ID)
+		return nil
+	}
+	user, err := r.userStore.AddUser(users.User{Login: "john@example.com", OIDCID: "oidc-1"})
+	if err != nil {
+		t.Fatalf("add user error: %s", err)
+	}
+	err = r.oidcStore.SaveOAuth2Data(oidc.OAuthData{
+		ID:               "oidc-1",
+		OIDCProviderID:   "prov-1",
+		CreatedAt:        time.Now().Add(-2 * time.Hour),
+		LastTokenRenewal: time.Now().Add(-2 * time.Hour),
+		RenewalRetries:   RENEWAL_RETRIES - 1,
+		Token:            oidc.Token{AccessToken: fakeAccessToken(t, time.Now().Add(-time.Minute)), RefreshToken: "refresh"},
+	}, "state-1")
+	if err != nil {
+		t.Fatalf("save error: %s", err)
+	}
+
+	r.renewAll()
+
+	if len(disabled) != 1 || disabled[0] != user.ID {
+		t.Fatalf("expected disable hook for %s, got %v", user.ID, disabled)
+	}
+	updated, err := r.userStore.GetUserByID(user.ID)
+	if err != nil {
+		t.Fatalf("get user error: %s", err)
+	}
+	if !updated.ConnectionsDisabledOnAuthFailure {
+		t.Fatalf("expected ConnectionsDisabledOnAuthFailure to be set")
+	}
+	oauth2Data, _ := r.oidcStore.GetOAuth2DataByKey("state-1")
+	if !oauth2Data.RenewalFailed {
+		t.Fatalf("expected renewal to be marked as failed")
+	}
+}
+
+// TestWorkerOnlyBacksOffAfterRenewal: entries that aren't renewed (pending
+// logins without token) don't make the worker sleep.
+func TestWorkerOnlyBacksOffAfterRenewal(t *testing.T) {
+	sleeps := 0
+	backoffSleep = func() { sleeps++ }
+	r, _ := newTestRenewal(t, http.StatusBadRequest)
+	for _, state := range []string{"a", "b", "c"} {
+		if err := r.oidcStore.SaveOAuth2Data(oidc.OAuthData{ID: state, OIDCProviderID: "prov-1", CreatedAt: time.Now()}, state); err != nil {
+			t.Fatalf("save error: %s", err)
+		}
+	}
+	r.renewAll()
+	if sleeps != 0 {
+		t.Fatalf("expected no backoff for entries without renewal, got %d", sleeps)
+	}
+}

auth/oidc/store/save.go

@@ -3,6 +3,7 @@ package oidcstore
 import (
 	"encoding/json"
 	"fmt"
+	"time"
 
 	"github.com/in4it/go-devops-platform/auth/oidc"
 )
@@ -24,6 +25,10 @@ func (store *Store) SaveOIDCStore() error {
 
 func (store *Store) SaveOAuth2Data(oauth2Data oidc.OAuthData, key string) error {
 	store.Mu.Lock()
+	if _, exists := store.OAuth2Data[key]; !exists && oauth2Data.Token.AccessToken == "" {
+		// new login started: make sure pending states can't grow without bound
+		store.prunePendingStates(time.Now())
+	}
 	store.OAuth2Data[key] = oauth2Data
 	store.Mu.Unlock()
 	return store.SaveOIDCStore()