Merge pull request argoproj#7 from alexmt/bugs/rate-limit-failed-logins

Bugs/rate limit failed logins

docs/operator-manual/user-management/index.md

@@ -90,6 +90,22 @@ argocd account update-password \
 argocd account generate-token --account <username> 
 ```
 
+### Failed logins rate limiting
+
+Argo CD rejects login attempts after too many failed in order to prevent password brute-forcing.
+The following environments variables are available to control throttling settings:
+
+* `ARGOCD_SESSION_MAX_FAIL_COUNT`: Maximum number of failed logins before Argo CD starts
+rejecting login attempts. Default: 5.
+
+* `ARGOCD_SESSION_FAILURE_WINDOW_SECONDS`: Number of seconds for the failure window.
+Default: 300 (5 minutes). If this is set to 0, the failure window is
+disabled and the login attempts gets rejected after 10 consecutive logon failures,
+regardless of the time frame they happened.
+
+* `ARGOCD_SESSION_MAX_CACHE_SIZE`: Maximum number of entries allowed in the
+cache. Default: 1000
+
 ## SSO
 
 There are two ways that SSO can be configured:

server/account/account_test.go

@@ -18,7 +18,6 @@ import (
 	"github.com/argoproj/argo-cd/pkg/apiclient/account"
 	sessionpkg "github.com/argoproj/argo-cd/pkg/apiclient/session"
 	"github.com/argoproj/argo-cd/server/session"
-	"github.com/argoproj/argo-cd/util/cache"
 	"github.com/argoproj/argo-cd/util/password"
 	"github.com/argoproj/argo-cd/util/rbac"
 	sessionutil "github.com/argoproj/argo-cd/util/session"
@@ -64,8 +63,7 @@ func newTestAccountServerExt(ctx context.Context, enforceFn rbac.ClaimsEnforcerF
 	}
 	kubeclientset := fake.NewSimpleClientset(cm, secret)
 	settingsMgr := settings.NewSettingsManager(ctx, kubeclientset, testNamespace)
-	sessionCache := cache.NewCache(cache.NewInMemoryCache(0))
-	sessionMgr := sessionutil.NewSessionManager(settingsMgr, "", sessionCache)
+	sessionMgr := sessionutil.NewSessionManager(settingsMgr, "", sessionutil.NewInMemoryUserStateStorage())
 	enforcer := rbac.NewEnforcer(kubeclientset, testNamespace, common.ArgoCDRBACConfigMapName, nil)
 	enforcer.SetClaimsEnforcerFunc(enforceFn)
 

server/cache/cache.go

@@ -9,6 +9,8 @@ import (
 	appv1 "github.com/argoproj/argo-cd/pkg/apis/application/v1alpha1"
 	cacheutil "github.com/argoproj/argo-cd/util/cache"
 	appstatecache "github.com/argoproj/argo-cd/util/cache/appstate"
+	"github.com/argoproj/argo-cd/util/oidc"
+	"github.com/argoproj/argo-cd/util/session"
 )
 
 var ErrCacheMiss = appstatecache.ErrCacheMiss
@@ -17,19 +19,16 @@ type Cache struct {
 	cache                           *appstatecache.Cache
 	connectionStatusCacheExpiration time.Duration
 	oidcCacheExpiration             time.Duration
+	loginAttemptsExpiration         time.Duration
 }
 
 func NewCache(
 	cache *appstatecache.Cache,
 	connectionStatusCacheExpiration time.Duration,
 	oidcCacheExpiration time.Duration,
+	loginAttemptsExpiration time.Duration,
 ) *Cache {
-	return &Cache{cache, connectionStatusCacheExpiration, oidcCacheExpiration}
-}
-
-type OIDCState struct {
-	// ReturnURL is the URL in which to redirect a user back to after completing an OAuth2 login
-	ReturnURL string `json:"returnURL"`
+	return &Cache{cache, connectionStatusCacheExpiration, oidcCacheExpiration, loginAttemptsExpiration}
 }
 
 type ClusterInfo struct {
@@ -40,9 +39,11 @@ type ClusterInfo struct {
 func AddCacheFlagsToCmd(cmd *cobra.Command) func() (*Cache, error) {
 	var connectionStatusCacheExpiration time.Duration
 	var oidcCacheExpiration time.Duration
+	var loginAttemptsExpiration time.Duration
 
 	cmd.Flags().DurationVar(&connectionStatusCacheExpiration, "connection-status-cache-expiration", 1*time.Hour, "Cache expiration for cluster/repo connection status")
 	cmd.Flags().DurationVar(&oidcCacheExpiration, "oidc-cache-expiration", 3*time.Minute, "Cache expiration for OIDC state")
+	cmd.Flags().DurationVar(&loginAttemptsExpiration, "login-attempts-expiration", 24*time.Hour, "Cache expiration for failed login attempts")
 
 	fn := appstatecache.AddCacheFlagsToCmd(cmd)
 
@@ -52,7 +53,7 @@ func AddCacheFlagsToCmd(cmd *cobra.Command) func() (*Cache, error) {
 			return nil, err
 		}
 
-		return NewCache(cache, connectionStatusCacheExpiration, oidcCacheExpiration), nil
+		return NewCache(cache, connectionStatusCacheExpiration, oidcCacheExpiration, loginAttemptsExpiration), nil
 	}
 }
 
@@ -64,6 +65,14 @@ func (c *Cache) GetAppManagedResources(appName string, res *[]*appv1.ResourceDif
 	return c.cache.GetAppManagedResources(appName, res)
 }
 
+func (c *Cache) GetLoginAttempts(attempts *map[string]session.LoginAttempts) error {
+	return c.cache.GetItem("session|login.attempts", attempts)
+}
+
+func (c *Cache) SetLoginAttempts(attempts map[string]session.LoginAttempts) error {
+	return c.cache.SetItem("session|login.attempts", attempts, c.loginAttemptsExpiration, attempts == nil)
+}
+
 func clusterConnectionStateKey(server string) string {
 	return fmt.Sprintf("cluster|%s|connection-state", server)
 }
@@ -96,13 +105,13 @@ func oidcStateKey(key string) string {
 	return fmt.Sprintf("oidc|%s", key)
 }
 
-func (c *Cache) GetOIDCState(key string) (*OIDCState, error) {
-	res := OIDCState{}
+func (c *Cache) GetOIDCState(key string) (*oidc.OIDCState, error) {
+	res := oidc.OIDCState{}
 	err := c.cache.GetItem(oidcStateKey(key), &res)
 	return &res, err
 }
 
-func (c *Cache) SetOIDCState(key string, state *OIDCState) error {
+func (c *Cache) SetOIDCState(key string, state *oidc.OIDCState) error {
 	return c.cache.SetItem(oidcStateKey(key), state, c.oidcCacheExpiration, state == nil)
 }
 

server/cache/cache_test.go

@@ -10,6 +10,7 @@ import (
 	. "github.com/argoproj/argo-cd/pkg/apis/application/v1alpha1"
 	cacheutil "github.com/argoproj/argo-cd/util/cache"
 	appstatecache "github.com/argoproj/argo-cd/util/cache/appstate"
+	"github.com/argoproj/argo-cd/util/oidc"
 )
 
 type fixtures struct {
@@ -24,6 +25,7 @@ func newFixtures() *fixtures {
 		),
 		1*time.Minute,
 		1*time.Minute,
+		1*time.Minute,
 	)}
 }
 
@@ -67,15 +69,15 @@ func TestCache_GetOIDCState(t *testing.T) {
 	_, err := cache.GetOIDCState("my-key")
 	assert.Equal(t, ErrCacheMiss, err)
 	// populate cache
-	err = cache.SetOIDCState("my-key", &OIDCState{ReturnURL: "my-return-url"})
+	err = cache.SetOIDCState("my-key", &oidc.OIDCState{ReturnURL: "my-return-url"})
 	assert.NoError(t, err)
 	//cache miss
 	_, err = cache.GetOIDCState("other-key")
 	assert.Equal(t, ErrCacheMiss, err)
 	// cache hit
 	value, err := cache.GetOIDCState("my-key")
 	assert.NoError(t, err)
-	assert.Equal(t, &OIDCState{ReturnURL: "my-return-url"}, value)
+	assert.Equal(t, &oidc.OIDCState{ReturnURL: "my-return-url"}, value)
 }
 
 func TestAddCacheFlagsToCmd(t *testing.T) {

server/project/project_test.go

@@ -6,9 +6,8 @@ import (
 	"strings"
 	"testing"
 
-	"github.com/google/uuid"
-
 	"github.com/dgrijalva/jwt-go"
+	"github.com/google/uuid"
 	"github.com/stretchr/testify/assert"
 	"google.golang.org/grpc/codes"
 	"google.golang.org/grpc/status"
@@ -23,7 +22,6 @@ import (
 	apps "github.com/argoproj/argo-cd/pkg/client/clientset/versioned/fake"
 	"github.com/argoproj/argo-cd/util"
 	"github.com/argoproj/argo-cd/util/assets"
-	"github.com/argoproj/argo-cd/util/cache"
 	jwtutil "github.com/argoproj/argo-cd/util/jwt"
 	"github.com/argoproj/argo-cd/util/rbac"
 	"github.com/argoproj/argo-cd/util/session"
@@ -32,11 +30,6 @@ import (
 
 const testNamespace = "default"
 
-func newSessionCache() *cache.Cache {
-	sessionCache := cache.NewCache(cache.NewInMemoryCache(0))
-	return sessionCache
-}
-
 func TestProjectServer(t *testing.T) {
 	kubeclientset := fake.NewSimpleClientset(&corev1.ConfigMap{
 		ObjectMeta: v1.ObjectMeta{
@@ -292,7 +285,7 @@ func TestProjectServer(t *testing.T) {
 	id := "testId"
 
 	t.Run("TestCreateTokenDenied", func(t *testing.T) {
-		sessionMgr := session.NewSessionManager(settingsMgr, "", newSessionCache())
+		sessionMgr := session.NewSessionManager(settingsMgr, "", session.NewInMemoryUserStateStorage())
 		projectWithRole := existingProj.DeepCopy()
 		projectWithRole.Spec.Roles = []v1alpha1.ProjectRole{{Name: tokenName}}
 		projectServer := NewServer("default", fake.NewSimpleClientset(), apps.NewSimpleClientset(projectWithRole), enforcer, util.NewKeyLock(), sessionMgr)
@@ -301,7 +294,7 @@ func TestProjectServer(t *testing.T) {
 	})
 
 	t.Run("TestCreateTokenSuccessfullyUsingGroup", func(t *testing.T) {
-		sessionMgr := session.NewSessionManager(settingsMgr, "", newSessionCache())
+		sessionMgr := session.NewSessionManager(settingsMgr, "", session.NewInMemoryUserStateStorage())
 		projectWithRole := existingProj.DeepCopy()
 		projectWithRole.Spec.Roles = []v1alpha1.ProjectRole{{Name: tokenName, Groups: []string{"my-group"}}}
 		projectServer := NewServer("default", fake.NewSimpleClientset(), apps.NewSimpleClientset(projectWithRole), enforcer, util.NewKeyLock(), sessionMgr)
@@ -312,7 +305,7 @@ func TestProjectServer(t *testing.T) {
 	_ = enforcer.SetBuiltinPolicy(`p, role:admin, projects, update, *, allow`)
 
 	t.Run("TestCreateTokenSuccessfully", func(t *testing.T) {
-		sessionMgr := session.NewSessionManager(settingsMgr, "", newSessionCache())
+		sessionMgr := session.NewSessionManager(settingsMgr, "", session.NewInMemoryUserStateStorage())
 		projectWithRole := existingProj.DeepCopy()
 		projectWithRole.Spec.Roles = []v1alpha1.ProjectRole{{Name: tokenName}}
 		projectServer := NewServer("default", fake.NewSimpleClientset(), apps.NewSimpleClientset(projectWithRole), enforcer, util.NewKeyLock(), sessionMgr)
@@ -330,7 +323,7 @@ func TestProjectServer(t *testing.T) {
 	})
 
 	t.Run("TestCreateTokenWithIDSuccessfully", func(t *testing.T) {
-		sessionMgr := session.NewSessionManager(settingsMgr, "", newSessionCache())
+		sessionMgr := session.NewSessionManager(settingsMgr, "", session.NewInMemoryUserStateStorage())
 		projectWithRole := existingProj.DeepCopy()
 		projectWithRole.Spec.Roles = []v1alpha1.ProjectRole{{Name: tokenName}}
 		projectServer := NewServer("default", fake.NewSimpleClientset(), apps.NewSimpleClientset(projectWithRole), enforcer, util.NewKeyLock(), sessionMgr)
@@ -348,7 +341,7 @@ func TestProjectServer(t *testing.T) {
 	})
 
 	t.Run("TestCreateTokenWithSameIdDeny", func(t *testing.T) {
-		sessionMgr := session.NewSessionManager(settingsMgr, "", newSessionCache())
+		sessionMgr := session.NewSessionManager(settingsMgr, "", session.NewInMemoryUserStateStorage())
 		projectWithRole := existingProj.DeepCopy()
 		projectWithRole.Spec.Roles = []v1alpha1.ProjectRole{{Name: tokenName}}
 		projectServer := NewServer("default", fake.NewSimpleClientset(), apps.NewSimpleClientset(projectWithRole), enforcer, util.NewKeyLock(), sessionMgr)
@@ -373,7 +366,7 @@ func TestProjectServer(t *testing.T) {
 	_ = enforcer.SetBuiltinPolicy(`p, *, *, *, *, deny`)
 
 	t.Run("TestDeleteTokenDenied", func(t *testing.T) {
-		sessionMgr := session.NewSessionManager(settingsMgr, "", newSessionCache())
+		sessionMgr := session.NewSessionManager(settingsMgr, "", session.NewInMemoryUserStateStorage())
 		projWithToken := existingProj.DeepCopy()
 		issuedAt := int64(1)
 		secondIssuedAt := issuedAt + 1
@@ -386,7 +379,7 @@ func TestProjectServer(t *testing.T) {
 	})
 
 	t.Run("TestDeleteTokenSuccessfullyWithGroup", func(t *testing.T) {
-		sessionMgr := session.NewSessionManager(settingsMgr, "", newSessionCache())
+		sessionMgr := session.NewSessionManager(settingsMgr, "", session.NewInMemoryUserStateStorage())
 		projWithToken := existingProj.DeepCopy()
 		issuedAt := int64(1)
 		secondIssuedAt := issuedAt + 1
@@ -402,7 +395,7 @@ func TestProjectServer(t *testing.T) {
 p, role:admin, projects, update, *, allow`)
 
 	t.Run("TestDeleteTokenSuccessfully", func(t *testing.T) {
-		sessionMgr := session.NewSessionManager(settingsMgr, "", newSessionCache())
+		sessionMgr := session.NewSessionManager(settingsMgr, "", session.NewInMemoryUserStateStorage())
 		projWithToken := existingProj.DeepCopy()
 		issuedAt := int64(1)
 		secondIssuedAt := issuedAt + 1
@@ -423,7 +416,7 @@ p, role:admin, projects, update, *, allow`)
 p, role:admin, projects, update, *, allow`)
 
 	t.Run("TestDeleteTokenByIdSuccessfully", func(t *testing.T) {
-		sessionMgr := session.NewSessionManager(settingsMgr, "", newSessionCache())
+		sessionMgr := session.NewSessionManager(settingsMgr, "", session.NewInMemoryUserStateStorage())
 		projWithToken := existingProj.DeepCopy()
 		issuedAt := int64(1)
 		secondIssuedAt := issuedAt + 1
@@ -446,7 +439,7 @@ p, role:admin, projects, update, *, allow`)
 	enforcer = newEnforcer(kubeclientset)
 
 	t.Run("TestCreateTwoTokensInRoleSuccess", func(t *testing.T) {
-		sessionMgr := session.NewSessionManager(settingsMgr, "", newSessionCache())
+		sessionMgr := session.NewSessionManager(settingsMgr, "", session.NewInMemoryUserStateStorage())
 		projWithToken := existingProj.DeepCopy()
 		tokenName := "testToken"
 		token := v1alpha1.ProjectRole{Name: tokenName, JWTTokens: []v1alpha1.JWTToken{{IssuedAt: 1}}}
@@ -611,7 +604,7 @@ p, role:admin, projects, update, *, allow`)
 	})
 
 	t.Run("TestSyncWindowsActive", func(t *testing.T) {
-		sessionMgr := session.NewSessionManager(settingsMgr, "", newSessionCache())
+		sessionMgr := session.NewSessionManager(settingsMgr, "", session.NewInMemoryUserStateStorage())
 		projectWithSyncWindows := existingProj.DeepCopy()
 		projectWithSyncWindows.Spec.SyncWindows = v1alpha1.SyncWindows{}
 		win := &v1alpha1.SyncWindow{Kind: "allow", Schedule: "* * * * *", Duration: "1h"}
@@ -624,7 +617,7 @@ p, role:admin, projects, update, *, allow`)
 	})
 
 	t.Run("TestGetSyncWindowsStateCannotGetProjectDetails", func(t *testing.T) {
-		sessionMgr := session.NewSessionManager(settingsMgr, "", newSessionCache())
+		sessionMgr := session.NewSessionManager(settingsMgr, "", session.NewInMemoryUserStateStorage())
 		projectWithSyncWindows := existingProj.DeepCopy()
 		projectWithSyncWindows.Spec.SyncWindows = v1alpha1.SyncWindows{}
 		win := &v1alpha1.SyncWindow{Kind: "allow", Schedule: "* * * * *", Duration: "1h"}
@@ -642,7 +635,7 @@ p, role:admin, projects, update, *, allow`)
 		enforcer.SetClaimsEnforcerFunc(nil)
 		ctx := context.WithValue(context.Background(), "claims", &jwt.MapClaims{"groups": []string{"my-group"}})
 
-		sessionMgr := session.NewSessionManager(settingsMgr, "", newSessionCache())
+		sessionMgr := session.NewSessionManager(settingsMgr, "", session.NewInMemoryUserStateStorage())
 		projectWithSyncWindows := existingProj.DeepCopy()
 		win := &v1alpha1.SyncWindow{Kind: "allow", Schedule: "* * * * *", Duration: "1h"}
 		projectWithSyncWindows.Spec.SyncWindows = append(projectWithSyncWindows.Spec.SyncWindows, win)

server/server.go

@@ -74,7 +74,6 @@ import (
 	"github.com/argoproj/argo-cd/server/version"
 	"github.com/argoproj/argo-cd/util"
 	"github.com/argoproj/argo-cd/util/assets"
-	cacheutil "github.com/argoproj/argo-cd/util/cache"
 	"github.com/argoproj/argo-cd/util/db"
 	"github.com/argoproj/argo-cd/util/dex"
 	dexutil "github.com/argoproj/argo-cd/util/dex"
@@ -179,14 +178,7 @@ func NewServer(ctx context.Context, opts ArgoCDServerOpts) *ArgoCDServer {
 	err = initializeDefaultProject(opts)
 	errors.CheckError(err)
 
-	var smgrCache *cacheutil.Cache
-	if opts.Cache != nil && opts.Cache.GetCache() != nil {
-		smgrCache = opts.Cache.GetCache()
-	} else {
-		log.Warnf("Running session manager with InMemory cache, which is not recommended.")
-		smgrCache = cacheutil.NewCache(cacheutil.NewInMemoryCache(0))
-	}
-	sessionMgr := util_session.NewSessionManager(settingsMgr, opts.DexServerAddr, smgrCache)
+	sessionMgr := util_session.NewSessionManager(settingsMgr, opts.DexServerAddr, opts.Cache)
 
 	factory := appinformer.NewFilteredSharedInformerFactory(opts.AppClientset, 0, opts.Namespace, func(options *metav1.ListOptions) {})
 	projInformer := factory.Argoproj().V1alpha1().AppProjects().Informer()

ui/src/app/login/components/login.tsx

@@ -18,6 +18,7 @@ export interface LoginForm {
 interface State {
     authSettings: AuthSettings;
     loginError: string;
+    loginInProgress: boolean;
     returnUrl: string;
     ssoLoginError: string;
 }
@@ -36,7 +37,7 @@ export class Login extends React.Component<RouteComponentProps<{}>, State> {
 
     constructor(props: RouteComponentProps<{}>) {
         super(props);
-        this.state = {authSettings: null, loginError: null, returnUrl: null, ssoLoginError: null};
+        this.state = {authSettings: null, loginError: null, returnUrl: null, ssoLoginError: null, loginInProgress: false};
     }
 
     public async componentDidMount() {
@@ -93,7 +94,7 @@ export class Login extends React.Component<RouteComponentProps<{}>, State> {
                                         {this.state.loginError && <div className='argo-form-row__error-msg'>{this.state.loginError}</div>}
                                     </div>
                                     <div className='login__form-row'>
-                                        <button className='argo-button argo-button--full-width argo-button--xlg' type='submit'>
+                                        <button disabled={this.state.loginInProgress} className='argo-button argo-button--full-width argo-button--xlg' type='submit'>
                                             Sign In
                                         </button>
                                     </div>
@@ -116,17 +117,18 @@ export class Login extends React.Component<RouteComponentProps<{}>, State> {
 
     private async login(username: string, password: string, returnURL: string) {
         try {
-            this.setState({loginError: ''});
+            this.setState({loginError: '', loginInProgress: true});
             this.appContext.apis.navigation.goto('.', {sso_error: null});
             await services.users.login(username, password);
+            this.setState({loginInProgress: false});
             if (returnURL) {
                 const url = new URL(returnURL);
                 this.appContext.apis.navigation.goto(url.pathname + url.search);
             } else {
                 this.appContext.apis.navigation.goto('/applications');
             }
         } catch (e) {
-            this.setState({loginError: e.response.body.error});
+            this.setState({loginError: e.response.body.error, loginInProgress: false});
         }
     }
 

util/cache/appstate/cache.go

@@ -21,11 +21,6 @@ func NewCache(cache *cacheutil.Cache, appStateCacheExpiration time.Duration) *Ca
 	return &Cache{cache, appStateCacheExpiration}
 }
 
-type OIDCState struct {
-	// ReturnURL is the URL in which to redirect a user back to after completing an OAuth2 login
-	ReturnURL string `json:"returnURL"`
-}
-
 func AddCacheFlagsToCmd(cmd *cobra.Command) func() (*Cache, error) {
 	var appStateCacheExpiration time.Duration