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

Automatic verification of access token hash. #174

Open
gibbz00 opened this issue Jun 5, 2024 · 1 comment
Open

Automatic verification of access token hash. #174

gibbz00 opened this issue Jun 5, 2024 · 1 comment

Comments

@gibbz00
Copy link

gibbz00 commented Jun 5, 2024

From the root module example:

let id_token_verifier = client.id_token_verifier();
let claims = id_token.claims(&id_token_verifier, &nonce)?;

// Verify the access token hash to ensure that the access token hasn't been substituted for
// another user's.
if let Some(expected_access_token_hash) = claims.access_token_hash() {
    let actual_access_token_hash = AccessTokenHash::from_token(
        token_response.access_token(),
        id_token.signing_alg()?,
        id_token.signing_key(&id_token_verifier)?,
    )?;
    if actual_access_token_hash != *expected_access_token_hash {
        return Err(anyhow!("Invalid access token"));
    }
}

Would it perhaps be possible to have this validation be done "automatically", (as in when configured), in client.exchange_code(...), and the necessary values exists?

@ramosbugs
Copy link
Owner

exchange_code() doesn't currently have access to the ID token due to the way the generic types are defined (see the openidconnect::TokenResponse trait). however, even if it did, accessing the (verified) ID token claims requires an IdTokenVerifier and nonce verifier, which would need to be added to the exchange_code() API. I'm hesitant to verify the at_hash using unverified ID token claims, since verification errors could be more useful to users than just seeing that the hash doesn't match.

the proposed changes would make exchange_code() fetch and verify the response in a single call, but users may want access to the raw ID token before trying to verify it (e.g., for better logging and error handling).

I agree that it would be nice for users not to have to manually verify the at_hash, but I'm not sure how to do that without introducing even more undesirable trade-offs. I'd be open to other API suggestions though, to see if we can make at_hash verification less manual without sacrificing flexibility and observability. I should also note that if we're verifying at_hash automatically, it seems like we should do the same for c_hash.

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

No branches or pull requests

2 participants