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

Resolve incorrect handling of tokens in logout flow #2795

Merged
merged 3 commits into from
May 28, 2024
Merged
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
70 changes: 41 additions & 29 deletions tools/cli/src/cli/session.rs
Original file line number Diff line number Diff line change
Expand Up @@ -589,7 +589,17 @@ impl LogoutOpt {
}

pub async fn exec(&self) {
let mut tokens = read_tokens(&self.copt.get_token_cache_path()).unwrap_or_else(|_| {
error!("Error retrieving authentication token store");
std::process::exit(1);
});

let instance_name = &self.copt.instance;
let n_lookup = instance_name.clone().unwrap_or_default();
let Some(token_instance) = tokens.instances.get_mut(&n_lookup) else {
println!("No sessions for instance {}", n_lookup);
return;
};

let spn: String = if self.local_only {
// For now we just remove this from the token store.
Expand Down Expand Up @@ -624,61 +634,63 @@ impl LogoutOpt {
std::process::exit(1);
}
};
// Parse it for the username.

if let Err(e) = client.logout().await {
error!("Failed to logout - {:?}", e);
std::process::exit(1);
}

// Server acked the logout, lets proceed with the local cleanup now.

// Parse it for the SPN. Annoying but it's what we have to do
// because we don't know what token was used in the lower to client calls.
let jwsc = match JwsCompact::from_str(&token) {
Ok(j) => j,
Err(err) => {
error!(?err, "Unable to parse token");
info!("The token can be removed locally with `--local-only`");
std::process::exit(1);
}
};

let jws_verifier = if let Some(pub_jwk) = jwsc.get_jwk_pubkey() {
match JwsEs256Verifier::try_from(pub_jwk) {
Ok(verifier) => verifier,
Err(err) => {
error!(?err, "Unable to configure jws verifier");
std::process::exit(1);
}
}
} else {
error!("Unable to access token public key");
let Some(key_id) = jwsc.kid() else {
error!("Invalid token, missing KeyID");
Firstyear marked this conversation as resolved.
Show resolved Hide resolved
info!("The token can be removed locally with `--local-only`");
std::process::exit(1);
};

let Some(pub_jwk) = token_instance.keys().get(key_id) else {
error!("Invalid instance, no signing keys are available");
info!("The token can be removed locally with `--local-only`");
std::process::exit(1);
};

let jws_verifier = match JwsEs256Verifier::try_from(pub_jwk) {
Ok(verifier) => verifier,
Err(err) => {
error!(?err, "Unable to configure jws verifier");
info!("The token can be removed locally with `--local-only`");
std::process::exit(1);
}
};

let uat = match jws_verifier.verify(&jwsc).and_then(|jws| {
jws.from_json::<UserAuthToken>().map_err(|serde_err| {
error!(?serde_err);
info!("The token can be removed locally with `--local-only`");
JwtError::InvalidJwt
})
}) {
Ok(uat) => uat,
Err(e) => {
error!(?e, "Unable to verify token signature, may be corrupt");
info!("The token can be removed locally with `--local-only`");
std::process::exit(1);
}
};

uat.spn
};

let mut tokens = read_tokens(&self.copt.get_token_cache_path()).unwrap_or_else(|_| {
error!("Error retrieving authentication token store");
std::process::exit(1);
});
// Now we know we have a valid(ish) token, call the server to do the logout.
if let Err(e) = client.logout().await {
error!("Failed to logout - {:?}", e);
std::process::exit(1);
}

let n_lookup = instance_name.clone().unwrap_or_default();
let Some(token_instance) = tokens.instances.get_mut(&n_lookup) else {
println!("No sessions for {}", spn);
return;
// Server acked the logout, lets proceed with the local cleanup now, return
// the spn so the outer parts know what to remove.
uat.spn
};

// Remove our old one
Expand Down
4 changes: 2 additions & 2 deletions tools/cli/src/opt/kanidm.rs
Original file line number Diff line number Diff line change
Expand Up @@ -782,8 +782,8 @@ pub struct ReauthOpt {
pub struct LogoutOpt {
#[clap(flatten)]
copt: CommonOpt,
#[clap(short, long, hide = true)]
/// Do not send the logout to the server - only remove the session token locally
#[clap(short, long)]
/// Do not send a logout request to the server - only remove the session token locally.
local_only: bool,
}

Expand Down
Loading