mirror of https://github.com/grafana/grafana.git
				
				
				
			Provisioning: Fix check of who can update (#110835)
This commit is contained in:
		
							parent
							
								
									8805e93b1d
								
							
						
					
					
						commit
						323738d191
					
				|  | @ -6,6 +6,7 @@ import ( | |||
| 	"net/http" | ||||
| 
 | ||||
| 	"github.com/grafana/authlib/authn" | ||||
| 	"github.com/grafana/grafana/apps/provisioning/pkg/apis/provisioning/v0alpha1" | ||||
| 	utilnet "k8s.io/apimachinery/pkg/util/net" | ||||
| ) | ||||
| 
 | ||||
|  | @ -32,8 +33,15 @@ func NewRoundTripper(tokenExchangeClient tokenExchanger, base http.RoundTripper, | |||
| } | ||||
| 
 | ||||
| func (t *RoundTripper) RoundTrip(req *http.Request) (*http.Response, error) { | ||||
| 	// when we want to write resources with the provisioning API, the audience needs to include provisioning
 | ||||
| 	// so that it passes the check in enforceManagerProperties, which prevents others from updating provisioned resources
 | ||||
| 	audiences := []string{t.audience} | ||||
| 	if t.audience != v0alpha1.GROUP { | ||||
| 		audiences = append(audiences, v0alpha1.GROUP) | ||||
| 	} | ||||
| 
 | ||||
| 	tokenResponse, err := t.client.Exchange(req.Context(), authn.TokenExchangeRequest{ | ||||
| 		Audiences: []string{t.audience}, | ||||
| 		Audiences: audiences, | ||||
| 		Namespace: "*", | ||||
| 	}) | ||||
| 	if err != nil { | ||||
|  |  | |||
|  | @ -5,17 +5,22 @@ import ( | |||
| 	"io" | ||||
| 	"net/http" | ||||
| 	"net/http/httptest" | ||||
| 	"reflect" | ||||
| 	"testing" | ||||
| 
 | ||||
| 	"github.com/grafana/authlib/authn" | ||||
| 	"github.com/grafana/grafana/apps/provisioning/pkg/apis/provisioning/v0alpha1" | ||||
| 	"github.com/stretchr/testify/require" | ||||
| ) | ||||
| 
 | ||||
| type fakeExchanger struct { | ||||
| 	resp   *authn.TokenExchangeResponse | ||||
| 	err    error | ||||
| 	gotReq *authn.TokenExchangeRequest | ||||
| } | ||||
| 
 | ||||
| func (f *fakeExchanger) Exchange(_ context.Context, req authn.TokenExchangeRequest) (*authn.TokenExchangeResponse, error) { | ||||
| 	f.gotReq = &req | ||||
| 	return f.resp, f.err | ||||
| } | ||||
| 
 | ||||
|  | @ -61,3 +66,41 @@ func TestRoundTripper_PropagatesExchangeError(t *testing.T) { | |||
| 		t.Fatalf("expected error, got nil") | ||||
| 	} | ||||
| } | ||||
| 
 | ||||
| func TestRoundTripper_AudiencesAndNamespace(t *testing.T) { | ||||
| 	tests := []struct { | ||||
| 		name          string | ||||
| 		audience      string | ||||
| 		wantAudiences []string | ||||
| 	}{ | ||||
| 		{ | ||||
| 			name:          "adds group when custom audience", | ||||
| 			audience:      "example-audience", | ||||
| 			wantAudiences: []string{"example-audience", v0alpha1.GROUP}, | ||||
| 		}, | ||||
| 		{ | ||||
| 			name:          "no duplicate when group audience", | ||||
| 			audience:      v0alpha1.GROUP, | ||||
| 			wantAudiences: []string{v0alpha1.GROUP}, | ||||
| 		}, | ||||
| 	} | ||||
| 
 | ||||
| 	for _, tt := range tests { | ||||
| 		t.Run(tt.name, func(t *testing.T) { | ||||
| 			fx := &fakeExchanger{resp: &authn.TokenExchangeResponse{Token: "abc123"}} | ||||
| 			tr := NewRoundTripper(fx, roundTripperFunc(func(_ *http.Request) (*http.Response, error) { | ||||
| 				rr := httptest.NewRecorder() | ||||
| 				rr.WriteHeader(http.StatusOK) | ||||
| 				return rr.Result(), nil | ||||
| 			}), tt.audience) | ||||
| 
 | ||||
| 			req, _ := http.NewRequestWithContext(context.Background(), http.MethodGet, "http://example", nil) | ||||
| 			resp, err := tr.RoundTrip(req) | ||||
| 			require.NoError(t, err) | ||||
| 			_, _ = io.Copy(io.Discard, resp.Body) | ||||
| 			_ = resp.Body.Close() | ||||
| 			require.NotNil(t, fx.gotReq) | ||||
| 			require.True(t, reflect.DeepEqual(fx.gotReq.Audiences, tt.wantAudiences)) | ||||
| 		}) | ||||
| 	} | ||||
| } | ||||
|  |  | |||
|  | @ -5,6 +5,7 @@ import ( | |||
| 	"errors" | ||||
| 	"fmt" | ||||
| 	"net/http" | ||||
| 	"slices" | ||||
| 
 | ||||
| 	apierrors "k8s.io/apimachinery/pkg/api/errors" | ||||
| 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" | ||||
|  | @ -17,6 +18,7 @@ import ( | |||
| 
 | ||||
| 	authtypes "github.com/grafana/authlib/types" | ||||
| 
 | ||||
| 	provisioning "github.com/grafana/grafana/apps/provisioning/pkg/apis/provisioning/v0alpha1" | ||||
| 	"github.com/grafana/grafana/pkg/apimachinery/utils" | ||||
| 	"github.com/grafana/grafana/pkg/storage/unified/resourcepb" | ||||
| ) | ||||
|  | @ -75,7 +77,7 @@ func enforceManagerProperties(auth authtypes.AuthInfo, obj utils.GrafanaMetaAcce | |||
| 		return nil // not managed
 | ||||
| 
 | ||||
| 	case utils.ManagerKindRepo: | ||||
| 		if auth.GetUID() == "access-policy:provisioning" { | ||||
| 		if auth.GetUID() == "access-policy:provisioning" || slices.Contains(auth.GetAudience(), provisioning.GROUP) { | ||||
| 			return nil // OK!
 | ||||
| 		} | ||||
| 		// This can fallback to writing the value with a provisioning client
 | ||||
|  |  | |||
|  | @ -4,15 +4,19 @@ import ( | |||
| 	"context" | ||||
| 	"testing" | ||||
| 
 | ||||
| 	"github.com/go-jose/go-jose/v3/jwt" | ||||
| 	"github.com/stretchr/testify/require" | ||||
| 	v1 "k8s.io/apimachinery/pkg/apis/meta/v1" | ||||
| 	"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" | ||||
| 	"k8s.io/apimachinery/pkg/runtime" | ||||
| 
 | ||||
| 	authnlib "github.com/grafana/authlib/authn" | ||||
| 	authtypes "github.com/grafana/authlib/types" | ||||
| 	dashboard "github.com/grafana/grafana/apps/dashboard/pkg/apis/dashboard/v1beta1" | ||||
| 	provisioning "github.com/grafana/grafana/apps/provisioning/pkg/apis/provisioning/v0alpha1" | ||||
| 	"github.com/grafana/grafana/pkg/apimachinery/identity" | ||||
| 	"github.com/grafana/grafana/pkg/apimachinery/utils" | ||||
| 	serviceauthn "github.com/grafana/grafana/pkg/services/authn" | ||||
| ) | ||||
| 
 | ||||
| func TestManagedAuthorizer(t *testing.T) { | ||||
|  | @ -112,6 +116,25 @@ func TestManagedAuthorizer(t *testing.T) { | |||
| 				}, | ||||
| 			}, | ||||
| 		}, | ||||
| 		{ | ||||
| 			name: "audience includes provisioning group", | ||||
| 			auth: &serviceauthn.Identity{ | ||||
| 				Type: authtypes.TypeAccessPolicy, | ||||
| 				UID:  "access-policy:random-uid", | ||||
| 				AccessTokenClaims: &authnlib.Claims[authnlib.AccessTokenClaims]{ | ||||
| 					Claims: jwt.Claims{ | ||||
| 						Audience: []string{provisioning.GROUP}, | ||||
| 					}, | ||||
| 				}, | ||||
| 			}, | ||||
| 			obj: &dashboard.Dashboard{ | ||||
| 				ObjectMeta: v1.ObjectMeta{ | ||||
| 					Annotations: map[string]string{ | ||||
| 						utils.AnnoKeyManagerKind: string(utils.ManagerKindRepo), | ||||
| 					}, | ||||
| 				}, | ||||
| 			}, | ||||
| 		}, | ||||
| 	} | ||||
| 
 | ||||
| 	for _, tt := range tests { | ||||
|  |  | |||
		Loading…
	
		Reference in New Issue