Compare commits

...

5 Commits

Author SHA1 Message Date
argo-bot
38755a4c1e Bump version to 2.2.9 2022-05-18 11:55:29 +00:00
argo-bot
1c559fd7ba Bump version to 2.2.9 2022-05-18 11:55:16 +00:00
jannfis
0fc0d10a4e Merge pull request from GHSA-r642-gv9p-2wjj
Signed-off-by: jannfis <jann@mistrust.net>

Co-authored-by: Michael Crenshaw <michael@crenshaw.dev>

Co-authored-by: Michael Crenshaw <michael@crenshaw.dev>
2022-05-18 13:16:21 +02:00
Michael Crenshaw
5e767a4b9e Merge pull request from GHSA-6gcg-hp2x-q54h
* fix: do not allow symlinks from directory-type applications

Signed-off-by: Michael Crenshaw <michael@crenshaw.dev>

* chore: fix imports and unnecessary parameters

Signed-off-by: Michael Crenshaw <michael@crenshaw.dev>

* chore: lint

Signed-off-by: Michael Crenshaw <michael@crenshaw.dev>

* chore: use t.TempDir for simpler tests

Signed-off-by: Michael Crenshaw <michael@crenshaw.dev>

* address comments

Signed-off-by: Michael Crenshaw <michael@crenshaw.dev>
2022-05-18 13:13:41 +02:00
jannfis
5cee8f84e3 Merge pull request from GHSA-xmg8-99r8-jc2j
Signed-off-by: Michael Crenshaw <michael@crenshaw.dev>

Co-authored-by: Michael Crenshaw <michael@crenshaw.dev>
2022-05-18 13:06:31 +02:00
19 changed files with 644 additions and 65 deletions

View File

@@ -1 +1 @@
2.2.8
2.2.9

View File

@@ -5,7 +5,7 @@ kind: Kustomization
images:
- name: quay.io/argoproj/argocd
newName: quay.io/argoproj/argocd
newTag: v2.2.8
newTag: v2.2.9
resources:
- ./application-controller
- ./dex

View File

@@ -3018,7 +3018,7 @@ spec:
value: /helm-working-dir
- name: HELM_DATA_HOME
value: /helm-working-dir
image: quay.io/argoproj/argocd:v2.2.8
image: quay.io/argoproj/argocd:v2.2.9
imagePullPolicy: Always
livenessProbe:
failureThreshold: 3
@@ -3067,7 +3067,7 @@ spec:
- -n
- /usr/local/bin/argocd
- /var/run/argocd/argocd-cmp-server
image: quay.io/argoproj/argocd:v2.2.8
image: quay.io/argoproj/argocd:v2.2.9
name: copyutil
volumeMounts:
- mountPath: /var/run/argocd
@@ -3232,7 +3232,7 @@ spec:
key: controller.default.cache.expiration
name: argocd-cmd-params-cm
optional: true
image: quay.io/argoproj/argocd:v2.2.8
image: quay.io/argoproj/argocd:v2.2.9
imagePullPolicy: Always
livenessProbe:
httpGet:

View File

@@ -11,4 +11,4 @@ resources:
images:
- name: quay.io/argoproj/argocd
newName: quay.io/argoproj/argocd
newTag: v2.2.8
newTag: v2.2.9

View File

@@ -11,7 +11,7 @@ patchesStrategicMerge:
images:
- name: quay.io/argoproj/argocd
newName: quay.io/argoproj/argocd
newTag: v2.2.8
newTag: v2.2.9
resources:
- ../../base/application-controller
- ../../base/dex

View File

@@ -3709,7 +3709,7 @@ spec:
- -n
- /usr/local/bin/argocd
- /shared/argocd-dex
image: quay.io/argoproj/argocd:v2.2.8
image: quay.io/argoproj/argocd:v2.2.9
imagePullPolicy: Always
name: copyutil
volumeMounts:
@@ -3926,7 +3926,7 @@ spec:
value: /helm-working-dir
- name: HELM_DATA_HOME
value: /helm-working-dir
image: quay.io/argoproj/argocd:v2.2.8
image: quay.io/argoproj/argocd:v2.2.9
imagePullPolicy: Always
livenessProbe:
failureThreshold: 3
@@ -3975,7 +3975,7 @@ spec:
- -n
- /usr/local/bin/argocd
- /var/run/argocd/argocd-cmp-server
image: quay.io/argoproj/argocd:v2.2.8
image: quay.io/argoproj/argocd:v2.2.9
name: copyutil
volumeMounts:
- mountPath: /var/run/argocd
@@ -4202,7 +4202,7 @@ spec:
key: server.http.cookie.maxnumber
name: argocd-cmd-params-cm
optional: true
image: quay.io/argoproj/argocd:v2.2.8
image: quay.io/argoproj/argocd:v2.2.9
imagePullPolicy: Always
livenessProbe:
httpGet:
@@ -4398,7 +4398,7 @@ spec:
key: controller.default.cache.expiration
name: argocd-cmd-params-cm
optional: true
image: quay.io/argoproj/argocd:v2.2.8
image: quay.io/argoproj/argocd:v2.2.9
imagePullPolicy: Always
livenessProbe:
httpGet:

View File

@@ -1068,7 +1068,7 @@ spec:
- -n
- /usr/local/bin/argocd
- /shared/argocd-dex
image: quay.io/argoproj/argocd:v2.2.8
image: quay.io/argoproj/argocd:v2.2.9
imagePullPolicy: Always
name: copyutil
volumeMounts:
@@ -1285,7 +1285,7 @@ spec:
value: /helm-working-dir
- name: HELM_DATA_HOME
value: /helm-working-dir
image: quay.io/argoproj/argocd:v2.2.8
image: quay.io/argoproj/argocd:v2.2.9
imagePullPolicy: Always
livenessProbe:
failureThreshold: 3
@@ -1334,7 +1334,7 @@ spec:
- -n
- /usr/local/bin/argocd
- /var/run/argocd/argocd-cmp-server
image: quay.io/argoproj/argocd:v2.2.8
image: quay.io/argoproj/argocd:v2.2.9
name: copyutil
volumeMounts:
- mountPath: /var/run/argocd
@@ -1561,7 +1561,7 @@ spec:
key: server.http.cookie.maxnumber
name: argocd-cmd-params-cm
optional: true
image: quay.io/argoproj/argocd:v2.2.8
image: quay.io/argoproj/argocd:v2.2.9
imagePullPolicy: Always
livenessProbe:
httpGet:
@@ -1757,7 +1757,7 @@ spec:
key: controller.default.cache.expiration
name: argocd-cmd-params-cm
optional: true
image: quay.io/argoproj/argocd:v2.2.8
image: quay.io/argoproj/argocd:v2.2.9
imagePullPolicy: Always
livenessProbe:
httpGet:

View File

@@ -3079,7 +3079,7 @@ spec:
- -n
- /usr/local/bin/argocd
- /shared/argocd-dex
image: quay.io/argoproj/argocd:v2.2.8
image: quay.io/argoproj/argocd:v2.2.9
imagePullPolicy: Always
name: copyutil
volumeMounts:
@@ -3260,7 +3260,7 @@ spec:
value: /helm-working-dir
- name: HELM_DATA_HOME
value: /helm-working-dir
image: quay.io/argoproj/argocd:v2.2.8
image: quay.io/argoproj/argocd:v2.2.9
imagePullPolicy: Always
livenessProbe:
failureThreshold: 3
@@ -3309,7 +3309,7 @@ spec:
- -n
- /usr/local/bin/argocd
- /var/run/argocd/argocd-cmp-server
image: quay.io/argoproj/argocd:v2.2.8
image: quay.io/argoproj/argocd:v2.2.9
name: copyutil
volumeMounts:
- mountPath: /var/run/argocd
@@ -3532,7 +3532,7 @@ spec:
key: server.http.cookie.maxnumber
name: argocd-cmd-params-cm
optional: true
image: quay.io/argoproj/argocd:v2.2.8
image: quay.io/argoproj/argocd:v2.2.9
imagePullPolicy: Always
livenessProbe:
httpGet:
@@ -3722,7 +3722,7 @@ spec:
key: controller.default.cache.expiration
name: argocd-cmd-params-cm
optional: true
image: quay.io/argoproj/argocd:v2.2.8
image: quay.io/argoproj/argocd:v2.2.9
imagePullPolicy: Always
livenessProbe:
httpGet:

View File

@@ -438,7 +438,7 @@ spec:
- -n
- /usr/local/bin/argocd
- /shared/argocd-dex
image: quay.io/argoproj/argocd:v2.2.8
image: quay.io/argoproj/argocd:v2.2.9
imagePullPolicy: Always
name: copyutil
volumeMounts:
@@ -619,7 +619,7 @@ spec:
value: /helm-working-dir
- name: HELM_DATA_HOME
value: /helm-working-dir
image: quay.io/argoproj/argocd:v2.2.8
image: quay.io/argoproj/argocd:v2.2.9
imagePullPolicy: Always
livenessProbe:
failureThreshold: 3
@@ -668,7 +668,7 @@ spec:
- -n
- /usr/local/bin/argocd
- /var/run/argocd/argocd-cmp-server
image: quay.io/argoproj/argocd:v2.2.8
image: quay.io/argoproj/argocd:v2.2.9
name: copyutil
volumeMounts:
- mountPath: /var/run/argocd
@@ -891,7 +891,7 @@ spec:
key: server.http.cookie.maxnumber
name: argocd-cmd-params-cm
optional: true
image: quay.io/argoproj/argocd:v2.2.8
image: quay.io/argoproj/argocd:v2.2.9
imagePullPolicy: Always
livenessProbe:
httpGet:
@@ -1081,7 +1081,7 @@ spec:
key: controller.default.cache.expiration
name: argocd-cmd-params-cm
optional: true
image: quay.io/argoproj/argocd:v2.2.8
image: quay.io/argoproj/argocd:v2.2.9
imagePullPolicy: Always
livenessProbe:
httpGet:

View File

