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
2 changes: 1 addition & 1 deletion src/frontend/src/lib/generated/internet_identity_idl.js
Original file line number Diff line number Diff line change
Expand Up @@ -455,7 +455,7 @@ export const idlFactory = ({ IDL }) => {
'Unauthorized' : IDL.Principal,
'JwtVerificationFailed' : IDL.Null,
});
const OpenIdCredentialKey = IDL.Tuple(Iss, Sub);
const OpenIdCredentialKey = IDL.Tuple(Iss, Sub, Aud);
const OpenIdCredentialRemoveError = IDL.Variant({
'InternalCanisterError' : IDL.Text,
'OpenIdCredentialNotFound' : IDL.Null,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -846,7 +846,7 @@ export type OpenIdCredentialAddError = {
{ 'JwtExpired' : null } |
{ 'Unauthorized' : Principal } |
{ 'JwtVerificationFailed' : null };
export type OpenIdCredentialKey = [Iss, Sub];
export type OpenIdCredentialKey = [Iss, Sub, Aud];
export type OpenIdCredentialRemoveError = { 'InternalCanisterError' : string } |
{ 'OpenIdCredentialNotFound' : null } |
{ 'Unauthorized' : Principal };
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -187,6 +187,7 @@
.openid_credential_remove($authenticatedStore.identityNumber, [
removingAccessMethod.openid.iss,
removingAccessMethod.openid.sub,
removingAccessMethod.openid.aud,
])
.then(throwCanisterError);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,8 @@ export const toAccessMethods = (

/**
* Returns a unique string key for an access method.
* Uses credential ID for passkeys and issuer+subject for OpenID.
* Uses credential ID for passkeys and a JSON-encoded (iss, sub, aud) triple
* for OpenID so components can't collide via concatenation.
*/
export const toKey = (accessMethod: AccessMethod): string => {
if (
Expand All @@ -67,7 +68,11 @@ export const toKey = (accessMethod: AccessMethod): string => {
);
}
if ("openid" in accessMethod) {
return accessMethod.openid.iss + accessMethod.openid.sub;
return JSON.stringify([
accessMethod.openid.iss,
accessMethod.openid.sub,
accessMethod.openid.aud,
]);
}
throw new Error("Unknown access method type");
};
Expand Down
2 changes: 1 addition & 1 deletion src/internet_identity/internet_identity.did
Original file line number Diff line number Diff line change
Expand Up @@ -403,7 +403,7 @@ type OidcConfig = record {
issuer : opt text;
};

type OpenIdCredentialKey = record { Iss; Sub };
type OpenIdCredentialKey = record { Iss; Sub; Aud };

type AnalyticsConfig = variant {
Plausible : record {
Expand Down
2 changes: 1 addition & 1 deletion src/internet_identity/src/anchor_management.rs
Original file line number Diff line number Diff line change
Expand Up @@ -229,7 +229,7 @@ pub fn remove_openid_credential(
key: &OpenIdCredentialKey,
) -> Result<Operation, AnchorError> {
anchor.remove_openid_credential(key)?;
let (iss, _) = key;
let (iss, _, _) = key;
Ok(Operation::RemoveOpenIdCredential { iss: iss.clone() })
}

Expand Down
58 changes: 49 additions & 9 deletions src/internet_identity/src/attributes.rs
Original file line number Diff line number Diff line change
Expand Up @@ -978,7 +978,11 @@ mod tests {
assert!(requested.is_empty());

let attrs = result
.get(&(GOOGLE_ISSUER.to_string(), "google-user-123".to_string()))
.get(&(
GOOGLE_ISSUER.to_string(),
"google-user-123".to_string(),
"test-client-id".to_string(),
))
.expect("google verified email attributes should be present");

pretty_assert_eq!(attrs.len(), 1);
Expand Down Expand Up @@ -1011,7 +1015,11 @@ mod tests {
assert!(requested.is_empty());

let attrs = result
.get(&(GOOGLE_ISSUER.to_string(), "google-user-123".to_string()))
.get(&(
GOOGLE_ISSUER.to_string(),
"google-user-123".to_string(),
"test-client-id".to_string(),
))
.expect("credential key should be present");
pretty_assert_eq!(
attrs.len(),
Expand All @@ -1034,7 +1042,11 @@ mod tests {
assert!(requested.is_empty());

let attrs = result
.get(&(GOOGLE_ISSUER.to_string(), "google-user-123".to_string()))
.get(&(
GOOGLE_ISSUER.to_string(),
"google-user-123".to_string(),
"test-client-id".to_string(),
))
.expect("credential key should be present");
pretty_assert_eq!(
attrs.len(),
Expand All @@ -1057,7 +1069,11 @@ mod tests {
assert!(requested.is_empty());

let attrs = result
.get(&(GOOGLE_ISSUER.to_string(), "google-user-123".to_string()))
.get(&(
GOOGLE_ISSUER.to_string(),
"google-user-123".to_string(),
"test-client-id".to_string(),
))
.expect("credential key should be present");
pretty_assert_eq!(
attrs.len(),
Expand Down Expand Up @@ -1087,7 +1103,11 @@ mod tests {
assert!(requested.is_empty());

let attrs = result
.get(&(GOOGLE_ISSUER.to_string(), "google-user-123".to_string()))
.get(&(
GOOGLE_ISSUER.to_string(),
"google-user-123".to_string(),
"test-client-id".to_string(),
))
.expect("credential key should be present");
pretty_assert_eq!(
attrs.len(),
Expand Down Expand Up @@ -1123,7 +1143,11 @@ mod tests {
assert!(requested.is_empty());

let attrs = result
.get(&(GOOGLE_ISSUER.to_string(), "google-user-123".to_string()))
.get(&(
GOOGLE_ISSUER.to_string(),
"google-user-123".to_string(),
"test-client-id".to_string(),
))
.expect("credential key should be present");
pretty_assert_eq!(
attrs.len(),
Expand Down Expand Up @@ -1157,6 +1181,7 @@ mod tests {
.get(&(
MICROSOFT_RESOLVED_ISSUER.to_string(),
"ms-user-456".to_string(),
"test-client-id".to_string(),
))
.expect("microsoft verified email attributes should be present");

Expand Down Expand Up @@ -1200,7 +1225,11 @@ mod tests {
assert!(requested.is_empty());

let attrs = result
.get(&(enterprise_iss, "enterprise-user".to_string()))
.get(&(
enterprise_iss,
"enterprise-user".to_string(),
"test-client-id".to_string(),
))
.expect("credential key should be present");
pretty_assert_eq!(
attrs.len(),
Expand Down Expand Up @@ -1259,6 +1288,7 @@ mod tests {
.get(&(
MICROSOFT_RESOLVED_ISSUER.to_string(),
"ms-user-456".to_string(),
"test-client-id".to_string(),
))
.expect("credential key should be present");
pretty_assert_eq!(
Expand Down Expand Up @@ -1310,7 +1340,11 @@ mod tests {
pretty_assert_eq!(result.len(), 2, "both credentials should be present");

let google_attrs = result
.get(&(GOOGLE_ISSUER.to_string(), "google-user-123".to_string()))
.get(&(
GOOGLE_ISSUER.to_string(),
"google-user-123".to_string(),
"test-client-id".to_string(),
))
.expect("google attributes");
pretty_assert_eq!(
attribute_pairs(google_attrs),
Expand All @@ -1324,6 +1358,7 @@ mod tests {
.get(&(
MICROSOFT_RESOLVED_ISSUER.to_string(),
"ms-user-456".to_string(),
"test-client-id".to_string(),
))
.expect("microsoft attributes");
pretty_assert_eq!(
Expand Down Expand Up @@ -1367,7 +1402,11 @@ mod tests {
assert!(requested.is_empty());

let attrs = result
.get(&(GOOGLE_ISSUER.to_string(), "google-user-123".to_string()))
.get(&(
GOOGLE_ISSUER.to_string(),
"google-user-123".to_string(),
"test-client-id".to_string(),
))
.expect("google attributes");

pretty_assert_eq!(attrs.len(), 3, "all three attributes should be returned");
Expand Down Expand Up @@ -1540,6 +1579,7 @@ mod tests {
.get(&(
MICROSOFT_RESOLVED_ISSUER.to_string(),
"ms-user-456".to_string(),
"test-client-id".to_string(),
))
.expect("credential key should be present");
pretty_assert_eq!(attrs.len(), 1, "email attribute should be returned");
Expand Down
97 changes: 97 additions & 0 deletions src/internet_identity/src/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,9 @@ use ic_canister_sig_creation::signature_map::LABEL_SIG;
use ic_cdk::api::{caller, set_certified_data, trap};
use ic_cdk::call;
use ic_cdk_macros::{init, post_upgrade, pre_upgrade, query, update};
use ic_cdk_timers::TimerId;
use std::cell::RefCell;
use std::time::Duration;

use internet_identity_interface::archive::types::{BufferedEntry, Operation};
use internet_identity_interface::http_gateway::{HttpRequest, HttpResponse};
Expand Down Expand Up @@ -75,6 +78,95 @@ const INTERNETCOMPUTER_ORG_ORIGIN: &str = "https://identity.internetcomputer.org
const ID_AI_DOMAIN: &str = "id.ai";
const ID_AI_ORIGIN: &str = "https://id.ai";

// ---- OpenID credential key migration (temporary, see PR #3784) ----
//
// The `OpenIdCredentialKey` type grew an `aud` field. Existing index entries
// were written in the legacy `(iss, sub)` CBOR array shape and must be
// upgraded to the new `(iss, sub, aud)` CBOR map shape. Upgrading all entries
// synchronously in `post_upgrade` would blow the instruction limit on II's
// production canister, so we batch the work via an interval timer using the
// same convention as the prior anchor migration (#3713).

/// How long to wait between migration batches.
const OIDC_KEY_MIGRATION_BATCH_BACKOFF_SECONDS: Duration = Duration::from_secs(1);

/// Maximum number of index entries to upgrade per batch (= per ingress message).
/// Matches the 2 000-anchor-per-batch convention used for previous migrations.
const OIDC_KEY_MIGRATION_BATCH_SIZE: u64 = 2_000;

thread_local! {
// TODO: Remove these after the data migration is complete.
static OIDC_KEY_MIGRATION_DONE: RefCell<bool> = const { RefCell::new(false) };
static OIDC_KEY_MIGRATION_PROCESSED: RefCell<u64> = const { RefCell::new(0) };
static OIDC_KEY_MIGRATION_ERRORS: RefCell<Vec<String>> = const { RefCell::new(Vec::new()) };
static OIDC_KEY_MIGRATION_TIMER_ID: RefCell<Option<TimerId>> = const { RefCell::new(None) };
}

/// Temporary hidden endpoint: returns any orphan-entry errors encountered
/// during the OpenID credential key migration.
#[update(hidden = true)]
fn list_oidc_key_migration_errors() -> Vec<String> {
OIDC_KEY_MIGRATION_ERRORS.with_borrow(|errors| errors.clone())
}

/// Temporary hidden endpoint: returns `(processed_entries, is_done)` so
/// monitoring can track migration progress.
#[query(hidden = true)]
fn oidc_key_migration_status() -> (u64, bool) {
(
OIDC_KEY_MIGRATION_PROCESSED.with_borrow(|c| *c),
OIDC_KEY_MIGRATION_DONE.with_borrow(|d| *d),
)
}

/// Process one batch of the OpenID credential key migration. Bound to the
/// interval timer set up in [`init_oidc_key_migration_timer`]; clears the
/// timer once the migration signals completion.
fn run_oidc_key_migration_batch() {
if OIDC_KEY_MIGRATION_DONE.with_borrow(|done| *done) {
return;
}

let outcome = state::storage_borrow_mut(|storage| {
storage.migrate_openid_credential_keys_batch(OIDC_KEY_MIGRATION_BATCH_SIZE)
});

OIDC_KEY_MIGRATION_PROCESSED.with_borrow_mut(|count| {
*count = count.saturating_add(outcome.processed);
});
if !outcome.errors.is_empty() {
OIDC_KEY_MIGRATION_ERRORS.with_borrow_mut(|errors| errors.extend(outcome.errors));
}

if outcome.is_done {
OIDC_KEY_MIGRATION_DONE.replace(true);
OIDC_KEY_MIGRATION_TIMER_ID.with_borrow_mut(|id_slot| {
if let Some(timer_id) = id_slot.take() {
ic_cdk_timers::clear_timer(timer_id);
}
});
let processed = OIDC_KEY_MIGRATION_PROCESSED.with_borrow(|c| *c);
ic_cdk::println!(
"OpenID credential key migration COMPLETED ({processed} entries processed)."
);
}
}

/// Start the interval timer driving [`run_oidc_key_migration_batch`]. Safe to
/// call from both `init` (the first batch will immediately see an empty index
/// and mark the migration done) and `post_upgrade`.
fn init_oidc_key_migration_timer() {
let timer_id = ic_cdk_timers::set_timer_interval(
OIDC_KEY_MIGRATION_BATCH_BACKOFF_SECONDS,
run_oidc_key_migration_batch,
);
OIDC_KEY_MIGRATION_TIMER_ID.with_borrow_mut(|id_slot| {
if let Some(old_id) = id_slot.replace(timer_id) {
ic_cdk_timers::clear_timer(old_id);
}
});
}

#[update]
async fn init_salt() {
state::init_salt().await;
Expand Down Expand Up @@ -639,6 +731,11 @@ fn initialize(maybe_arg: Option<InternetIdentityInit>) {
if let Some(oidc_configs) = config.oidc_configs {
openid::setup_oidc(oidc_configs);
}

// Kick off the OpenID credential key batch migration. Processes at most
// `OIDC_KEY_MIGRATION_BATCH_SIZE` entries per tick so each batch fits in
// one ingress message; timer self-clears once the migration signals done.
init_oidc_key_migration_timer();
}

fn apply_install_arg(maybe_arg: Option<InternetIdentityInit>) {
Expand Down
Loading
Loading