Skip to content
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

Add introspect (can hopefully be used for NGINX) #174

Open
wants to merge 3 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

30 changes: 30 additions & 0 deletions src/controllers/oauth_controller.rs
Original file line number Diff line number Diff line change
Expand Up @@ -316,3 +316,33 @@ pub async fn token(
}))
}
}

#[derive(Serialize, Debug)]
pub struct IntrospectResp {
active: bool,
}

#[derive(FromForm, Debug)]
pub struct IntrospectFormData {
token: String,
token_type_hint: Option<String>,
}

#[post("/oauth/introspect", data = "<form>")]
pub async fn introspect(
auth: Option<BasicAuthentication>,
form: Form<IntrospectFormData>,
db: DbConn,
) -> Result<Json<IntrospectResp>> {
let auth = auth
.map(|auth| (auth.user, auth.password))
.ok_or(ZauthError::from(OAuthError::InvalidRequest))?;

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(),
}))
Comment on lines +341 to +347
Copy link
Collaborator

Choose a reason for hiding this comment

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

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, ...).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good catch! I'll look at it at a later time, I don't think this pr is high prio

}
1 change: 1 addition & 0 deletions src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -101,6 +101,7 @@ fn assemble(rocket: Rocket<Build>) -> Rocket<Build> {
oauth_controller::grant_get,
oauth_controller::grant_post,
oauth_controller::token,
oauth_controller::introspect,
pages_controller::home_page,
sessions_controller::create_session,
sessions_controller::new_session,
Expand Down
62 changes: 59 additions & 3 deletions tests/oauth.rs
Original file line number Diff line number Diff line change
Expand Up @@ -170,6 +170,27 @@ async fn normal_flow() {

assert_eq!(response.status(), Status::SeeOther);

let credentials =
base64::encode(&format!("{}:{}", client_id, client.secret));

let form_body = format!("token=1234");
let req = http_client
.post("/oauth/introspect")
.header(ContentType::Form)
.header(Header::new(
"Authorization",
format!("Basic {}", credentials),
))
.body(form_body);

let response = req.dispatch().await;
assert_eq!(response.status(), Status::Ok);
let response_body =
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);
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
assert_eq!(data["active"], false);
assert_eq!(data["active"], false, "introspect with invalid token should not be active");


// 7a. Client requests access code while sending its credentials
// trough HTTP Auth.
let token_url = "/oauth/token";
Expand All @@ -178,9 +199,6 @@ async fn normal_flow() {
authorization_code, redirect_uri
);

let credentials =
base64::encode(&format!("{}:{}", client_id, client.secret));

let req = http_client
.post(token_url)
.header(ContentType::Form)
Expand Down Expand Up @@ -256,6 +274,24 @@ async fn normal_flow() {
assert_eq!(data["token_type"], "bearer");
let token = data["access_token"].as_str().expect("access token");

let form_body = format!("token={}", token);
let req = http_client
.post("/oauth/introspect")
.header(ContentType::Form)
.header(Header::new(
"Authorization",
format!("Basic {}", credentials),
))
.body(form_body);

let response = req.dispatch().await;
assert_eq!(response.status(), Status::Ok);
let response_body =
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);
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
assert_eq!(data["active"], true);
assert_eq!(data["active"], true, "introspect with valid token should be active");


let response = http_client
.get("/current_user")
.header(Accept::JSON)
Expand All @@ -276,6 +312,26 @@ async fn normal_flow() {

assert!(data["id"].is_number());
assert_eq!(data["username"], user_username);

http_client.post("/logout").dispatch().await;

let form_body = format!("token=1234");
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
let form_body = format!("token=1234");
let form_body = format!("token={}", token);

I think you meant to test if the token is invalidated?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yes and the test fails 😢

let req = http_client
.post("/oauth/introspect")
.header(ContentType::Form)
.header(Header::new(
"Authorization",
format!("Basic {}", credentials),
))
.body(form_body);

let response = req.dispatch().await;
assert_eq!(response.status(), Status::Ok);
let response_body =
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);
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
assert_eq!(data["active"], false);
assert_eq!(data["active"], false, "introspection on invalidated token should return false");

})
.await;
}