@@ -16,6 +16,8 @@ import (
"strings"
"time"
"github.com/argoproj/argo-cd/v2/util/io/files"
"github.com/argoproj/argo-cd/v2/util/argo"
"github.com/Masterminds/semver"
@@ -752,7 +754,8 @@ func GenerateManifests(appPath, repoRoot, revision string, q *apiclient.Manifest
if directory = q.ApplicationSource.Directory; directory == nil {
directory = &v1alpha1.ApplicationSourceDirectory{}
}
targetObjs, err = findManifests(appPath, repoRoot, env, *directory)
logCtx := log.WithField("application", q.AppName)
targetObjs, err = findManifests(logCtx, appPath, repoRoot, env, *directory)
}
if err != nil {
return nil, err
@@ -950,12 +953,32 @@ func ksShow(appLabelKey, appPath string, ksonnetOpts *v1alpha1.ApplicationSource
var manifestFile = regexp.MustCompile(`^.*\.(yaml|yml|json|jsonnet)$`)
// findManifests looks at all yaml files in a directory and unmarshals them into a list of unstructured objects
func findManifests(appPath string, repoRoot string, env *v1alpha1.Env, directory v1alpha1.ApplicationSourceDirectory) ([]*unstructured.Unstructured, error) {
func findManifests(logCtx *log.Entry, appPath string, repoRoot string, env *v1alpha1.Env, directory v1alpha1.ApplicationSourceDirectory) ([]*unstructured.Unstructured, error) {
var objs []*unstructured.Unstructured
err := filepath.Walk(appPath, func(path string, f os.FileInfo, err error) error {
if err != nil {
return err
}
relPath, err := filepath.Rel(appPath, path)
if err != nil {
return fmt.Errorf("failed to get relative path of symlink: %w", err)
}
if files.IsSymlink(f) {
realPath, err := filepath.EvalSymlinks(path)
if err != nil {
logCtx.Debugf("error checking symlink realpath: %s", err)
if os.IsNotExist(err) {
log.Warnf("ignoring out-of-bounds symlink at %q: %s", relPath, err)
return nil
} else {
return fmt.Errorf("failed to evaluate symlink at %q: %w", relPath, err)
}
}
if !files.Inbound(realPath, appPath) {
logCtx.Warnf("illegal filepath in symlink: %s", realPath)
return fmt.Errorf("illegal filepath in symlink at %q", relPath)
}
}
if f.IsDir() {
if path != appPath && !directory.Recurse {
return filepath.SkipDir
@@ -968,10 +991,6 @@ func findManifests(appPath string, repoRoot string, env *v1alpha1.Env, directory
return nil
}
relPath, err := filepath.Rel(appPath, path)
if err != nil {
return err
}
if directory.Exclude != "" && glob.Match(directory.Exclude, relPath) {
return nil
}

View File

@@ -8,13 +8,14 @@ import (
"io/ioutil"
"os"
"os/exec"
"path"
"path/filepath"
"regexp"
"strings"
"testing"
"time"
"github.com/argoproj/argo-cd/v2/util/argo"
log "github.com/sirupsen/logrus"
"github.com/ghodss/yaml"
"github.com/stretchr/testify/assert"
@@ -29,6 +30,7 @@ import (
"github.com/argoproj/argo-cd/v2/reposerver/cache"
"github.com/argoproj/argo-cd/v2/reposerver/metrics"
fileutil "github.com/argoproj/argo-cd/v2/test/fixture/path"
"github.com/argoproj/argo-cd/v2/util/argo"
cacheutil "github.com/argoproj/argo-cd/v2/util/cache"
"github.com/argoproj/argo-cd/v2/util/git"
gitmocks "github.com/argoproj/argo-cd/v2/util/git/mocks"
@@ -52,7 +54,7 @@ func newServiceWithMocks(root string, signed bool) (*Service, *gitmocks.Client)
return newServiceWithOpt(func(gitClient *gitmocks.Client) {
gitClient.On("Init").Return(nil)
gitClient.On("Fetch", mock.Anything).Return(nil)
gitClient.On("Checkout", mock.Anything).Return(nil)
gitClient.On("Checkout", mock.Anything, mock.Anything).Return(nil)
gitClient.On("LsRemote", mock.Anything).Return(mock.Anything, nil)
gitClient.On("CommitSHA").Return(mock.Anything, nil)
gitClient.On("Root").Return(root)
@@ -81,7 +83,6 @@ func newServiceWithOpt(cf clientFunc) (*Service, *gitmocks.Client) {
}}, nil)
helmClient.On("ExtractChart", chart, version).Return("./testdata/my-chart", io.NopCloser, nil)
helmClient.On("CleanChartCache", chart, version).Return(nil)
service.newGitClient = func(rawRepoURL string, creds git.Creds, insecure bool, enableLfs bool, prosy string, opts ...git.ClientOpts) (client git.Client, e error) {
return gitClient, nil
}
@@ -112,7 +113,7 @@ func newServiceWithCommitSHA(root, revision string) *Service {
service, gitClient := newServiceWithOpt(func(gitClient *gitmocks.Client) {
gitClient.On("Init").Return(nil)
gitClient.On("Fetch", mock.Anything).Return(nil)
gitClient.On("Checkout", mock.Anything).Return(nil)
gitClient.On("Checkout", mock.Anything, mock.Anything).Return(nil)
gitClient.On("LsRemote", revision).Return(revision, revisionErr)
gitClient.On("CommitSHA").Return("632039659e542ed7de0c170a4fcc1c571b288fc0", nil)
gitClient.On("Root").Return(root)
@@ -132,7 +133,7 @@ func TestGenerateYamlManifestInDir(t *testing.T) {
q := apiclient.ManifestRequest{Repo: &argoappv1.Repository{}, ApplicationSource: &src}
// update this value if we add/remove manifests
const countOfManifests = 34
const countOfManifests = 41
res1, err := service.GenerateManifest(context.Background(), &q)
@@ -145,6 +146,76 @@ func TestGenerateYamlManifestInDir(t *testing.T) {
assert.Equal(t, 3, len(res2.Manifests))
}
func Test_GenerateManifests_NoOutOfBoundsAccess(t *testing.T) {
testCases := []struct {
name string
outOfBoundsFilename string
outOfBoundsFileContents string
mustNotContain string // Optional string that must not appear in error or manifest output. If empty, use outOfBoundsFileContents.
}{
{
name: "out of bounds JSON file should not appear in error output",
outOfBoundsFilename: "test.json",
outOfBoundsFileContents: `{"some": "json"}`,
},
{
name: "malformed JSON file contents should not appear in error output",
outOfBoundsFilename: "test.json",
outOfBoundsFileContents: "$",
},
{
name: "out of bounds JSON manifest should not appear in manifest output",
outOfBoundsFilename: "test.json",
// JSON marshalling is deterministic. So if there's a leak, exactly this should appear in the manifests.
outOfBoundsFileContents: `{"apiVersion":"v1","kind":"Secret","metadata":{"name":"test","namespace":"default"},"type":"Opaque"}`,
},
{
name: "out of bounds YAML manifest should not appear in manifest output",
outOfBoundsFilename: "test.yaml",
outOfBoundsFileContents: "apiVersion: v1\nkind: Secret\nmetadata:\n name: test\n namespace: default\ntype: Opaque",
mustNotContain: `{"apiVersion":"v1","kind":"Secret","metadata":{"name":"test","namespace":"default"},"type":"Opaque"}`,
},
}
for _, testCase := range testCases {
testCaseCopy := testCase
t.Run(testCaseCopy.name, func(t *testing.T) {
t.Parallel()
outOfBoundsDir := t.TempDir()
outOfBoundsFile := path.Join(outOfBoundsDir, testCaseCopy.outOfBoundsFilename)
err := os.WriteFile(outOfBoundsFile, []byte(testCaseCopy.outOfBoundsFileContents), os.FileMode(0444))
require.NoError(t, err)
repoDir := t.TempDir()
err = os.Symlink(outOfBoundsFile, path.Join(repoDir, testCaseCopy.outOfBoundsFilename))
require.NoError(t, err)
var mustNotContain = testCaseCopy.outOfBoundsFileContents
if testCaseCopy.mustNotContain != "" {
mustNotContain = testCaseCopy.mustNotContain
}
q := apiclient.ManifestRequest{Repo: &argoappv1.Repository{}, ApplicationSource: &argoappv1.ApplicationSource{}}
res, err := GenerateManifests(repoDir, "", "", &q, false)
require.Error(t, err)
assert.NotContains(t, err.Error(), mustNotContain)
assert.Contains(t, err.Error(), "illegal filepath")
assert.Nil(t, res)
})
}
}
func TestGenerateManifests_MissingSymlinkDestination(t *testing.T) {
repoDir := t.TempDir()
err := os.Symlink("/obviously/does/not/exist", path.Join(repoDir, "test.yaml"))
require.NoError(t, err)
q := apiclient.ManifestRequest{Repo: &argoappv1.Repository{}, ApplicationSource: &argoappv1.ApplicationSource{}}
_, err = GenerateManifests(repoDir, "", "", &q, false)
require.NoError(t, err)
}
func TestGenerateManifests_K8SAPIResetCache(t *testing.T) {
service := newService("../..")
@@ -1612,7 +1683,7 @@ func TestFindResources(t *testing.T) {
for i := range testCases {
tc := testCases[i]
t.Run(tc.name, func(t *testing.T) {
objs, err := findManifests("testdata/app-include-exclude", ".", nil, argoappv1.ApplicationSourceDirectory{
objs, err := findManifests(&log.Entry{}, "testdata/app-include-exclude", ".", nil, argoappv1.ApplicationSourceDirectory{
Recurse: true,
Include: tc.include,
Exclude: tc.exclude,
@@ -1630,7 +1701,7 @@ func TestFindResources(t *testing.T) {
}
func TestFindManifests_Exclude(t *testing.T) {
objs, err := findManifests("testdata/app-include-exclude", ".", nil, argoappv1.ApplicationSourceDirectory{
objs, err := findManifests(&log.Entry{}, "testdata/app-include-exclude", ".", nil, argoappv1.ApplicationSourceDirectory{
Recurse: true,
Exclude: "subdir/deploymentSub.yaml",
})
@@ -1643,7 +1714,7 @@ func TestFindManifests_Exclude(t *testing.T) {
}
func TestFindManifests_Exclude_NothingMatches(t *testing.T) {
objs, err := findManifests("testdata/app-include-exclude", ".", nil, argoappv1.ApplicationSourceDirectory{
objs, err := findManifests(&log.Entry{}, "testdata/app-include-exclude", ".", nil, argoappv1.ApplicationSourceDirectory{
Recurse: true,
Exclude: "nothing.yaml",
})

View File

@@ -949,6 +949,8 @@ func (a *ArgoCDServer) Authenticate(ctx context.Context) (context.Context, error
}
if !argoCDSettings.AnonymousUserEnabled {
return ctx, claimsErr
} else {
ctx = context.WithValue(ctx, "claims", "")
}
}

View File

@@ -3,6 +3,8 @@ package server
import (
"context"
"fmt"
"io"
"net/http"
"net/http/httptest"
"strings"
"testing"
@@ -11,6 +13,7 @@ import (
"github.com/dgrijalva/jwt-go/v4"
log "github.com/sirupsen/logrus"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
"google.golang.org/grpc/metadata"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/client-go/kubernetes/fake"
@@ -430,6 +433,386 @@ func TestAuthenticate(t *testing.T) {
}
}
func dexMockHandler(t *testing.T, url string) func(http.ResponseWriter, *http.Request) {
return func(w http.ResponseWriter, r *http.Request) {
w.Header().Set("Content-Type", "application/json")
switch r.RequestURI {
case "/api/dex/.well-known/openid-configuration":
_, err := io.WriteString(w, fmt.Sprintf(`
{
"issuer": "%[1]s/api/dex",
"authorization_endpoint": "%[1]s/api/dex/auth",
"token_endpoint": "%[1]s/api/dex/token",
"jwks_uri": "%[1]s/api/dex/keys",
"userinfo_endpoint": "%[1]s/api/dex/userinfo",
"device_authorization_endpoint": "%[1]s/api/dex/device/code",
"grant_types_supported": [
"authorization_code",
"refresh_token",
"urn:ietf:params:oauth:grant-type:device_code"
],
"response_types_supported": [
"code"
],
"subject_types_supported": [
"public"
],
"id_token_signing_alg_values_supported": [
"RS256", "HS256"
],
"code_challenge_methods_supported": [
"S256",
"plain"
],
"scopes_supported": [
"openid",
"email",
"groups",
"profile",
"offline_access"
],
"token_endpoint_auth_methods_supported": [
"client_secret_basic",
"client_secret_post"
],
"claims_supported": [
"iss",
"sub",
"aud",
"iat",
"exp",
"email",
"email_verified",
"locale",
"name",
"preferred_username",
"at_hash"
]
}`, url))
if err != nil {
t.Fail()
}
default:
w.WriteHeader(404)
}
}
}
func getTestServer(t *testing.T, anonymousEnabled bool, withFakeSSO bool) (argocd *ArgoCDServer, dexURL string) {
cm := test.NewFakeConfigMap()
if anonymousEnabled {
cm.Data["users.anonymous.enabled"] = "true"
}
ts := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
return // Start with a placeholder. We need the server URL before setting up the real handler.
}))
ts.Config.Handler = http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
dexMockHandler(t, ts.URL)(w, r)
})
if withFakeSSO {
cm.Data["url"] = ts.URL
cm.Data["dex.config"] = `
connectors:
# OIDC
- type: OIDC
id: oidc
name: OIDC
config:
issuer: https://auth.example.gom
clientID: test-client
clientSecret: $dex.oidc.clientSecret`
}
secret := test.NewFakeSecret()
kubeclientset := fake.NewSimpleClientset(cm, secret)
appClientSet := apps.NewSimpleClientset()
argoCDOpts := ArgoCDServerOpts{
Namespace: test.FakeArgoCDNamespace,
KubeClientset: kubeclientset,
AppClientset: appClientSet,
}
if withFakeSSO {
argoCDOpts.DexServerAddr = ts.URL
}
argocd = NewServer(context.Background(), argoCDOpts)
return argocd, ts.URL
}
func TestAuthenticate_3rd_party_JWTs(t *testing.T) {
type testData struct {
test string
anonymousEnabled bool
claims jwt.StandardClaims
expectedErrorContains string
expectedClaims interface{}
}
var tests = []testData{
{
test: "anonymous disabled, no audience",
anonymousEnabled: false,
claims: jwt.StandardClaims{},
expectedErrorContains: "no audience found in the token",
expectedClaims: nil,
},
{
test: "anonymous enabled, no audience",
anonymousEnabled: true,
claims: jwt.StandardClaims{},
expectedErrorContains: "",
expectedClaims: "",
},
{
test: "anonymous disabled, unexpired token, admin claim",
anonymousEnabled: false,
claims: jwt.StandardClaims{Audience: jwt.ClaimStrings{"test-client"}, Subject: "admin", ExpiresAt: jwt.NewTime(float64(time.Now().Add(time.Hour * 24).Unix()))},
expectedErrorContains: "id token signed with unsupported algorithm",
expectedClaims: nil,
},
{
test: "anonymous enabled, unexpired token, admin claim",
anonymousEnabled: true,
claims: jwt.StandardClaims{Audience: jwt.ClaimStrings{"test-client"}, Subject: "admin", ExpiresAt: jwt.NewTime(float64(time.Now().Add(time.Hour * 24).Unix()))},
expectedErrorContains: "",
expectedClaims: "",
},
{
test: "anonymous disabled, expired token, admin claim",
anonymousEnabled: false,
claims: jwt.StandardClaims{Audience: jwt.ClaimStrings{"test-client"}, Subject: "admin", ExpiresAt: jwt.NewTime(float64(time.Now().Unix()))},
expectedErrorContains: "token is expired",
expectedClaims: jwt.StandardClaims{Issuer: "sso"},
},
{
test: "anonymous enabled, expired token, admin claim",
anonymousEnabled: true,
claims: jwt.StandardClaims{Audience: jwt.ClaimStrings{"test-client"}, Subject: "admin", ExpiresAt: jwt.NewTime(float64(time.Now().Unix()))},
expectedErrorContains: "",
expectedClaims: "",
},
}
for _, testData := range tests {
testDataCopy := testData
t.Run(testDataCopy.test, func(t *testing.T) {
t.Parallel()
argocd, dexURL := getTestServer(t, testDataCopy.anonymousEnabled, true)
ctx := context.Background()
testDataCopy.claims.Issuer = fmt.Sprintf("%s/api/dex", dexURL)
token := jwt.NewWithClaims(jwt.SigningMethodHS256, testDataCopy.claims)
tokenString, err := token.SignedString([]byte("key"))
require.NoError(t, err)
ctx = metadata.NewIncomingContext(context.Background(), metadata.Pairs(apiclient.MetaDataTokenKey, tokenString))
ctx, err = argocd.Authenticate(ctx)
claims := ctx.Value("claims")
if testDataCopy.expectedClaims == nil {
assert.Nil(t, claims)
} else {
assert.Equal(t, testDataCopy.expectedClaims, claims)
}
if testDataCopy.expectedErrorContains != "" {
assert.Contains(t, err.Error(), testDataCopy.expectedErrorContains, "Authenticate should have thrown an error and blocked the request")
} else {
assert.NoError(t, err)
}
})
}
}
func TestAuthenticate_no_request_metadata(t *testing.T) {
type testData struct {
test string
anonymousEnabled bool
expectedErrorContains string
expectedClaims interface{}
}
var tests = []testData{
{
test: "anonymous disabled",
anonymousEnabled: false,
expectedErrorContains: "no session information",
expectedClaims: nil,
},
{
test: "anonymous enabled",
anonymousEnabled: true,
expectedErrorContains: "",
expectedClaims: "",
},
}
for _, testData := range tests {
testDataCopy := testData
t.Run(testDataCopy.test, func(t *testing.T) {
t.Parallel()
argocd, _ := getTestServer(t, testDataCopy.anonymousEnabled, true)
ctx := context.Background()
ctx, err := argocd.Authenticate(ctx)
claims := ctx.Value("claims")
assert.Equal(t, testDataCopy.expectedClaims, claims)
if testDataCopy.expectedErrorContains != "" {
assert.Contains(t, err.Error(), testDataCopy.expectedErrorContains, "Authenticate should have thrown an error and blocked the request")
} else {
assert.NoError(t, err)
}
})
}
}
func TestAuthenticate_no_SSO(t *testing.T) {
type testData struct {
test string
anonymousEnabled bool
expectedErrorMessage string
expectedClaims interface{}
}
var tests = []testData{
{
test: "anonymous disabled",
anonymousEnabled: false,
expectedErrorMessage: "SSO is not configured",
expectedClaims: nil,
},
{
test: "anonymous enabled",
anonymousEnabled: true,
expectedErrorMessage: "",
expectedClaims: "",
},
}
for _, testData := range tests {
testDataCopy := testData
t.Run(testDataCopy.test, func(t *testing.T) {
t.Parallel()
argocd, dexURL := getTestServer(t, testDataCopy.anonymousEnabled, false)
ctx := context.Background()
token := jwt.NewWithClaims(jwt.SigningMethodHS256, jwt.StandardClaims{Issuer: fmt.Sprintf("%s/api/dex", dexURL)})
tokenString, err := token.SignedString([]byte("key"))
require.NoError(t, err)
ctx = metadata.NewIncomingContext(context.Background(), metadata.Pairs(apiclient.MetaDataTokenKey, tokenString))
ctx, err = argocd.Authenticate(ctx)
claims := ctx.Value("claims")
assert.Equal(t, testDataCopy.expectedClaims, claims)
if testDataCopy.expectedErrorMessage != "" {
assert.Contains(t, err.Error(), testDataCopy.expectedErrorMessage, "Authenticate should have thrown an error and blocked the request")
} else {
assert.NoError(t, err)
}
})
}
}
func TestAuthenticate_bad_request_metadata(t *testing.T) {
type testData struct {
test string
anonymousEnabled bool
metadata metadata.MD
expectedErrorMessage string
expectedClaims interface{}
}
var tests = []testData{
{
test: "anonymous disabled, empty metadata",
anonymousEnabled: false,
metadata: metadata.MD{},
expectedErrorMessage: "no session information",
expectedClaims: nil,
},
{
test: "anonymous enabled, empty metadata",
anonymousEnabled: true,
metadata: metadata.MD{},
expectedErrorMessage: "",
expectedClaims: "",
},
{
test: "anonymous disabled, empty tokens",
anonymousEnabled: false,
metadata: metadata.MD{apiclient.MetaDataTokenKey: []string{}},
expectedErrorMessage: "no session information",
expectedClaims: nil,
},
{
test: "anonymous enabled, empty tokens",
anonymousEnabled: true,
metadata: metadata.MD{apiclient.MetaDataTokenKey: []string{}},
expectedErrorMessage: "",
expectedClaims: "",
},
{
test: "anonymous disabled, bad tokens",
anonymousEnabled: false,
metadata: metadata.Pairs(apiclient.MetaDataTokenKey, "bad"),
expectedErrorMessage: "token contains an invalid number of segments",
expectedClaims: nil,
},
{
test: "anonymous enabled, bad tokens",
anonymousEnabled: true,
metadata: metadata.Pairs(apiclient.MetaDataTokenKey, "bad"),
expectedErrorMessage: "",
expectedClaims: "",
},
{
test: "anonymous disabled, bad auth header",
anonymousEnabled: false,
metadata: metadata.MD{"authorization": []string{"Bearer eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJzdWIiOiJhZG1pbiJ9.TGGTTHuuGpEU8WgobXxkrBtW3NiR3dgw5LR-1DEW3BQ"}},
expectedErrorMessage: "no audience found in the token",
expectedClaims: nil,
},
{
test: "anonymous enabled, bad auth header",
anonymousEnabled: true,
metadata: metadata.MD{"authorization": []string{"Bearer eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJzdWIiOiJhZG1pbiJ9.TGGTTHuuGpEU8WgobXxkrBtW3NiR3dgw5LR-1DEW3BQ"}},
expectedErrorMessage: "",
expectedClaims: "",
},
{
test: "anonymous disabled, bad auth cookie",
anonymousEnabled: false,
metadata: metadata.MD{"grpcgateway-cookie": []string{"argocd.token=eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJzdWIiOiJhZG1pbiJ9.TGGTTHuuGpEU8WgobXxkrBtW3NiR3dgw5LR-1DEW3BQ"}},
expectedErrorMessage: "no audience found in the token",
expectedClaims: nil,
},
{
test: "anonymous enabled, bad auth cookie",
anonymousEnabled: true,
metadata: metadata.MD{"grpcgateway-cookie": []string{"argocd.token=eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJzdWIiOiJhZG1pbiJ9.TGGTTHuuGpEU8WgobXxkrBtW3NiR3dgw5LR-1DEW3BQ"}},
expectedErrorMessage: "",
expectedClaims: "",
},
}
for _, testData := range tests {
testDataCopy := testData
t.Run(testDataCopy.test, func(t *testing.T) {
t.Parallel()
argocd, _ := getTestServer(t, testDataCopy.anonymousEnabled, true)
ctx := context.Background()
ctx = metadata.NewIncomingContext(context.Background(), testDataCopy.metadata)
ctx, err := argocd.Authenticate(ctx)
claims := ctx.Value("claims")
assert.Equal(t, testDataCopy.expectedClaims, claims)
if testDataCopy.expectedErrorMessage != "" {
assert.Contains(t, err.Error(), testDataCopy.expectedErrorMessage, "Authenticate should have thrown an error and blocked the request")
} else {
assert.NoError(t, err)
}
})
}
}
func Test_getToken(t *testing.T) {
token := "eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJzdWIiOiIxMjM0NTY3ODkwIiwibmFtZSI6IkpvaG4gRG9lIiwiaWF0IjoxNTE2MjM5MDIyfQ.SflKxwRJSMeKKF2QT4fwpMeJf36POk6yJV_adQssw5c"
t.Run("Empty", func(t *testing.T) {

View File

@@ -20,7 +20,7 @@ interface State {
loginError: string;
loginInProgress: boolean;
returnUrl: string;
ssoLoginError: string;
hasSsoLoginError: boolean;
}
export class Login extends React.Component<RouteComponentProps<{}>, State> {
@@ -31,13 +31,13 @@ export class Login extends React.Component<RouteComponentProps<{}>, State> {
public static getDerivedStateFromProps(props: RouteComponentProps<{}>): Partial<State> {
const search = new URLSearchParams(props.history.location.search);
const returnUrl = search.get('return_url') || '';
const ssoLoginError = search.get('sso_error') || '';
return {ssoLoginError, returnUrl};
const hasSsoLoginError = search.get('has_sso_error') === 'true';
return {hasSsoLoginError, returnUrl};
}
constructor(props: RouteComponentProps<{}>) {
super(props);
this.state = {authSettings: null, loginError: null, returnUrl: null, ssoLoginError: null, loginInProgress: false};
this.state = {authSettings: null, loginError: null, returnUrl: null, hasSsoLoginError: false, loginInProgress: false};
}
public async componentDidMount() {
@@ -69,7 +69,7 @@ export class Login extends React.Component<RouteComponentProps<{}>, State> {
)}
</button>
</a>
{this.state.ssoLoginError && <div className='argo-form-row__error-msg'>{this.state.ssoLoginError}</div>}
{this.state.hasSsoLoginError && <div className='argo-form-row__error-msg'>Login failed.</div>}
{authSettings && !authSettings.userLoginsDisabled && (
<div className='login__saml-separator'>
<span>or</span>

View File

@@ -3,20 +3,18 @@ package dex
import (
"bytes"
"fmt"
"html"
"io/ioutil"
"net/http"
"net/http/httputil"
"net/url"
"path"
"regexp"
"strconv"
log "github.com/sirupsen/logrus"
"github.com/argoproj/argo-cd/v2/util/errors"
)
var messageRe = regexp.MustCompile(`<p>(.*)([\s\S]*?)<\/p>`)
func decorateDirector(director func(req *http.Request), target *url.URL) func(req *http.Request) {
return func(req *http.Request) {
director(req)
@@ -44,16 +42,10 @@ func NewDexHTTPReverseProxy(serverAddr string, baseHRef string) func(writer http
if err != nil {
return err
}
var message string
matches := messageRe.FindSubmatch(b)
if len(matches) > 1 {
message = html.UnescapeString(string(matches[1]))
} else {
message = "Unknown error"
}
log.Errorf("received error from dex: %s", string(b))
resp.ContentLength = 0
resp.Header.Set("Content-Length", strconv.Itoa(0))
resp.Header.Set("Location", fmt.Sprintf("%s?sso_error=%s", path.Join(baseHRef, "login"), url.QueryEscape(message)))
resp.Header.Set("Location", fmt.Sprintf("%s?has_sso_error=true", path.Join(baseHRef, "login")))
resp.StatusCode = http.StatusSeeOther
resp.Body = ioutil.NopCloser(bytes.NewReader(make([]byte, 0)))
return nil

View File

@@ -408,7 +408,7 @@ func Test_DexReverseProxy(t *testing.T) {
assert.Equal(t, http.StatusSeeOther, resp.StatusCode)
location, _ := resp.Location()
fmt.Printf("%s %s\n", resp.Status, location.RequestURI())
assert.True(t, strings.HasPrefix(location.RequestURI(), "/login?sso_error"))
assert.True(t, strings.HasPrefix(location.RequestURI(), "/login?has_sso_error=true"))
})
t.Run("Invalid URL for Dex reverse proxy", func(t *testing.T) {

35
util/io/files/util.go Normal file
View File

@@ -0,0 +1,35 @@
package files
import (
"io/fs"
"os"
"path/filepath"
"strings"
)
// Inbound will validate if the given candidate path is inside the
// baseDir. This is useful to make sure that malicious candidates
// are not targeting a file outside of baseDir boundaries.
// Considerations:
// - baseDir must be absolute path. Will return false otherwise
// - candidate can be absolute or relative path
// - candidate should not be symlink as only syntatic validation is
// applied by this function
func Inbound(candidate, baseDir string) bool {
if !filepath.IsAbs(baseDir) {
return false
}
var target string
if filepath.IsAbs(candidate) {
target = filepath.Clean(candidate)
} else {
target = filepath.Join(baseDir, candidate)
}
return strings.HasPrefix(target, filepath.Clean(baseDir)+string(os.PathSeparator))
}
// IsSymlink return true if the given FileInfo relates to a
// symlink file. Returns false otherwise.
func IsSymlink(fi os.FileInfo) bool {
return fi.Mode()&fs.ModeSymlink == fs.ModeSymlink
}

View File

@@ -0,0 +1,63 @@
package files_test
import (
"testing"
"github.com/stretchr/testify/assert"
"github.com/argoproj/argo-cd/v2/util/io/files"
)
func TestInbound(t *testing.T) {
type testcase struct {
name string
candidate string
basedir string
expected bool
}
cases := []testcase{
{
name: "will return true if candidate is inbound",
candidate: "/home/test/app/readme.md",
basedir: "/home/test",
expected: true,
},
{
name: "will return false if candidate is not inbound",
candidate: "/home/test/../readme.md",
basedir: "/home/test",
expected: false,
},
{
name: "will return true if candidate is relative inbound",
candidate: "./readme.md",
basedir: "/home/test",
expected: true,
},
{
name: "will return false if candidate is relative outbound",
candidate: "../readme.md",
basedir: "/home/test",
expected: false,
},
{
name: "will return false if basedir is relative",
candidate: "/home/test/app/readme.md",
basedir: "./test",
expected: false,
},
}
for _, c := range cases {
c := c
t.Run(c.name, func(t *testing.T) {
// given
t.Parallel()
// when
inbound := files.Inbound(c.candidate, c.basedir)
// then
assert.Equal(t, c.expected, inbound)
})
}
}

View File

@@ -246,7 +246,7 @@ func (mgr *SessionManager) Parse(tokenString string) (jwt.Claims, string, error)
token, err := jwt.ParseWithClaims(tokenString, &claims, func(token *jwt.Token) (interface{}, error) {
// Don't forget to validate the alg is what you expect:
if _, ok := token.Method.(*jwt.SigningMethodHMAC); !ok {
return nil, fmt.Errorf("Unexpected signing method: %v", token.Header["alg"])
return nil, fmt.Errorf("unexpected signing method: %v", token.Header["alg"])
}
return argoCDSettings.ServerSignature, nil
})
@@ -298,7 +298,7 @@ func (mgr *SessionManager) Parse(tokenString string) (jwt.Claims, string, error)
}
if account.PasswordMtime != nil && issuedAt.Before(*account.PasswordMtime) {
return nil, "", fmt.Errorf("Account password has changed since token issued")
return nil, "", fmt.Errorf("account password has changed since token issued")
}
newToken := ""
@@ -515,7 +515,7 @@ func (mgr *SessionManager) VerifyToken(tokenString string) (jwt.Claims, string,
// IDP signed token
prov, err := mgr.provider()
if err != nil {
return claims, "", err
return nil, "", err
}
// Token must be verified for at least one audience
@@ -527,16 +527,30 @@ func (mgr *SessionManager) VerifyToken(tokenString string) (jwt.Claims, string,
break
}
}
// The token verification has failed. If the token has expired, we will
// return a dummy claims only containing a value for the issuer, so the
// UI can handle expired tokens appropriately.
if err != nil {
return claims, "", err
if strings.HasPrefix(err.Error(), "oidc: token is expired") {
claims = jwt.StandardClaims{
Issuer: "sso",
}
return claims, "", err
}
return nil, "", err
}
if idToken == nil {
return claims, "", fmt.Errorf("No audience found in the token")
return nil, "", fmt.Errorf("no audience found in the token")
}
var claims jwt.MapClaims
err = idToken.Claims(&claims)
return claims, "", err
if err != nil {
return nil, "", err
}
return claims, "", nil
}
}