fix(oauth2): use the short name in the userinfo (#1259)

This implements #1023 for the userinfo endpoint which was an oversight
in #1043. I've also added a testcase to verify this behaviour
This commit is contained in:
Dominik Süß 2022-12-14 01:30:27 +01:00 committed by GitHub
parent aa67d6f08c
commit c036f81d4e
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23

View file

@ -1396,6 +1396,12 @@ impl Oauth2ResourceServersReadTransaction {
}
};
let preferred_username = if o2rs.prefer_short_username {
Some(account.name)
} else {
Some(account.spn)
};
let (email, email_verified) = if scopes.contains(&"email".to_string()) {
if let Some(mp) = account.mail_primary {
(Some(mp), Some(true))
@ -1429,9 +1435,8 @@ impl Oauth2ResourceServersReadTransaction {
s_claims: OidcClaims {
// Map from displayname
name: Some(account.displayname.clone()),
// Map from spn
preferred_username: Some(account.spn),
scopes: scopes,
preferred_username,
email,
email_verified,
..Default::default()
@ -1644,6 +1649,7 @@ mod tests {
ct: Duration,
enable_pkce: bool,
enable_legacy_crypto: bool,
prefer_short_username: bool,
) -> (String, UserAuthToken, Identity, Uuid) {
let mut idms_prox_write = task::block_on(idms.proxy_write(ct));
@ -1686,6 +1692,10 @@ mod tests {
(
"oauth2_jwt_legacy_crypto_enable",
Value::new_bool(enable_legacy_crypto)
),
(
"oauth2_prefer_short_username",
Value::new_bool(prefer_short_username)
)
);
let ce = CreateEvent::new_internal(vec![e]);
@ -1749,7 +1759,8 @@ mod tests {
run_idm_test!(
|_qs: &QueryServer, idms: &IdmServer, idms_delayed: &mut IdmServerDelayed| {
let ct = Duration::from_secs(TEST_CURRENT_TIME);
let (secret, uat, ident, _) = setup_oauth2_resource_server(idms, ct, true, false);
let (secret, uat, ident, _) =
setup_oauth2_resource_server(idms, ct, true, false, false);
let idms_prox_read = task::block_on(idms.proxy_read());
@ -1820,7 +1831,8 @@ mod tests {
_idms_delayed: &mut IdmServerDelayed| {
// Test invalid oauth2 authorisation states/requests.
let ct = Duration::from_secs(TEST_CURRENT_TIME);
let (_secret, uat, ident, _) = setup_oauth2_resource_server(idms, ct, true, false);
let (_secret, uat, ident, _) =
setup_oauth2_resource_server(idms, ct, true, false, false);
let (anon_uat, anon_ident) = setup_idm_admin(idms, ct, AuthType::Anonymous);
let (idm_admin_uat, idm_admin_ident) = setup_idm_admin(idms, ct, AuthType::PasswordMfa);
@ -1984,7 +1996,8 @@ mod tests {
_idms_delayed: &mut IdmServerDelayed| {
// Test invalid oauth2 authorisation states/requests.
let ct = Duration::from_secs(TEST_CURRENT_TIME);
let (_secret, uat, ident, _) = setup_oauth2_resource_server(idms, ct, true, false);
let (_secret, uat, ident, _) =
setup_oauth2_resource_server(idms, ct, true, false, false);
let (uat2, ident2) = {
let mut idms_prox_write = task::block_on(idms.proxy_write(ct));
@ -2063,7 +2076,7 @@ mod tests {
|_qs: &QueryServer, idms: &IdmServer, idms_delayed: &mut IdmServerDelayed| {
let ct = Duration::from_secs(TEST_CURRENT_TIME);
let (secret, mut uat, ident, _) =
setup_oauth2_resource_server(idms, ct, true, false);
setup_oauth2_resource_server(idms, ct, true, false, false);
// ⚠️ We set the uat expiry time to 5 seconds from TEST_CURRENT_TIME. This
// allows all our other tests to pass, but it means when we specifically put the
@ -2237,7 +2250,8 @@ mod tests {
run_idm_test!(
|_qs: &QueryServer, idms: &IdmServer, idms_delayed: &mut IdmServerDelayed| {
let ct = Duration::from_secs(TEST_CURRENT_TIME);
let (secret, uat, ident, _) = setup_oauth2_resource_server(idms, ct, true, false);
let (secret, uat, ident, _) =
setup_oauth2_resource_server(idms, ct, true, false, false);
let client_authz = Some(base64::encode(format!("test_resource_server:{}", secret)));
let idms_prox_read = task::block_on(idms.proxy_read());
@ -2345,7 +2359,8 @@ mod tests {
|_qs: &QueryServer, idms: &IdmServer, idms_delayed: &mut IdmServerDelayed| {
// First, setup to get a token.
let ct = Duration::from_secs(TEST_CURRENT_TIME);
let (secret, uat, ident, _) = setup_oauth2_resource_server(idms, ct, true, false);
let (secret, uat, ident, _) =
setup_oauth2_resource_server(idms, ct, true, false, false);
let client_authz = Some(base64::encode(format!("test_resource_server:{}", secret)));
let idms_prox_read = task::block_on(idms.proxy_read());
@ -2519,7 +2534,8 @@ mod tests {
|_qs: &QueryServer, idms: &IdmServer, idms_delayed: &mut IdmServerDelayed| {
// First, setup to get a token.
let ct = Duration::from_secs(TEST_CURRENT_TIME);
let (secret, uat, ident, _) = setup_oauth2_resource_server(idms, ct, true, false);
let (secret, uat, ident, _) =
setup_oauth2_resource_server(idms, ct, true, false, false);
let client_authz = Some(base64::encode(format!("test_resource_server:{}", secret)));
let idms_prox_read = task::block_on(idms.proxy_read());
@ -2622,7 +2638,8 @@ mod tests {
idms: &IdmServer,
_idms_delayed: &mut IdmServerDelayed| {
let ct = Duration::from_secs(TEST_CURRENT_TIME);
let (_secret, uat, ident, _) = setup_oauth2_resource_server(idms, ct, true, false);
let (_secret, uat, ident, _) =
setup_oauth2_resource_server(idms, ct, true, false, false);
let (uat2, ident2) = {
let mut idms_prox_write = task::block_on(idms.proxy_write(ct));
@ -2707,7 +2724,8 @@ mod tests {
idms: &IdmServer,
_idms_delayed: &mut IdmServerDelayed| {
let ct = Duration::from_secs(TEST_CURRENT_TIME);
let (_secret, _uat, _ident, _) = setup_oauth2_resource_server(idms, ct, true, false);
let (_secret, _uat, _ident, _) =
setup_oauth2_resource_server(idms, ct, true, false, false);
let idms_prox_read = task::block_on(idms.proxy_read());
@ -2847,7 +2865,8 @@ mod tests {
run_idm_test!(
|_qs: &QueryServer, idms: &IdmServer, idms_delayed: &mut IdmServerDelayed| {
let ct = Duration::from_secs(TEST_CURRENT_TIME);
let (secret, uat, ident, _) = setup_oauth2_resource_server(idms, ct, true, false);
let (secret, uat, ident, _) =
setup_oauth2_resource_server(idms, ct, true, false, false);
let client_authz = Some(base64::encode(format!("test_resource_server:{}", secret)));
let idms_prox_read = task::block_on(idms.proxy_read());
@ -2971,6 +2990,97 @@ mod tests {
)
}
#[test]
fn test_idm_oauth2_openid_short_username() {
run_idm_test!(
|_qs: &QueryServer, idms: &IdmServer, idms_delayed: &mut IdmServerDelayed| {
// we run the same test as test_idm_oauth2_openid_extensions()
// but change the preferred_username setting on the RS
let ct = Duration::from_secs(TEST_CURRENT_TIME);
let (secret, uat, ident, _) =
setup_oauth2_resource_server(idms, ct, true, false, true);
let client_authz = Some(base64::encode(format!("test_resource_server:{}", secret)));
let idms_prox_read = task::block_on(idms.proxy_read());
let (code_verifier, code_challenge) = create_code_verifier!("Whar Garble");
let consent_request =
good_authorisation_request!(idms_prox_read, &ident, &uat, ct, code_challenge);
let consent_token =
if let AuthoriseResponse::ConsentRequested { consent_token, .. } =
consent_request
{
consent_token
} else {
unreachable!();
};
// == Manually submit the consent token to the permit for the permit_success
let permit_success = idms_prox_read
.check_oauth2_authorise_permit(&ident, &uat, &consent_token, ct)
.expect("Failed to perform oauth2 permit");
// Assert that the consent was submitted
match idms_delayed.async_rx.blocking_recv() {
Some(DelayedAction::Oauth2ConsentGrant(_)) => {}
_ => assert!(false),
}
// == Submit the token exchange code.
let token_req = AccessTokenRequest {
grant_type: "authorization_code".to_string(),
code: permit_success.code,
redirect_uri: Url::parse("https://demo.example.com/oauth2/result").unwrap(),
client_id: None,
client_secret: None,
// From the first step.
code_verifier,
};
let token_response = idms_prox_read
.check_oauth2_token_exchange(client_authz.as_deref(), &token_req, ct)
.expect("Failed to perform oauth2 token exchange");
// Assert that the session creation was submitted
match idms_delayed.async_rx.blocking_recv() {
Some(DelayedAction::Oauth2SessionRecord(_)) => {}
_ => assert!(false),
}
let id_token = token_response.id_token.expect("No id_token in response!");
let access_token = token_response.access_token;
let mut jwkset = idms_prox_read
.oauth2_openid_publickey("test_resource_server")
.expect("Failed to get public key");
let public_jwk = jwkset.keys.pop().expect("no such jwk");
let jws_validator =
JwsValidator::try_from(&public_jwk).expect("failed to build validator");
let oidc_unverified =
OidcUnverified::from_str(&id_token).expect("Failed to parse id_token");
let iat = ct.as_secs() as i64;
let oidc = oidc_unverified
.validate(&jws_validator, iat)
.expect("Failed to verify oidc");
// Do we have the short username in the token claims?
assert!(oidc.s_claims.preferred_username == Some("admin".to_string()));
// Do the id_token details line up to the userinfo?
let userinfo = idms_prox_read
.oauth2_openid_userinfo("test_resource_server", &access_token, ct)
.expect("failed to get userinfo");
assert!(oidc.s_claims == userinfo.s_claims);
}
)
}
// Check insecure pkce behaviour.
#[test]
fn test_idm_oauth2_insecure_pkce() {
@ -2978,7 +3088,8 @@ mod tests {
idms: &IdmServer,
_idms_delayed: &mut IdmServerDelayed| {
let ct = Duration::from_secs(TEST_CURRENT_TIME);
let (_secret, uat, ident, _) = setup_oauth2_resource_server(idms, ct, false, false);
let (_secret, uat, ident, _) =
setup_oauth2_resource_server(idms, ct, false, false, false);
let idms_prox_read = task::block_on(idms.proxy_read());
@ -3013,7 +3124,8 @@ mod tests {
run_idm_test!(
|_qs: &QueryServer, idms: &IdmServer, idms_delayed: &mut IdmServerDelayed| {
let ct = Duration::from_secs(TEST_CURRENT_TIME);
let (secret, uat, ident, _) = setup_oauth2_resource_server(idms, ct, false, true);
let (secret, uat, ident, _) =
setup_oauth2_resource_server(idms, ct, false, true, false);
let idms_prox_read = task::block_on(idms.proxy_read());
// The public key url should offer an rs key
// discovery should offer RS256
@ -3116,7 +3228,8 @@ mod tests {
run_idm_test!(
|_qs: &QueryServer, idms: &IdmServer, idms_delayed: &mut IdmServerDelayed| {
let ct = Duration::from_secs(TEST_CURRENT_TIME);
let (_secret, uat, ident, _) = setup_oauth2_resource_server(idms, ct, true, false);
let (_secret, uat, ident, _) =
setup_oauth2_resource_server(idms, ct, true, false, false);
let idms_prox_read = task::block_on(idms.proxy_read());
@ -3317,7 +3430,7 @@ mod tests {
|_qs: &QueryServer, idms: &IdmServer, idms_delayed: &mut IdmServerDelayed| {
let ct = Duration::from_secs(TEST_CURRENT_TIME);
let (_secret, uat, ident, o2rs_uuid) =
setup_oauth2_resource_server(idms, ct, true, false);
setup_oauth2_resource_server(idms, ct, true, false, false);
// Assert there are no consent maps yet.
assert!(ident.get_oauth2_consent_scopes(o2rs_uuid).is_none());
@ -3410,7 +3523,8 @@ mod tests {
|_qs: &QueryServer, idms: &IdmServer, idms_delayed: &mut IdmServerDelayed| {
let ct = Duration::from_secs(TEST_CURRENT_TIME);
// Enable pkce is set to FALSE
let (secret, uat, ident, _) = setup_oauth2_resource_server(idms, ct, false, false);
let (secret, uat, ident, _) =
setup_oauth2_resource_server(idms, ct, false, false, false);
let idms_prox_read = task::block_on(idms.proxy_read());
@ -3489,7 +3603,8 @@ mod tests {
|_qs: &QueryServer, idms: &IdmServer, idms_delayed: &mut IdmServerDelayed| {
let ct = Duration::from_secs(TEST_CURRENT_TIME);
// Enable pkce is set to FALSE
let (secret, uat, ident, _) = setup_oauth2_resource_server(idms, ct, false, false);
let (secret, uat, ident, _) =
setup_oauth2_resource_server(idms, ct, false, false, false);
let idms_prox_read = task::block_on(idms.proxy_read());