View Issue Details
| ID | Project | Category | View Status | Date Submitted | Last Update |
|---|---|---|---|---|---|
| 0010172 | Taler | merchant backend | public | 2025-07-09 14:55 | 2025-12-21 20:46 |
| Reporter | sebasjm | Assigned To | Christian Grothoff | Resolution deadline | |
| Implementer | Reviewer | ||||
| Release signed by | Deployed by | ||||
| Verifier | Communicator | ||||
| Data protection authority notification required | No | ||||
| CSIRT notification required | No | ||||
| Estimated effort (hours) | Actual effort (hours) | 0 | |||
| Fast-track | No | ||||
| Work package | none | ||||
| Priority | normal | Severity | minor | Reproducibility | have not tried |
| Status | closed | Resolution | fixed | ||
| Product Version | 1.0 | ||||
| Target Version | 1.3 | Fixed in Version | 1.3 | ||
| Summary | 0010172: should not be able to create to create tokens with a token | ||||
| Description | This works, and I can use the token returned to create more tokens (on any scope) $ curl 'http://merchant.taler.test/private/token' -H 'Authorization: Bearer secret-token:GVP8MZ2R22XGNY1KA9Q72RVMHZZ0E7K35E924ZJQECJTT0TFQ6NG' --data-raw '{"scope":"all","duration":{"d_us":"forever"}}' { "access_token": "secret-token:H6QTCSWAD2458EDG09J3JCH98N63ARM0SCXKG4EM3PMP6W942CP0", "token": "secret-token:H6QTCSWAD2458EDG09J3JCH98N63ARM0SCXKG4EM3PMP6W942CP0", "scope": "all", "refreshable": false, "expiration": { "t_s": "never" } } The problem is that this is the normal token used by the merchant SPA. I think we should either have a special scope for tokens to create new tokens (1) or only allow the creation of tokens with Basic, not Bearer (2). I think (2) is the simplest and safest. We can also go for (1) for a "spa-session" scope. This allows the full management (like all) but prevents the creation of new tokens if leaked. | ||||
| Tags | security | ||||
| You should not be able to create a token with a broader scope than the one you present. I also though it had to be refreshable. Need to check the code. | |
| What is the "normal token used by the SPA"? Because we do not have a scope for that, so I guess you already have a scope with "all". So your description would not be correct. We can define a special scope for SPAs, if you can provide me with a clear requirement and set of permissions required. | |
| (Note: "all" always was and still is defined as "can do anything including refresh") | |
| We cannot limit the token endpoint to only Basic, because that is what ":refreshable" is about. | |
| Your proposal (1) also does not work, because you can request scope "spa-session:refeshable" and get a token that can be refreshed. | |
| The SPA clearly should ask for a non-refreshable token, possibly with a lifetime restriction that the user can configure on login. When the SPA is used to create *another* token for export, I think it makes sense for the SPA to require the user to re-enter username/password for this. Maybe this is a pure SPA issue and not something that requires changes in the backend? | |
| 1) the only way to create a new session should be using username and password (or even 2fa) 2) the refresh mechanism re-create the __same__ token (not a new one, in fact, refreshing should invalidate the previous one) This way the user can keep control of the number of open sessions for an account. I have shown a way to overcome this two restriction, having a token I can create a new token without user/password and a token without refresh. a) do we want to have a scope that allow user of the token to create new tokens? (like the "all" scope) (consider that the user of the token may be a script or third party) I think this is wrong, I don't see any reason why this would be a good idea and the merchant could potentially find an unreasonable amount of new open sessions. b) how can we fix this? I proposed 2 ways: - only allow the creation of tokens using usr/pwd, the refreshing explained in (2) should care about the rest. - modify the scope definition to exclude token creation permission or to ask for it if needed (again I don't see why but you sound like you have a reason) If you insist on having a scope that can create tokens, then we need a scope that have "all - creation_of_tokens" __by definition__ since the SPA backoffice should have access to all API but the UI will ask (and test) user password before creating a new token. > The SPA clearly should ask for a non-refreshable token, possibly with a lifetime restriction that the user can configure on login. The spa will always ask for refreshable and max duration, this allows the backend server to impose the expiration (spa always check expiration independently of duration). It needs to be refreshable so the session only ends with the user hitting "log out" (or app close for too long) Asking for non-refreshable token will make the SPA prompt for "enter the username and password again" before expiration (manually refresh) this is worse UX. > When the SPA is used to create *another* token for export, I think it makes sense for the SPA to require the user to re-enter username/password for this. The creation of new token is working as you describe. SPA ask for password (and test it) even if not required right now (that's why I'm creating this issue against backend). Password is always thrown away after used, the SPA never ever saves the password. > You should not be able to create a token with a broader scope than the one you present Off course, this is not the case. In the example I have started the issue I'm asking for a token (H6QT) with refreshable:false using the token (GVP8). I succeded to create a new token without user/password (using token GVP8 and also using token H6QT) with a token which is not refreshable which makes refreshable property useless. I can also use H6QT again in the same way. > What is the "normal token used by the SPA"? A token with scope "all", since the spa session should have access to all endpoints no other scope make sense. | |
| The token with scope all is always refreshable. That is actually a bug: The "refreshable" field is set incorrectly in this special case and the refreshability is not indicated. You claim (1) is still unsubstantiated and everything hinges on the argument why this is a requirement. Why should this not be possible? See also https://oauth.net/2/refresh-tokens/ on how this is a common pattern. If you mean that the token refresh should invalidate an old token I agree. That would also be a new requirement an behaviour, but I can see why it makes sense. Tokens are also not sessions. Maybe that is the case for the SPA but a token is simply a token. The bigger problem is that we are mixing access tokens from login (session tokens) with third party access tokens. Those are actually thwo different things. Maybe this requires a bigger refactor: Separate login tokens (/private/token) from third party tokens (/private/third_party/token) I think in general we are using terms here in the wrong way. Like access token, session and refresh. | |
| Ok, I'm also not sure that we need this, schanzen's arguments here seem quite reasonable. Shall we "wontfix" it? | |
| @schanzen: So the difference between login tokens and third party tokens would just be that third party tokens can't create other (third party / login) tokens? Is there a reason why we'd need separate endpoints or could we model it with scopes? | |
| You can use a non-refreshable token as a third party token. But you have to be very careful that it is not refreshable. Otherwise, you cannot revoke it sanely, because the third party can create a lot of tokens that are not bound to it (you do not know who is the subject ob the token). | |
| Summary: - SPA creates at login a refreshable token with scope "all" - SPA can use this to create other tokens of all other scopes; - if your token is refreshable, you can create tokens with lifetimes that exceed the lifetime of the current token (but not the scope of the current token) - Usually (by default in the SPA) the (likely) 3rd party tokens will be non-refreshable - For PoS app tokens and E-commerce frontends, this still works nicely if you use a non-refreshable token of infinite lifetime (which we permit in the SPA and the backend) Assuming the above is done, I propose the following: TODO: - revoking a refreshable token can be tricky, as there could have been many tokens spawned from the original token; so we build a dependency tree where a token that has been created via refresh has a "parent" field pointing to the original token it was refreshed from; if a parent token is revoked (not merely expired), we DELETE it and then ON DELETE CASCADE should revoke all dependents; - we'll need to write some slightly more funky GC routine to collect non-revoked refresh-chains of expired tokens | |
| IF the UI forces non-refresh for 3rd party tokens, I do not think we need this. We may want to delete old tokens upon refresh in any case however. | |
| Instead, we may want to have a "bearer" subject associated with the token in order to revoke all tokens for an entity | |
| Talked with Christian and we came up with the following proposal: - When authenticating with ID/password (+2FA) a refreshable token may be requested (SPA use case) - A refreshable token can be used to refresh its own token (SPA use case) - 3rd party tokens may be created refreshable, but it is HIGHLY DISCOURAGED. Any admin may shoot himself in the foot if required. The UI should hide this or make it clear that it is dangerous. Required changes: - Merchant API: None - UI: Hide option refreshable for token creation / or give stern warning - Doc: Document behaviour for token endpoint. | |
| I added a notification banne when refreshable tokens are selected. For the documentation improvements we have 0010575 to track now. Proposing to mark this resolved. | |
| Date Modified | Username | Field | Change |
|---|---|---|---|
| 2025-07-09 14:55 | sebasjm | New Issue | |
| 2025-07-09 15:03 | Christian Grothoff | Assigned To | => schanzen |
| 2025-07-09 15:03 | Christian Grothoff | Status | new => assigned |
| 2025-07-09 16:07 | schanzen | Note Added: 0025469 | |
| 2025-07-09 16:09 | schanzen | Note Added: 0025470 | |
| 2025-07-09 16:11 | schanzen | Note Added: 0025471 | |
| 2025-07-09 16:12 | schanzen | Note Added: 0025472 | |
| 2025-07-09 16:12 | schanzen | Note Added: 0025473 | |
| 2025-07-09 18:37 | Christian Grothoff | Note Added: 0025474 | |
| 2025-07-09 18:37 | Christian Grothoff | Assigned To | schanzen => sebasjm |
| 2025-07-09 18:37 | Christian Grothoff | Status | assigned => feedback |
| 2025-07-10 02:23 | sebasjm | Note Added: 0025479 | |
| 2025-07-10 05:53 | schanzen | Note Added: 0025480 | |
| 2025-07-10 07:25 | Christian Grothoff | Note Added: 0025485 | |
| 2025-07-10 07:25 | Christian Grothoff | Assigned To | sebasjm => Florian Dold |
| 2025-08-04 20:28 | Florian Dold | Target Version | => 1.6 |
| 2025-08-04 20:29 | Florian Dold | Note Added: 0025620 | |
| 2025-08-04 20:29 | Florian Dold | Assigned To | Florian Dold => schanzen |
| 2025-08-06 08:38 | schanzen | Note Added: 0025631 | |
| 2025-08-28 08:36 | Christian Grothoff | Note Added: 0025704 | |
| 2025-08-28 08:36 | Christian Grothoff | Status | feedback => assigned |
| 2025-08-28 08:45 | schanzen | Note Added: 0025705 | |
| 2025-08-28 08:46 | schanzen | Note Added: 0025706 | |
| 2025-08-28 09:23 | schanzen | Note Added: 0025709 | |
| 2025-08-31 17:28 | Christian Grothoff | Tag Attached: security | |
| 2025-08-31 17:33 | Christian Grothoff | Severity | major => minor |
| 2025-08-31 17:33 | Christian Grothoff | Product Version | => 1.0 |
| 2025-08-31 17:33 | Christian Grothoff | Target Version | 1.6 => post-1.0 |
| 2025-11-18 21:05 | Stefan | Relationship added | related to 0010630 |
| 2025-11-20 11:00 | sebasjm | Relationship added | related to 0010639 |
| 2025-11-25 09:57 | schanzen | ||
| 2025-11-25 09:58 | schanzen | Relationship added | related to 0010575 |
| 2025-11-25 09:59 | schanzen | Assigned To | schanzen => Christian Grothoff |
| 2025-11-25 09:59 | schanzen | Status | assigned => feedback |
| 2025-11-25 09:59 | schanzen | Note Added: 0026613 | |
| 2025-11-25 10:34 | Christian Grothoff | Status | feedback => informing |
| 2025-11-25 10:34 | Christian Grothoff | Resolution | open => fixed |
| 2025-11-25 10:34 | Christian Grothoff | Fixed in Version | => 1.3 |
| 2025-11-25 10:34 | Christian Grothoff | Target Version | post-1.0 => 1.3 |
| 2025-12-03 14:07 | vecirex | Relationship replaced | parent of 0010630 |
| 2025-12-03 14:09 | vecirex | Relationship replaced | child of 0010630 |
| 2025-12-11 23:53 | vecirex | Relationship deleted | child of 0010630 |
| 2025-12-21 20:46 | Christian Grothoff | Status | informing => closed |