Merge branch 'main' into sea-snake/redesign-identity-switcher

Author: sea-snake

src/internet_identity/src/attributes.rs

@@ -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
+                    // 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();
@@ -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",);
+        }
+    }
 }

src/internet_identity/tests/integration/attributes.rs

@@ -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,
+    );
+}

src/internet_identity/tests/integration/openid.rs

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

src/internet_identity_interface/src/internet_identity/types/attributes.rs

@@ -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,
@@ -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,