Skip to content

Commit

Permalink
feat: UI changes for Authz caching (#88)
Browse files Browse the repository at this point in the history
* Introduce MaxKeyExpirySeconds in authz layer

* Add info in the UI about cache expiry when editing user roles

* Format authz message

* Conditionally set the maxCacheExpiryMinutes on the UI

* Document the cache expiry env var

* Do not set REACT_APP_MAX_AUTHZ_CACHE_EXPIRY_MINUTES conditionally
  • Loading branch information
krithika369 authored Jun 13, 2023
1 parent 6ae81a8 commit dd63f43
Show file tree
Hide file tree
Showing 9 changed files with 111 additions and 17 deletions.
20 changes: 13 additions & 7 deletions api/cmd/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,10 +3,12 @@ package main
import (
"encoding/json"
"fmt"
"math"
"net/http"
"os"
"path/filepath"
"strings"
"time"

"github.com/gorilla/mux"
"github.com/heptiolabs/healthcheck"
Expand All @@ -18,6 +20,7 @@ import (
"github.com/caraml-dev/mlp/api/config"
"github.com/caraml-dev/mlp/api/database"
"github.com/caraml-dev/mlp/api/log"
"github.com/caraml-dev/mlp/api/pkg/authz/enforcer"
)

func main() {
Expand Down Expand Up @@ -65,7 +68,9 @@ func main() {
SentryDSN: cfg.SentryDSN,
Streams: cfg.Streams,
Docs: cfg.Docs,
UIConfig: cfg.UI,
MaxAuthzCacheExpiryMinutes: fmt.Sprintf("%.0f",
math.Ceil((time.Duration(enforcer.MaxKeyExpirySeconds) * time.Second).Minutes())),
UIConfig: cfg.UI,
}

router.Methods("GET").Path("/env.js").HandlerFunc(uiEnv.handler)
Expand All @@ -89,12 +94,13 @@ func mount(r *mux.Router, path string, handler http.Handler) {
type uiEnvHandler struct {
*config.UIConfig

APIURL string `json:"REACT_APP_API_URL,omitempty"`
OauthClientID string `json:"REACT_APP_OAUTH_CLIENT_ID,omitempty"`
Environment string `json:"REACT_APP_ENVIRONMENT,omitempty"`
SentryDSN string `json:"REACT_APP_SENTRY_DSN,omitempty"`
Streams config.Streams `json:"REACT_APP_STREAMS"`
Docs config.Documentations `json:"REACT_APP_DOC_LINKS"`
APIURL string `json:"REACT_APP_API_URL,omitempty"`
OauthClientID string `json:"REACT_APP_OAUTH_CLIENT_ID,omitempty"`
Environment string `json:"REACT_APP_ENVIRONMENT,omitempty"`
SentryDSN string `json:"REACT_APP_SENTRY_DSN,omitempty"`
Streams config.Streams `json:"REACT_APP_STREAMS"`
Docs config.Documentations `json:"REACT_APP_DOC_LINKS"`
MaxAuthzCacheExpiryMinutes string `json:"REACT_APP_MAX_AUTHZ_CACHE_EXPIRY_MINUTES"`
}

func (h uiEnvHandler) handler(w http.ResponseWriter, r *http.Request) {
Expand Down
6 changes: 3 additions & 3 deletions api/config/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -190,9 +190,9 @@ var defaultConfig = &Config{
Environment: "dev",
Port: 8080,

Streams: Streams{},
Docs: Documentations{},

Streams: Streams{},
Docs: Documentations{},
Applications: []modelsv2.Application{},
Authorization: &AuthorizationConfig{
Enabled: false,
Caching: &InMemoryCacheConfig{
Expand Down
10 changes: 6 additions & 4 deletions api/config/config_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -76,8 +76,9 @@ func TestLoad(t *testing.T) {
MaxIdleConns: 10,
MaxOpenConns: 20,
},
Mlflow: &config.MlflowConfig{},
Docs: []config.Documentation{},
Mlflow: &config.MlflowConfig{},
Docs: []config.Documentation{},
Applications: []modelsv2.Application{},
Streams: map[string][]string{
"stream-1": {"team-a", "team-b"},
"SecondStream": {"MyTeam"},
Expand Down Expand Up @@ -132,8 +133,9 @@ func TestLoad(t *testing.T) {
MaxIdleConns: 10,
MaxOpenConns: 20,
},
Mlflow: &config.MlflowConfig{},
Docs: []config.Documentation{},
Mlflow: &config.MlflowConfig{},
Docs: []config.Documentation{},
Applications: []modelsv2.Application{},
Streams: map[string][]string{
"stream-1": {"team-a", "team-b"},
"SecondStream": {"MyTeam"},
Expand Down
9 changes: 8 additions & 1 deletion api/pkg/authz/enforcer/enforcer.go
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,9 @@ type CacheConfig struct {
CacheCleanUpIntervalSeconds int
}

// MaxKeyExpirySeconds is the max allowed value for the KeyExpirySeconds.
const MaxKeyExpirySeconds = 600

// Enforcer thin client providing interface for authorizing users
type Enforcer interface {
// Enforce check whether user is authorized to do certain action against a resource
Expand Down Expand Up @@ -84,7 +87,7 @@ func newEnforcer(
flavor Flavor,
timeout time.Duration,
cacheConfig *CacheConfig,
) (Enforcer, error) {
) (*enforcer, error) {
u, err := url.ParseRequestURI(hostURL)
if err != nil {
return nil, err
Expand All @@ -102,6 +105,10 @@ func newEnforcer(
timeout: timeout,
}
if cacheConfig != nil {
if cacheConfig.KeyExpirySeconds > MaxKeyExpirySeconds {
return nil, fmt.Errorf("Configured KeyExpirySeconds is larger than the max permitted value of %d",
MaxKeyExpirySeconds)
}
enforcer.cache = newInMemoryCache(cacheConfig.KeyExpirySeconds, cacheConfig.CacheCleanUpIntervalSeconds)
}
return enforcer, nil
Expand Down
52 changes: 52 additions & 0 deletions api/pkg/authz/enforcer/enforcer_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,58 @@ var BootstrapPolicy = []testPolicy{
},
}

func TestNewEnforcer(t *testing.T) {
tests := map[string]struct {
hostURL string
productName string
flavor Flavor
timeout time.Duration
cacheConfig *CacheConfig

expectedError string
}{
"success | no cache": {
hostURL: "http://authz.com",
productName: "mlp",
flavor: FlavorExact,
timeout: time.Second,
},
"success | with cache": {
hostURL: "http://authz.com",
productName: "mlp",
flavor: FlavorExact,
timeout: time.Second,
cacheConfig: &CacheConfig{
KeyExpirySeconds: 30,
CacheCleanUpIntervalSeconds: 60,
},
},
"failure | large cache expiry": {
hostURL: "http://authz.com",
productName: "mlp",
flavor: FlavorExact,
timeout: time.Second,
cacheConfig: &CacheConfig{
KeyExpirySeconds: 3000,
CacheCleanUpIntervalSeconds: 60,
},
expectedError: "Configured KeyExpirySeconds is larger than the max permitted value of 600",
},
}

for name, tt := range tests {
t.Run(name, func(t *testing.T) {
_, err := newEnforcer(tt.hostURL, tt.productName, tt.flavor, tt.timeout, tt.cacheConfig)

if tt.expectedError != "" {
assert.EqualError(t, err, tt.expectedError)
} else {
assert.NoError(t, err)
}
})
}
}

func TestEnforcer_Enforce(t *testing.T) {
enforcer, err := NewEnforcerBuilder().URL(KetoURL).Product(ProductName).Build()
assert.NoError(t, err)
Expand Down
2 changes: 1 addition & 1 deletion go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ require (
github.com/newrelic/go-agent v3.19.2+incompatible
github.com/opentracing/opentracing-go v1.1.0
github.com/ory/keto-client-go v0.4.4-alpha.1
github.com/patrickmn/go-cache v2.1.0+incompatible
github.com/pkg/errors v0.9.1
github.com/prometheus/client_golang v1.11.1
github.com/prometheus/client_model v0.2.0
Expand Down Expand Up @@ -107,7 +108,6 @@ require (
github.com/mitchellh/reflectwalk v1.0.2 // indirect
github.com/modern-go/concurrent v0.0.0-20180306012644-bacd9c7ef1dd // indirect
github.com/modern-go/reflect2 v1.0.2 // indirect
github.com/patrickmn/go-cache v2.1.0+incompatible // indirect
github.com/pelletier/go-toml v1.9.5 // indirect
github.com/pmezard/go-difflib v1.0.0 // indirect
github.com/prometheus/common v0.26.0 // indirect
Expand Down
3 changes: 3 additions & 0 deletions go.sum
Original file line number Diff line number Diff line change
Expand Up @@ -126,8 +126,10 @@ github.com/cloudflare/golz4 v0.0.0-20150217214814-ef862a3cdc58/go.mod h1:EOBUe0h
github.com/cncf/udpa/go v0.0.0-20191209042840-269d4d468f6f/go.mod h1:M8M6+tZqaGXZJjfX53e64911xZQV5JYwmTeXPW+k8Sc=
github.com/cncf/udpa/go v0.0.0-20200629203442-efcf912fb354/go.mod h1:WmhPx2Nbnhtbo57+VJT5O0JRkEi1Wbu0z5j0R8u5Hbk=
github.com/cncf/udpa/go v0.0.0-20201120205902-5459f2c99403/go.mod h1:WmhPx2Nbnhtbo57+VJT5O0JRkEi1Wbu0z5j0R8u5Hbk=
github.com/cncf/udpa/go v0.0.0-20210930031921-04548b0d99d4/go.mod h1:6pvJx4me5XPnfI9Z40ddWsdw2W/uZgQLFXToKeRcDiI=
github.com/cncf/xds/go v0.0.0-20210312221358-fbca930ec8ed/go.mod h1:eXthEFrGJvWHgFFCl3hGmgk+/aYT6PnTQLykKQRLhEs=
github.com/cncf/xds/go v0.0.0-20210805033703-aa0b78936158/go.mod h1:eXthEFrGJvWHgFFCl3hGmgk+/aYT6PnTQLykKQRLhEs=
github.com/cncf/xds/go v0.0.0-20211011173535-cb28da3451f1/go.mod h1:eXthEFrGJvWHgFFCl3hGmgk+/aYT6PnTQLykKQRLhEs=
github.com/cockroachdb/apd v1.1.0/go.mod h1:8Sl8LxpKi29FqWXR16WEFZRNSz3SoPzUzeMeY4+DwBQ=
github.com/cockroachdb/cockroach-go v0.0.0-20190925194419-606b3d062051/go.mod h1:XGLbWH/ujMcbPbhZq52Nv6UrCghb1yGn//133kEsvDk=
github.com/codahale/hdrhistogram v0.0.0-20161010025455-3a0bb77429bd h1:qMd81Ts1T2OTKmB4acZcyKaMtRnY5Y44NuXGX2GFJ1w=
Expand Down Expand Up @@ -173,6 +175,7 @@ github.com/envoyproxy/go-control-plane v0.9.9-0.20201210154907-fd9021fe5dad/go.m
github.com/envoyproxy/go-control-plane v0.9.9-0.20210217033140-668b12f5399d/go.mod h1:cXg6YxExXjJnVBQHBLXeUAgxn2UodCpnH306RInaBQk=
github.com/envoyproxy/go-control-plane v0.9.9-0.20210512163311-63b5d3c536b0/go.mod h1:hliV/p42l8fGbc6Y9bQ70uLwIvmJyVE5k4iMKlh8wCQ=
github.com/envoyproxy/go-control-plane v0.9.10-0.20210907150352-cf90f659a021/go.mod h1:AFq3mo9L8Lqqiid3OhADV3RfLJnjiw63cSpi+fDTRC0=
github.com/envoyproxy/go-control-plane v0.10.2-0.20220325020618-49ff273808a1/go.mod h1:KJwIaB5Mv44NWtYuAOFCVOjcI94vtpEz2JU/D2v6IjE=
github.com/envoyproxy/protoc-gen-validate v0.1.0/go.mod h1:iSmxcyjqTsJpI2R4NaDN7+kN2VEUnK/pcBlmesArF7c=
github.com/erikstmartin/go-testdb v0.0.0-20160219214506-8d10e4a1bae5 h1:Yzb9+7DPaBjB8zlTR87/ElzFsnQfuHnVUVqpZZIcV5Y=
github.com/erikstmartin/go-testdb v0.0.0-20160219214506-8d10e4a1bae5/go.mod h1:a2zkGnVExMxdzMo3M0Hi/3sEU+cWnZpSni0O6/Yb/P0=
Expand Down
8 changes: 7 additions & 1 deletion ui/packages/app/src/config.js
Original file line number Diff line number Diff line change
Expand Up @@ -19,12 +19,18 @@ const config = {
"https://github.com/caraml-dev/merlin/blob/main/docs/getting-started/README.md",
label: "Merlin User Guide"
},
{ href: "https://github.com/caraml-dev/turing", label: "Turing User Guide" },
{
href: "https://github.com/caraml-dev/turing",
label: "Turing User Guide"
},
{
href: "https://docs.feast.dev/user-guide/overview",
label: "Feast User Guide"
}
],
MAX_AUTHZ_CACHE_EXPIRY_MINUTES: parseInt(
getEnv("REACT_APP_MAX_AUTHZ_CACHE_EXPIRY_MINUTES") || "0"
),

CLOCKWORK_UI_HOMEPAGE: getEnv("REACT_APP_CLOCKWORK_UI_HOMEPAGE"),
KUBEFLOW_UI_HOMEPAGE: getEnv("REACT_APP_KUBEFLOW_UI_HOMEPAGE")
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ import React, { useCallback, useEffect, useState } from "react";
import {
EuiButton,
EuiButtonEmpty,
EuiCallOut,
EuiFieldText,
EuiFlexGroup,
EuiFlexItem,
Expand All @@ -14,6 +15,7 @@ import {
EuiTitle,
EuiToolTip
} from "@elastic/eui";
import config from "../../config";
import { validateEmail } from "../../validation/validation";
import { addToast, useMlpApi } from "@caraml-dev/ui-lib";
import UserRoleSelection from "./UserRoleSelection";
Expand Down Expand Up @@ -77,6 +79,7 @@ const SubmitUserRoleForm = ({ userRole, project, fetchUpdates, toggleAdd }) => {
onChange
]);

const isAuthzCacheEnabled = !!config.MAX_AUTHZ_CACHE_EXPIRY_MINUTES;
return (
<EuiPanel paddingSize="m">
<EuiFlexGroup justifyContent="spaceAround" direction="column">
Expand Down Expand Up @@ -124,6 +127,21 @@ const SubmitUserRoleForm = ({ userRole, project, fetchUpdates, toggleAdd }) => {
</EuiFlexGroup>
</EuiForm>
</EuiFlexItem>
<EuiFlexItem>
{isAuthzCacheEnabled && (
<EuiCallOut
title={`Permission changes may take up to ${
config.MAX_AUTHZ_CACHE_EXPIRY_MINUTES
}
${
config.MAX_AUTHZ_CACHE_EXPIRY_MINUTES > 1
? "minutes"
: "minute"
} to take effect in all components.`}
iconType="iInCircle"
/>
)}
</EuiFlexItem>
<EuiFlexItem grow={false}>
<EuiFlexGroup direction="row">
<EuiFlexItem grow={false}>
Expand Down

0 comments on commit dd63f43

Please sign in to comment.