Add introspect (can hopefully be used for NGINX)#174
Conversation
rien
left a comment
rien
left a comment
There was a problem hiding this comment.
This PR is looking okay, but there is a minor security issue: I think any client could try to fish for valid tokens for other clients (see the respective comment).
Can you:
- Add a test confirming whether this is possible
- Add a check whether the session belongs to the client doing the introspection
Additionally, we might want to consider making this route optional through the configuration. I wonder, in which scenario is this used? Do you think this is a common requirement?
| Client::find_and_authenticate(auth.0.to_string(), &auth.1, &db).await?; | ||
|
|
||
| let IntrospectFormData { token, .. } = form.into_inner(); | ||
| let maybe_session = Session::find_by_key(token, &db).await; | ||
| Ok(Json(IntrospectResp { | ||
| active: maybe_session.is_ok(), | ||
| })) |
There was a problem hiding this comment.
I think this is missing the following check required by the RFC:
o If the token can be used only at certain resource servers, the
authorization server MUST determine whether or not the token can
be used at the resource server making the introspection call.
With the current code, any client could try to fish for valid tokens.
The other checks required are checked by the Session::find_by_key function (token is not expired, not invalidated, ...).
There was a problem hiding this comment.
Good catch! I'll look at it at a later time, I don't think this pr is high prio
| response.into_string().await.expect("response body"); | ||
| let data: Value = | ||
| serde_json::from_str(&response_body).expect("response json values"); | ||
| assert_eq!(data["active"], false); |
There was a problem hiding this comment.
| assert_eq!(data["active"], false); | |
| assert_eq!(data["active"], false, "introspect with invalid token should not be active"); |
| response.into_string().await.expect("response body"); | ||
| let data: Value = | ||
| serde_json::from_str(&response_body).expect("response json values"); | ||
| assert_eq!(data["active"], true); |
There was a problem hiding this comment.
| assert_eq!(data["active"], true); | |
| assert_eq!(data["active"], true, "introspect with valid token should be active"); |
|
|
||
| http_client.post("/logout").dispatch().await; | ||
|
|
||
| let form_body = format!("token=1234"); |
There was a problem hiding this comment.
| let form_body = format!("token=1234"); | |
| let form_body = format!("token={}", token); |
I think you meant to test if the token is invalidated?
There was a problem hiding this comment.
yes and the test fails 😢
| response.into_string().await.expect("response body"); | ||
| let data: Value = | ||
| serde_json::from_str(&response_body).expect("response json values"); | ||
| assert_eq!(data["active"], false); |
There was a problem hiding this comment.
| assert_eq!(data["active"], false); | |
| assert_eq!(data["active"], false, "introspection on invalidated token should return false"); |
2 participants
Standard: https://datatracker.ietf.org/doc/html/rfc7662#page-4
Hopefully works with https://www.nginx.com/blog/validating-oauth-2-0-access-tokens-nginx/