Repository navigation
fix(auth): refresh with granted scopes #1331
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
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -2306,10 +2306,8 @@ impl AuthorizationManager { | |
| .exchange_refresh_token(&refresh_token_value) | ||
| // RFC 8707: the resource indicator is required on token requests, including refreshes | ||
| .add_extra_param("resource", self.oauth_resource().await); | ||
| let mut refresh_scopes = stored_credentials.granted_scopes.clone(); | ||
| self.add_offline_access_if_supported(&mut refresh_scopes); | ||
| let requested_scopes = refresh_scopes.clone(); | ||
| for scope in refresh_scopes { | ||
| let requested_scopes = stored_credentials.granted_scopes.clone(); | ||
| for scope in requested_scopes.iter().cloned() { | ||
| refresh_request = refresh_request.add_scope(Scope::new(scope)); | ||
| } | ||
| let mut token_result = match refresh_request | ||
|
|
@@ -9052,7 +9050,7 @@ mod tests { | |
| } | ||
|
|
||
| #[tokio::test] | ||
| async fn refresh_token_adds_offline_access_when_as_supports_it() { | ||
| async fn refresh_token_does_not_add_ungranted_offline_access() { | ||
| let (base_url, captured) = start_token_server().await; | ||
|
|
||
| let mut manager = manager_with_metadata(Some(AuthorizationMetadata { | ||
|
|
@@ -9078,6 +9076,45 @@ mod tests { | |
|
|
||
| manager.refresh_token().await.unwrap(); | ||
|
|
||
| let body = captured.lock().unwrap().take().unwrap(); | ||
| let params: std::collections::HashMap<_, _> = url::form_urlencoded::parse(body.as_bytes()) | ||
| .into_owned() | ||
| .collect(); | ||
| let scope = params | ||
| .get("scope") | ||
| .expect("scope should be present in refresh request"); | ||
| let mut scope_parts: Vec<&str> = scope.split_whitespace().collect(); | ||
| scope_parts.sort_unstable(); | ||
| assert_eq!(scope_parts, vec!["read"]); | ||
|
Comment on lines
+9086
to
+9088
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The test token server leaves |
||
| } | ||
|
|
||
| #[tokio::test] | ||
| async fn refresh_token_keeps_offline_access_when_it_was_granted() { | ||
| let (base_url, captured) = start_token_server().await; | ||
|
|
||
| let mut manager = manager_with_metadata(Some(AuthorizationMetadata { | ||
| authorization_endpoint: format!("{}/authorize", base_url), | ||
| token_endpoint: format!("{}/token", base_url), | ||
| scopes_supported: Some(vec!["read".to_string(), "offline_access".to_string()]), | ||
| ..Default::default() | ||
| })) | ||
| .await; | ||
| manager.configure_client(test_client_config()).unwrap(); | ||
|
|
||
| let stored = StoredCredentials { | ||
| client_id: "my-client".to_string(), | ||
| token_response: Some(make_token_response_with_refresh( | ||
| "old-token", | ||
| "my-refresh-token", | ||
| )), | ||
| granted_scopes: vec!["read".to_string(), "offline_access".to_string()], | ||
| token_received_at: Some(AuthorizationManager::now_epoch_secs()), | ||
| issuer: None, | ||
| }; | ||
| manager.credential_store.save(stored).await.unwrap(); | ||
|
|
||
| manager.refresh_token().await.unwrap(); | ||
|
|
||
| let body = captured.lock().unwrap().take().unwrap(); | ||
| let params: std::collections::HashMap<_, _> = url::form_urlencoded::parse(body.as_bytes()) | ||
| .into_owned() | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
granted_scopesis cloned into a newVec, and then each element is cloned again. So theresolve_granted_scopescall below can takerequested_scopesdirectly instead of&requested_scopes.