refactor: add attestation middleware #506
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
gusinacio
changed the title
gusinacio
force-pushed
the
gustavo/tap-308-create-middleware-for-attestation
branch
2 times, most recently
from
7d8ebcb to
490fe8f
Compare
Base automatically changed from
gustavo/tap-307-create-middleware-for-auth
to
main
November 22, 2024 20:00
gusinacio
force-pushed
the
gustavo/tap-308-create-middleware-for-attestation
branch
from
490fe8f to
4db0df2
Compare
Pull Request Test Coverage Report for Build 12013926275Details
💛 - Coveralls |
Signed-off-by: Gustavo Inacio <[email protected]>
Signed-off-by: Gustavo Inacio <[email protected]>
Signed-off-by: Gustavo Inacio <[email protected]>
Signed-off-by: Gustavo Inacio <[email protected]>
Signed-off-by: Gustavo Inacio <[email protected]>
gusinacio
force-pushed
the
gustavo/tap-308-create-middleware-for-attestation
branch
from
91130fd to
36de277
Compare
anirudh2
requested changes
Nov 25, 2024
Member
anirudh2
left a comment
anirudh2
left a comment
There was a problem hiding this comment.
Looks mostly good! Just fix the docstring!
| attestation: Option<Attestation>, | ||
| } | ||
|
|
||
| /// Check if the query can be attestable generates attestation |
Member
There was a problem hiding this comment.
can be attestable generates attestation
This is worded strangely. I think you meant to say "is attestable?"
pedrohba1
reviewed
Nov 25, 2024
| let app = Router::new().route("/", get(handle)).layer(middleware); | ||
|
|
||
| // with signer | ||
| let res = app |
Contributor
There was a problem hiding this comment.
You use this same code three times in a row in the 3 tests in this file, I suggest you do the change below. Should economize a few lines.
// add helper send_request function
async fn send_test_request(app: &Router, signer: Option<AttestationSigner>) -> Response<Body> {
let mut request = Request::builder().uri("/").body(Body::empty()).unwrap();
if let Some(s) = signer {
request.extensions_mut().insert(s);
}
app.clone().oneshot(request).await.unwrap()
}
//then substitute according in the tests:
#[tokio::test]
async fn test_create_attestation() {
let (allocation, signer) = allocation_signer();
let middleware = from_fn(attestation_middleware);
let handle = move |_: Request<Body>| async move {
let mut res = Response::new(RESPONSE.to_string());
res.extensions_mut().insert(AttestationInput::Attestable {
req: REQUEST.to_string(),
});
res
};
let app = Router::new().route("/", get(handle)).layer(middleware);
// Use helper function to send request
let res = send_test_request(&app, Some(signer.clone())).await;
assert_eq!(res.status(), StatusCode::OK);
let response = payload_from_response(res).await;
assert_eq!(response.graphql_response, RESPONSE.to_string());
let attestation = response.attestation.unwrap();
assert!(signer
.verify(&attestation, REQUEST, RESPONSE, &allocation.id)
.is_ok());
}
#[tokio::test]
async fn test_non_assignable() {
let (_, signer) = allocation_signer();
let handle = move |_: Request<Body>| async move { Response::new(RESPONSE.to_string()) };
let middleware = from_fn(attestation_middleware);
let app = Router::new().route("/", get(handle)).layer(middleware);
// Use helper function to send request
let res = send_test_request(&app, Some(signer.clone())).await;
assert_eq!(res.status(), StatusCode::OK);
let response = payload_from_response(res).await;
assert_eq!(response.graphql_response, RESPONSE.to_string());
assert!(response.attestation.is_none());
}
#[tokio::test]
async fn test_no_signer() {
let handle = move |_: Request<Body>| async move { Response::new(RESPONSE.to_string()) };
let middleware = from_fn(attestation_middleware);
let app = Router::new().route("/", get(handle)).layer(middleware);
// Use helper function to send request without signer
let res = send_test_request(&app, None).await;
assert_eq!(res.status(), StatusCode::INTERNAL_SERVER_ERROR);
}Signed-off-by: Gustavo Inacio <[email protected]>
Signed-off-by: Gustavo Inacio <[email protected]>
Signed-off-by: Gustavo Inacio <[email protected]>
anirudh2
approved these changes
Nov 25, 2024
Member
anirudh2
left a comment
anirudh2
left a comment
There was a problem hiding this comment.
Good from my side. Once Pedro is good, feel free to merge!
pedrohba1
approved these changes
Nov 25, 2024
gusinacio
deleted the
gustavo/tap-308-create-middleware-for-attestation
branch
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Signed-off-by: Gustavo Inacio [email protected]Signed-off-by: Gustavo Inacio [email protected]