-
Notifications
You must be signed in to change notification settings - Fork 0
feat(perms): Builder permissioning Tower
layer
#46
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
src/perms/middleware.rs
Outdated
StatusCode::BAD_REQUEST, | ||
Json(ApiError { | ||
error: "INVALID_HEADER_ENCODING", | ||
message: "Invalid header encoding", |
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.
Some Hint:
text in the human-readable strings would be great, re: expectations., what header is missing/invalid, and what does a valid one look like?
for next one,
why was permission denied?
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.
these are user facing API errors. I thought we wanted to be opaque here?
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.
we dont want to leak server internals by returning unmodified error objects that may contain things like stack traces. we do want to make them usefuk and helpful
src/perms/middleware.rs
Outdated
} | ||
|
||
info!(%sub, current_slot = %this.builders.calc().current_slot(), "builder permissioned successfully"); |
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.
same questions
src/perms/middleware.rs
Outdated
} | ||
}, | ||
None => { | ||
error!("builder request missing header"); |
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.
this is probably not error, as it is not a problem with our system
src/perms/middleware.rs
Outdated
Err(_) => { | ||
return Ok((StatusCode::BAD_REQUEST, "invalid header").into_response()); | ||
error!("builder request has invalid header encoding"); |
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.
this is probably not error, as it is not a problem with our system
2b9d2d9
to
91c4c4e
Compare
0f671dc
to
0dcec62
Compare
Closes ENG-1165