-
Notifications
You must be signed in to change notification settings - Fork 1.9k
Add capability API controller, refactor test code #19459
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.
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
Conversation
| "github.com/smartcontractkit/chainlink/v2/core/capabilities/vault/vaulttypes" | ||
| ) | ||
|
|
||
| func FetchVaultPublicKey(t *testing.T, gatewayURL string) (publicKey string) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we keep this private unless there's some reason for it to be public? (I don't see anything in this PR but I may have missed it)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Will result in downstream duplication as we use this in our E2E test, DM'ing. If you prefer it stay private, that's fine, just some duplicate code.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah no then it's OK 👍
core/web/router.go
Outdated
| authv2.GET("/find_lca", auth.RequiresRunRole(lcaC.FindLCA)) | ||
|
|
||
| capContr := CapabilityController{app} | ||
| authv2.POST("/execute_capability", auth.RequiresRunRole(capContr.ExecuteCapability)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hmmm... the implementation of RequiresRunRole is suspicious:
// RequiresRunRole extracts the user object from the context, and asserts the user's role is at least
// 'run'
func RequiresRunRole(handler func(*gin.Context)) func(*gin.Context) {
return func(c *gin.Context) {
user, ok := GetAuthenticatedUser(c)
if !ok {
c.Abort()
jsonAPIError(c, http.StatusUnauthorized, errors.New("not a valid session"))
return
}
if user.Role == clsessions.UserRoleView {
c.Abort()
jsonAPIError(c, http.StatusUnauthorized, errors.New("Unauthorized"))
return
}
handler(c)
}
}
Looks like any view role user can execute this, probably a long-standing bug that we would fix.
I'm not sure we would want to enable this endpoint in production builds -- can we use build.IsDev() or similar and only register it if that's true?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done: 8e811fd.
|




This PR adds a capability API web controller that can be used to make calls to the capabilities on the node directly. It is useful for E2E tests.
The PR also does some refactoring on test code:
writer_don_load_test.goandworkflow_don_load_test.goAll of which makes it easier to test 3rd party capabilities on CRE.