Skip to content

Commit

Permalink
feat: max cookie length (#7515)
Browse files Browse the repository at this point in the history
feat: max cookie length (#7515)

Signed-off-by: pashavictorovich <pavel@codefresh.io>
  • Loading branch information
pasha-codefresh committed Oct 21, 2021
1 parent d6d620e commit aa6aed3
Show file tree
Hide file tree
Showing 8 changed files with 41 additions and 4 deletions.
2 changes: 2 additions & 0 deletions common/common.go
Expand Up @@ -181,6 +181,8 @@ const (
EnvLogFormat = "ARGOCD_LOG_FORMAT"
// EnvLogLevel log level that is defined by `--loglevel` option
EnvLogLevel = "ARGOCD_LOG_LEVEL"
// EnvMaxCookieNumber max number of chunks a cookie can be broken into
EnvMaxCookieNumber = "ARGOCD_MAX_COOKIE_NUMBER"
)

const (
Expand Down
6 changes: 6 additions & 0 deletions manifests/base/server/argocd-server-deployment.yaml
Expand Up @@ -166,6 +166,12 @@ spec:
name: argocd-cmd-params-cm
key: server.default.cache.expiration
optional: true
- name: ARGOCD_MAX_COOKIE_NUMBER
valueFrom:
configMapKeyRef:
name: argocd-cmd-params-cm
key: server.http.cookie.maxnumber
optional: true
volumeMounts:
- name: ssh-known-hosts
mountPath: /app/config/ssh
Expand Down
6 changes: 6 additions & 0 deletions manifests/ha/install.yaml
Expand Up @@ -4149,6 +4149,12 @@ spec:
key: server.default.cache.expiration
name: argocd-cmd-params-cm
optional: true
- name: ARGOCD_MAX_COOKIE_NUMBER
valueFrom:
configMapKeyRef:
key: server.http.cookie.maxnumber
name: argocd-cmd-params-cm
optional: true
image: quay.io/argoproj/argocd:latest
imagePullPolicy: Always
livenessProbe:
Expand Down
6 changes: 6 additions & 0 deletions manifests/ha/namespace-install.yaml
Expand Up @@ -1536,6 +1536,12 @@ spec:
key: server.default.cache.expiration
name: argocd-cmd-params-cm
optional: true
- name: ARGOCD_MAX_COOKIE_NUMBER
valueFrom:
configMapKeyRef:
key: server.http.cookie.maxnumber
name: argocd-cmd-params-cm
optional: true
image: quay.io/argoproj/argocd:latest
imagePullPolicy: Always
livenessProbe:
Expand Down
6 changes: 6 additions & 0 deletions manifests/install.yaml
Expand Up @@ -3479,6 +3479,12 @@ spec:
key: server.default.cache.expiration
name: argocd-cmd-params-cm
optional: true
- name: ARGOCD_MAX_COOKIE_NUMBER
valueFrom:
configMapKeyRef:
key: server.http.cookie.maxnumber
name: argocd-cmd-params-cm
optional: true
image: quay.io/argoproj/argocd:latest
imagePullPolicy: Always
livenessProbe:
Expand Down
6 changes: 6 additions & 0 deletions manifests/namespace-install.yaml
Expand Up @@ -866,6 +866,12 @@ spec:
key: server.default.cache.expiration
name: argocd-cmd-params-cm
optional: true
- name: ARGOCD_MAX_COOKIE_NUMBER
valueFrom:
configMapKeyRef:
key: server.http.cookie.maxnumber
name: argocd-cmd-params-cm
optional: true
image: quay.io/argoproj/argocd:latest
imagePullPolicy: Always
livenessProbe:
Expand Down
11 changes: 8 additions & 3 deletions util/http/http.go
Expand Up @@ -8,13 +8,18 @@ import (
"strconv"
"strings"

"github.com/argoproj/argo-cd/v2/util/env"

"github.com/argoproj/argo-cd/v2/common"

log "github.com/sirupsen/logrus"
)

const maxCookieLength = 4093

// max number of chunks a cookie can be broken into. To be compatible with
// widest range of browsers, we shouldn't create more than 30 cookies per domain
const maxCookieNumber = 5
const maxCookieLength = 4093
var maxCookieNumber = env.ParseNumFromEnv(common.EnvMaxCookieNumber, 10, 0, 30)

// MakeCookieMetadata generates a string representing a Web cookie. Yum!
func MakeCookieMetadata(key, value string, flags ...string) ([]string, error) {
Expand All @@ -24,7 +29,7 @@ func MakeCookieMetadata(key, value string, flags ...string) ([]string, error) {
maxValueLength := maxCookieValueLength(key, attributes)
numberOfCookies := int(math.Ceil(float64(len(value)) / float64(maxValueLength)))
if numberOfCookies > maxCookieNumber {
return nil, fmt.Errorf("invalid cookie value, at %d long it is longer than the max length of %d", len(value), maxValueLength*maxCookieNumber)
return nil, fmt.Errorf("the authentication token is %d characters long and requires %d cookies but the max number of cookies is %d. Contact your Argo CD administrator to increase the max number of cookies", len(value), numberOfCookies, maxCookieNumber)
}

return splitCookie(key, value, attributes), nil
Expand Down
2 changes: 1 addition & 1 deletion util/http/http_test.go
Expand Up @@ -15,7 +15,7 @@ func TestCookieMaxLength(t *testing.T) {

// keys will be of format foo, foo-1, foo-2 ..
cookies, err = MakeCookieMetadata("foo", strings.Repeat("_", (maxCookieLength-5)*maxCookieNumber))
assert.EqualError(t, err, "invalid cookie value, at 20440 long it is longer than the max length of 20435")
assert.EqualError(t, err, "the authentication token is 40880 characters long and requires 11 cookies but the max number of cookies is 10. Contact your Argo CD administrator to increase the max number of cookies")
assert.Equal(t, 0, len(cookies))
}

Expand Down

0 comments on commit aa6aed3

Please sign in to comment.