Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Bugs/rate limit failed logins #7

Closed
16 changes: 16 additions & 0 deletions docs/operator-manual/user-management/index.md
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down
4 changes: 1 addition & 3 deletions server/account/account_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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)

Expand Down
29 changes: 19 additions & 10 deletions server/cache/cache.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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 {
Expand All @@ -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)

Expand All @@ -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
}
}

Expand All @@ -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)
}
Expand Down Expand Up @@ -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)
}

Expand Down
6 changes: 4 additions & 2 deletions server/cache/cache_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand All @@ -24,6 +25,7 @@ func newFixtures() *fixtures {
),
1*time.Minute,
1*time.Minute,
1*time.Minute,
)}
}

Expand Down Expand Up @@ -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) {
Expand Down
35 changes: 14 additions & 21 deletions server/project/project_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand All @@ -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"
Expand All @@ -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{
Expand Down Expand Up @@ -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)
Expand All @@ -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)
Expand All @@ -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)
Expand All @@ -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)
Expand All @@ -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)
Expand All @@ -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
Expand All @@ -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
Expand All @@ -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
Expand All @@ -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
Expand All @@ -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}}}
Expand Down Expand Up @@ -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"}
Expand All @@ -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"}
Expand All @@ -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)
Expand Down
10 changes: 1 addition & 9 deletions server/server.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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()
Expand Down
10 changes: 6 additions & 4 deletions ui/src/app/login/components/login.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ export interface LoginForm {
interface State {
authSettings: AuthSettings;
loginError: string;
loginInProgress: boolean;
returnUrl: string;
ssoLoginError: string;
}
Expand All @@ -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() {
Expand Down Expand Up @@ -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>
Expand All @@ -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});
}
}

Expand Down
5 changes: 0 additions & 5 deletions util/cache/appstate/cache.go
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
Loading