Skip to content

test(oidc-device-organization): pin the identity swap and _deviceId stability - #91

Merged
guimard merged 1 commit into
fix/sshca-rate-limitfrom
test/device-organization
Sep 6, 2026
Merged

guimard merged 1 commit into
fix/sshca-rate-limitfrom
test/device-organization

Conversation

@guimard

@guimard guimard commented Sep 6, 2026

Copy link
Copy Markdown
Member

Closes #71. Stacked on #90#89#88#87.

The issue was filed when the plugin had no t/ at all. The #72 fix added one
(52 assertions, the fail-closed path), but the three gaps the issue actually
names were still untested. This closes them: 52 → 125 assertions.

_deviceId stability across a refresh

It is documented as "deterministic (stable across refreshes)", and
PamAccess::_callerId returns it as the bastion identity — a device whose id
changed at refresh would silently lose its vouchers, and the symptom would show
up hours later as voucher_mismatch.

Exercised end to end through /pam/heartbeat, the path Open Bastion actually
uses, across two consecutive heartbeats. pam-access is pulled in as a
test-only dependency (test_depends in plugin.json), which the tooling
already supports and CI already honours.

The renewed token carries no user_session_id — an offline refresh token is
standalone by design — which is precisely why _deviceId has to be stable on
its own: there is nothing else to key on.

AllowOffline=0 + ownership=organization

An access token is issued, no refresh token of either kind, offline_access
is not advertised as granted, and the identity swap still happens with
_deviceId stamped.

The identity swap itself

The wholesale %$session_data = %{ $session->data } copy and the
user_session_id reassignment:

  • the token points at the synthetic session, not the admin's;
  • no admin attribute (mail, cn, uid, _session_uid) survives the copy;
  • userinfo answers as the device and carries none of the admin's attributes;
  • the device token still works after the approving admin's SSO session is
    removed
    — the entire point of organizational ownership;
  • and the negative: an RP without ownership=organization gets no _deviceId
    and keeps the admin identity.

One constraint pinned

The plugin's design comment dismisses the "offline refresh re-resolves the user
in the UserDB" objection on the grounds that Open Bastion never uses the core
refresh grant. Nothing checked that. It now does: the core refresh_token
grant answers 400 invalid_grant for these tokens. If that ever changes,
the test says so before a deployment finds out.

No production code changed, apart from the test_depends entry.

https://claude.ai/code/session_01GfBG36HfzjGy8W9rJQbxBL

…tability

The plugin carries the most identity-critical code in the chain — it replaces
the approving admin's session with a synthetic one and derives _deviceId, the
value the whole bastion vouching chain keys on — and three things about it
were pinned nowhere.

_deviceId stability across a refresh. It is documented as "stable across
refreshes" and PamAccess::_callerId returns it as the bastion identity, so a
device whose id changed at refresh would silently lose its vouchers. Now
exercised end to end through /pam/heartbeat (pam-access is pulled in as a
test-only dependency), including two consecutive heartbeats.

The AllowOffline=0 + ownership=organization combination: an access token is
issued, no refresh token of either kind, and the identity swap still happens.

The swap itself: the token points at the synthetic session and not the admin's,
no admin attribute survives the wholesale session copy, userinfo answers as the
device, and the device token keeps working after the approving admin's SSO
session is removed — which is the entire point of organizational ownership.
Plus the negative: an RP without ownership=organization gets no _deviceId.

It also pins the constraint the plugin's own design comment leans on: the core
/oauth2/token refresh grant answers invalid_grant for these tokens, which is
why Open Bastion refreshes through /pam/heartbeat.

52 -> 125 assertions.

Closes #71

Claude-Session: https://claude.ai/code/session_01GfBG36HfzjGy8W9rJQbxBL

@guimard guimard left a comment

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Test-only, well targeted at the gaps in #71. test_depends is honoured by mcp/lib.js and CI. Pinning the core invalid_grant behaviour is a deliberate, documented tripwire — good.

@guimard
guimard merged commit 83ba88b into fix/sshca-rate-limit Sep 6, 2026
44 checks passed
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