Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

(BEDS-1173) using correct table for api keys #1293

Open
wants to merge 1 commit into
base: staging
Choose a base branch
from
+50 −14
Open

(BEDS-1173) using correct table for api keys #1293

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
25 changes: 22 additions & 3 deletions backend/pkg/api/api_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -95,7 +95,7 @@ func setup() error {

for _, user := range testUsers {
pHash, _ := bcrypt.GenerateFromPassword([]byte(user.Password), 10)
insertDs := goqu.Dialect("postgres").
userDs := goqu.Dialect("postgres").
Insert("users").
Rows(struct {
User
Expand All @@ -107,7 +107,26 @@ func setup() error {
string(pHash),
})

query, args, err := insertDs.Prepared(true).ToSQL()
apiKeyDs := goqu.Dialect("postgres").
Insert("api_keys").
Rows(struct {
ApiKey string `db:"api_key"`
UserId uint `db:"user_id"`
}{
user.ApiKey,
user.Id,
})

query, args, err := userDs.Prepared(true).ToSQL()
if err != nil {
return fmt.Errorf("error preparing query: %w", err)
}
_, err = tempDb.Exec(query, args...)
if err != nil {
return err
}

query, args, err = apiKeyDs.Prepared(true).ToSQL()
if err != nil {
return fmt.Errorf("error preparing query: %w", err)
}
Expand Down Expand Up @@ -503,7 +522,7 @@ func TestPublicAndSharedDashboards(t *testing.T) {
type User struct {
Email string `db:"email"`
Password string
ApiKey string `db:"api_key"`
ApiKey string
// optional
Id uint `db:"id" goqu:"omitempty"`
UserGroup string `db:"user_group" goqu:"omitempty"`
Expand Down
37 changes: 27 additions & 10 deletions backend/pkg/api/data_access/user.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -47,21 +47,38 @@ func (d *DataAccessService) GetUserByEmail(ctx context.Context, email string) (u

func (d *DataAccessService) CreateUser(ctx context.Context, email, password string) (uint64, error) {
// (password is already hashed)
var result uint64

apiKey, err := utils.GenerateRandomAPIKey()
var userId uint64
err := d.userWriter.GetContext(ctx, &userId, `
INSERT INTO users (password, email, register_ts)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does it make sense to wrap SQL query in goqu same as we did for test

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For simpler queries like this it's usually not required as it's not really worth it, with the additional error handling etc. readability isn't really increased. I'd consider it optional in such cases but if you insist, I can change it

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM as soon as we added test

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think the benefit of goqu is to validate the query before executing it. Wrapping or not is up to you.

VALUES ($1, $2, NOW())
RETURNING id`,
password, email,
)
if err != nil {
return 0, err
}
err = d.AddApiKey(ctx, userId, "")

err = d.userWriter.GetContext(ctx, &result, `
INSERT INTO users (password, email, register_ts, api_key)
VALUES ($1, $2, NOW(), $3)
RETURNING id`,
password, email, apiKey,
return userId, err
}

// generates new key if empty
func (d *DataAccessService) AddApiKey(ctx context.Context, userId uint64, apiKey string) error {
var err error
if apiKey == "" {
apiKey, err = utils.GenerateRandomAPIKey()
if err != nil {
return err
}
}

_, err = d.userWriter.ExecContext(ctx, `
INSERT INTO api_keys (api_key, user_id, valid_until, changed_at)
VALUES ($1, $2, to_timestamp('9999-12-31 23:59:59', 'YYYY-MM-DD HH24:MI:SS'), NOW())`,
apiKey, userId,
)

return result, err
return err
}

func (d *DataAccessService) RemoveUser(ctx context.Context, userId uint64) error {
Expand Down Expand Up @@ -228,7 +245,7 @@ func (d *DataAccessService) GetUserCredentialInfo(ctx context.Context, userId ui

func (d *DataAccessService) GetUserIdByApiKey(ctx context.Context, apiKey string) (uint64, error) {
var userId uint64
err := d.userReader.GetContext(ctx, &userId, `SELECT user_id FROM api_keys WHERE api_key = $1 LIMIT 1`, apiKey)
err := d.userReader.GetContext(ctx, &userId, `SELECT user_id FROM api_keys WHERE api_key = $1 AND NOW() < valid_until`, apiKey)
if errors.Is(err, sql.ErrNoRows) {
return 0, fmt.Errorf("%w: user for api_key not found", ErrNotFound)
}
Expand Down
2 changes: 1 addition & 1 deletion backend/pkg/commons/db/user.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -119,7 +119,7 @@ func GetUserInfo(ctx context.Context, userId uint64, userDbReader *sqlx.DB) (*t.

userInfo.Email = utils.CensorEmail(userInfo.Email)

err = userDbReader.SelectContext(ctx, &userInfo.ApiKeys, `SELECT api_key FROM api_keys WHERE user_id = $1`, userId)
err = userDbReader.SelectContext(ctx, &userInfo.ApiKeys, `SELECT api_key FROM api_keys WHERE user_id = $1 AND NOW() < valid_until`, userId)
if err != nil && err != sql.ErrNoRows {
return nil, fmt.Errorf("error getting userApiKeys for user %v: %w", userId, err)
}
Expand Down
Loading