[Security/UX] Subwallets should be able to obtain their own tokens without admin API key

[manavdeep]

Context: In the current ACA-Py multi-tenant setup, the only way for a subwallet to obtain its JWT token is via the endpoint: POST /multitenancy/wallet/{wallet_id}/token However, this endpoint is protected by the --admin-api-key, meaning the subwallet must:

• Either know the admin key (a security issue), or
• Rely on the admin to generate the token (a UX and trust issue)

Problem:

1. Anyone with the admin-api-key can generate a token for any subwallet, without knowing the wallet's wallet_key. This effectively allows impersonation of tenants by the admin.
2. Subwallets cannot independently authenticate themselves using their own wallet ID/key.
3. This breaks tenant isolation assumptions and makes secure delegation or self-service auth impossible in multi-tenant apps.

Expected Behavior / Feature Request:

Introduce a new endpoint like: POST /auth/token

Which:

• Accepts a wallet label (or ID) and wallet key.
• Returns a JWT token scoped only to that wallet.
• Does not require the admin-api-key.
• Optionally: supports rate-limiting or throttling to prevent abuse.

Why this matters:

• In many hosted use-cases (e.g., police agents, university agents), tenants should not know the admin key.
• Relying on the admin to hand out tokens is fragile, especially at scale.
• Better self-service auth will significantly improve security and operability of ACA-Py's multi-tenant mode.

Environment:

• ACA-Py version: v0.12.x
• Wallet type: askar
• Multi-tenant with JWTs enabled
• Admin API key used: --admin-api-key

[swcurran]

@jamshale @esune -- comments on this? Has this been addressed in versions later than 0.12.x?

[esune]

This was addressed by #2860, which appears to have been included starting in 1.0.0. I believe we decided not to backport the change to the 0.12.x branch as there are ways to work around this (i.e.: having an intermediary service that injects the api-key as needed, like in the Traction proxy service (example configuration here), and because we thought we would move on from 0.12.x as default image sinbce 1.x.x has several other improvements and new features that cannot be backported.

If there is no compelling reason to stay on 0.12.x I would recommend upgrading to a later (post 1.0.0) base image.

[manavdeep]

Sorry for the wrong version confusion. I was using 1.2.4 already. image: ghcr.io/openwallet-foundation/acapy-agent:py3.12-1.2.4

I have attached the swagger.json/openApi.json for the same. I don't see any API here to do individual wallet authentication based on wallet key. The "/multitenancy/wallet/{wallet_id}/token" API only uses wallet_id as required parameter.

openApi.json

[esune]

Okay, I think I got it figured out.

The way the /multitenancy/wallet/{wallet_id}/token endpoint is supposed to work does not require the api-key header: tenants should be able to fetch their own tokens without knowing the admin credentials - as you noted - just by providing their own id/secret combination. However, the @admin_authentication decorator is protecting that route, which is preventing this from happening.

