Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

bug: Check to see if the user still exists before registering channels #713

Draft
wants to merge 4 commits into
base: master
Choose a base branch
from
Draft
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
Original file line number Diff line number Diff line change
Expand Up @@ -96,6 +96,17 @@ impl WebPushClient {
);
self.app_state.db.add_user(user).await?;
self.deferred_add_user = None;
} else {
// Check to see if the UAID is still valid.
if !self
.app_state
.db
.check_user(&self.uaid)
.await
.map_err(|_| SMErrorKind::UaidReset)?
{
return Err(SMErrorKind::UaidReset);
}
}

let endpoint = make_endpoint(
Expand Down Expand Up @@ -328,3 +339,39 @@ impl WebPushClient {
}
}
}

#[cfg(test)]
mod tests {
use std::sync::Arc;

use super::*;
use crate::identified::{ClientFlags, SessionStatistics, WebPushClient};
use autopush_common::util::user_agent::UserAgentInfo;

/// Test that you can't register if you don't exist.
#[tokio::test]
async fn test_do_register_unregistered() {
// Mostly a garbage client, but note that the deferred_add_user is None.
let mut client = WebPushClient {
uaid: Uuid::new_v4(),
uid: Uuid::new_v4(),
ua_info: UserAgentInfo::default(),
broadcast_subs: Default::default(),
flags: ClientFlags::default(),
ack_state: Default::default(),
sent_from_storage: 0,
deferred_add_user: None,
connected_at: sec_since_epoch(),
last_ping: sec_since_epoch(),
current_timestamp: None,
stats: SessionStatistics::default(),
app_state: Arc::new(autoconnect_settings::AppState::default()),
};
let result = client.do_register(&Uuid::new_v4(), None).await;
assert!(result.is_err());
assert_eq!(
result.unwrap_err().to_string(),
SMErrorKind::UaidReset.to_string()
);
}
}
14 changes: 14 additions & 0 deletions autoendpoint/src/routes/registration.rs
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ use autopush_common::db::User;
use autopush_common::endpoint::make_endpoint;

/// Handle the `POST /v1/{router_type}/{app_id}/registration` route
/// This call is used to register a new user with a given bridge.
pub async fn register_uaid_route(
path_args: RegistrationPathArgs,
router_data_input: RouterDataInput,
Expand Down Expand Up @@ -78,6 +79,7 @@ pub async fn register_uaid_route(
}

/// Handle the `DELETE /v1/{router_type}/{app_id}/registration/{uaid}` route
/// This call is used to unregister a user from a given bridge.
pub async fn unregister_user_route(
_auth: AuthorizationCheck,
path_args: RegistrationPathArgsWithUaid,
Expand All @@ -90,6 +92,7 @@ pub async fn unregister_user_route(
}

/// Handle the `PUT /v1/{router_type}/{app_id}/registration/{uaid}` route
/// This call is used to update the token of a user with a given bridge.
pub async fn update_token_route(
_auth: AuthorizationCheck,
path_args: RegistrationPathArgsWithUaid,
Expand Down Expand Up @@ -124,6 +127,7 @@ pub async fn update_token_route(
}

/// Handle the `POST /v1/{router_type}/{app_id}/registration/{uaid}/subscription` route
/// This call is used to add a new channel to a user (provided the user still exists).
pub async fn new_channel_route(
_auth: AuthorizationCheck,
path_args: RegistrationPathArgsWithUaid,
Expand All @@ -132,6 +136,12 @@ pub async fn new_channel_route(
) -> ApiResult<HttpResponse> {
// Add the channel
let uaid = path_args.user.uaid;
// Make sure the UAID still exists before adding a channel.
// The UAID may have been dropped by the bridge, which the UA
// may be unaware of.
if !app_state.db.check_user(&uaid).await? {
return Err(ApiErrorKind::NoUser.into());
}
debug!("🌍 Adding a channel to UAID {uaid}");
let channel_data = channel_data.map(Json::into_inner).unwrap_or_default();
let channel_id = channel_data.channel_id.unwrap_or_else(Uuid::new_v4);
Expand All @@ -157,6 +167,9 @@ pub async fn new_channel_route(
}

/// Handle the `GET /v1/{router_type}/{app_id}/registration/{uaid}` route
/// This call is used to get the list of channel IDs for a user. The UA will
/// compare that list with it's own and if there are any discrepancies, the
/// UA will reset it's subscriptions.
pub async fn get_channels_route(
_auth: AuthorizationCheck,
path_args: RegistrationPathArgsWithUaid,
Expand All @@ -173,6 +186,7 @@ pub async fn get_channels_route(
}

/// Handle the `DELETE /v1/{router_type}/{app_id}/registration/{uaid}/subscription/{chid}` route
/// This call is used to remove a channel from a user.
pub async fn unregister_channel_route(
_auth: AuthorizationCheck,
path_args: RegistrationPathArgsWithUaid,
Expand Down
9 changes: 9 additions & 0 deletions autopush-common/src/db/bigtable/bigtable_client/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -862,6 +862,15 @@ impl DbClient for BigTableClientImpl {
Ok(predicate_matched)
}

async fn check_user(&self, uaid: &Uuid) -> DbResult<bool> {
let row_key = uaid.as_simple().to_string();
let mut req = self.read_row_request(&row_key);
let mut filters = vec![router_gc_policy_filter()];
filters.push(family_filter(format!("^{ROUTER_FAMILY}$")));
req.set_filter(filter_chain(filters));
Ok(self.read_row(req).await?.is_some())
}

async fn get_user(&self, uaid: &Uuid) -> DbResult<Option<User>> {
let row_key = uaid.as_simple().to_string();
let mut req = self.read_row_request(&row_key);
Expand Down
5 changes: 5 additions & 0 deletions autopush-common/src/db/client.rs
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,11 @@ pub trait DbClient: Send + Sync {
/// Read a user from the database
async fn get_user(&self, uaid: &Uuid) -> DbResult<Option<User>>;

/// Does the user record still exist?
async fn check_user(&self, uaid: &Uuid) -> DbResult<bool> {
Ok(self.get_user(uaid).await?.is_some())
}

/// Delete a user from the router table
async fn remove_user(&self, uaid: &Uuid) -> DbResult<()>;

Expand Down