Skip to content

Commit 850a9ed

Browse files
MTRNordtonkku107
authored andcommitted
Link removal storage API
From #3245 with changes from review
1 parent 61091ff commit 850a9ed

9 files changed

+175
-16
lines changed

crates/data-model/src/upstream_oauth2/session.rs

Lines changed: 46 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,12 @@ pub enum UpstreamOAuthAuthorizationSessionState {
3030
extra_callback_parameters: Option<serde_json::Value>,
3131
userinfo: Option<serde_json::Value>,
3232
},
33+
Unlinked {
34+
completed_at: DateTime<Utc>,
35+
consumed_at: Option<DateTime<Utc>>,
36+
unlinked_at: DateTime<Utc>,
37+
id_token: Option<String>,
38+
},
3339
}
3440

3541
impl UpstreamOAuthAuthorizationSessionState {
@@ -57,7 +63,9 @@ impl UpstreamOAuthAuthorizationSessionState {
5763
extra_callback_parameters,
5864
userinfo,
5965
}),
60-
Self::Completed { .. } | Self::Consumed { .. } => Err(InvalidTransitionError),
66+
Self::Completed { .. } | Self::Consumed { .. } | Self::Unlinked { .. } => {
67+
Err(InvalidTransitionError)
68+
}
6169
}
6270
}
6371

@@ -85,7 +93,9 @@ impl UpstreamOAuthAuthorizationSessionState {
8593
extra_callback_parameters,
8694
userinfo,
8795
}),
88-
Self::Pending | Self::Consumed { .. } => Err(InvalidTransitionError),
96+
Self::Pending | Self::Consumed { .. } | Self::Unlinked { .. } => {
97+
Err(InvalidTransitionError)
98+
}
8999
}
90100
}
91101

@@ -98,7 +108,7 @@ impl UpstreamOAuthAuthorizationSessionState {
98108
#[must_use]
99109
pub fn link_id(&self) -> Option<Ulid> {
100110
match self {
101-
Self::Pending => None,
111+
Self::Pending | Self::Unlinked { .. } => None,
102112
Self::Completed { link_id, .. } | Self::Consumed { link_id, .. } => Some(*link_id),
103113
}
104114
}
@@ -114,9 +124,9 @@ impl UpstreamOAuthAuthorizationSessionState {
114124
pub fn completed_at(&self) -> Option<DateTime<Utc>> {
115125
match self {
116126
Self::Pending => None,
117-
Self::Completed { completed_at, .. } | Self::Consumed { completed_at, .. } => {
118-
Some(*completed_at)
119-
}
127+
Self::Completed { completed_at, .. }
128+
| Self::Consumed { completed_at, .. }
129+
| Self::Unlinked { completed_at, .. } => Some(*completed_at),
120130
}
121131
}
122132

@@ -130,9 +140,9 @@ impl UpstreamOAuthAuthorizationSessionState {
130140
pub fn id_token(&self) -> Option<&str> {
131141
match self {
132142
Self::Pending => None,
133-
Self::Completed { id_token, .. } | Self::Consumed { id_token, .. } => {
134-
id_token.as_deref()
135-
}
143+
Self::Completed { id_token, .. }
144+
| Self::Consumed { id_token, .. }
145+
| Self::Unlinked { id_token, .. } => id_token.as_deref(),
136146
}
137147
}
138148

@@ -145,7 +155,7 @@ impl UpstreamOAuthAuthorizationSessionState {
145155
#[must_use]
146156
pub fn extra_callback_parameters(&self) -> Option<&serde_json::Value> {
147157
match self {
148-
Self::Pending => None,
158+
Self::Pending | Self::Unlinked { .. } => None,
149159
Self::Completed {
150160
extra_callback_parameters,
151161
..
@@ -160,7 +170,7 @@ impl UpstreamOAuthAuthorizationSessionState {
160170
#[must_use]
161171
pub fn userinfo(&self) -> Option<&serde_json::Value> {
162172
match self {
163-
Self::Pending => None,
173+
Self::Pending | Self::Unlinked { .. } => None,
164174
Self::Completed { userinfo, .. } | Self::Consumed { userinfo, .. } => userinfo.as_ref(),
165175
}
166176
}
@@ -177,6 +187,22 @@ impl UpstreamOAuthAuthorizationSessionState {
177187
match self {
178188
Self::Pending | Self::Completed { .. } => None,
179189
Self::Consumed { consumed_at, .. } => Some(*consumed_at),
190+
Self::Unlinked { consumed_at, .. } => *consumed_at,
191+
}
192+
}
193+
194+
/// Get the time at which the upstream OAuth 2.0 authorization session was
195+
/// unlinked.
196+
///
197+
/// Returns `None` if the upstream OAuth 2.0 authorization session state is
198+
/// not [`Unlinked`].
199+
///
200+
/// [`Unlinked`]: UpstreamOAuthAuthorizationSessionState::Unlinked
201+
#[must_use]
202+
pub fn unlinked_at(&self) -> Option<DateTime<Utc>> {
203+
match self {
204+
Self::Pending | Self::Completed { .. } | Self::Consumed { .. } => None,
205+
Self::Unlinked { unlinked_at, .. } => Some(*unlinked_at),
180206
}
181207
}
182208

@@ -206,6 +232,15 @@ impl UpstreamOAuthAuthorizationSessionState {
206232
pub fn is_consumed(&self) -> bool {
207233
matches!(self, Self::Consumed { .. })
208234
}
235+
236+
/// Returns `true` if the upstream OAuth 2.0 authorization session state is
237+
/// [`Unlinked`].
238+
///
239+
/// [`Unlinked`]: UpstreamOAuthAuthorizationSessionState::Unlinked
240+
#[must_use]
241+
pub fn is_unlinked(&self) -> bool {
242+
matches!(self, Self::Unlinked { .. })
243+
}
209244
}
210245

211246
#[derive(Debug, Clone, PartialEq, Eq, Serialize)]
Lines changed: 8 additions & 2 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

crates/storage-pg/.sqlx/query-3ed73cfce8ef6a1108f454e18b1668f64b76975dba07e67d04ed7a52e2e8107f.json

Lines changed: 15 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

crates/storage-pg/.sqlx/query-cc60ad934d347fb4546205d1fe07e9d2f127cb15b1bb650d1ea3805a4c55b196.json

Lines changed: 14 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

crates/storage-pg/.sqlx/query-fcd8b4b9e003d1540357c6bf1ff9c715560d011d4c01112703a9c046170c84f1.json

Lines changed: 1 addition & 1 deletion
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.
Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,7 @@
1+
-- Copyright 2025 New Vector Ltd.
2+
--
3+
-- SPDX-License-Identifier: AGPL-3.0-only
4+
-- Please see LICENSE in the repository root for full details.
5+
6+
ALTER TABLE upstream_oauth_authorization_sessions
7+
ADD COLUMN unlinked_at TIMESTAMP WITH TIME ZONE;

crates/storage-pg/src/upstream_oauth2/link.rs

Lines changed: 49 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -374,4 +374,53 @@ impl UpstreamOAuthLinkRepository for PgUpstreamOAuthLinkRepository<'_> {
374374
.try_into()
375375
.map_err(DatabaseError::to_invalid_operation)
376376
}
377+
378+
#[tracing::instrument(
379+
name = "db.upstream_oauth_link.remove",
380+
skip_all,
381+
fields(
382+
db.query.text,
383+
upstream_oauth_link.id,
384+
upstream_oauth_link.provider_id,
385+
%upstream_oauth_link.subject,
386+
),
387+
err,
388+
)]
389+
async fn remove(
390+
&mut self,
391+
clock: &dyn Clock,
392+
upstream_oauth_link: UpstreamOAuthLink,
393+
) -> Result<(), Self::Error> {
394+
// Unlink the authorization sessions first, as they have a foreign key
395+
// constraint on the links.
396+
sqlx::query!(
397+
r#"
398+
UPDATE upstream_oauth_authorization_sessions SET
399+
upstream_oauth_link_id = NULL,
400+
unlinked_at = $2
401+
WHERE upstream_oauth_link_id = $1
402+
"#,
403+
Uuid::from(upstream_oauth_link.id),
404+
clock.now()
405+
)
406+
.traced()
407+
.execute(&mut *self.conn)
408+
.await?;
409+
410+
// Then delete the link itself
411+
let res = sqlx::query!(
412+
r#"
413+
DELETE FROM upstream_oauth_links
414+
WHERE upstream_oauth_link_id = $1
415+
"#,
416+
Uuid::from(upstream_oauth_link.id),
417+
)
418+
.traced()
419+
.execute(&mut *self.conn)
420+
.await?;
421+
422+
DatabaseError::ensure_affected_rows(&res, 1)?;
423+
424+
Ok(())
425+
}
377426
}

crates/storage-pg/src/upstream_oauth2/session.rs

Lines changed: 17 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -45,6 +45,7 @@ struct SessionLookup {
4545
completed_at: Option<DateTime<Utc>>,
4646
consumed_at: Option<DateTime<Utc>>,
4747
extra_callback_parameters: Option<serde_json::Value>,
48+
unlinked_at: Option<DateTime<Utc>>,
4849
}
4950

5051
impl TryFrom<SessionLookup> for UpstreamOAuthAuthorizationSession {
@@ -59,15 +60,19 @@ impl TryFrom<SessionLookup> for UpstreamOAuthAuthorizationSession {
5960
value.userinfo,
6061
value.completed_at,
6162
value.consumed_at,
63+
value.unlinked_at,
6264
) {
63-
(None, None, None, None, None, None) => UpstreamOAuthAuthorizationSessionState::Pending,
65+
(None, None, None, None, None, None, None) => {
66+
UpstreamOAuthAuthorizationSessionState::Pending
67+
}
6468
(
6569
Some(link_id),
6670
id_token,
6771
extra_callback_parameters,
6872
userinfo,
6973
Some(completed_at),
7074
None,
75+
None,
7176
) => UpstreamOAuthAuthorizationSessionState::Completed {
7277
completed_at,
7378
link_id: link_id.into(),
@@ -82,6 +87,7 @@ impl TryFrom<SessionLookup> for UpstreamOAuthAuthorizationSession {
8287
userinfo,
8388
Some(completed_at),
8489
Some(consumed_at),
90+
None,
8591
) => UpstreamOAuthAuthorizationSessionState::Consumed {
8692
completed_at,
8793
link_id: link_id.into(),
@@ -90,6 +96,14 @@ impl TryFrom<SessionLookup> for UpstreamOAuthAuthorizationSession {
9096
userinfo,
9197
consumed_at,
9298
},
99+
(_, id_token, _, _, Some(completed_at), consumed_at, Some(unlinked_at)) => {
100+
UpstreamOAuthAuthorizationSessionState::Unlinked {
101+
completed_at,
102+
id_token,
103+
consumed_at,
104+
unlinked_at,
105+
}
106+
}
93107
_ => {
94108
return Err(DatabaseInconsistencyError::on(
95109
"upstream_oauth_authorization_sessions",
@@ -142,7 +156,8 @@ impl UpstreamOAuthSessionRepository for PgUpstreamOAuthSessionRepository<'_> {
142156
userinfo,
143157
created_at,
144158
completed_at,
145-
consumed_at
159+
consumed_at,
160+
unlinked_at
146161
FROM upstream_oauth_authorization_sessions
147162
WHERE upstream_oauth_authorization_session_id = $1
148163
"#,

crates/storage/src/upstream_oauth2/link.rs

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -200,6 +200,22 @@ pub trait UpstreamOAuthLinkRepository: Send + Sync {
200200
///
201201
/// Returns [`Self::Error`] if the underlying repository fails
202202
async fn count(&mut self, filter: UpstreamOAuthLinkFilter<'_>) -> Result<usize, Self::Error>;
203+
204+
/// Delete a [`UpstreamOAuthLink`]
205+
///
206+
/// # Parameters
207+
///
208+
/// * `clock`: The clock used to generate timestamps
209+
/// * `upstream_oauth_link`: The [`UpstreamOAuthLink`] to delete
210+
///
211+
/// # Errors
212+
///
213+
/// Returns [`Self::Error`] if the underlying repository fails
214+
async fn remove(
215+
&mut self,
216+
clock: &dyn Clock,
217+
upstream_oauth_link: UpstreamOAuthLink,
218+
) -> Result<(), Self::Error>;
203219
}
204220

205221
repository_impl!(UpstreamOAuthLinkRepository:
@@ -233,4 +249,6 @@ repository_impl!(UpstreamOAuthLinkRepository:
233249
) -> Result<Page<UpstreamOAuthLink>, Self::Error>;
234250

235251
async fn count(&mut self, filter: UpstreamOAuthLinkFilter<'_>) -> Result<usize, Self::Error>;
252+
253+
async fn remove(&mut self, clock: &dyn Clock, upstream_oauth_link: UpstreamOAuthLink) -> Result<(), Self::Error>;
236254
);

0 commit comments

Comments
 (0)