#468

fix(server): enforce settings.tokens privileges in the REST token handlers

Openbenjaminkincseswants to merge intofrom

Problem

The GraphQL token resolvers check the settings.tokens privileges before touching a token: createAPIToken requires settings.tokens.create, updateAPIToken requires settings.tokens.edit, deleteAPIToken requires settings.tokens.delete, and the queries check settings.tokens.view. The REST handlers in backend/pkg/server/services/api_tokens.go check none of them. CreateToken, ListTokens, GetToken, UpdateToken and DeleteToken only check ownership, with settings.tokens.admin widening access to other users' tokens.

So a custom role that was deliberately given no token privileges can still create, list, read, update and delete its own tokens over REST, including minting a token with a TTL of up to three years. Both seeded roles hold all of these privileges, so they see no change.

Solution

Add the same per-operation privilege check at the top of each REST token handler, returning 403 NotPermitted when the privilege is missing. settings.tokens.admin keeps its current meaning, whose tokens a caller can reach, and does not stand in for the operation privileges, same as in the resolvers.

Type of change

  • Bug fix (non-breaking change which fixes an issue)

Areas affected

  • Backend / API

Testing

Test cases

  1. New unit test TestAPITokens_Handlers_RequireTheSettingsTokensPrivileges: all five handlers called with no privileges and with settings.tokens.admin only. Expected: 403 NotPermitted from every handler, and the seeded token is still active and unchanged afterwards, with nothing published.
  2. Existing token tests: they already pass the matching privilege for each handler (create with settings.tokens.create, list and get with settings.tokens.view, and so on), so they cover the allowed paths as before.

Verification steps

  1. The existing api_tokens_test.go cases already supplied the privileges this change now enforces, which is what pointed at the gap.
  2. gofmt is clean on both changed files.

Security considerations

  • This PR does not introduce security vulnerabilities

Checklist

  • I have performed a self-review of my code
  • I have commented my code, particularly in hard-to-understand areas
  • I have made corresponding changes to the documentation
  • My changes generate no new warnings
  • I have added tests that prove my fix is effective or that my feature works
  • New and existing unit tests pass locally with my changes
  • Any dependent changes have been merged and published

Additional notes

The swagger comments on these handlers already declared a 403 for "not permitted" cases. This change makes the handlers actually produce it.