[public-api] Implement regenerate token #14867
Conversation
jeanp413
changed the title
|
/werft run 👍 started the job as gitpod-build-jp-public-api-regenerate.2 |
jeanp413
force-pushed
the
jp/public-api-regenerate
branch
from
a796b47 to
5c62016
Compare
| err := db.Transaction(func(tx *gorm.DB) error { | ||
| txErr := tx. | ||
| Where("id = ?", tokenID). | ||
| Where("userId = ?", userID). | ||
| Where("deleted = ?", 0). | ||
| Select("hash", "expirationTime").Updates(PersonalAccessToken{Hash: hash, ExpirationTime: expirationTime}).Error | ||
| if txErr != nil { | ||
| return txErr | ||
| } | ||
|
|
||
| txErr = tx.Where("id = ?", tokenID).Where("userId = ?", userID).Where("deleted = ?", 0).First(&token).Error | ||
| if txErr != nil { | ||
| return txErr | ||
| } | ||
|
|
||
| return nil | ||
| }) |
There was a problem hiding this comment.
I believe we can write this query using https://gorm.io/docs/update.html#Returning-Data-From-Modified-Rows
This would remove the need for
- The explicit transaction
- The lookup afterwards
I'm just spinning up a workspace to test this out, will comment if I can make it work.
There was a problem hiding this comment.
Okay, looks like it doesn't work because it won't return the full PersonalAccessToken object, but only the rows which have actually changed. It's also dependant on the DB supporting Returning values.
There was a problem hiding this comment.
Okay, looks like it doesn't work because it won't return the full PersonalAccessToken object, but only the rows which have actually changed. It's also dependant on the DB supporting Returning values.
Yep, we tried it (clause.Returning{}) yesterday, and it was not worked as expected, so we switched to update and select to avoid transaction.
Update:
I saw JP switched it to transaction 😂, I will make it back to update and select only
There was a problem hiding this comment.
Okay, looks like it doesn't work because it won't return the full PersonalAccessToken object, but only the rows which have actually changed. It's also dependant on the DB supporting Returning values.
yep it's because mysql doesn't support returning sql clause
| require.Equal(t, origResponse.Msg.Token.Id, response.Msg.Token.Id) | ||
| require.NotEqual(t, "", response.Msg.Token.Value) | ||
| require.Equal(t, origResponse.Msg.Token.Name, response.Msg.Token.Name) | ||
| require.Equal(t, origResponse.Msg.Token.Description, response.Msg.Token.Description) | ||
| require.Equal(t, origResponse.Msg.Token.Scopes, response.Msg.Token.Scopes) | ||
| require.Equal(t, newTimestamp.AsTime(), response.Msg.Token.ExpirationTime.AsTime()) | ||
| require.Equal(t, origResponse.Msg.Token.CreatedAt, response.Msg.Token.CreatedAt) |
There was a problem hiding this comment.
As a follow-up (in another PR), we can add a helper method which compares the token and performs a custom check for the Value, ensuring it exists (and possibly trying to parse it) without matching on specific contents.
The alternative to be able to use require.Equal(t, struct, struct) would also be to have a seeded implementation of the Signer.
|
/hold |
mustard-mh
force-pushed
the
jp/public-api-regenerate
branch
from
69a2b63 to
6c8676b
Compare
Co-authored-by: Milan Pavlik <[email protected]> Co-authored-by: mustard <[email protected]>
mustard-mh
force-pushed
the
jp/public-api-regenerate
branch
from
6c8676b to
4917f6e
Compare
roboquat
added
deployed: webapp
Description
Implements regenerate token
Related Issue(s)
Fixes #14611
Release Notes
Documentation
Werft options:
If enabled this will build
install/previewValid options are
all,workspace,webapp,ide,jetbrains,vscode,ssh