Describe the bug
As mentioned in the discussion on #3673, legacy opaque tokens are identified by their actual token string in the audit log, meaning the actual, usable tokens are leaking everywhere in the logs. This is just plain stupid and a borderline security issue.
This stems from the fact that the APIToken model has a str implementation that returns the opaque token value, which is just silly.
To Reproduce
- Go to the
Toolbox, then User and API Administration and then the API Token List tab.
- Create a new token and save it.
- Go back to the token list and click the
Audit log icon.
- Observe the newly created token string is fully visible in the audit log.
Expected behavior
Some other way of describing an APIToken instance as text should be used. Its comment field could be used, but that may be too long to make sense in the audit log. The most opaque but still secure way could be to just refer to it via its integer primary key.
Things to consider as part of the fix:
-
The token string is also used to identify each token in the API Token list, although only a truncated version is shown as an identifier here. It still does not make much sense though. One suggestion could be to use the comment as the primary identifier in this list - and, since the comment field is optional, creating a token with an empty comment should maybe be automatically filled with a comment that describes what the token give access to.
-
When upgrading from an older NAV version, tokens will still be all over the existing audit log. A migration script should be created to find all audit log entries that identify API tokens in this manner and rewrite those entries to use the same type of identification as the bugfixed version does.
Environment (please complete the following information):
- NAV version installed: 5.14
Describe the bug
As mentioned in the discussion on #3673, legacy opaque tokens are identified by their actual token string in the audit log, meaning the actual, usable tokens are leaking everywhere in the logs. This is just plain stupid and a borderline security issue.
This stems from the fact that the
APITokenmodel has astrimplementation that returns the opaque token value, which is just silly.To Reproduce
Toolbox, thenUser and API Administrationand then theAPI Token Listtab.Audit logicon.Expected behavior
Some other way of describing an
APITokeninstance as text should be used. Its comment field could be used, but that may be too long to make sense in the audit log. The most opaque but still secure way could be to just refer to it via its integer primary key.Things to consider as part of the fix:
The token string is also used to identify each token in the API Token list, although only a truncated version is shown as an identifier here. It still does not make much sense though. One suggestion could be to use the comment as the primary identifier in this list - and, since the comment field is optional, creating a token with an empty comment should maybe be automatically filled with a comment that describes what the token give access to.
When upgrading from an older NAV version, tokens will still be all over the existing audit log. A migration script should be created to find all audit log entries that identify API tokens in this manner and rewrite those entries to use the same type of identification as the bugfixed version does.
Environment (please complete the following information):