Skip to content

[Auth] Cover the auth middleware with tests#25

Merged
MohamedBassem merged 1 commit intomainfrom
pr25
Aug 1, 2023
Merged

[Auth] Cover the auth middleware with tests#25
MohamedBassem merged 1 commit intomainfrom
pr25

Conversation

@MohamedBassem
Copy link
Contributor

[Auth] Cover the auth middleware with tests

AhmedSoliman
AhmedSoliman previously approved these changes Jul 31, 2023
Copy link
Contributor

@AhmedSoliman AhmedSoliman left a comment

Choose a reason for hiding this comment

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

Thanks for covering those with tests 💯

Comment on lines +23 to +30
impl FromRef<Arc<AppState>> for AuthenticationState {
fn from_ref(input: &Arc<AppState>) -> Self {
Self {
authenticator: input.authenticator.clone(),
config: input.context.service_config(),
}
}
}
Copy link
Contributor

Choose a reason for hiding this comment

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

I thought FromRef was implemented for all T: Clone already, no? I wonder why this wasn't the case for AuthenticationState.

Also, did you try to #[derive(FromRef, Clone)]?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

FromRef<T> for T is implemented for all Ts where T: Clone but here I'm defining how to get an AuthenticationState (custom struct) from an AppState, there's no way the compiler can infer this on its own or even with a derive macro, right?

I think the derive macro might work only if I'm getting a state member from the state struct, but here it's a completely custom new struct.

/// AuthenticationStatus for a certain route.
pub async fn authenticate<B>(
State(state): State<Arc<AppState>>,
State(state): State<AuthenticationState>,
Copy link
Contributor

Choose a reason for hiding this comment

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

Very happy that you started to use partial states 🚀

unknown_secret_key: StatusCode,
}

async fn run_tests(
Copy link
Contributor

Choose a reason for hiding this comment

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

Should we put #[track_caller] here to get helpful messages when assertions fail? Not super sure if this works with async functions though.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

TIL. But it doesn't work with track_caller indeed: rust-lang/rust#87417

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants