Skip to content
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
125 changes: 122 additions & 3 deletions src/internet_identity/src/attributes.rs
Original file line number Diff line number Diff line change
Expand Up @@ -42,9 +42,18 @@ impl Anchor {
self.openid_credentials
.iter()
.flat_map(|openid_credential| {
let scope = Some(AttributeScope::OpenId {
issuer: openid_credential.iss.clone(),
});
// It is important here to rely on the issuer ID as opposed to `openid_credential.iss`
// becasue, e.g., for Microsoft accounts, the issuer has a tenant ID parameter
Copy link

Copilot AI Mar 6, 2026

Choose a reason for hiding this comment

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

Typo in comment: "becasue" should be "because".

Suggested change
// becasue, e.g., for Microsoft accounts, the issuer has a tenant ID parameter
// because, e.g., for Microsoft accounts, the issuer has a tenant ID parameter

Copilot uses AI. Check for mistakes.
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Will fix this in a follow-up PR

// which should *not* be instantiated for a general Microsoft attribute scope.
let Some(issuer) = openid_credential.config_issuer() else {
ic_cdk::println!(
"No matching OpenID provider for issuer: {}, skipping credential",
openid_credential.iss
);
return BTreeSet::new();
};

let scope = Some(AttributeScope::OpenId { issuer });

let Some(attributes_to_fetch) = attributes.remove(&scope) else {
return BTreeSet::new();
Expand Down Expand Up @@ -1220,4 +1229,114 @@ mod tests {
);
}
}

/// Regression test for the `get_attributes` fix.
///
/// The bug: `get_attributes` built its scope key from `openid_credential.iss`
/// (the *resolved* issuer, e.g. `…/9188040d-…/v2.0` for Microsoft),
/// while callers pass the *template* issuer (`…/{tid}/v2.0`) as the scope
/// key. This caused `attributes.remove(&scope)` to always miss for
/// Microsoft credentials.
///
/// We test via `prepare_openid_attributes` because:
/// 1. It uses the identical `config_issuer()` lookup that the fix applied
/// to `get_attributes`.
/// 2. `get_attributes` calls into canister runtime (`state::assets_and_signatures`,
/// `ic_cdk::println!`) and cannot be exercised in a unit test.
/// 3. A test that passes for `prepare_openid_attributes` with the
/// template-scope key proves that the same key will work in
/// `get_attributes` after the fix.
mod get_attributes_issuer_regression_test {
use super::*;
use internet_identity_interface::internet_identity::types::{
OpenIdConfig, OpenIdEmailVerificationScheme,
};

const MICROSOFT_ISSUER_TEMPLATE: &str = "https://login.microsoftonline.com/{tid}/v2.0";
const MICROSOFT_PERSONAL_TID: &str = "9188040d-6c67-4c5b-b112-36a304b66dad";
const MICROSOFT_RESOLVED_ISSUER: &str =
"https://login.microsoftonline.com/9188040d-6c67-4c5b-b112-36a304b66dad/v2.0";
const ANCHOR_NUMBER: u64 = 4242;

fn setup_microsoft_provider() {
crate::openid::setup(vec![OpenIdConfig {
name: "Microsoft".to_string(),
logo: String::new(),
issuer: MICROSOFT_ISSUER_TEMPLATE.to_string(),
client_id: "test-client-id".to_string(),
jwks_uri: String::new(),
auth_uri: String::new(),
auth_scope: vec![],
fedcm_uri: None,
email_verification: Some(OpenIdEmailVerificationScheme::Microsoft),
}]);
}

/// Callers of `get_attributes` / `prepare_attributes` always use the
/// *template* issuer (`{tid}` placeholder) as the scope key.
fn microsoft_template_scope() -> Option<AttributeScope> {
Some(AttributeScope::OpenId {
issuer: MICROSOFT_ISSUER_TEMPLATE.to_string(),
})
}

/// A Microsoft credential whose `.iss` is the *resolved* issuer
/// (real tenant ID), as it would be after JWT verification.
fn microsoft_credential_with_email(email: &str) -> OpenIdCredential {
OpenIdCredential {
iss: MICROSOFT_RESOLVED_ISSUER.to_string(),
sub: "ms-user-456".to_string(),
aud: "test-client-id".to_string(),
last_usage_timestamp: None,
metadata: HashMap::from([
(
"email".to_string(),
MetadataEntryV2::String(email.to_string()),
),
(
"tid".to_string(),
MetadataEntryV2::String(MICROSOFT_PERSONAL_TID.to_string()),
),
]),
}
}

/// The scope key uses the *template* issuer while the credential's
/// `.iss` contains the *resolved* issuer. Before the fix,
/// `get_attributes` used `.iss` directly and the lookup would miss.
/// `prepare_openid_attributes` (and now `get_attributes`) uses
/// `config_issuer()` which returns the template, so the key matches.
#[test]
fn test_template_scope_resolves_microsoft_credential() {
setup_microsoft_provider();

let mut anchor = Anchor::new(ANCHOR_NUMBER, 0);
anchor.openid_credentials = vec![microsoft_credential_with_email("user@outlook.com")];

// Key the request by the *template* scope — exactly as callers do.
let mut requested = BTreeMap::from([(
microsoft_template_scope(),
BTreeSet::from([AttributeName::Email]),
)]);

let result = anchor.prepare_openid_attributes(&mut requested);

// The scope must have been consumed (matched).
assert!(
requested.is_empty(),
"template scope should match the credential via config_issuer()"
);

// Exactly one credential key should be present with one attribute.
pretty_assert_eq!(result.len(), 1, "one credential should match");
let attrs = result
.get(&(
MICROSOFT_RESOLVED_ISSUER.to_string(),
"ms-user-456".to_string(),
))
.expect("credential key should be present");
pretty_assert_eq!(attrs.len(), 1, "email attribute should be returned");
pretty_assert_eq!(String::from_utf8_lossy(&attrs[0].value), "user@outlook.com",);
}
}
}
182 changes: 182 additions & 0 deletions src/internet_identity/tests/integration/attributes.rs
Original file line number Diff line number Diff line change
Expand Up @@ -243,3 +243,185 @@ fn should_get_certified_attributes() {
get_response.expires_at_timestamp_ns,
);
}

