Jeff Mitchell
2c61bd4963
The existing logic was to ignore Forbidden errors when recursively listing in favor of looking for downstream scopes with appropriate permissions. The problem is that when there is actually no authentication token at all, what comes back is an Unauthenticated error. Therefore if you didn't have permissions on the ingress scope (e.g. the default of `global`) but did downstream for the anonymous user, you'd get back an error. I don't think there's any security issue with doing this; the user could always have simply started the search at a downstream scope to begin with so nothing more is being revealed, this just fixes behavior to match expectations in this case. A test was added for one of the services; the code is exactly the same across them. The reason that the existing tests that were added with the output fields testing didn't catch this is that even though they were specifying `u_anon` as the user, the disabled test context skipped the logic that would return a 401 in this case. It turns out that fixing this is very complicated because we don't have a great way in most tests of defining grants to put onto the generated scopes, so even if a true anonymous context was added each test would require the anonymous user to have list permissions on whatever resource was under test, requiring a lot of role and grant and principal finagling. This is probably a good longer-term TODO but not one that seemed worth it for this fix. Fixes ICU-2301
75 lines
2.3 KiB
Go
75 lines
2.3 KiB
Go
package cluster
|
|
|
|
import (
|
|
"strings"
|
|
"testing"
|
|
|
|
amapi "github.com/hashicorp/boundary/api/authmethods"
|
|
rolesapi "github.com/hashicorp/boundary/api/roles"
|
|
"github.com/hashicorp/boundary/internal/servers/controller"
|
|
"github.com/hashicorp/boundary/internal/types/scope"
|
|
"github.com/stretchr/testify/require"
|
|
)
|
|
|
|
// This test validates the fix for ICU-2301
|
|
func TestListAnonymousRecursing(t *testing.T) {
|
|
require := require.New(t)
|
|
tc := controller.NewTestController(t, nil)
|
|
defer tc.Shutdown()
|
|
|
|
client := tc.Client()
|
|
token := tc.Token()
|
|
client.SetToken(token.Token)
|
|
amClient := amapi.NewClient(client)
|
|
rolesClient := rolesapi.NewClient(client)
|
|
orgScopeId := "o_1234567890"
|
|
|
|
// Create an auth method in org scope for the test
|
|
am, err := amClient.Create(tc.Context(), "password", orgScopeId)
|
|
require.NoError(err)
|
|
require.NotNil(am)
|
|
|
|
// We expect to see all three with the normal token
|
|
l, err := amClient.List(tc.Context(), scope.Global.String(), amapi.WithRecursive(true))
|
|
require.NoError(err)
|
|
require.NotNil(l)
|
|
require.Len(l.GetItems().([]*amapi.AuthMethod), 3)
|
|
|
|
// Originally we also expect to see all three as anon user
|
|
amClient.ApiClient().SetToken("")
|
|
l, err = amClient.List(tc.Context(), scope.Global.String(), amapi.WithRecursive(true))
|
|
require.NoError(err)
|
|
require.NotNil(l)
|
|
require.Len(l.GetItems().([]*amapi.AuthMethod), 3)
|
|
|
|
// Find the global roles and delete them
|
|
rl, err := rolesClient.List(tc.Context(), scope.Global.String())
|
|
require.NoError(err)
|
|
require.NotNil(rl)
|
|
require.Len(rl.GetItems().([]*rolesapi.Role), 2)
|
|
|
|
// Find the non-admin one and delete that first
|
|
adminIdx := 0
|
|
defaultIdx := 1
|
|
roles := rl.GetItems().([]*rolesapi.Role)
|
|
if strings.Contains(roles[0].Name, "Default") {
|
|
adminIdx, defaultIdx = 1, 0
|
|
}
|
|
_, err = rolesClient.Delete(tc.Context(), roles[defaultIdx].Id)
|
|
require.NoError(err)
|
|
_, err = rolesClient.Delete(tc.Context(), roles[adminIdx].Id)
|
|
require.NoError(err)
|
|
|
|
// Make sure we can't list in global
|
|
_, err = amClient.List(tc.Context(), scope.Global.String())
|
|
require.Error(err)
|
|
|
|
// But we can still see 1 when recursing, from the org scope
|
|
l, err = amClient.List(tc.Context(), scope.Global.String(), amapi.WithRecursive(true))
|
|
require.NoError(err)
|
|
require.NotNil(l)
|
|
ams := l.GetItems().([]*amapi.AuthMethod)
|
|
require.Len(ams, 1)
|
|
require.Equal(orgScopeId, ams[0].ScopeId)
|
|
}
|