Skip to content

Comments

feat: Integration test for verifying grant revocation#573

Open
konac-hamza wants to merge 1 commit intoopenstack-experimental:mainfrom
konac-hamza:feature/integration-test-grant-revocation
Open

feat: Integration test for verifying grant revocation#573
konac-hamza wants to merge 1 commit intoopenstack-experimental:mainfrom
konac-hamza:feature/integration-test-grant-revocation

Conversation

@konac-hamza
Copy link
Collaborator

@konac-hamza konac-hamza commented Feb 19, 2026

No description provided.

gtema
gtema requested changes Feb 20, 2026
View reviewed changes
tests/integration/src/assignment/grant/revoke.rs

use eyre::Result;
use tracing_test::traced_test;

Copy link
Collaborator

@gtema gtema Feb 20, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

please keep the spaces between import groups. First group is typically std (+ whatever is std nowadays like tracing, eyre), second group are the other foreign crates, 3rd are the own crates (i.e. when a project spreads across multiple crates, what in our case is going to be openstack_keystone_api_types, openstack_keystone_storage, etc). The last group are the imports from the current crate itself.

tests/integration/src/assignment/grant/revoke.rs
access_rules: None,
name: Uuid::new_v4().to_string(),
project_id: "project_a".into(),
roles: vec![Role {
Copy link
Collaborator

@gtema gtema Feb 20, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

you should be having multiple roles. Explanation further down.

tests/integration/src/assignment/grant/revoke.rs
.get_token_provider()
.encode_token(&post_revoke_token)?;

assert!(
Copy link
Collaborator

@gtema gtema Feb 20, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

intention with having multiple roles is to still have a working authentication just ensuring the revoked role is not present anymore.

tests/integration/src/assignment/grant/revoke.rs

// CHECK 3: existing auth (issued before revocation) is no longer accepted
assert!(
matches!(
Copy link
Collaborator

@gtema gtema Feb 20, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this shows the real problem and is exactly the purpose of the test. The token must be considered as a revoked and not that the user is not having roles. In the combination with having more roles initially this would lead to the vulnerability.
I am not sure why exactly, but we should get the TokenProviderError::TokenRevoked error

tests/integration/src/assignment/grant.rs
use eyre::Report;
use sea_orm::{DbConn, entity::*};
use std::sync::Arc;

Copy link
Collaborator

@gtema gtema Feb 20, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

same comment regarding the imports sorting is applicable here.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@gtema gtema gtema requested changes

Requested changes must be addressed to merge this pull request.

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@konac-hamza @gtema