/// Regression test: Microsoft credentials use a template issuer with `{tid}` placeholder.
/// `get_attributes` must use the template issuer (from the OpenID config) as the scope key,
/// NOT the resolved issuer (from the JWT `iss` claim with the actual tenant ID).
/// Without the fix, `get_attributes` would look up signatures under the resolved issuer scope
/// but `prepare_attributes` stores them under the template issuer scope, resulting in
/// no certified attributes being returned.
#[test]
fn should_get_certified_attributes_microsoft() {
let env = env();
#[allow(unused_variables)]
let (jwt, salt, claims, test_time, test_principal, test_authn_method) =
crate::openid::one_openid_microsoft_test_data();

let mut init_args = arg_with_wasm_hash(ARCHIVE_WASM.clone()).unwrap();
init_args.openid_configs = Some(vec![OpenIdConfig {
name: "Microsoft".into(),
logo: "logo".into(),
issuer: "https://login.microsoftonline.com/{tid}/v2.0".into(),
client_id: "d948c073-eebd-4ab8-861d-055f7ab49e17".into(),
jwks_uri: "https://login.microsoftonline.com/common/discovery/v2.0/keys".into(),
auth_uri: "https://login.microsoftonline.com/common/oauth2/v2.0/authorize".into(),
auth_scope: vec!["openid".into(), "profile".into(), "email".into()],
fedcm_uri: Some("".into()),
email_verification: None,
}]);

let ii_backend_canister_id = install_ii_canister_with_arg_and_cycles(
&env,
II_WASM.clone(),
Some(init_args),
1_000_000_000_000_000, // 1P cycles for HTTP outcalls
);
// Mock certs response so openid_credential_add can verify the JWT
crate::openid::mock_microsoft_certs_response(&env);

deploy_archive_via_ii(&env, ii_backend_canister_id);

// Create an identity with the required authn method
let identity_number =
crate::v2_api::authn_method_test_helpers::create_identity_with_authn_method(
&env,
ii_backend_canister_id,
&test_authn_method,
);

// Sync time to the JWT iat
let time_to_advance = Duration::from_millis(test_time) - Duration::from_nanos(time(&env));
env.advance_time(time_to_advance);

// Add OpenID credential
api::openid_credential_add(
&env,
ii_backend_canister_id,
test_principal,
identity_number,
&jwt,
&salt,
)
.expect("failed to add openid credential")
.expect("openid_credential_add error");

let origin = "https://some-dapp.com";

// The attribute key MUST use the template issuer with {tid}, not the resolved tenant ID.
let microsoft_template_issuer = "https://login.microsoftonline.com/{tid}/v2.0";

// 1. Prepare attributes

env.advance_time(Duration::from_secs(15));

let prepare_request = PrepareAttributeRequest {
identity_number,
origin: origin.to_string(),
account_number: None,
attribute_keys: vec![format!("openid:{microsoft_template_issuer}:name")],
};

let prepare_response = api::prepare_attributes(
&env,
ii_backend_canister_id,
test_principal,
prepare_request,
)
.expect("failed to call prepare_attributes")
.expect("prepare_attributes error");

assert_eq!(prepare_response.attributes.len(), 1);

// 2. Get attributes
env.advance_time(Duration::from_secs(5));

let get_request = GetAttributesRequest {
identity_number,
origin: origin.to_string(),
account_number: None,
issued_at_timestamp_ns: prepare_response.issued_at_timestamp_ns,
attributes: prepare_response.attributes.clone(),
};

let get_response =
api::get_attributes(&env, ii_backend_canister_id, test_principal, get_request)
.expect("failed to call get_attributes")
.expect("get_attributes error");

// Check that we actually got a certified attribute back (the regression would return 0)
assert_eq!(
get_response.certified_attributes.len(),
1,
"get_attributes should return 1 certified attribute for Microsoft name; \
if 0, get_attributes is likely using the resolved issuer instead of the template issuer"
);

// Verify the CBOR signature prefix (smoke test)
assert!(get_response.certified_attributes[0]
.signature
.starts_with(&[217, 217, 247, 162, 107, 99, 101, 114]));

// Redact signatures for comparison
let mut redacted_response = get_response.clone();
redacted_response
.certified_attributes
.iter_mut()
.for_each(|attr| attr.signature = vec![]);

assert_eq!(
redacted_response,
CertifiedAttributes {
certified_attributes: vec![CertifiedAttribute {
key: format!("openid:{microsoft_template_issuer}:name"),
value: "Llorenç Muntaner Perello".as_bytes().to_vec(),
signature: vec![], // redacted
}],
expires_at_timestamp_ns: prepare_response.issued_at_timestamp_ns
+ Duration::from_secs(30 * 60).as_nanos() as u64,
}
);

// Verify the signatures; this relies on delegation verification for the same (origin, user).

let session_public_key = ByteBuf::from("session public key");

env.advance_time(Duration::from_secs(35));

let (canister_sig_key, expiration) = api::prepare_delegation(
&env,
ii_backend_canister_id,
test_principal,
identity_number,
origin,
&session_public_key,
None,
)
.unwrap();

env.advance_time(Duration::from_secs(5));

let signed_delegation = match api::get_delegation(
&env,
ii_backend_canister_id,
test_principal,
identity_number,
origin,
&session_public_key,
expiration,
)
.unwrap()
{
GetDelegationResponse::SignedDelegation(delegation) => delegation,
GetDelegationResponse::NoSuchDelegation => panic!("failed to get delegation"),
};

verify_delegation_and_attribute_signatures(
&env,
session_public_key,
canister_sig_key,
ii_backend_canister_id,
signed_delegation,
get_response.certified_attributes,
get_response.expires_at_timestamp_ns,
);
}
5 changes: 3 additions & 2 deletions src/internet_identity/tests/integration/openid.rs
Original file line number Diff line number Diff line change
Expand Up @@ -619,7 +619,7 @@ pub fn mock_google_certs_response(env: &PocketIc) {
mock_certs_response(env, url, mock_certs);
}

