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

fix(api): fix admin user access #3441

Merged
merged 1 commit into from
Jan 14, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 14 additions & 0 deletions src/acceptance/api/api_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -200,6 +200,20 @@ var _ = Describe("AutoScaler Public API", func() {
})
})

When("an admin user tries to access the api", func() {
BeforeEach(func() {
workflowhelpers.AsUser(setup.AdminUserContext(), cfg.DefaultTimeoutDuration(), func() {
oauthToken = OauthToken(cfg)
})
})

It("should succeed to get a policy", func() {
gotPolicy, status := getPolicy()
Expect(status).To(Equal(200))
Expect(string(gotPolicy)).Should(MatchJSON(policy))
})
})

When("a scale out is triggered ", func() {
BeforeEach(func() {
totalTime := time.Duration(cfg.AggregateInterval*2)*time.Second + 3*time.Minute
Expand Down
29 changes: 15 additions & 14 deletions src/autoscaler/api/publicapiserver/middleware.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ package publicapiserver

import (
"errors"
"fmt"
"net/http"
"strings"

Expand Down Expand Up @@ -34,15 +35,16 @@ func (mw *Middleware) Oauth(next http.Handler) http.Handler {
return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
vars := mux.Vars(r)

userToken := r.Header.Get("Authorization")
if userToken == "" {
mw.logger.Error("userToken is not present", nil, lager.Data{"url": r.URL.String()})
authHeaderValue := r.Header.Get("Authorization")
if authHeaderValue == "" {
mw.logger.Error("authorization-header-is-not-present", nil, lager.Data{"url": r.URL.String()})
handlers.WriteJSONResponse(w, http.StatusUnauthorized, models.ErrorResponse{
Code: "Unauthorized",
Message: "User token is not present in Authorization header"})
Message: "Authorization header is not present"})
return
}
if !mw.isValidUserToken(userToken) {
userToken, err := mw.extractBearerToken(authHeaderValue)
if err != nil {
handlers.WriteJSONResponse(w, http.StatusUnauthorized, models.ErrorResponse{
Code: "Unauthorized",
Message: "Invalid bearer token"})
Expand Down Expand Up @@ -125,19 +127,18 @@ func (mw *Middleware) CheckServiceBinding(next http.Handler) http.Handler {
})
}

func (mw *Middleware) isValidUserToken(userToken string) bool {
lowerCaseToken := strings.ToLower(userToken)
if !strings.HasPrefix(lowerCaseToken, "bearer ") {
mw.logger.Error("Token should start with bearer", cf.ErrInvalidTokenFormat)
return false
func (mw *Middleware) extractBearerToken(token string) (string, error) {
if !strings.HasPrefix(strings.ToLower(token), "bearer ") {
mw.logger.Error("check-bearer-scheme", fmt.Errorf("authorization credentials should specify bearer scheme"))
return "", cf.ErrInvalidTokenFormat
}
tokenSplitted := strings.Split(lowerCaseToken, " ")
tokenSplitted := strings.Split(token, " ")
if len(tokenSplitted) != 2 {
mw.logger.Error("Token should contain two parts separated by space", cf.ErrInvalidTokenFormat)
return false
mw.logger.Error("split-auth-credentials", fmt.Errorf("authorization credentials should contain scheme and token separated by space"))
return "", cf.ErrInvalidTokenFormat
}

return true
return tokenSplitted[1], nil
}

func (mw *Middleware) HasClientToken(next http.Handler) http.Handler {
Expand Down
18 changes: 13 additions & 5 deletions src/autoscaler/api/publicapiserver/middleware_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -52,27 +52,27 @@ var _ = Describe("Middleware", func() {
router.ServeHTTP(resp, req)
})

Context("User token is not present in Authorization header", func() {
When("Authorization header is not preset", func() {
BeforeEach(func() {
req = httptest.NewRequest(http.MethodGet, "/v1/apps/"+TEST_APP_ID, nil)
})
It("should fail with 401", func() {
CheckResponse(resp, http.StatusUnauthorized, models.ErrorResponse{
Code: "Unauthorized",
Message: "User token is not present in Authorization header",
Message: "Authorization header is not present",
})

})
})

Context("Invalid user token format", func() {
Context("when user token is not a bearer token", func() {
When("Authorization header does not contain a bearer token", func() {
BeforeEach(func() {
req = httptest.NewRequest(http.MethodGet, "/v1/apps/"+TEST_APP_ID, nil)
req.Header.Add("Authorization", INVALID_USER_TOKEN_WITHOUT_BEARER)
})
It("should fail with 401", func() {
Eventually(logger.Buffer).Should(Say("Token should start with bearer"))
Eventually(logger.Buffer).Should(Say("authorization credentials should specify bearer scheme"))
CheckResponse(resp, http.StatusUnauthorized, models.ErrorResponse{
Code: "Unauthorized",
Message: "Invalid bearer token",
Expand All @@ -86,7 +86,7 @@ var _ = Describe("Middleware", func() {
req.Header.Add("Authorization", INVALID_USER_TOKEN)
})
It("should fail with 401", func() {
Eventually(logger.Buffer).Should(Say("Token should contain two parts separated by space"))
Eventually(logger.Buffer).Should(Say("authorization credentials should contain scheme and token separated by space"))
CheckResponse(resp, http.StatusUnauthorized, models.ErrorResponse{
Code: "Unauthorized",
Message: "Invalid bearer token",
Expand Down Expand Up @@ -130,6 +130,10 @@ var _ = Describe("Middleware", func() {
req.Header.Add("Authorization", TEST_USER_TOKEN)
})
It("should fail with 500", func() {
By("checking if the token is from an admin user")
Expect(fakeCFClient.IsUserAdminCallCount()).To(Equal(1))
Expect(fakeCFClient.IsUserAdminArgsForCall(0)).To(Equal(TEST_BEARER_TOKEN))

CheckResponse(resp, http.StatusInternalServerError, models.ErrorResponse{
Code: http.StatusText(http.StatusInternalServerError),
Message: "Failed to check if user is admin",
Expand All @@ -145,6 +149,10 @@ var _ = Describe("Middleware", func() {
req.Header.Add("Authorization", TEST_USER_TOKEN)
})
It("should succeed with 200", func() {
By("checking if the token is from an admin user")
Expect(fakeCFClient.IsUserAdminCallCount()).To(Equal(1))
Expect(fakeCFClient.IsUserAdminArgsForCall(0)).To(Equal(TEST_BEARER_TOKEN))

Expect(resp.Code).To(Equal(http.StatusOK))
})
})
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,8 @@ const (
CLIENT_ID = "client-id"
CLIENT_SECRET = "client-secret"
TEST_APP_ID = "deadbeef-dead-beef-dead-beef00000075"
TEST_USER_TOKEN = "bearer testusertoken"
TEST_BEARER_TOKEN = "testusertoken"
TEST_USER_TOKEN = "bearer " + TEST_BEARER_TOKEN
INVALID_USER_TOKEN = "bearer invalid_user_token invalid_user_token"
INVALID_USER_TOKEN_WITHOUT_BEARER = "not-bearer testusertoken"
TEST_INVALID_USER_TOKEN = "bearer testinvalidusertoken"
Expand Down
2 changes: 1 addition & 1 deletion src/autoscaler/cf/oauth.go
Original file line number Diff line number Diff line change
Expand Up @@ -85,7 +85,7 @@ func (c *CtxClient) getUserId(ctx context.Context, userToken string) (UserId, er
c.logger.Error("Failed to get user info, create request failed", err, lager.Data{"userInfoEndpoint": userInfoEndpoint})
return "", err
}
req.Header.Set("Authorization", userToken)
req.Header.Set("Authorization", "Bearer "+userToken)
req.Header.Set("Content-Type", "application/json")

resp, err := c.Client.Do(req)
Expand Down
Loading