I am not too sure about how this was not caught until now, all I can think is that everyone involved (including us) has been using an intermediary service that masks this bit so it never became a blocker, but I agree we should fix this to make it work as expected. I am also thinking the ability of the admin to fetch tokens without knowing the correct key should be reviewed: while it is useful for recovery scenarios, I think there may be a better pattern to it (maybe rotating the tenant's keys for situations where the original ones were lost, similar to a password reset?).

In doing so, we will also want to add key validation to the request: it does not appear to be enforced by default currently, the multitenant_provider plugin BCGov uses with Traction adds this as well as other functionality, but it should be enforced by the acapy code in the first place.

[manavdeep]

Yeah correct, I think you got it perfectly fine. So I will summarize the 2 things:

1. The /multitenancy/wallet/{wallet_id}/token API should not be restricted by admin key.
2. Token should be generated on the basis of tenant's wallet id and wallet key.

whereas Right now:

1. It is restricted by Admin key.
2. Token is generated just by using wallet Id, no key validation takes place.

For Recovery purposes there are many other better patterns.

[esune]

@swcurran @jamshale @ff137 I may need som insight on these lines: I don;t think they make sense for this route, but don't fully understand why the check was added.

The check seems to be based off the storage type and whether the wallet is managed/unmanaged, but I do not understand WHY we would want to check this condition when fetching a wallet auth token. Is it okay/safe to remove?

[ff137]

@swcurran @jamshale @ff137 I may need som insight on these lines: I don;t think they make sense for this route, but don't fully understand why the check was added.

That appears to have been added here: https://github.com/openwallet-foundation/acapy/pull/1439, 4 years ago.

I've not looked closely at this before, but it does seem that the check is logically reversed. I would assume that if wallet_record.requires_external_key is True, then it should prevent requests that don't have wallet_key... but it's only asserting wallet_key should not be provided if wallet_record.requires_external_key is False.

Definitely worth it to revisit / refactor this. The fact that tenant-admins can rotate access keys, without tenant involvement, has always been a big concern for me.

[esune]

I don't think I fully understand the concept that the requires_external_key flag gravitates around. I initially thought it was about managed vs. unmanaged, but it seems to be something more or different?

Regarding the tenant admins being able to rotate keys without tenant involvement: I think I understand your point of view and somewhat agree, however providing the possibility for an admin to initiate some sort of key rotation/recovery for a tenant that has lost or compromised their credentials seems like an important feature to me.

[esune]

@TimoGlastra wondering if you might have insight on this - I see you committed parts of the code related to requires_external_key

[jamshale]

I've always been confused by this, but my limited understanding is that we currently only have managed mode where the base (admin) wallet has all the tenant keys. The unmanaged mode is for the use case described in this ticket but I don't believe it was ever completed. Unless these docs were never updated. https://github.com/openwallet-foundation/acapy/blob/main/docs/features/Multitenancy.md#single-wallet-vs-multiple-wallets

Also worth noting is this line It is important to note unmanaged mode doesn't provide a lot of security over managed mode. The key is still processed by the agent, and therefore trust is required. It could however provide some benefit in the case a multi-tenant agent is compromised, as the agent doesn't store the key to unlock the wallet.

I do agree that while having the admin wallet contain all the keys doesn't support the case described here, the unmanaged mode doesn't really supply the security desired and losing the tenant key would essentially kill the tenant. So, we could basically implement the unmanaged mode knowing that the keys still require a level of trust with the admin wallet and that losing a key would be fatal, or, only support managed mode as it is now and implementations that fully required trust should just be standalone agents instead of leveraging multi-tenant mode.

Another side note, not really relevant to the problem, but you are able to re-key individual tenants now.

[jamshale]

I don't think I really answered the question, though, about tenants needing to know the admin-key. I think that the token endpoint should be protected by the tenant key only, and they can still be found by the admin in managed mode. Requiring the admin-key doesn't make sense and this requires_external_key thing should probably just be removed.

[esune]

I agree with the above and would be inclined towards "bypassing" the managed mode checks since it cannot really be used. IF we finally complete the implementation of unmanaged mode the behaviour for this endpoint - as you described - would likely not need to change much or at all from the desired behaviour; IF we decide unmanaged mode is really not something we are wanting to implement then we may want to go further and clean up the corresponding code to simplify the project.

Thoughts?

[jamshale]

See the linked PR, but I'm going to close this ticket as won't do. If someone wants to enable an unmanaged multitenancy mode despite the implied security not being fully achieved then go ahead and you can re-open this. I don't think the ticket description is really understanding of what multitenancy is in acapy. There will always be an admin agent where data flows through. We could not save tenant keys in the admin wallet but they will still flow through the admin agent. Then despite this drawback, I don't think we can just use an tenant key as an api key. There's too much security implications by doing this. To enable an unmanaged mode a better authorization system, somewhat like the multitenant provider plugin would need to be added to the core.

If a fully isolated, self managed tenants/subwallets are required then I would consider a custom solution of a controller that's can spawn/create individual agents.