fn mock_microsoft_certs_response(env: &PocketIc) {
pub fn mock_microsoft_certs_response(env: &PocketIc) {
// This is the URL that the canister will fetch the Microsoft certificates
let url = "https://login.microsoftonline.com/common/discovery/v2.0/keys";
// These are the certificates at the time of the related Microsoft JWT mocked data was created.
Expand Down Expand Up @@ -785,7 +785,8 @@ fn second_openid_google_test_data() -> (String, [u8; 32], Claims, u64, Principal
)
}

fn one_openid_microsoft_test_data() -> (String, [u8; 32], Claims, u64, Principal, AuthnMethodData) {
pub fn one_openid_microsoft_test_data(
) -> (String, [u8; 32], Claims, u64, Principal, AuthnMethodData) {
let jwt = "eyJ0eXAiOiJKV1QiLCJhbGciOiJSUzI1NiIsImtpZCI6IkpZaEFjVFBNWl9MWDZEQmxPV1E3SG4wTmVYRSJ9.eyJhdWQiOiJkOTQ4YzA3My1lZWJkLTRhYjgtODYxZC0wNTVmN2FiNDllMTciLCJpc3MiOiJodHRwczovL2xvZ2luLm1pY3Jvc29mdG9ubGluZS5jb20vNGE0MzVjNWUtNjQ1MS00YzFhLWE4MWYtYWI5NjY2YjZkZThmL3YyLjAiLCJpYXQiOjE3NTY4MDgzMjQsIm5iZiI6MTc1NjgwODMyNCwiZXhwIjoxNzU2ODEyMjI0LCJhaW8iOiJBVlFBcS84WkFBQUExQjhrYVdEVWp6V0xnSUxVT2hIQ1pWVndhbk1wNGVnVzdURzZwTytnVSsyYzdKRVRJckV5VHlySkxQQ0h1VkZINkUrbzRlMzhCQjZ6dmlWSU9kTzkxNVRHVDhEaUR3bkhCazYxTSt2bTdJaz0iLCJjX2hhc2giOiJGUzJsWllUYTIwcWozZVl1enczUXBnIiwibmFtZSI6Ikxsb3JlbsOnIE11bnRhbmVyIFBlcmVsbG8iLCJub25jZSI6ImN1UmM4VlNEN1ZkQU9ISmpsX1UxbkNWdlpvamQtMGJoUE81X0lGbTc0N2MiLCJvaWQiOiIxYjI2NDVmNy04YjdmLTQyMTAtYjQxYy01MDM1MmQ1OWYyZTgiLCJwcmVmZXJyZWRfdXNlcm5hbWUiOiJsbG9yZW5jLm11bnRhbmVyQGRmaW5pdHkub3JnIiwicmgiOiIxLkFTNEFYbHhEU2xGa0dreW9INnVXWnJiZWozUEFTTm05N3JoS2hoMEZYM3EwbmhlNUFPd3VBQS4iLCJzaWQiOiIwMDdkZWE3OS0yNjY5LWZjNTItMzQwOS01Y2NjZDkxOTAzMjEiLCJzdWIiOiJydkF0eGluNk1TblRsN1RnUlg4RlhYQ0tQbEVlTklmUHI0bHdQT1lfd293IiwidGlkIjoiNGE0MzVjNWUtNjQ1MS00YzFhLWE4MWYtYWI5NjY2YjZkZThmIiwidXRpIjoiU3pLR0k3cG44MC1ZdnRmMmxuZ0RBUSIsInZlciI6IjIuMCJ9.kS8C8IlRoMaYoFyru-D06WzdeS8mHA3LupXyrOqXwwb4AIMMUDETlJEznAQ6iZxK4iAhAPAqAnC9TS_j0sacRCTBA3Rks-tkuwV2sA3XdwDsoFOnJdBs-N5GEXJNv45TzQ0jQANnXBJwwgH9hS-ledFZiutvzaTfDGpAymxx58qj7VDG5fTMxpiPMNCr42sNidw7B8ifUJgcfcxt_8wsTN_mui4Q6wtWRQvPnbesyTvRaOg2S6LMG3m8RBNYtHvXlICwD1kaKS5wUiYcrN3gg6wqOXCI3w57S5yfnGNo1tF4sWCfR0ZkfyHfVzdXK_6BwCty7rt4udp-NFsCAVXNRQ";
let salt: [u8; 32] = [
196, 116, 153, 227, 8, 104, 231, 67, 202, 28, 156, 132, 101, 84, 170, 111, 86, 233, 29, 54,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -212,7 +212,7 @@ impl std::fmt::Display for AttributeKey {
}
}

#[derive(CandidType, Deserialize)]
#[derive(CandidType, Debug, Deserialize)]
pub struct PrepareAttributeRequest {
pub identity_number: AnchorNumber,
pub origin: FrontendHostname,
Expand Down Expand Up @@ -302,7 +302,7 @@ pub enum PrepareAttributeError {
GetAccountError(GetAccountError),
}

#[derive(CandidType, Deserialize)]
#[derive(CandidType, Debug, Deserialize)]
pub struct GetAttributesRequest {
pub identity_number: AnchorNumber,
pub origin: FrontendHostname,
Expand Down
Loading