Skip to content

Commit

Permalink
[CCXDEV-12871] Allow user access even if RBAC is enabled and remove A…
Browse files Browse the repository at this point in the history
…CM wildcard (#1372)

* allow user requests even if USE_RBAC is true

* remove wildcard for acm user agent

* add support for ephemeral RBAC url

* add more debug logging

* use INFO level

* use RBAC on ephemeral

* Revert "use INFO level"

This reverts commit b144df3.

* don't enforce RBAC by default
  • Loading branch information
juandspy authored Nov 22, 2024
1 parent 35b8b98 commit 33dc400
Show file tree
Hide file tree
Showing 6 changed files with 17 additions and 39 deletions.
2 changes: 1 addition & 1 deletion auth/configuration.go
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ func constructRBACURL(config *RBACConfig) (string, string, error) {
}

// Add the /access/ endpoint and ocp-advisor parameter
baseURL.Path = "/access/"
baseURL.Path += "/access/"

params := url.Values{}
params.Add("application", "ocp-advisor")
Expand Down
9 changes: 8 additions & 1 deletion auth/configuration_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -41,9 +41,16 @@ func TestConstructRBACURL(t *testing.T) {
expectedHost: "example.com",
expectError: false,
},
{
// ephemeral case:
config: &auth.RBACConfig{URL: "http://rbac-service:8000/api/rbac/v1"},
expectedURL: "http://rbac-service:8000/api/rbac/v1/access/?application=ocp-advisor&limit=100",
expectedHost: "rbac-service:8000",
expectError: false,
},
{
config: &auth.RBACConfig{URL: "wrong-schema:/invalid"},
expectedURL: "https://wrong-schema:/access/?application=ocp-advisor&limit=100",
expectedURL: "https://wrong-schema:/invalid/access/?application=ocp-advisor&limit=100",
expectedHost: "wrong-schema:",
expectError: false, //url.Parse and url.ParseURI can't catch bad URLs, careful!
},
Expand Down
5 changes: 4 additions & 1 deletion auth/rbac.go
Original file line number Diff line number Diff line change
Expand Up @@ -65,19 +65,22 @@ func (rc *rbacClientImpl) IsEnforcing() bool {
// IsAuthorized checks if an account has the correct permissions to access our resources
func (rc *rbacClientImpl) IsAuthorized(token string) bool {
permissions := rc.getPermissions(token)
log.Debug().Interface("permissions", permissions).Msg("Account openshift permissions")
return permissions != nil
}

func (rc *rbacClientImpl) getPermissions(identityToken string) map[string][]string {
acls := rc.requestAccess(rc.uri, identityToken)
if len(acls) > 0 {
log.Debug().Interface("acls", acls).Msg("Account all permissions")
permissions := aggregatePermissions(acls)
if len(permissions) > 0 {
log.Info().Any("RBAC permissions", permissions).Send()
log.Info().Any("RBAC openshift permissions", permissions).Send()
return permissions
}
return nil
}
log.Debug().Msg("Account has no ACLs")
return nil
}

Expand Down
2 changes: 1 addition & 1 deletion deploy/clowdapp.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -322,6 +322,6 @@ parameters:
- name: USE_RBAC
value: "false"
- name: RBAC_SERVER_URL
value: "https://console.stage.redhat.com/api/rbac/v1"
value: "http://rbac-service:8000/api/rbac/v1"
- name: ENFORCE_RBAC
value: "false"
10 changes: 3 additions & 7 deletions server/auth_middleware.go
Original file line number Diff line number Diff line change
Expand Up @@ -127,12 +127,6 @@ func (server *HTTPServer) Authorization(next http.Handler, noAuthURLs []string)
return
}

// We also leave ACM folks out of this for now
if server.getKnownUserAgentProduct(r) == acmUserAgent {
next.ServeHTTP(w, r)
return
}

token, err := auth.DecodeTokenFromHeader(w, r, server.Config.AuthType)
if err != nil {
handleServerError(w, err)
Expand All @@ -143,6 +137,7 @@ func (server *HTTPServer) Authorization(next http.Handler, noAuthURLs []string)
// be for all users, but let's first make sure we won't disturb existing users by only
// logging unauthorized service accounts
if token.Identity.Type == "ServiceAccount" {
log.Debug().Str("client ID", token.Identity.ServiceAccount.ClientID).Msg("Received a request from a service account")
// Check permissions for service accounts
if !server.rbacClient.IsAuthorized(auth.GetAuthTokenHeader(r)) {
log.Warn().Str(accountType, token.Identity.Type).Msg(accountNotAuthorized)
Expand All @@ -154,7 +149,8 @@ func (server *HTTPServer) Authorization(next http.Handler, noAuthURLs []string)
} else {
log.Debug().Str(accountType, token.Identity.Type).Msg("RBAC is only used with service accounts for now")
// handleServerError(w, &auth.AuthorizationError{ErrString: "unknown identity type"})
return
// We don't use return because RBAC is not mandatory for users yet
// return
}

// Access is authorized, proceed with the request
Expand Down
28 changes: 0 additions & 28 deletions server/auth_middleware_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -353,31 +353,3 @@ func TestAuthorization_NoAuthURLs(t *testing.T) {
// Verify that the response is OK and RBAC was not used
assert.Equal(t, http.StatusOK, rr.Code)
}

func TestAuthorization_ACMUserAgent(t *testing.T) {
// Setup the server and the request
rbacClient := &MockRBACClient{authorized: false, enforcing: true}
testServer := server.HTTPServer{
Config: server.Configuration{
Auth: true,
AuthType: "xrh",
UseRBAC: true,
},
}
testServer.SetRBACClient(rbacClient)

// Create a request with the ACM user agent
req := httptest.NewRequest(http.MethodGet, "/some-endpoint", nil)
req.Header.Set("User-Agent", server.AcmUserAgent)
rr := httptest.NewRecorder()

// Call the authorization middleware
handler := testServer.Authorization(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
w.WriteHeader(http.StatusOK)
}), []string{})

handler.ServeHTTP(rr, req)

// Verify that the response is OK and RBAC was not used
assert.Equal(t, http.StatusOK, rr.Code)
}

0 comments on commit 33dc400

Please sign in to comment.