Auth: Allow expiration of API keys (#17678)
* Modify backend to allow expiration of API Keys * Add middleware test for expired api keys * Modify frontend to enable expiration of API Keys * Fix frontend tests * Fix migration and add index for `expires` field * Add api key tests for database access * Substitude time.Now() by a mock for test usage * Front-end modifications * Change input label to `Time to live` * Change input behavior to comply with the other similar * Add tooltip * Modify AddApiKey api call response Expiration should be *time.Time instead of string * Present expiration date in the selected timezone * Use kbn for transforming intervals to seconds * Use `assert` library for tests * Frontend fixes Add checks for empty/undefined/null values * Change expires column from datetime to integer * Restrict api key duration input It should be interval not number * AddApiKey must complain if SecondsToLive is negative * Declare ErrInvalidApiKeyExpiration * Move configuration to auth section * Update docs * Eliminate alias for models in modified files * Omit expiration from api response if empty * Eliminate Goconvey from test file * Fix test Do not sleep, use mocked timeNow() instead * Remove index for expires from api_key table The index should be anyway on both org_id and expires fields. However this commit eliminates completely the index for now since not many rows are expected to be in this table. * Use getTimeZone function * Minor change in api key listing The frontend should display a message instead of empty string if the key does not expire.
This commit is contained in:
@@ -5,7 +5,7 @@ import (
|
||||
"time"
|
||||
|
||||
"github.com/grafana/grafana/pkg/bus"
|
||||
m "github.com/grafana/grafana/pkg/models"
|
||||
"github.com/grafana/grafana/pkg/models"
|
||||
)
|
||||
|
||||
func init() {
|
||||
@@ -16,14 +16,18 @@ func init() {
|
||||
bus.AddHandler("sql", AddApiKey)
|
||||
}
|
||||
|
||||
func GetApiKeys(query *m.GetApiKeysQuery) error {
|
||||
sess := x.Limit(100, 0).Where("org_id=?", query.OrgId).Asc("name")
|
||||
func GetApiKeys(query *models.GetApiKeysQuery) error {
|
||||
sess := x.Limit(100, 0).Where("org_id=? and ( expires IS NULL or expires >= ?)",
|
||||
query.OrgId, timeNow().Unix()).Asc("name")
|
||||
if query.IncludeInvalid {
|
||||
sess = x.Limit(100, 0).Where("org_id=?", query.OrgId).Asc("name")
|
||||
}
|
||||
|
||||
query.Result = make([]*m.ApiKey, 0)
|
||||
query.Result = make([]*models.ApiKey, 0)
|
||||
return sess.Find(&query.Result)
|
||||
}
|
||||
|
||||
func DeleteApiKeyCtx(ctx context.Context, cmd *m.DeleteApiKeyCommand) error {
|
||||
func DeleteApiKeyCtx(ctx context.Context, cmd *models.DeleteApiKeyCommand) error {
|
||||
return withDbSession(ctx, func(sess *DBSession) error {
|
||||
var rawSql = "DELETE FROM api_key WHERE id=? and org_id=?"
|
||||
_, err := sess.Exec(rawSql, cmd.Id, cmd.OrgId)
|
||||
@@ -31,15 +35,24 @@ func DeleteApiKeyCtx(ctx context.Context, cmd *m.DeleteApiKeyCommand) error {
|
||||
})
|
||||
}
|
||||
|
||||
func AddApiKey(cmd *m.AddApiKeyCommand) error {
|
||||
func AddApiKey(cmd *models.AddApiKeyCommand) error {
|
||||
return inTransaction(func(sess *DBSession) error {
|
||||
t := m.ApiKey{
|
||||
updated := timeNow()
|
||||
var expires *int64 = nil
|
||||
if cmd.SecondsToLive > 0 {
|
||||
v := updated.Add(time.Second * time.Duration(cmd.SecondsToLive)).Unix()
|
||||
expires = &v
|
||||
} else if cmd.SecondsToLive < 0 {
|
||||
return models.ErrInvalidApiKeyExpiration
|
||||
}
|
||||
t := models.ApiKey{
|
||||
OrgId: cmd.OrgId,
|
||||
Name: cmd.Name,
|
||||
Role: cmd.Role,
|
||||
Key: cmd.Key,
|
||||
Created: time.Now(),
|
||||
Updated: time.Now(),
|
||||
Created: updated,
|
||||
Updated: updated,
|
||||
Expires: expires,
|
||||
}
|
||||
|
||||
if _, err := sess.Insert(&t); err != nil {
|
||||
@@ -50,28 +63,28 @@ func AddApiKey(cmd *m.AddApiKeyCommand) error {
|
||||
})
|
||||
}
|
||||
|
||||
func GetApiKeyById(query *m.GetApiKeyByIdQuery) error {
|
||||
var apikey m.ApiKey
|
||||
func GetApiKeyById(query *models.GetApiKeyByIdQuery) error {
|
||||
var apikey models.ApiKey
|
||||
has, err := x.Id(query.ApiKeyId).Get(&apikey)
|
||||
|
||||
if err != nil {
|
||||
return err
|
||||
} else if !has {
|
||||
return m.ErrInvalidApiKey
|
||||
return models.ErrInvalidApiKey
|
||||
}
|
||||
|
||||
query.Result = &apikey
|
||||
return nil
|
||||
}
|
||||
|
||||
func GetApiKeyByName(query *m.GetApiKeyByNameQuery) error {
|
||||
var apikey m.ApiKey
|
||||
func GetApiKeyByName(query *models.GetApiKeyByNameQuery) error {
|
||||
var apikey models.ApiKey
|
||||
has, err := x.Where("org_id=? AND name=?", query.OrgId, query.KeyName).Get(&apikey)
|
||||
|
||||
if err != nil {
|
||||
return err
|
||||
} else if !has {
|
||||
return m.ErrInvalidApiKey
|
||||
return models.ErrInvalidApiKey
|
||||
}
|
||||
|
||||
query.Result = &apikey
|
||||
|
||||
@@ -1,31 +1,117 @@
|
||||
package sqlstore
|
||||
|
||||
import (
|
||||
"github.com/grafana/grafana/pkg/models"
|
||||
"github.com/stretchr/testify/assert"
|
||||
"testing"
|
||||
|
||||
. "github.com/smartystreets/goconvey/convey"
|
||||
|
||||
m "github.com/grafana/grafana/pkg/models"
|
||||
"time"
|
||||
)
|
||||
|
||||
func TestApiKeyDataAccess(t *testing.T) {
|
||||
mockTimeNow()
|
||||
defer resetTimeNow()
|
||||
|
||||
Convey("Testing API Key data access", t, func() {
|
||||
t.Run("Testing API Key data access", func(t *testing.T) {
|
||||
InitTestDB(t)
|
||||
|
||||
Convey("Given saved api key", func() {
|
||||
cmd := m.AddApiKeyCommand{OrgId: 1, Name: "hello", Key: "asd"}
|
||||
t.Run("Given saved api key", func(t *testing.T) {
|
||||
cmd := models.AddApiKeyCommand{OrgId: 1, Name: "hello", Key: "asd"}
|
||||
err := AddApiKey(&cmd)
|
||||
So(err, ShouldBeNil)
|
||||
assert.Nil(t, err)
|
||||
|
||||
Convey("Should be able to get key by name", func() {
|
||||
query := m.GetApiKeyByNameQuery{KeyName: "hello", OrgId: 1}
|
||||
t.Run("Should be able to get key by name", func(t *testing.T) {
|
||||
query := models.GetApiKeyByNameQuery{KeyName: "hello", OrgId: 1}
|
||||
err = GetApiKeyByName(&query)
|
||||
|
||||
So(err, ShouldBeNil)
|
||||
So(query.Result, ShouldNotBeNil)
|
||||
assert.Nil(t, err)
|
||||
assert.NotNil(t, query.Result)
|
||||
})
|
||||
|
||||
})
|
||||
|
||||
t.Run("Add non expiring key", func(t *testing.T) {
|
||||
cmd := models.AddApiKeyCommand{OrgId: 1, Name: "non-expiring", Key: "asd1", SecondsToLive: 0}
|
||||
err := AddApiKey(&cmd)
|
||||
assert.Nil(t, err)
|
||||
|
||||
query := models.GetApiKeyByNameQuery{KeyName: "non-expiring", OrgId: 1}
|
||||
err = GetApiKeyByName(&query)
|
||||
assert.Nil(t, err)
|
||||
|
||||
assert.Nil(t, query.Result.Expires)
|
||||
})
|
||||
|
||||
t.Run("Add an expiring key", func(t *testing.T) {
|
||||
//expires in one hour
|
||||
cmd := models.AddApiKeyCommand{OrgId: 1, Name: "expiring-in-an-hour", Key: "asd2", SecondsToLive: 3600}
|
||||
err := AddApiKey(&cmd)
|
||||
assert.Nil(t, err)
|
||||
|
||||
query := models.GetApiKeyByNameQuery{KeyName: "expiring-in-an-hour", OrgId: 1}
|
||||
err = GetApiKeyByName(&query)
|
||||
assert.Nil(t, err)
|
||||
|
||||
assert.True(t, *query.Result.Expires >= timeNow().Unix())
|
||||
|
||||
// timeNow() has been called twice since creation; once by AddApiKey and once by GetApiKeyByName
|
||||
// therefore two seconds should be subtracted by next value retuned by timeNow()
|
||||
// that equals the number by which timeSeed has been advanced
|
||||
then := timeNow().Add(-2 * time.Second)
|
||||
expected := then.Add(1 * time.Hour).UTC().Unix()
|
||||
assert.Equal(t, *query.Result.Expires, expected)
|
||||
})
|
||||
|
||||
t.Run("Add a key with negative lifespan", func(t *testing.T) {
|
||||
//expires in one day
|
||||
cmd := models.AddApiKeyCommand{OrgId: 1, Name: "key-with-negative-lifespan", Key: "asd3", SecondsToLive: -3600}
|
||||
err := AddApiKey(&cmd)
|
||||
assert.EqualError(t, err, models.ErrInvalidApiKeyExpiration.Error())
|
||||
|
||||
query := models.GetApiKeyByNameQuery{KeyName: "key-with-negative-lifespan", OrgId: 1}
|
||||
err = GetApiKeyByName(&query)
|
||||
assert.EqualError(t, err, "Invalid API Key")
|
||||
})
|
||||
|
||||
t.Run("Add keys", func(t *testing.T) {
|
||||
//never expires
|
||||
cmd := models.AddApiKeyCommand{OrgId: 1, Name: "key1", Key: "key1", SecondsToLive: 0}
|
||||
err := AddApiKey(&cmd)
|
||||
assert.Nil(t, err)
|
||||
|
||||
//expires in 1s
|
||||
cmd = models.AddApiKeyCommand{OrgId: 1, Name: "key2", Key: "key2", SecondsToLive: 1}
|
||||
err = AddApiKey(&cmd)
|
||||
assert.Nil(t, err)
|
||||
|
||||
//expires in one hour
|
||||
cmd = models.AddApiKeyCommand{OrgId: 1, Name: "key3", Key: "key3", SecondsToLive: 3600}
|
||||
err = AddApiKey(&cmd)
|
||||
assert.Nil(t, err)
|
||||
|
||||
// advance mocked getTime by 1s
|
||||
timeNow()
|
||||
|
||||
query := models.GetApiKeysQuery{OrgId: 1, IncludeInvalid: false}
|
||||
err = GetApiKeys(&query)
|
||||
assert.Nil(t, err)
|
||||
|
||||
for _, k := range query.Result {
|
||||
if k.Name == "key2" {
|
||||
t.Fatalf("key2 should not be there")
|
||||
}
|
||||
}
|
||||
|
||||
query = models.GetApiKeysQuery{OrgId: 1, IncludeInvalid: true}
|
||||
err = GetApiKeys(&query)
|
||||
assert.Nil(t, err)
|
||||
|
||||
found := false
|
||||
for _, k := range query.Result {
|
||||
if k.Name == "key2" {
|
||||
found = true
|
||||
}
|
||||
}
|
||||
assert.True(t, found)
|
||||
})
|
||||
})
|
||||
}
|
||||
|
||||
@@ -78,4 +78,8 @@ func addApiKeyMigrations(mg *Migrator) {
|
||||
{Name: "key", Type: DB_Varchar, Length: 190, Nullable: false},
|
||||
{Name: "role", Type: DB_NVarchar, Length: 255, Nullable: false},
|
||||
}))
|
||||
|
||||
mg.AddMigration("Add expires to api_key table", NewAddColumnMigration(apiKeyV2, &Column{
|
||||
Name: "expires", Type: DB_BigInt, Nullable: true,
|
||||
}))
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user