mirror of https://github.com/grafana/grafana
Add/Delete API keys to Service accounts (#44871)
* ServiceAccounts: move token handlers to specific file * ServiceAccounts: move Add API key to Service account * APIKeys: api keys can still be used even when service accounts are enabled * APIKeys: legacy endpoint can't be used to add SA tokens * ServiceAccount: add tests for creation with nil and non-nil service account ids * ServiceAccounts: fix unnasigned cfg and AC typo * Test: test service account token adding * fix linting error * ServiceAccounts: Handle Token deletion * rename token funcs * rename token funcs and api wrapping * add token deletion tests * review Co-authored-by: eleijonmarck <eric.leijonmarck@gmail.com> * remove bus * Update pkg/api/apikey.go Co-authored-by: eleijonmarck <eric.leijonmarck@gmail.com>pull/44645/head^2
J Guerreiro
3 years ago
committed by
GitHub
parent
12176e24ef
commit
94820e1f29
@ -0,0 +1,165 @@ |
||||
package api |
||||
|
||||
import ( |
||||
"errors" |
||||
"net/http" |
||||
"strconv" |
||||
"time" |
||||
|
||||
"github.com/grafana/grafana/pkg/api/dtos" |
||||
"github.com/grafana/grafana/pkg/api/response" |
||||
"github.com/grafana/grafana/pkg/components/apikeygen" |
||||
"github.com/grafana/grafana/pkg/models" |
||||
"github.com/grafana/grafana/pkg/services/serviceaccounts" |
||||
"github.com/grafana/grafana/pkg/web" |
||||
) |
||||
|
||||
const failedToDeleteMsg = "Failed to delete API key" |
||||
|
||||
func (api *ServiceAccountsAPI) ListTokens(ctx *models.ReqContext) response.Response { |
||||
saID, err := strconv.ParseInt(web.Params(ctx.Req)[":serviceAccountId"], 10, 64) |
||||
if err != nil { |
||||
return response.Error(http.StatusBadRequest, "serviceAccountId is invalid", err) |
||||
} |
||||
|
||||
if saTokens, err := api.store.ListTokens(ctx.Req.Context(), ctx.OrgId, saID); err == nil { |
||||
result := make([]*models.ApiKeyDTO, len(saTokens)) |
||||
for i, t := range saTokens { |
||||
var expiration *time.Time = nil |
||||
if t.Expires != nil { |
||||
v := time.Unix(*t.Expires, 0) |
||||
expiration = &v |
||||
} |
||||
result[i] = &models.ApiKeyDTO{ |
||||
Id: t.Id, |
||||
Name: t.Name, |
||||
Role: t.Role, |
||||
Expiration: expiration, |
||||
} |
||||
} |
||||
|
||||
return response.JSON(200, result) |
||||
} else { |
||||
return response.Error(500, "Internal server error", err) |
||||
} |
||||
} |
||||
|
||||
// CreateNewToken adds an API key to a service account
|
||||
func (api *ServiceAccountsAPI) CreateToken(c *models.ReqContext) response.Response { |
||||
saID, err := strconv.ParseInt(web.Params(c.Req)[":serviceAccountId"], 10, 64) |
||||
if err != nil { |
||||
return response.Error(http.StatusBadRequest, "serviceAccountId is invalid", err) |
||||
} |
||||
|
||||
// confirm service account exists
|
||||
if _, err := api.store.RetrieveServiceAccount(c.Req.Context(), c.OrgId, saID); err != nil { |
||||
switch { |
||||
case errors.Is(err, serviceaccounts.ErrServiceAccountNotFound): |
||||
return response.Error(http.StatusNotFound, "Failed to retrieve service account", err) |
||||
default: |
||||
return response.Error(http.StatusInternalServerError, "Failed to retrieve service account", err) |
||||
} |
||||
} |
||||
|
||||
cmd := models.AddApiKeyCommand{} |
||||
if err := web.Bind(c.Req, &cmd); err != nil { |
||||
return response.Error(http.StatusBadRequest, "bad request data", err) |
||||
} |
||||
|
||||
// Force affected service account to be the one referenced in the URL
|
||||
cmd.ServiceAccountId = &saID |
||||
cmd.OrgId = c.OrgId |
||||
|
||||
if !cmd.Role.IsValid() { |
||||
return response.Error(400, "Invalid role specified", nil) |
||||
} |
||||
|
||||
if api.cfg.ApiKeyMaxSecondsToLive != -1 { |
||||
if cmd.SecondsToLive == 0 { |
||||
return response.Error(400, "Number of seconds before expiration should be set", nil) |
||||
} |
||||
if cmd.SecondsToLive > api.cfg.ApiKeyMaxSecondsToLive { |
||||
return response.Error(400, "Number of seconds before expiration is greater than the global limit", nil) |
||||
} |
||||
} |
||||
|
||||
newKeyInfo, err := apikeygen.New(cmd.OrgId, cmd.Name) |
||||
if err != nil { |
||||
return response.Error(500, "Generating API key failed", err) |
||||
} |
||||
|
||||
cmd.Key = newKeyInfo.HashedKey |
||||
|
||||
if err := api.apiKeyStore.AddAPIKey(c.Req.Context(), &cmd); err != nil { |
||||
if errors.Is(err, models.ErrInvalidApiKeyExpiration) { |
||||
return response.Error(400, err.Error(), nil) |
||||
} |
||||
if errors.Is(err, models.ErrDuplicateApiKey) { |
||||
return response.Error(409, err.Error(), nil) |
||||
} |
||||
return response.Error(500, "Failed to add API Key", err) |
||||
} |
||||
|
||||
result := &dtos.NewApiKeyResult{ |
||||
ID: cmd.Result.Id, |
||||
Name: cmd.Result.Name, |
||||
Key: newKeyInfo.ClientSecret, |
||||
} |
||||
|
||||
return response.JSON(200, result) |
||||
} |
||||
|
||||
// DeleteToken deletes service account tokens
|
||||
func (api *ServiceAccountsAPI) DeleteToken(c *models.ReqContext) response.Response { |
||||
saID, err := strconv.ParseInt(web.Params(c.Req)[":serviceAccountId"], 10, 64) |
||||
if err != nil { |
||||
return response.Error(http.StatusBadRequest, "serviceAccountId is invalid", err) |
||||
} |
||||
|
||||
// confirm service account exists
|
||||
if _, err := api.store.RetrieveServiceAccount(c.Req.Context(), c.OrgId, saID); err != nil { |
||||
switch { |
||||
case errors.Is(err, serviceaccounts.ErrServiceAccountNotFound): |
||||
return response.Error(http.StatusNotFound, "Failed to retrieve service account", err) |
||||
default: |
||||
return response.Error(http.StatusInternalServerError, "Failed to retrieve service account", err) |
||||
} |
||||
} |
||||
|
||||
tokenID, err := strconv.ParseInt(web.Params(c.Req)[":tokenId"], 10, 64) |
||||
if err != nil { |
||||
return response.Error(http.StatusBadRequest, "serviceAccountId is invalid", err) |
||||
} |
||||
|
||||
// confirm API key belongs to service account. TODO: refactor get & delete to single call
|
||||
cmdGet := &models.GetApiKeyByIdQuery{ApiKeyId: tokenID} |
||||
if err = api.apiKeyStore.GetApiKeyById(c.Req.Context(), cmdGet); err != nil { |
||||
status := 404 |
||||
if err != nil && !errors.Is(err, models.ErrApiKeyNotFound) { |
||||
status = 500 |
||||
} else { |
||||
err = models.ErrApiKeyNotFound |
||||
} |
||||
|
||||
return response.Error(status, failedToDeleteMsg, err) |
||||
} |
||||
|
||||
// verify service account ID matches the URL
|
||||
if *cmdGet.Result.ServiceAccountId != saID { |
||||
return response.Error(404, failedToDeleteMsg, err) |
||||
} |
||||
|
||||
cmdDel := &models.DeleteApiKeyCommand{Id: tokenID, OrgId: c.OrgId} |
||||
if err = api.apiKeyStore.DeleteApiKey(c.Req.Context(), cmdDel); err != nil { |
||||
status := 404 |
||||
if err != nil && !errors.Is(err, models.ErrApiKeyNotFound) { |
||||
status = 500 |
||||
} else { |
||||
err = models.ErrApiKeyNotFound |
||||
} |
||||
|
||||
return response.Error(status, failedToDeleteMsg, err) |
||||
} |
||||
|
||||
return response.Success("API key deleted") |
||||
} |
||||
@ -0,0 +1,238 @@ |
||||
package api |
||||
|
||||
import ( |
||||
"context" |
||||
"encoding/json" |
||||
"fmt" |
||||
"io" |
||||
"net/http" |
||||
"net/http/httptest" |
||||
"strings" |
||||
"testing" |
||||
|
||||
"github.com/grafana/grafana/pkg/api/routing" |
||||
"github.com/grafana/grafana/pkg/bus" |
||||
"github.com/grafana/grafana/pkg/components/apikeygen" |
||||
"github.com/grafana/grafana/pkg/models" |
||||
"github.com/grafana/grafana/pkg/services/accesscontrol" |
||||
accesscontrolmock "github.com/grafana/grafana/pkg/services/accesscontrol/mock" |
||||
"github.com/grafana/grafana/pkg/services/serviceaccounts" |
||||
"github.com/grafana/grafana/pkg/services/serviceaccounts/tests" |
||||
"github.com/grafana/grafana/pkg/services/sqlstore" |
||||
"github.com/grafana/grafana/pkg/web" |
||||
"github.com/stretchr/testify/assert" |
||||
"github.com/stretchr/testify/require" |
||||
) |
||||
|
||||
const ( |
||||
serviceaccountIDTokensPath = "/api/serviceaccounts/%v/tokens" // #nosec G101
|
||||
serviceaccountIDTokensDetailPath = "/api/serviceaccounts/%v/tokens/%v" // #nosec G101
|
||||
) |
||||
|
||||
func createTokenforSA(t *testing.T, keyName string, orgID int64, saID int64) *models.ApiKey { |
||||
key, err := apikeygen.New(orgID, keyName) |
||||
require.NoError(t, err) |
||||
cmd := models.AddApiKeyCommand{ |
||||
Name: keyName, |
||||
Role: "Viewer", |
||||
OrgId: orgID, |
||||
Key: key.HashedKey, |
||||
SecondsToLive: 0, |
||||
ServiceAccountId: &saID, |
||||
Result: &models.ApiKey{}, |
||||
} |
||||
err = bus.Dispatch(context.Background(), &cmd) |
||||
require.NoError(t, err) |
||||
|
||||
return cmd.Result |
||||
} |
||||
|
||||
func TestServiceAccountsAPI_CreateToken(t *testing.T) { |
||||
store := sqlstore.InitTestDB(t) |
||||
svcmock := tests.ServiceAccountMock{} |
||||
sa := tests.SetupUserServiceAccount(t, store, tests.TestUser{Login: "sa", IsServiceAccount: true}) |
||||
|
||||
type testCreateSAToken struct { |
||||
desc string |
||||
expectedCode int |
||||
body map[string]interface{} |
||||
acmock *accesscontrolmock.Mock |
||||
} |
||||
|
||||
testCases := []testCreateSAToken{ |
||||
{ |
||||
desc: "should be ok to create serviceaccount token with scope all permissions", |
||||
acmock: tests.SetupMockAccesscontrol( |
||||
t, |
||||
func(c context.Context, siu *models.SignedInUser) ([]*accesscontrol.Permission, error) { |
||||
return []*accesscontrol.Permission{{Action: serviceaccounts.ActionWrite, Scope: serviceaccounts.ScopeAll}}, nil |
||||
}, |
||||
false, |
||||
), |
||||
body: map[string]interface{}{"name": "Test1", "role": "Viewer", "secondsToLive": 1}, |
||||
expectedCode: http.StatusOK, |
||||
}, |
||||
{ |
||||
desc: "serviceaccount token should match SA orgID and SA provided in parameters even if specified in body", |
||||
acmock: tests.SetupMockAccesscontrol( |
||||
t, |
||||
func(c context.Context, siu *models.SignedInUser) ([]*accesscontrol.Permission, error) { |
||||
return []*accesscontrol.Permission{{Action: serviceaccounts.ActionWrite, Scope: serviceaccounts.ScopeAll}}, nil |
||||
}, |
||||
false, |
||||
), |
||||
body: map[string]interface{}{"name": "Test2", "role": "Viewer", "secondsToLive": 1, "orgId": 4, "serviceAccountId": 4}, |
||||
expectedCode: http.StatusOK, |
||||
}, |
||||
{ |
||||
desc: "should be ok to create serviceaccount token with scope id permissions", |
||||
acmock: tests.SetupMockAccesscontrol( |
||||
t, |
||||
func(c context.Context, siu *models.SignedInUser) ([]*accesscontrol.Permission, error) { |
||||
return []*accesscontrol.Permission{{Action: serviceaccounts.ActionWrite, Scope: "serviceaccounts:id:1"}}, nil |
||||
}, |
||||
false, |
||||
), |
||||
body: map[string]interface{}{"name": "Test3", "role": "Viewer", "secondsToLive": 1}, |
||||
expectedCode: http.StatusOK, |
||||
}, |
||||
{ |
||||
desc: "should be forbidden to create serviceaccount token if wrong scoped", |
||||
acmock: tests.SetupMockAccesscontrol( |
||||
t, |
||||
func(c context.Context, siu *models.SignedInUser) ([]*accesscontrol.Permission, error) { |
||||
return []*accesscontrol.Permission{{Action: serviceaccounts.ActionWrite, Scope: "serviceaccounts:id:2"}}, nil |
||||
}, |
||||
false, |
||||
), |
||||
body: map[string]interface{}{"name": "Test4", "role": "Viewer"}, |
||||
expectedCode: http.StatusForbidden, |
||||
}, |
||||
} |
||||
|
||||
var requestResponse = func(server *web.Mux, httpMethod, requestpath string, requestBody io.Reader) *httptest.ResponseRecorder { |
||||
req, err := http.NewRequest(httpMethod, requestpath, requestBody) |
||||
require.NoError(t, err) |
||||
recorder := httptest.NewRecorder() |
||||
server.ServeHTTP(recorder, req) |
||||
return recorder |
||||
} |
||||
|
||||
for _, tc := range testCases { |
||||
t.Run(tc.desc, func(t *testing.T) { |
||||
endpoint := fmt.Sprintf(serviceaccountIDTokensPath, sa.Id) |
||||
bodyString := "" |
||||
if tc.body != nil { |
||||
b, err := json.Marshal(tc.body) |
||||
require.NoError(t, err) |
||||
bodyString = string(b) |
||||
} |
||||
|
||||
server := setupTestServer(t, &svcmock, routing.NewRouteRegister(), tc.acmock, store) |
||||
actual := requestResponse(server, http.MethodPost, endpoint, strings.NewReader(bodyString)) |
||||
|
||||
actualCode := actual.Code |
||||
actualBody := map[string]interface{}{} |
||||
|
||||
err := json.Unmarshal(actual.Body.Bytes(), &actualBody) |
||||
require.NoError(t, err) |
||||
require.Equal(t, tc.expectedCode, actualCode, endpoint, actualBody) |
||||
|
||||
if actualCode == http.StatusOK { |
||||
assert.Equal(t, tc.body["name"], actualBody["name"]) |
||||
|
||||
query := models.GetApiKeyByNameQuery{KeyName: tc.body["name"].(string), OrgId: sa.OrgId} |
||||
err = store.GetApiKeyByName(context.Background(), &query) |
||||
require.NoError(t, err) |
||||
|
||||
assert.Equal(t, sa.Id, *query.Result.ServiceAccountId) |
||||
assert.Equal(t, sa.OrgId, query.Result.OrgId) |
||||
} |
||||
}) |
||||
} |
||||
} |
||||
|
||||
func TestServiceAccountsAPI_DeleteToken(t *testing.T) { |
||||
store := sqlstore.InitTestDB(t) |
||||
svcmock := tests.ServiceAccountMock{} |
||||
sa := tests.SetupUserServiceAccount(t, store, tests.TestUser{Login: "sa", IsServiceAccount: true}) |
||||
|
||||
type testCreateSAToken struct { |
||||
desc string |
||||
keyName string |
||||
expectedCode int |
||||
acmock *accesscontrolmock.Mock |
||||
} |
||||
|
||||
testCases := []testCreateSAToken{ |
||||
{ |
||||
desc: "should be ok to delete serviceaccount token with scope id permissions", |
||||
keyName: "Test1", |
||||
acmock: tests.SetupMockAccesscontrol( |
||||
t, |
||||
func(c context.Context, siu *models.SignedInUser) ([]*accesscontrol.Permission, error) { |
||||
return []*accesscontrol.Permission{{Action: serviceaccounts.ActionWrite, Scope: "serviceaccounts:id:1"}}, nil |
||||
}, |
||||
false, |
||||
), |
||||
expectedCode: http.StatusOK, |
||||
}, |
||||
{ |
||||
desc: "should be ok to delete serviceaccount token with scope all permissions", |
||||
keyName: "Test2", |
||||
acmock: tests.SetupMockAccesscontrol( |
||||
t, |
||||
func(c context.Context, siu *models.SignedInUser) ([]*accesscontrol.Permission, error) { |
||||
return []*accesscontrol.Permission{{Action: serviceaccounts.ActionWrite, Scope: serviceaccounts.ScopeAll}}, nil |
||||
}, |
||||
false, |
||||
), |
||||
expectedCode: http.StatusOK, |
||||
}, |
||||
{ |
||||
desc: "should be forbidden to delete serviceaccount token if wrong scoped", |
||||
keyName: "Test3", |
||||
acmock: tests.SetupMockAccesscontrol( |
||||
t, |
||||
func(c context.Context, siu *models.SignedInUser) ([]*accesscontrol.Permission, error) { |
||||
return []*accesscontrol.Permission{{Action: serviceaccounts.ActionWrite, Scope: "serviceaccounts:id:10"}}, nil |
||||
}, |
||||
false, |
||||
), |
||||
expectedCode: http.StatusForbidden, |
||||
}, |
||||
} |
||||
|
||||
var requestResponse = func(server *web.Mux, httpMethod, requestpath string, requestBody io.Reader) *httptest.ResponseRecorder { |
||||
req, err := http.NewRequest(httpMethod, requestpath, requestBody) |
||||
require.NoError(t, err) |
||||
recorder := httptest.NewRecorder() |
||||
server.ServeHTTP(recorder, req) |
||||
return recorder |
||||
} |
||||
|
||||
for _, tc := range testCases { |
||||
t.Run(tc.desc, func(t *testing.T) { |
||||
token := createTokenforSA(t, tc.keyName, sa.OrgId, sa.Id) |
||||
|
||||
endpoint := fmt.Sprintf(serviceaccountIDTokensDetailPath, sa.Id, token.Id) |
||||
bodyString := "" |
||||
server := setupTestServer(t, &svcmock, routing.NewRouteRegister(), tc.acmock, store) |
||||
actual := requestResponse(server, http.MethodDelete, endpoint, strings.NewReader(bodyString)) |
||||
|
||||
actualCode := actual.Code |
||||
actualBody := map[string]interface{}{} |
||||
|
||||
_ = json.Unmarshal(actual.Body.Bytes(), &actualBody) |
||||
require.Equal(t, tc.expectedCode, actualCode, endpoint, actualBody) |
||||
|
||||
query := models.GetApiKeyByNameQuery{KeyName: tc.keyName, OrgId: sa.OrgId} |
||||
err := store.GetApiKeyByName(context.Background(), &query) |
||||
if actualCode == http.StatusOK { |
||||
require.Error(t, err) |
||||
} else { |
||||
require.NoError(t, err) |
||||
} |
||||
}) |
||||
} |
||||
} |
||||
Loading…
Reference in new issue