From 059ba2597357a84083ab5c726dc920ae9f8644f2 Mon Sep 17 00:00:00 2001 From: Gabriel MABILLE Date: Wed, 29 Nov 2023 10:25:46 +0100 Subject: [PATCH] AuthN: Check API Key is not trying to access another organization (#78749) * AuthN: Check API Key is not trying to access another organization * Revert local change * Add test * Discussed with Kalle we should set r.OrgID * Syntax sugar * Suggestion org-mismatch --- pkg/services/authn/clients/api_key.go | 13 ++++++++++--- pkg/services/authn/clients/api_key_test.go | 19 ++++++++++++++++--- 2 files changed, 26 insertions(+), 6 deletions(-) diff --git a/pkg/services/authn/clients/api_key.go b/pkg/services/authn/clients/api_key.go index 11e0e689aba..558340cdaa0 100644 --- a/pkg/services/authn/clients/api_key.go +++ b/pkg/services/authn/clients/api_key.go @@ -19,9 +19,10 @@ import ( ) var ( - errAPIKeyInvalid = errutil.Unauthorized("api-key.invalid", errutil.WithPublicMessage("Invalid API key")) - errAPIKeyExpired = errutil.Unauthorized("api-key.expired", errutil.WithPublicMessage("Expired API key")) - errAPIKeyRevoked = errutil.Unauthorized("api-key.revoked", errutil.WithPublicMessage("Revoked API key")) + errAPIKeyInvalid = errutil.Unauthorized("api-key.invalid", errutil.WithPublicMessage("Invalid API key")) + errAPIKeyExpired = errutil.Unauthorized("api-key.expired", errutil.WithPublicMessage("Expired API key")) + errAPIKeyRevoked = errutil.Unauthorized("api-key.revoked", errutil.WithPublicMessage("Revoked API key")) + errAPIKeyOrgMismatch = errutil.Unauthorized("api-key.organization-mismatch", errutil.WithPublicMessage("API key does not belong to the requested organization")) ) var _ authn.HookClient = new(APIKey) @@ -62,6 +63,12 @@ func (s *APIKey) Authenticate(ctx context.Context, r *authn.Request) (*authn.Ide return nil, errAPIKeyRevoked.Errorf("Api key is revoked") } + if r.OrgID == 0 { + r.OrgID = apiKey.OrgID + } else if r.OrgID != apiKey.OrgID { + return nil, errAPIKeyOrgMismatch.Errorf("API does not belong in Organization %v", r.OrgID) + } + // if the api key don't belong to a service account construct the identity and return it if apiKey.ServiceAccountId == nil || *apiKey.ServiceAccountId < 1 { return &authn.Identity{ diff --git a/pkg/services/authn/clients/api_key_test.go b/pkg/services/authn/clients/api_key_test.go index a996dcca3f7..4848eb38358 100644 --- a/pkg/services/authn/clients/api_key_test.go +++ b/pkg/services/authn/clients/api_key_test.go @@ -109,6 +109,17 @@ func TestAPIKey_Authenticate(t *testing.T) { }, expectedErr: errAPIKeyRevoked, }, + { + desc: "should fail for api key in another organization", + req: &authn.Request{OrgID: 1, HTTPRequest: &http.Request{Header: map[string][]string{"Authorization": {"Bearer " + secret}}}}, + expectedKey: &apikey.APIKey{ + ID: 1, + OrgID: 2, + Key: hash, + ServiceAccountId: intPtr(1), + }, + expectedErr: errAPIKeyOrgMismatch, + }, } for _, tt := range tests { @@ -123,10 +134,12 @@ func TestAPIKey_Authenticate(t *testing.T) { if tt.expectedErr != nil { assert.Nil(t, identity) assert.ErrorIs(t, err, tt.expectedErr) - } else { - assert.NoError(t, err) - assert.EqualValues(t, *tt.expectedIdentity, *identity) + return } + + assert.NoError(t, err) + assert.EqualValues(t, *tt.expectedIdentity, *identity) + assert.Equal(t, tt.req.OrgID, tt.expectedIdentity.OrgID, "the request organization should match the identity's one") }) } }