Skip to content

feat: add FxA event webhook endpoint#2108

Open
chenba wants to merge 1 commit intomasterfrom
feat/fxa-event-webhook-stor-128
Open

feat: add FxA event webhook endpoint#2108
chenba wants to merge 1 commit intomasterfrom
feat/fxa-event-webhook-stor-128

Conversation

@chenba
Copy link
Collaborator

@chenba chenba commented Mar 10, 2026


/// An authentication token as parsed from the `Authorization` header.
/// OAuth tokens are opaque to Tokenserver and must be verified via FxA.
/// Signed JWTs can be verified locally or via FxA.
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I did some renaming in numerous spots since JWTs can be used outside of an OAuth context.

pub init_node_capacity: i32,
/// Whether to enable the FxA webhook endpoint.
/// Defaults to false.
pub fxa_webhook_enabled: bool,
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I'll update the config.md to add this.

@chenba chenba force-pushed the feat/fxa-event-webhook-stor-128 branch 3 times, most recently from 361d156 to 6715c01 Compare March 11, 2026 21:24
pub fn with_capture() -> (Self, Arc<Mutex<Vec<params::PutUser>>>) {
let pool = MockDbPool::default();
let put_user_calls = Arc::clone(&pool.put_user_calls);
(pool, put_user_calls)
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Maybe we should consider using mockall instead. But I'll leave that to another PR, if we want to make that change.

@chenba chenba requested review from pjenvey and taddes March 12, 2026 01:16
@chenba chenba force-pushed the feat/fxa-event-webhook-stor-128 branch from 6715c01 to 3dee63c Compare March 12, 2026 17:41
Comment on lines +305 to +308
let events = match claims.events.as_object() {
Some(map) => map.clone(),
None => return Ok(HttpResponse::Ok().finish()),
};
Copy link
Member

Choose a reason for hiding this comment

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

looks like the clone isn't needed either

Suggested change
let events = match claims.events.as_object() {
Some(map) => map.clone(),
None => return Ok(HttpResponse::Ok().finish()),
};
let Some(events) = claims.events.as_object() else {
return Ok(HttpResponse::Ok().finish());
};

Self::InvalidKey => "oauth.error.invalid_key",
Self::InvalidSignature => "oauth.error.invalid_signature",
Self::DecodingError => "oauth.error.decoding_error",
Self::ExpiredSignature => "jwt.error.expired_signature",
Copy link
Member

Choose a reason for hiding this comment

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

I'll note we don't seem to graph any of these in our dashboards so 👍 to renaming

pub fn new(jwk: &Jwk, client_id: &str) -> Result<Self, JWTVerifyError> {
let decoding_key = DecodingKey::from_jwk(jwk).map_err(|_| JWTVerifyError::InvalidKey)?;
let mut validation = Validation::new(Algorithm::RS256);
validation.set_audience(&[client_id]);
Copy link
Member

@pjenvey pjenvey Mar 13, 2026

Choose a reason for hiding this comment

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

Maybe we should verify iss as well?

Suggested change
validation.set_audience(&[client_id]);
validation.set_audience(&[client_id]);
validation.set_issuer(&[<SYNC_TOKENSERVER__FXA_OAUTH_SERVER_URL?>]);

for event_type in events.keys() {
match event_type.as_str() {
"https://schemas.accounts.firefox.com/event/delete-user" => {
db.put_user(tokenserver_db::params::PutUser {
Copy link
Member

Choose a reason for hiding this comment

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

We need a new Db::retire_user here like the one in database.py called by process_account_events.py, which sets a replaced_at on the record

.get("changeTime")
.and_then(|t| t.as_i64())
{
db.put_user(tokenserver_db::params::PutUser {
Copy link
Member

Choose a reason for hiding this comment

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

I think we also need a new Db::update_user_generation for this (see my comment in https://mozilla-hub.atlassian.net/browse/STOR-128?focusedCommentId=1281438)

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

Successfully merging this pull request may close these issues.

2 participants