diff --git a/book/src/database_maintenance.md b/book/src/database_maintenance.md index 2aa39ff71..e8c583d12 100644 --- a/book/src/database_maintenance.md +++ b/book/src/database_maintenance.md @@ -25,7 +25,7 @@ definitions. ```bash docker stop docker run --rm -i -t -v kanidmd:/data \ - kanidm/server:latest /sbin/kanidmd reindex -c /data/server.toml + kanidm/server:latest /sbin/kanidmd database reindex -c /data/server.toml docker start ``` @@ -41,7 +41,7 @@ affects pagesize) in server.toml, you must run a vacuum for this to take effect: ```bash docker stop docker run --rm -i -t -v kanidmd:/data \ - kanidm/server:latest /sbin/kanidmd vacuum -c /data/server.toml + kanidm/server:latest /sbin/kanidmd database vacuum -c /data/server.toml docker start ``` @@ -59,7 +59,7 @@ You can run a verification with: ```bash docker stop docker run --rm -i -t -v kanidmd:/data \ - kanidm/server:latest /sbin/kanidmd verify -c /data/server.toml + kanidm/server:latest /sbin/kanidmd database verify -c /data/server.toml docker start ``` diff --git a/server/lib/src/idm/oauth2.rs b/server/lib/src/idm/oauth2.rs index 8468e49fd..e48140e63 100644 --- a/server/lib/src/idm/oauth2.rs +++ b/server/lib/src/idm/oauth2.rs @@ -1153,7 +1153,7 @@ impl<'a> IdmServerProxyWriteTransaction<'a> { // Oauth2TokenType::Access { .. } | Oauth2TokenType::ClientAccess { .. } => { admin_error!("attempt to refresh with an access token"); - Err(Oauth2Error::InvalidToken) + Err(Oauth2Error::InvalidRequest) } Oauth2TokenType::Refresh { scopes, @@ -1167,7 +1167,7 @@ impl<'a> IdmServerProxyWriteTransaction<'a> { } => { if exp <= ct.as_secs() as i64 { security_info!(?uuid, "refresh token has expired, "); - return Err(Oauth2Error::InvalidToken); + return Err(Oauth2Error::InvalidGrant); } // Check the session is still valid. This call checks the parent session @@ -1187,7 +1187,7 @@ impl<'a> IdmServerProxyWriteTransaction<'a> { ?uuid, "access token has no account not valid, returning inactive" ); - return Err(Oauth2Error::InvalidToken); + return Err(Oauth2Error::InvalidGrant); }; // Check the not issued before of the session relative to this refresh iat @@ -1199,7 +1199,7 @@ impl<'a> IdmServerProxyWriteTransaction<'a> { ?session_id, "No OAuth2 session found, unable to proceed with refresh" ); - Oauth2Error::InvalidToken + Oauth2Error::InvalidGrant })?; // If the refresh token was issued previous to the time listed in our oauth2_session @@ -1231,16 +1231,21 @@ impl<'a> IdmServerProxyWriteTransaction<'a> { return Err(Oauth2Error::InvalidGrant); } - // Check the scopes are identical, or None. - if let Some(req_scopes) = req_scopes { - if &scopes != req_scopes { + // Check the scopes are equal or subset, OR none. + let update_scopes = if let Some(req_scopes) = req_scopes { + if req_scopes.is_subset(&scopes) { + debug!("oauth2 scopes requested, checked as valid."); + // We have to return the requested set since it + // may be constrained. + req_scopes.clone() + } else { warn!("oauth2 scopes requested, invalid."); return Err(Oauth2Error::InvalidScope); - } else { - debug!("oauth2 scopes requested, checked as valid."); } } else { debug!("No OAuth2 scopes requested, this is valid."); + // Return the initial set of scopes. + scopes }; // ---------- @@ -1251,7 +1256,7 @@ impl<'a> IdmServerProxyWriteTransaction<'a> { self.generate_access_token_response( o2rs, ct, - scopes, + update_scopes, account_uuid, parent_session_id, session_id, @@ -5525,6 +5530,29 @@ mod tests { .check_oauth2_token_exchange(&client_authz, &token_req, ct) .expect("Unable to exchange for OAuth2 token"); + // Get the user entry to check the session life was extended. + + let entry = idms_prox_write + .qs_write + .internal_search_uuid(UUID_TESTPERSON_1) + .expect("failed"); + let session = entry + .get_ava_as_oauth2session_map(Attribute::OAuth2Session) + .and_then(|sessions| sessions.first_key_value()) + // If there is no map, then something is wrong. + .unwrap(); + + trace!(?session); + // The Oauth2 Session must be updated with a newer session time. + assert_eq!( + SessionState::ExpiresAt( + time::OffsetDateTime::UNIX_EPOCH + + ct + + Duration::from_secs(OAUTH_REFRESH_TOKEN_EXPIRY) + ), + session.1.state + ); + assert!(idms_prox_write.commit().is_ok()); trace!(?access_token_response_3); @@ -5559,7 +5587,7 @@ mod tests { .check_oauth2_token_exchange(&client_authz, &token_req, ct) .unwrap_err(); - assert!(access_token_response_4 == Oauth2Error::InvalidToken); + assert!(access_token_response_4 == Oauth2Error::InvalidGrant); assert!(idms_prox_write.commit().is_ok()); } @@ -5609,7 +5637,7 @@ mod tests { // Should be unable to exchange. .unwrap_err(); - assert!(access_token_response_2 == Oauth2Error::InvalidToken); + assert!(access_token_response_2 == Oauth2Error::InvalidGrant); assert!(idms_prox_write.commit().is_ok()); } @@ -5832,6 +5860,154 @@ mod tests { // Success! } + #[idm_test] + async fn test_idm_oauth2_refresh_token_scope_constraints( + idms: &IdmServer, + idms_delayed: &mut IdmServerDelayed, + ) { + // First, setup to get a token. + let ct = Duration::from_secs(TEST_CURRENT_TIME); + + let (access_token_response_1, client_authz) = + setup_refresh_token(idms, idms_delayed, ct).await; + + // https://www.rfc-editor.org/rfc/rfc6749#section-1.5 + // Refresh tokens are issued to the client by the authorization + // server and are used to obtain a new access token when the + // current access token becomes invalid or expires, or to obtain + // additional access tokens with identical or narrower scope + // (access tokens may have a shorter lifetime and fewer + // permissions than authorized by the resource owner). + + let ct = Duration::from_secs(TEST_CURRENT_TIME + 10); + let mut idms_prox_write = idms.proxy_write(ct).await.unwrap(); + + let refresh_token = access_token_response_1 + .refresh_token + .as_ref() + .expect("no refresh token was issued") + .clone(); + + // Get the initial scopes. + let jws_verifier = JwsDangerReleaseWithoutVerify::default(); + + let access_token_unverified = JwsCompact::from_str(&access_token_response_1.access_token) + .expect("Invalid Access Token"); + + let reflected_token = jws_verifier + .verify(&access_token_unverified) + .unwrap() + .from_json::>() + .expect("Failed to access internals of the refresh token"); + + trace!(?reflected_token); + let initial_scopes = reflected_token.extensions.scope; + trace!(?initial_scopes); + + // Should be the same scopes as initial. + let token_req: AccessTokenRequest = GrantTypeReq::RefreshToken { + refresh_token, + scope: None, + } + .into(); + + let access_token_response_2 = idms_prox_write + .check_oauth2_token_exchange(&client_authz, &token_req, ct) + .expect("Unable to exchange for OAuth2 token"); + + let access_token_unverified = JwsCompact::from_str(&access_token_response_2.access_token) + .expect("Invalid Access Token"); + + let reflected_token = jws_verifier + .verify(&access_token_unverified) + .unwrap() + .from_json::>() + .expect("Failed to access internals of the refresh token"); + + assert_eq!(initial_scopes, reflected_token.extensions.scope); + + let refresh_token = access_token_response_2 + .refresh_token + .as_ref() + .expect("no refresh token was issued") + .clone(); + + // Now the scopes can be constrained. + let token_req: AccessTokenRequest = GrantTypeReq::RefreshToken { + refresh_token, + scope: Some(["openid".to_string()].into()), + } + .into(); + + let access_token_response_3 = idms_prox_write + .check_oauth2_token_exchange(&client_authz, &token_req, ct) + .expect("Unable to exchange for OAuth2 token"); + + let access_token_unverified = JwsCompact::from_str(&access_token_response_3.access_token) + .expect("Invalid Access Token"); + + let reflected_token = jws_verifier + .verify(&access_token_unverified) + .unwrap() + .from_json::>() + .expect("Failed to access internals of the refresh token"); + + assert_ne!(initial_scopes, reflected_token.extensions.scope); + + // Keep the constrained scopes. + let constrained_scopes = reflected_token.extensions.scope; + + let refresh_token = access_token_response_3 + .refresh_token + .as_ref() + .expect("no refresh token was issued") + .clone(); + + // No scope request still issues the constrained values. + let token_req: AccessTokenRequest = GrantTypeReq::RefreshToken { + refresh_token, + scope: None, + } + .into(); + + let access_token_response_4 = idms_prox_write + .check_oauth2_token_exchange(&client_authz, &token_req, ct) + .expect("Unable to exchange for OAuth2 token"); + + let access_token_unverified = JwsCompact::from_str(&access_token_response_4.access_token) + .expect("Invalid Access Token"); + + let reflected_token = jws_verifier + .verify(&access_token_unverified) + .unwrap() + .from_json::>() + .expect("Failed to access internals of the refresh token"); + + assert_ne!(initial_scopes, reflected_token.extensions.scope); + assert_eq!(constrained_scopes, reflected_token.extensions.scope); + + let refresh_token = access_token_response_4 + .refresh_token + .as_ref() + .expect("no refresh token was issued") + .clone(); + + // We can't now extend back to the initial scopes. + let token_req: AccessTokenRequest = GrantTypeReq::RefreshToken { + refresh_token, + scope: Some(initial_scopes), + } + .into(); + + let access_token_response_5_err = idms_prox_write + .check_oauth2_token_exchange(&client_authz, &token_req, ct) + .unwrap_err(); + + assert_eq!(access_token_response_5_err, Oauth2Error::InvalidScope); + + assert!(idms_prox_write.commit().is_ok()); + } + #[test] // I know this looks kinda dumb but at some point someone pointed out that our scope syntax wasn't compliant with rfc6749 //(https://datatracker.ietf.org/doc/html/rfc6749#section-3.3), so I'm just making sure that we don't break it again. diff --git a/server/lib/src/value.rs b/server/lib/src/value.rs index bb590022e..1abee030f 100644 --- a/server/lib/src/value.rs +++ b/server/lib/src/value.rs @@ -2409,4 +2409,20 @@ mod tests { assert!(KeyStatus::Valid < KeyStatus::Retained); assert!(KeyStatus::Retained < KeyStatus::Revoked); } + + #[test] + fn test_value_session_state_order() { + assert!( + SessionState::RevokedAt(Cid::new_zero()) > SessionState::RevokedAt(Cid::new_count(1)) + ); + assert!( + SessionState::RevokedAt(Cid::new_zero()) + > SessionState::ExpiresAt(OffsetDateTime::UNIX_EPOCH) + ); + assert!( + SessionState::ExpiresAt(OffsetDateTime::UNIX_EPOCH + Duration::from_secs(1)) + > SessionState::ExpiresAt(OffsetDateTime::UNIX_EPOCH) + ); + assert!(SessionState::ExpiresAt(OffsetDateTime::UNIX_EPOCH) > SessionState::NeverExpires); + } } diff --git a/server/lib/src/valueset/session.rs b/server/lib/src/valueset/session.rs index efa90e846..d0bbb6da1 100644 --- a/server/lib/src/valueset/session.rs +++ b/server/lib/src/valueset/session.rs @@ -391,7 +391,7 @@ impl ValueSetT for ValueSetSession { // session.state value and what it currently is set to. for (k_other, v_other) in b.iter() { if let Some(v_self) = self.map.get_mut(k_other) { - // We only update if lower. This is where RevokedAt + // We only update if greater. This is where RevokedAt // always proceeds other states, and lower revoked // cids will always take effect. if v_other.state > v_self.state { @@ -1030,7 +1030,7 @@ impl ValueSetT for ValueSetOauth2Session { let mut rs_filter = self.rs_filter; for (k_other, v_other) in b.iter() { if let Some(v_self) = map.get_mut(k_other) { - // We only update if lower. This is where RevokedAt + // We only update if greater. This is where RevokedAt // always proceeds other states, and lower revoked // cids will always take effect. if v_other.state > v_self.state {