Skip to content

Commit 81dbe20

Browse files
zecakehpoljar
zecakeh
authored and
poljar
committed
refactor(oidc): Remove support for ID tokens
ID tokens are a feature of OpenID Connect, we don't need them to support OAuth 2.0. Signed-off-by: Kévin Commaille <[email protected]>
1 parent fd0fca4 commit 81dbe20

File tree

9 files changed

+32
-275
lines changed

9 files changed

+32
-275
lines changed

‎bindings/matrix-sdk-ffi/src/client.rs

Lines changed: 12 additions & 68 deletions
Original file line numberDiff line numberDiff line change
@@ -10,12 +10,7 @@ use matrix_sdk::{
1010
authentication::oidc::{
1111
registrations::{ClientId, OidcRegistrations},
1212
requests::account_management::AccountManagementActionFull,
13-
types::{
14-
registration::{
15-
ClientMetadata, ClientMetadataVerificationError, VerifiedClientMetadata,
16-
},
17-
requests::Prompt as SdkOidcPrompt,
18-
},
13+
types::{registration::VerifiedClientMetadata, requests::Prompt as SdkOidcPrompt},
1914
OidcAuthorizationData, OidcSession,
2015
},
2116
event_cache::EventCacheError,
@@ -1613,19 +1608,11 @@ impl Session {
16131608
matrix_sdk::authentication::oidc::OidcSessionTokens {
16141609
access_token,
16151610
refresh_token,
1616-
latest_id_token,
16171611
},
16181612
issuer,
16191613
} = api.user_session().context("Missing session")?;
16201614
let client_id = api.client_id().context("OIDC client ID is missing.")?.0.clone();
1621-
let client_metadata =
1622-
api.client_metadata().context("OIDC client metadata is missing.")?.clone();
1623-
let oidc_data = OidcSessionData {
1624-
client_id,
1625-
client_metadata,
1626-
latest_id_token: latest_id_token.map(|t| t.to_string()),
1627-
issuer,
1628-
};
1615+
let oidc_data = OidcSessionData { client_id, issuer };
16291616

16301617
let oidc_data = serde_json::to_string(&oidc_data).ok();
16311618
Ok(Session {
@@ -1658,14 +1645,7 @@ impl TryFrom<Session> for AuthSession {
16581645

16591646
if let Some(oidc_data) = oidc_data {
16601647
// Create an OidcSession.
1661-
let oidc_data = serde_json::from_str::<OidcUnvalidatedSessionData>(&oidc_data)?
1662-
.validate()
1663-
.context("OIDC metadata validation failed.")?;
1664-
let latest_id_token = oidc_data
1665-
.latest_id_token
1666-
.map(TryInto::try_into)
1667-
.transpose()
1668-
.context("OIDC latest_id_token is invalid.")?;
1648+
let oidc_data = serde_json::from_str::<OidcSessionData>(&oidc_data)?;
16691649

16701650
let user_session = matrix_sdk::authentication::oidc::UserSession {
16711651
meta: matrix_sdk::SessionMeta {
@@ -1675,16 +1655,12 @@ impl TryFrom<Session> for AuthSession {
16751655
tokens: matrix_sdk::authentication::oidc::OidcSessionTokens {
16761656
access_token,
16771657
refresh_token,
1678-
latest_id_token,
16791658
},
16801659
issuer: oidc_data.issuer,
16811660
};
16821661

1683-
let session = OidcSession {
1684-
client_id: ClientId(oidc_data.client_id),
1685-
metadata: oidc_data.client_metadata,
1686-
user: user_session,
1687-
};
1662+
let session =
1663+
OidcSession { client_id: ClientId(oidc_data.client_id), user: user_session };
16881664

16891665
Ok(AuthSession::Oidc(session.into()))
16901666
} else {
@@ -1707,63 +1683,31 @@ impl TryFrom<Session> for AuthSession {
17071683

17081684
/// Represents a client registration against an OpenID Connect authentication
17091685
/// issuer.
1710-
#[derive(Serialize)]
1686+
#[derive(Serialize, Deserialize)]
1687+
#[serde(try_from = "OidcSessionDataDeHelper")]
17111688
pub(crate) struct OidcSessionData {
17121689
client_id: String,
1713-
client_metadata: VerifiedClientMetadata,
1714-
latest_id_token: Option<String>,
17151690
issuer: String,
17161691
}
17171692

1718-
/// Represents an unverified client registration against an OpenID Connect
1719-
/// authentication issuer. Call `validate` on this to use it for restoration.
17201693
#[derive(Deserialize)]
1721-
#[serde(try_from = "OidcUnvalidatedSessionDataDeHelper")]
1722-
pub(crate) struct OidcUnvalidatedSessionData {
1694+
struct OidcSessionDataDeHelper {
17231695
client_id: String,
1724-
client_metadata: ClientMetadata,
1725-
latest_id_token: Option<String>,
1726-
issuer: String,
1727-
}
1728-
1729-
impl OidcUnvalidatedSessionData {
1730-
/// Validates the data so that it can be used.
1731-
fn validate(self) -> Result<OidcSessionData, ClientMetadataVerificationError> {
1732-
Ok(OidcSessionData {
1733-
client_id: self.client_id,
1734-
client_metadata: self.client_metadata.validate()?,
1735-
latest_id_token: self.latest_id_token,
1736-
issuer: self.issuer,
1737-
})
1738-
}
1739-
}
1740-
1741-
#[derive(Deserialize)]
1742-
struct OidcUnvalidatedSessionDataDeHelper {
1743-
client_id: String,
1744-
client_metadata: ClientMetadata,
1745-
latest_id_token: Option<String>,
17461696
issuer_info: Option<AuthenticationServerInfo>,
17471697
issuer: Option<String>,
17481698
}
17491699

1750-
impl TryFrom<OidcUnvalidatedSessionDataDeHelper> for OidcUnvalidatedSessionData {
1700+
impl TryFrom<OidcSessionDataDeHelper> for OidcSessionData {
17511701
type Error = String;
17521702

1753-
fn try_from(value: OidcUnvalidatedSessionDataDeHelper) -> Result<Self, Self::Error> {
1754-
let OidcUnvalidatedSessionDataDeHelper {
1755-
client_id,
1756-
client_metadata,
1757-
latest_id_token,
1758-
issuer_info,
1759-
issuer,
1760-
} = value;
1703+
fn try_from(value: OidcSessionDataDeHelper) -> Result<Self, Self::Error> {
1704+
let OidcSessionDataDeHelper { client_id, issuer_info, issuer } = value;
17611705

17621706
let issuer = issuer
17631707
.or(issuer_info.map(|info| info.issuer))
17641708
.ok_or_else(|| "missing field `issuer`".to_owned())?;
17651709

1766-
Ok(Self { client_id, client_metadata, latest_id_token, issuer })
1710+
Ok(Self { client_id, issuer })
17671711
}
17681712
}
17691713

‎crates/matrix-sdk/src/authentication/oidc/backend/mock.rs

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -29,7 +29,6 @@ use mas_oidc_client::{
2929
iana::oauth::OAuthTokenTypeHint,
3030
oidc::{ProviderMetadataVerificationError, VerifiedProviderMetadata},
3131
registration::{ClientRegistrationResponse, VerifiedClientMetadata},
32-
IdToken,
3332
},
3433
};
3534
use url::Url;
@@ -118,7 +117,6 @@ impl OidcBackend for MockImpl {
118117
&self,
119118
_provider_metadata: VerifiedProviderMetadata,
120119
_credentials: ClientCredentials,
121-
_metadata: VerifiedClientMetadata,
122120
_auth_code: AuthorizationCode,
123121
_validation_data: AuthorizationValidationData,
124122
) -> Result<OidcSessionTokens, OidcError> {
@@ -158,9 +156,7 @@ impl OidcBackend for MockImpl {
158156
&self,
159157
_provider_metadata: VerifiedProviderMetadata,
160158
_credentials: ClientCredentials,
161-
_metadata: &VerifiedClientMetadata,
162159
refresh_token: String,
163-
_latest_id_token: Option<IdToken<'static>>,
164160
) -> Result<RefreshedSessionTokens, OidcError> {
165161
if Some(refresh_token) != self.expected_refresh_token {
166162
Err(OidcError::Oidc(OidcClientError::TokenRefresh(TokenRefreshError::Token(

‎crates/matrix-sdk/src/authentication/oidc/backend/mod.rs

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,6 @@ use mas_oidc_client::{
2323
iana::oauth::OAuthTokenTypeHint,
2424
oidc::VerifiedProviderMetadata,
2525
registration::{ClientRegistrationResponse, VerifiedClientMetadata},
26-
IdToken,
2726
},
2827
};
2928
use url::Url;
@@ -58,7 +57,6 @@ pub(super) trait OidcBackend: std::fmt::Debug + Send + Sync {
5857
&self,
5958
provider_metadata: VerifiedProviderMetadata,
6059
credentials: ClientCredentials,
61-
metadata: VerifiedClientMetadata,
6260
auth_code: AuthorizationCode,
6361
validation_data: AuthorizationValidationData,
6462
) -> Result<OidcSessionTokens, OidcError>;
@@ -67,9 +65,7 @@ pub(super) trait OidcBackend: std::fmt::Debug + Send + Sync {
6765
&self,
6866
provider_metadata: VerifiedProviderMetadata,
6967
credentials: ClientCredentials,
70-
metadata: &VerifiedClientMetadata,
7168
refresh_token: String,
72-
latest_id_token: Option<IdToken<'static>>,
7369
) -> Result<RefreshedSessionTokens, OidcError>;
7470

7571
async fn revoke_token(

‎crates/matrix-sdk/src/authentication/oidc/backend/server.rs

Lines changed: 4 additions & 38 deletions
Original file line numberDiff line numberDiff line change
@@ -21,11 +21,9 @@ use chrono::Utc;
2121
use http::StatusCode;
2222
use mas_oidc_client::{
2323
http_service::HttpService,
24-
jose::jwk::PublicJsonWebKeySet,
2524
requests::{
2625
authorization_code::{access_token_with_authorization_code, AuthorizationValidationData},
2726
discovery::{discover, insecure_discover},
28-
jose::{fetch_jwks, JwtVerificationData},
2927
refresh_token::refresh_access_token,
3028
registration::register_client,
3129
revocation::revoke_token,
@@ -35,7 +33,6 @@ use mas_oidc_client::{
3533
iana::oauth::OAuthTokenTypeHint,
3634
oidc::{ProviderMetadata, ProviderMetadataVerificationError, VerifiedProviderMetadata},
3735
registration::{ClientRegistrationResponse, VerifiedClientMetadata},
38-
IdToken,
3936
},
4037
};
4138
use oauth2::{AsyncHttpClient, HttpClientError, HttpRequest, HttpResponse};
@@ -66,14 +63,6 @@ impl OidcServer {
6663
fn http_service(&self) -> HttpService {
6764
HttpService::new(self.http_client().clone())
6865
}
69-
70-
/// Fetch the OpenID Connect JSON Web Key Set at the given URI.
71-
///
72-
/// Returns an error if the client registration was not restored, or if an
73-
/// error occurred when fetching the data.
74-
async fn fetch_jwks(&self, uri: &Url) -> Result<PublicJsonWebKeySet, OidcError> {
75-
fetch_jwks(&self.http_service(), uri).await.map_err(Into::into)
76-
}
7766
}
7867

7968
#[async_trait::async_trait]
@@ -138,26 +127,16 @@ impl OidcBackend for OidcServer {
138127
&self,
139128
provider_metadata: VerifiedProviderMetadata,
140129
credentials: ClientCredentials,
141-
metadata: VerifiedClientMetadata,
142130
auth_code: AuthorizationCode,
143131
validation_data: AuthorizationValidationData,
144132
) -> Result<OidcSessionTokens, OidcError> {
145-
let jwks = self.fetch_jwks(provider_metadata.jwks_uri()).await?;
146-
147-
let id_token_verification_data = JwtVerificationData {
148-
issuer: provider_metadata.issuer(),
149-
jwks: &jwks,
150-
client_id: &credentials.client_id().to_owned(),
151-
signing_algorithm: metadata.id_token_signed_response_alg(),
152-
};
153-
154-
let (response, id_token) = access_token_with_authorization_code(
133+
let (response, _) = access_token_with_authorization_code(
155134
&self.http_service(),
156135
credentials.clone(),
157136
provider_metadata.token_endpoint(),
158137
auth_code.code,
159138
validation_data,
160-
Some(id_token_verification_data),
139+
None,
161140
Utc::now(),
162141
&mut rng()?,
163142
)
@@ -166,35 +145,23 @@ impl OidcBackend for OidcServer {
166145
Ok(OidcSessionTokens {
167146
access_token: response.access_token,
168147
refresh_token: response.refresh_token,
169-
latest_id_token: id_token,
170148
})
171149
}
172150

173151
async fn refresh_access_token(
174152
&self,
175153
provider_metadata: VerifiedProviderMetadata,
176154
credentials: ClientCredentials,
177-
metadata: &VerifiedClientMetadata,
178155
refresh_token: String,
179-
latest_id_token: Option<IdToken<'static>>,
180156
) -> Result<RefreshedSessionTokens, OidcError> {
181-
let jwks = self.fetch_jwks(provider_metadata.jwks_uri()).await?;
182-
183-
let id_token_verification_data = JwtVerificationData {
184-
issuer: provider_metadata.issuer(),
185-
jwks: &jwks,
186-
client_id: &credentials.client_id().to_owned(),
187-
signing_algorithm: &metadata.id_token_signed_response_alg().clone(),
188-
};
189-
190157
refresh_access_token(
191158
&self.http_service(),
192159
credentials,
193160
provider_metadata.token_endpoint(),
194161
refresh_token,
195162
None,
196-
Some(id_token_verification_data),
197-
latest_id_token.as_ref(),
163+
None,
164+
None,
198165
Utc::now(),
199166
&mut rng()?,
200167
)
@@ -285,7 +252,6 @@ impl OidcBackend for OidcServer {
285252
let tokens = OidcSessionTokens {
286253
access_token: response.access_token().secret().to_owned(),
287254
refresh_token: response.refresh_token().map(|t| t.secret().to_owned()),
288-
latest_id_token: None,
289255
};
290256

291257
Ok(tokens)

‎crates/matrix-sdk/src/authentication/oidc/data_serde.rs

Lines changed: 0 additions & 63 deletions
This file was deleted.

0 commit comments

Comments
 (0)