Skip to content

refactor: remove the unused consumer-scoping branch from consent grants - #73

Merged
hongwei1 merged 1 commit into
develop-obpfrom
fix/consent-revoke-consumer-scope
Aug 9, 2026
Merged

refactor: remove the unused consumer-scoping branch from consent grants#73
hongwei1 merged 1 commit into
develop-obpfrom
fix/consent-revoke-consumer-scope

Conversation

@hongwei1

@hongwei1 hongwei1 commented Aug 9, 2026

Copy link
Copy Markdown
Owner

grantAccessToViews took a consumerId that no caller passed. All four call sites use the default ALL_CONSUMERS, and the two methods the branch reached for — grantAccessToSystemViewForConsumer / grantAccessToCustomViewForConsumer — have no other caller anywhere in the codebase.

Why dead code was worth removing rather than leaving

AccountAccess.consumer_id stores the literal string ALL_CONSUMERS; it is not a wildcard, and every lookup matches it by equality:

// MapperViews.accessGrantedToUserForConsumer
By(AccountAccess.user_fk, user.userPrimaryKey.value),
By(AccountAccess.consumer_id, consumerId)          // exact match

revokeConsentAccountAccess sweeps a revoked consent’s rows by asking for exactly ALL_CONSUMERS. So a row written under a real consumer id would have been invisible to it — access outliving the consent that created it, with nothing left to remove it.

Nobody had hit this, because nobody passed a consumerId. But the first person to use the parameter would have got it silently, and the grant and revoke sites are far enough apart that the connection is easy to miss.

Removing the branch makes the invariant hold by construction rather than by coincidence. Both sides now state it: grantAccessToViews carries the warning, and revokeConsentAccountAccess records that its sweep is complete only because of it.

The scaladoc also still claimed “UK is the exception — it grants to the real PSU — so it passes the consent’s own consumerId”. That stopped being true when UK moved to the per-consent shadow user; corrected.

The Views trait keeps both ForConsumer methods — only this caller goes.

Verification

Suite
code.api.berlin.group 183/183
code.api.UKOpenBanking 414/414
code.api.v5_1_0 246/246

The local probe matrix gains a check that reads the granted rows back out of the database and asserts every one is scoped ALL_CONSUMERS, so the property revoke depends on is pinned rather than assumed:

[PASS] UK v3.1   every consent row is scoped ALL_CONSUMERS, so revoke can find them all
[PASS] UK v4.0.1 every consent row is scoped ALL_CONSUMERS, so revoke can find them all

103/103 against a running instance.

grantAccessToViews took a consumerId that no caller passed. All four call
sites use the default, ALL_CONSUMERS, and the two methods the branch reached
for -- grantAccessToSystemViewForConsumer and grantAccessToCustomViewForConsumer
-- have no other caller anywhere in the codebase.

Dead, but not harmless. AccountAccess.consumer_id stores the literal string
ALL_CONSUMERS rather than acting as a wildcard, and every lookup matches it by
equality. revokeConsentAccountAccess sweeps a revoked consent's rows by asking
for exactly ALL_CONSUMERS, so any row written under a real consumer id would
have been invisible to it: access outliving the consent that created it, with
nothing left to remove it. Whoever first passed a consumerId would have got
that silently, and the two functions are far enough apart that the connection
is easy to miss.

Removing the branch makes the invariant hold by construction rather than by
coincidence, and both sides now say so: grantAccessToViews carries the warning,
revokeConsentAccountAccess records that its sweep is complete only because of
it. The scaladoc also claimed UK grants to the real PSU and passes the
consent's own consumerId, which stopped being true when UK moved to the shadow
user.

The Views trait keeps both ForConsumer methods; only this caller goes.

berlin.group 183/183, UKOpenBanking 414/414, v5_1_0 246/246. The local probe
matrix gains a check that reads the granted rows back and asserts every one is
scoped ALL_CONSUMERS, so the property revoke depends on is pinned rather than
assumed: 103/103.
@sonarqubecloud

sonarqubecloud Bot commented Aug 9, 2026

Copy link
Copy Markdown

@hongwei1
hongwei1 merged commit 96e7838 into develop-obp Aug 9, 2026
25 checks passed
@hongwei1
hongwei1 deleted the fix/consent-revoke-consumer-scope branch August 9, 2026 12:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant