Log user id and token id when applicable. #2644
Conversation
In rust-lang/rfcs#2947 (comment), @pietroalbini says, with regards to restricting the default authorization of API tokens: > this would break everyone using the token to authenticate to endpoints > not used by Cargo. Even though we're not providing stability guarantees > for them I'd be wary of blindly breaking them. At least I'd like some > stats on which endpoints are accessed with the cookie and which are used > with tokens. This change attempts to provide those stats via the logs. All authenticated requests receive a `uid` field in the log output. If the request was authenticated via an API token, the log output additionally contains a `tokenid` field. Because the method I used, log_request::add_custom_metadata, requires a mutable request, I had to make req.authenticate() take a mutable ref. Since that conflicted with many call sites that were already holding an immutable ref to the request's DB connection, I moved the taking of the DB connection reference after the authenticate call. In crate_owner_invitations.rs and follow.rs, this also meant removing duplicate authenticate calls and passing through the already-authenticated user ID from a calling function instead. In a few places, `req.authenticate(&conn)?.find_user(&conn)?` has been replaced with two lines, one to do the authentication, and one to do the database lookup for the user object, after `let conn = req.db_conn()?`
|
Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @smarnach (or someone else) soon. If any changes to this PR are deemed necessary, please add them as extra commits. This ensures that the reviewer can see what has changed since they last reviewed the code. Due to the way GitHub handles out-of-date commits, this should also make it reasonably obvious what issues have or haven't been addressed. Large or tricky changes may require several passes of review and changes. Please see the contribution instructions for more information. |
|
Thanks! r? @pietroalbini |
|
📌 Commit bbadd1e has been approved by |
|
☀️ Test successful - checks-travis |
In rust-lang/rfcs#2947 (comment),
@pietroalbini says, with regards to restricting the default
authorization of API tokens:
This change attempts to provide those stats via the logs. All
authenticated requests receive a
uidfield in the log output. If therequest was authenticated via an API token, the log output additionally
contains a
tokenidfield.Because the method I used, log_request::add_custom_metadata, requires a
mutable request, I had to make req.authenticate() take a mutable ref.
Since that conflicted with many call sites that were already holding an
immutable ref to the request's DB connection, I moved the taking of the
DB connection reference after the authenticate call.
In crate_owner_invitations.rs and follow.rs, this also meant removing
duplicate authenticate calls and passing through the
already-authenticated user ID from a calling function instead.
In a few places,
req.authenticate(&conn)?.find_user(&conn)?has beenreplaced with two lines, one to do the authentication, and one to do the
database lookup for the user object, after
let conn = req.db_conn()?