Skip to content

Commit 5b0cf09

Browse files
Alex Shanbwasmithwillmurphyscodetjvman
committed
Check that client exists before assigning them an org role
Previously the set-org-role command would not validate that the provided client ID actually corresponded to an existing client. Fetching a client from UAA requires the clients.read, clients.admin or zones.<zone.id>.admin scope so users who want to assign roles to clients will need to verify that they have one of those scopes if they want to use --client. [#159967411] Co-authored-by: Brendan Smith <[email protected]> Co-authored-by: Will Murphy <[email protected]> Co-authored-by: Tom Viehman <[email protected]>
1 parent 126c389 commit 5b0cf09

File tree

9 files changed

+396
-17
lines changed

9 files changed

+396
-17
lines changed

cf/api/apifakes/fake_client_repository.go

+115
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

cf/api/clients.go

+62
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,62 @@
1+
package api
2+
3+
import (
4+
"fmt"
5+
"net/http"
6+
7+
"code.cloudfoundry.org/cli/cf/api/resources"
8+
"code.cloudfoundry.org/cli/cf/configuration/coreconfig"
9+
"code.cloudfoundry.org/cli/cf/errors"
10+
. "code.cloudfoundry.org/cli/cf/i18n"
11+
"code.cloudfoundry.org/cli/cf/net"
12+
)
13+
14+
//go:generate counterfeiter . ClientRepository
15+
16+
type ClientRepository interface {
17+
ClientExists(clientID string) (exists bool, apiErr error)
18+
}
19+
20+
type CloudControllerClientRepository struct {
21+
config coreconfig.Reader
22+
uaaGateway net.Gateway
23+
}
24+
25+
func NewCloudControllerClientRepository(config coreconfig.Reader, uaaGateway net.Gateway) (repo CloudControllerClientRepository) {
26+
repo.config = config
27+
repo.uaaGateway = uaaGateway
28+
return
29+
}
30+
31+
func (repo CloudControllerClientRepository) ClientExists(clientID string) (exists bool, apiErr error) {
32+
exists = false
33+
uaaEndpoint, apiErr := repo.getAuthEndpoint()
34+
if apiErr != nil {
35+
return exists, apiErr
36+
}
37+
38+
path := fmt.Sprintf("%s/oauth/clients/%s", uaaEndpoint, clientID)
39+
40+
uaaResponse := new(resources.UAAUserResources)
41+
apiErr = repo.uaaGateway.GetResource(path, uaaResponse)
42+
if apiErr != nil {
43+
if errType, ok := apiErr.(errors.HTTPError); ok {
44+
switch errType.StatusCode() {
45+
case http.StatusNotFound:
46+
return false, errors.NewModelNotFoundError("Client", clientID)
47+
case http.StatusForbidden:
48+
return false, errors.NewAccessDeniedError()
49+
}
50+
}
51+
return false, apiErr
52+
}
53+
return true, nil
54+
}
55+
56+
func (repo CloudControllerClientRepository) getAuthEndpoint() (string, error) {
57+
uaaEndpoint := repo.config.UaaEndpoint()
58+
if uaaEndpoint == "" {
59+
return "", errors.New(T("UAA endpoint missing from config file"))
60+
}
61+
return uaaEndpoint, nil
62+
}

cf/api/clients_test.go

+125
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,125 @@
1+
package api_test
2+
3+
import (
4+
"fmt"
5+
"net/http"
6+
7+
"code.cloudfoundry.org/cli/cf/api"
8+
"code.cloudfoundry.org/cli/cf/configuration/coreconfig"
9+
"code.cloudfoundry.org/cli/cf/errors"
10+
"code.cloudfoundry.org/cli/cf/net"
11+
"code.cloudfoundry.org/cli/cf/terminal/terminalfakes"
12+
"code.cloudfoundry.org/cli/cf/trace/tracefakes"
13+
testconfig "code.cloudfoundry.org/cli/cf/util/testhelpers/configuration"
14+
. "github.com/onsi/ginkgo"
15+
. "github.com/onsi/gomega"
16+
17+
"github.com/onsi/gomega/ghttp"
18+
)
19+
20+
var _ = Describe("ClientRepository", func() {
21+
Describe("ClientExists", func() {
22+
var (
23+
client api.ClientRepository
24+
uaaServer *ghttp.Server
25+
uaaGateway net.Gateway
26+
config coreconfig.ReadWriter
27+
)
28+
29+
BeforeEach(func() {
30+
uaaServer = ghttp.NewServer()
31+
config = testconfig.NewRepositoryWithDefaults()
32+
33+
config.SetUaaEndpoint(uaaServer.URL())
34+
uaaGateway = net.NewUAAGateway(config, new(terminalfakes.FakeUI), new(tracefakes.FakePrinter), "")
35+
client = api.NewCloudControllerClientRepository(config, uaaGateway)
36+
})
37+
38+
Context("when a client does not exist", func() {
39+
var clientID string
40+
BeforeEach(func() {
41+
clientID = "some-client"
42+
43+
requestPath := fmt.Sprintf("/oauth/clients/%s", clientID)
44+
45+
uaaServer.AppendHandlers(
46+
ghttp.CombineHandlers(
47+
ghttp.VerifyRequest("GET", requestPath),
48+
ghttp.RespondWith(http.StatusNotFound, ""),
49+
),
50+
)
51+
})
52+
53+
It("returns a ModelNotFound error", func() {
54+
b, err := client.ClientExists("some-client")
55+
Expect(err).To(MatchError(&errors.ModelNotFoundError{
56+
ModelType: "Client",
57+
ModelName: "some-client",
58+
}))
59+
Expect(b).To(BeFalse())
60+
})
61+
})
62+
63+
Context("when the active user has insufficient permissions", func() {
64+
var clientID string
65+
BeforeEach(func() {
66+
clientID = "some-client"
67+
68+
requestPath := fmt.Sprintf("/oauth/clients/%s", clientID)
69+
70+
uaaServer.AppendHandlers(
71+
ghttp.CombineHandlers(
72+
ghttp.VerifyRequest("GET", requestPath),
73+
ghttp.RespondWith(http.StatusForbidden, ""),
74+
),
75+
)
76+
})
77+
78+
It("returns an AccessDenied error", func() {
79+
b, err := client.ClientExists(clientID)
80+
Expect(err).To(MatchError(&errors.AccessDeniedError{}))
81+
Expect(b).To(BeFalse())
82+
})
83+
})
84+
85+
Context("when a client does exist", func() {
86+
var clientID string
87+
BeforeEach(func() {
88+
clientID = "some-client"
89+
90+
requestPath := fmt.Sprintf("/oauth/clients/%s", clientID)
91+
92+
uaaServer.AppendHandlers(
93+
ghttp.CombineHandlers(
94+
ghttp.VerifyRequest("GET", requestPath),
95+
ghttp.RespondWith(http.StatusOK, ""),
96+
),
97+
)
98+
})
99+
100+
It("returns true and no error", func() {
101+
b, err := client.ClientExists("some-client")
102+
103+
Expect(b).To(BeTrue())
104+
Expect(err).To(BeNil())
105+
106+
})
107+
})
108+
109+
Context("when getAuthEndpoint fails", func() {
110+
var executeErr error
111+
112+
BeforeEach(func() {
113+
executeErr = errors.New("UAA endpoint missing from config file")
114+
config.SetUaaEndpoint("")
115+
})
116+
117+
It("returns that error", func() {
118+
b, err := client.ClientExists("some-client")
119+
Expect(b).To(BeFalse())
120+
Expect(err).To(MatchError(executeErr))
121+
})
122+
})
123+
})
124+
125+
})

cf/api/repository_locator.go

+11
Original file line numberDiff line numberDiff line change
@@ -57,6 +57,7 @@ type RepositoryLocator struct {
5757
routeServiceBindingRepo RouteServiceBindingRepository
5858
serviceSummaryRepo ServiceSummaryRepository
5959
userRepo UserRepository
60+
clientRepo ClientRepository
6061
passwordRepo password.Repository
6162
logsRepo logs.Repository
6263
authTokenRepo ServiceAuthTokenRepository
@@ -130,6 +131,7 @@ func NewRepositoryLocator(config coreconfig.ReadWriter, gatewaysByName map[strin
130131
loc.spaceRepo = spaces.NewCloudControllerSpaceRepository(config, cloudControllerGateway)
131132
loc.userProvidedServiceInstanceRepo = NewCCUserProvidedServiceInstanceRepository(config, cloudControllerGateway)
132133
loc.userRepo = NewCloudControllerUserRepository(config, uaaGateway, cloudControllerGateway)
134+
loc.clientRepo = NewCloudControllerClientRepository(config, uaaGateway)
133135
loc.buildpackRepo = NewCloudControllerBuildpackRepository(config, cloudControllerGateway)
134136
loc.buildpackBitsRepo = NewCloudControllerBuildpackBitsRepository(config, cloudControllerGateway, appfiles.ApplicationZipper{})
135137
loc.securityGroupRepo = securitygroups.NewSecurityGroupRepo(config, cloudControllerGateway)
@@ -336,6 +338,15 @@ func (locator RepositoryLocator) GetUserRepository() UserRepository {
336338
return locator.userRepo
337339
}
338340

341+
func (locator RepositoryLocator) GetClientRepository() ClientRepository {
342+
return locator.clientRepo
343+
}
344+
345+
func (locator RepositoryLocator) SetClientRepository(repo ClientRepository) RepositoryLocator {
346+
locator.clientRepo = repo
347+
return locator
348+
}
349+
339350
func (locator RepositoryLocator) SetPasswordRepository(repo password.Repository) RepositoryLocator {
340351
locator.passwordRepo = repo
341352
return locator

cf/requirements/factory.go

+1
Original file line numberDiff line numberDiff line change
@@ -110,6 +110,7 @@ func (f apiRequirementFactory) NewUserRequirement(username string, wantGUID bool
110110
func (f apiRequirementFactory) NewClientRequirement(username string) UserRequirement {
111111
return NewClientRequirement(
112112
username,
113+
f.repoLocator.GetClientRepository(),
113114
)
114115
}
115116

cf/requirements/user.go

+12-5
Original file line numberDiff line numberDiff line change
@@ -13,17 +13,19 @@ type UserRequirement interface {
1313
}
1414

1515
type userAPIRequirement struct {
16-
username string
17-
userRepo api.UserRepository
18-
wantGUID bool
19-
clientID string
16+
username string
17+
userRepo api.UserRepository
18+
clientRepo api.ClientRepository
19+
wantGUID bool
20+
clientID string
2021

2122
user models.UserFields
2223
}
2324

24-
func NewClientRequirement(clientID string) *userAPIRequirement {
25+
func NewClientRequirement(clientID string, clientRepo api.ClientRepository) *userAPIRequirement {
2526
req := new(userAPIRequirement)
2627
req.clientID = clientID
28+
req.clientRepo = clientRepo
2729
return req
2830
}
2931

@@ -48,6 +50,11 @@ func (req *userAPIRequirement) Execute() error {
4850
return err
4951
}
5052
} else if req.clientID != "" {
53+
var err error
54+
_, err = req.clientRepo.ClientExists(req.clientID)
55+
if err != nil {
56+
return err
57+
}
5158
req.user = models.UserFields{GUID: req.clientID, Username: req.clientID}
5259
} else {
5360
req.user = models.UserFields{Username: req.username}

0 commit comments

Comments
 (0)