Compare commits
2 Commits
main
...
claude/rev
| Author | SHA1 | Date |
|---|---|---|
Claude
|
e309130aea | |
|
Claude
|
d27951b882 |
|
|
@ -0,0 +1,567 @@
|
|||
# Implementation Plan: Bot Users
|
||||
|
||||
This plan implements bot users as first-class users with an `is_bot` flag, gated behind a `service.enablebotusers` config flag. It incorporates all gap resolutions from the design review.
|
||||
|
||||
Reference: [Design Plan](https://github.com/go-vikunja/vikunja/blob/claude/explore-openclaw-integration-KQEzg/plans/design-bot-users.md) | [Gap Analysis](plans/review-bot-design-plan.md)
|
||||
|
||||
---
|
||||
|
||||
## Phase 1: Backend Foundation
|
||||
|
||||
### Step 1.1: Config Key
|
||||
|
||||
**File:** `pkg/config/config.go`
|
||||
|
||||
- Add `ServiceEnableBotUsers Key = "service.enablebotusers"` alongside existing `ServiceEnable*` keys (around line 52-65).
|
||||
- In `InitDefaultConfig()`, add `ServiceEnableBotUsers.setDefault(false)`.
|
||||
|
||||
**File:** `config-raw.json`
|
||||
|
||||
- Add entry in the `service` section (after existing `enableuserdeletion` block around line 107):
|
||||
```json
|
||||
{
|
||||
"key": "enablebotusers",
|
||||
"default_value": "false",
|
||||
"comment": "If enabled, users can create bot users that interact with Vikunja via the API only."
|
||||
}
|
||||
```
|
||||
|
||||
---
|
||||
|
||||
### Step 1.2: User Struct Changes
|
||||
|
||||
**File:** `pkg/user/user.go`
|
||||
|
||||
- Add `IsBot` field to the `User` struct:
|
||||
```go
|
||||
IsBot bool `xorm:"bool default false index" json:"is_bot"`
|
||||
```
|
||||
|
||||
- Update `ShouldNotify()` (line 146-160) to return `false` for bots:
|
||||
```go
|
||||
if user.IsBot {
|
||||
return false, nil
|
||||
}
|
||||
```
|
||||
|
||||
---
|
||||
|
||||
### Step 1.3: Error Types
|
||||
|
||||
**File:** `pkg/user/error.go`
|
||||
|
||||
Add four new error types after `ErrorCodeTokenUserMismatch = 1029` (line 707+):
|
||||
|
||||
| Error Type | Code | HTTP Status | When |
|
||||
|---|---|---|---|
|
||||
| `ErrAccountIsBot` | 1030 | 412 | Bot attempts password login |
|
||||
| `ErrBotUsersDisabled` | 1031 | 403 | Bot endpoint called when feature is off |
|
||||
| `ErrBotNotOwned` | 1032 | 403 | User tries to manage a bot they don't own |
|
||||
| `ErrCannotMakeBotOwner` | 1033 | 400 | Trying to set a bot as owner of another bot |
|
||||
| `ErrBotUsernameMustHavePrefix` | 1034 | 400 | Bot username doesn't start with `bot-` |
|
||||
|
||||
Follow the existing error pattern: struct, `IsErr*()` check, `Error()` string, `ErrCode*` constant, `HTTPError()` method.
|
||||
|
||||
---
|
||||
|
||||
### Step 1.4: Bot Owner Model & Database Migration
|
||||
|
||||
**New file:** `pkg/models/bot_users.go`
|
||||
|
||||
Define `BotOwner` struct:
|
||||
```go
|
||||
type BotOwner struct {
|
||||
ID int64 `xorm:"bigint autoincr not null unique pk" json:"id"`
|
||||
BotID int64 `xorm:"bigint not null unique index" json:"bot_id"`
|
||||
OwnerID int64 `xorm:"bigint not null index" json:"owner_id"`
|
||||
Created time.Time `xorm:"created not null" json:"created"`
|
||||
}
|
||||
|
||||
func (*BotOwner) TableName() string {
|
||||
return "bot_owners"
|
||||
}
|
||||
```
|
||||
|
||||
**File:** `pkg/models/models.go`
|
||||
|
||||
- Add `&BotOwner{}` to `GetTables()` return slice (line 45-73).
|
||||
|
||||
**New file:** `pkg/migration/<timestamp>.go`
|
||||
|
||||
- Run `mage dev:make-migration BotOwner` to generate migration file.
|
||||
- Migration adds:
|
||||
1. `is_bot` column to `users` table (bool, default false, indexed)
|
||||
2. `bot_owners` table via `tx.Sync2(&BotOwner{})`
|
||||
|
||||
---
|
||||
|
||||
### Step 1.5: CreateBotUser Function
|
||||
|
||||
**File:** `pkg/user/user_create.go`
|
||||
|
||||
Add `CreateBotUser()` function. This does NOT call `checkIfUserIsValid()` or `checkIfUserExists()` — it has its own validation:
|
||||
|
||||
1. Validate username: not empty, no spaces, starts with `bot-`, not a duplicate (via `GetUserByUsername`)
|
||||
2. Validate name (display name): optional, free-form
|
||||
3. Create user with:
|
||||
- `IsBot = true`
|
||||
- `Status = StatusActive`
|
||||
- `Issuer = IssuerLocal`
|
||||
- `Password = ""` (no password)
|
||||
- `Email = ""` (no email)
|
||||
- `EmailRemindersEnabled = false`
|
||||
- `OverdueTasksRemindersEnabled = false`
|
||||
- All other defaults from config
|
||||
4. Insert user
|
||||
5. Dispatch `CreatedEvent`
|
||||
6. Return the new user
|
||||
|
||||
Also add to `checkIfUserIsValid()` (line 130-153): reject `bot-` prefix for non-bot users:
|
||||
```go
|
||||
if strings.HasPrefix(user.Username, "bot-") {
|
||||
return ErrUsernameReserved{Username: user.Username}
|
||||
}
|
||||
```
|
||||
|
||||
---
|
||||
|
||||
### Step 1.6: Block Login for Bots
|
||||
|
||||
**File:** `pkg/routes/api/v1/login.go`
|
||||
|
||||
After the status check at line 74-77, add:
|
||||
```go
|
||||
if user.IsBot {
|
||||
_ = s.Rollback()
|
||||
return &user2.ErrAccountIsBot{UserID: user.ID}
|
||||
}
|
||||
```
|
||||
|
||||
This blocks bots after both LDAP and local auth paths converge. Even if LDAP somehow resolves a bot user, it gets blocked here.
|
||||
|
||||
**File:** `pkg/routes/caldav/auth.go`
|
||||
|
||||
Before `c.Set("userBasicAuth", u)` at line 63-64:
|
||||
```go
|
||||
if u != nil && u.IsBot {
|
||||
log.Warningf("CalDAV basic auth rejected for bot user %d", u.ID)
|
||||
return false, nil
|
||||
}
|
||||
```
|
||||
|
||||
---
|
||||
|
||||
### Step 1.7: API Token Auth - User Status Check (Security Fix)
|
||||
|
||||
**File:** `pkg/routes/api_tokens.go`
|
||||
|
||||
In `checkAPITokenAndPutItInContext()`, after `user.GetUserByID()` at line 94-97, add:
|
||||
```go
|
||||
if u.Status == user.StatusDisabled {
|
||||
log.Debugf("[auth] Tried authenticating with token %d but user %d is disabled", token.ID, u.ID)
|
||||
return echo.NewHTTPError(http.StatusUnauthorized, "Unauthorized")
|
||||
}
|
||||
```
|
||||
|
||||
This ensures disabling a bot (or any user) immediately revokes API token access.
|
||||
|
||||
---
|
||||
|
||||
### Step 1.8: APIToken.Create() - Minimal Branching
|
||||
|
||||
**File:** `pkg/models/api_tokens.go`
|
||||
|
||||
In `Create()` (line 86-110), change line 102 from:
|
||||
```go
|
||||
t.OwnerID = a.GetID()
|
||||
```
|
||||
to:
|
||||
```go
|
||||
if t.OwnerID == 0 {
|
||||
t.OwnerID = a.GetID()
|
||||
}
|
||||
```
|
||||
|
||||
This allows bot token handlers to pre-set `OwnerID` to the bot's ID.
|
||||
|
||||
---
|
||||
|
||||
## Phase 2: Bot API Endpoints
|
||||
|
||||
### Step 2.1: Bot CRUD Model Methods
|
||||
|
||||
**File:** `pkg/models/bot_users.go` (extend from Step 1.4)
|
||||
|
||||
Add a `BotUser` struct for the API layer (wraps user + ownership info). This struct implements `web.CRUDable` and `web.Permissions` for use with generic WebHandlers where possible.
|
||||
|
||||
However, because bot CRUD involves creating users (not just inserting a model row), custom handlers are needed for Create and Delete. ReadAll and ReadOne can use generic patterns.
|
||||
|
||||
Define the following methods on `BotUser`:
|
||||
- `Create(s, a)`: Call `user.CreateBotUser()`, then insert `BotOwner{BotID: newUser.ID, OwnerID: a.GetID()}`
|
||||
- `ReadAll(s, a, search, page, perPage)`: Query `bot_owners WHERE owner_id = a.GetID()` joined with `users`
|
||||
- `ReadOne(s, a)`: Get bot by ID, verify ownership
|
||||
- `Update(s, a)`: Update bot's `name` or `username` (verify ownership, validate `bot-` prefix preserved)
|
||||
- `Delete(s, a)`: Call `DeleteUser()` from `pkg/models/user_delete.go` to hard-delete the bot and all related data. Also delete the `BotOwner` row. Skip `AccountDeletedNotification` since `ShouldNotify()` returns false for bots.
|
||||
|
||||
Add a `Disable`/`Enable` method (or use `Update` with a status field):
|
||||
- Set `user.Status = StatusDisabled` or `StatusActive`
|
||||
|
||||
**New file:** `pkg/models/bot_users_permissions.go`
|
||||
|
||||
Permission methods:
|
||||
- `CanCreate(s, a)`: Return true if `config.ServiceEnableBotUsers.GetBool()` and user is not a bot.
|
||||
- `CanRead(s, a)`: Return true if user owns this bot (check `bot_owners`).
|
||||
- `CanUpdate(s, a)`: Same as CanRead.
|
||||
- `CanDelete(s, a)`: Same as CanRead.
|
||||
|
||||
---
|
||||
|
||||
### Step 2.2: Bot Token Endpoints
|
||||
|
||||
**File:** `pkg/routes/api/v1/bots.go` (new file)
|
||||
|
||||
Custom handlers for bot token management since we need to override the token owner:
|
||||
|
||||
**`PUT /user/bots/:bot/tokens`** - Create token for bot:
|
||||
1. Parse bot ID from `:bot` param
|
||||
2. Verify current user owns this bot (query `bot_owners`)
|
||||
3. Bind `APIToken` from request body
|
||||
4. Set `token.OwnerID = bot.ID` (pre-set before calling Create)
|
||||
5. Call `token.Create(s, a)` — uses the branched logic from Step 1.8
|
||||
6. Return the token (visible only once)
|
||||
|
||||
**`GET /user/bots/:bot/tokens`** - List bot's tokens:
|
||||
1. Verify ownership
|
||||
2. Query `api_tokens WHERE owner_id = bot.ID`
|
||||
|
||||
**`DELETE /user/bots/:bot/tokens/:token`** - Delete bot's token:
|
||||
1. Verify ownership
|
||||
2. Delete `api_tokens WHERE id = :token AND owner_id = bot.ID`
|
||||
|
||||
---
|
||||
|
||||
### Step 2.3: Route Registration
|
||||
|
||||
**File:** `pkg/routes/routes.go`
|
||||
|
||||
In the user routes group (after line 462), add:
|
||||
```go
|
||||
if config.ServiceEnableBotUsers.GetBool() {
|
||||
botHandler := &handler.WebHandler{
|
||||
EmptyStruct: func() handler.CObject {
|
||||
return &models.BotUser{}
|
||||
},
|
||||
}
|
||||
u.PUT("/bots", botHandler.CreateWeb)
|
||||
u.GET("/bots", botHandler.ReadAllWeb)
|
||||
u.GET("/bots/:bot", botHandler.ReadOneWeb)
|
||||
u.POST("/bots/:bot", botHandler.UpdateWeb)
|
||||
u.DELETE("/bots/:bot", botHandler.DeleteWeb)
|
||||
|
||||
// Bot token management (custom handlers)
|
||||
u.PUT("/bots/:bot/tokens", apiv1.CreateBotToken)
|
||||
u.GET("/bots/:bot/tokens", apiv1.ListBotTokens)
|
||||
u.DELETE("/bots/:bot/tokens/:token", apiv1.DeleteBotToken)
|
||||
}
|
||||
```
|
||||
|
||||
---
|
||||
|
||||
### Step 2.4: Info Endpoint
|
||||
|
||||
**File:** `pkg/routes/api/v1/info.go`
|
||||
|
||||
- Add `BotUsersEnabled bool \`json:"bot_users_enabled"\`` to `vikunjaInfos` struct (line 35-55).
|
||||
- In the `Info` handler, set: `info.BotUsersEnabled = config.ServiceEnableBotUsers.GetBool()`.
|
||||
|
||||
---
|
||||
|
||||
## Phase 3: Frontend
|
||||
|
||||
### Step 3.1: Config Store
|
||||
|
||||
**File:** `frontend/src/stores/config.ts`
|
||||
|
||||
- Add to `ConfigState` interface (after `publicTeamsEnabled` at line 46):
|
||||
```typescript
|
||||
botUsersEnabled: boolean,
|
||||
```
|
||||
|
||||
- Add default in reactive state (after `publicTeamsEnabled: false` at line 85):
|
||||
```typescript
|
||||
botUsersEnabled: false,
|
||||
```
|
||||
|
||||
No other changes needed — `objectToCamelCase(config)` in `update()` automatically maps `bot_users_enabled` → `botUsersEnabled`.
|
||||
|
||||
---
|
||||
|
||||
### Step 3.2: User Type & Model
|
||||
|
||||
**File:** `frontend/src/modelTypes/IUser.ts`
|
||||
|
||||
Add to interface:
|
||||
```typescript
|
||||
isBot: boolean
|
||||
```
|
||||
|
||||
**File:** `frontend/src/models/user.ts`
|
||||
|
||||
Add default in constructor:
|
||||
```typescript
|
||||
isBot = false
|
||||
```
|
||||
|
||||
---
|
||||
|
||||
### Step 3.3: Bot Badge in User Component
|
||||
|
||||
**File:** `frontend/src/components/misc/User.vue`
|
||||
|
||||
After the username `<span>`, add:
|
||||
```vue
|
||||
<span v-if="user?.isBot" class="bot-badge">{{ $t('user.bot.badge') }}</span>
|
||||
```
|
||||
|
||||
Style:
|
||||
```scss
|
||||
.bot-badge {
|
||||
display: inline-flex;
|
||||
align-items: center;
|
||||
font-size: 0.65rem;
|
||||
font-weight: 600;
|
||||
padding: 0.1rem 0.35rem;
|
||||
border-radius: 0.25rem;
|
||||
background: var(--grey-200);
|
||||
color: var(--grey-600);
|
||||
margin-inline-start: 0.25rem;
|
||||
vertical-align: middle;
|
||||
text-transform: uppercase;
|
||||
letter-spacing: 0.02em;
|
||||
}
|
||||
```
|
||||
|
||||
---
|
||||
|
||||
### Step 3.4: Bot Service & Model
|
||||
|
||||
**New file:** `frontend/src/services/botUser.ts`
|
||||
|
||||
```typescript
|
||||
export default class BotUserService extends AbstractService<IBotUser> {
|
||||
constructor() {
|
||||
super({
|
||||
create: '/user/bots',
|
||||
getAll: '/user/bots',
|
||||
get: '/user/bots/{id}',
|
||||
update: '/user/bots/{id}',
|
||||
delete: '/user/bots/{id}',
|
||||
})
|
||||
}
|
||||
modelFactory(data) { return new BotUserModel(data) }
|
||||
}
|
||||
```
|
||||
|
||||
**New file:** `frontend/src/services/botToken.ts`
|
||||
|
||||
```typescript
|
||||
export default class BotTokenService extends AbstractService<IApiToken> {
|
||||
constructor(botId: number) {
|
||||
super({
|
||||
create: `/user/bots/${botId}/tokens`,
|
||||
getAll: `/user/bots/${botId}/tokens`,
|
||||
delete: `/user/bots/${botId}/tokens/{id}`,
|
||||
})
|
||||
}
|
||||
modelFactory(data) { return new ApiTokenModel(data) }
|
||||
}
|
||||
```
|
||||
|
||||
**New file:** `frontend/src/modelTypes/IBotUser.ts`
|
||||
|
||||
```typescript
|
||||
export interface IBotUser extends IAbstract {
|
||||
id: number
|
||||
username: string
|
||||
name: string
|
||||
isBot: boolean
|
||||
created: Date
|
||||
updated: Date
|
||||
}
|
||||
```
|
||||
|
||||
**New file:** `frontend/src/models/botUser.ts`
|
||||
|
||||
Model class extending `AbstractModel<IBotUser>`.
|
||||
|
||||
---
|
||||
|
||||
### Step 3.5: Bot Settings Page
|
||||
|
||||
**New file:** `frontend/src/views/user/settings/Bots.vue`
|
||||
|
||||
Settings page with:
|
||||
- List of bots owned by current user (fetched from `GET /user/bots`)
|
||||
- Create bot form: username (with `bot-` prefix pre-filled), display name
|
||||
- Per-bot actions:
|
||||
- Edit (update display name)
|
||||
- Disable/Enable toggle
|
||||
- Delete (with confirmation dialog — make it clear this is permanent and irreversible)
|
||||
- Per-bot token management:
|
||||
- Create token (show token value once, copy-to-clipboard)
|
||||
- List existing tokens
|
||||
- Delete token
|
||||
|
||||
Follow the pattern of `frontend/src/views/user/settings/ApiTokens.vue` for the token management UI.
|
||||
|
||||
---
|
||||
|
||||
### Step 3.6: Route & Navigation
|
||||
|
||||
**File:** `frontend/src/router/index.ts`
|
||||
|
||||
Add to `user.settings` children (after the webhooks route, line 147-151):
|
||||
```typescript
|
||||
{
|
||||
path: '/user/settings/bots',
|
||||
name: 'user.settings.bots',
|
||||
component: () => import('@/views/user/settings/Bots.vue'),
|
||||
},
|
||||
```
|
||||
|
||||
**File:** `frontend/src/views/user/Settings.vue`
|
||||
|
||||
Add computed:
|
||||
```typescript
|
||||
const botUsersEnabled = computed(() => configStore.botUsersEnabled)
|
||||
```
|
||||
|
||||
Add to `navigationItems` array (before the deletion entry):
|
||||
```typescript
|
||||
{
|
||||
title: t('user.settings.bots.title'),
|
||||
routeName: 'user.settings.bots',
|
||||
condition: botUsersEnabled.value,
|
||||
},
|
||||
```
|
||||
|
||||
---
|
||||
|
||||
### Step 3.7: Translation Strings
|
||||
|
||||
**File:** `frontend/src/i18n/lang/en.json`
|
||||
|
||||
Add under `user.settings`:
|
||||
```json
|
||||
"bots": {
|
||||
"title": "Bots",
|
||||
"create": "Create Bot",
|
||||
"createDescription": "Bot users can interact with Vikunja via the API only. Bot usernames must start with 'bot-'.",
|
||||
"username": "Bot Username",
|
||||
"usernamePlaceholder": "bot-my-assistant",
|
||||
"displayName": "Display Name",
|
||||
"displayNamePlaceholder": "My Assistant Bot",
|
||||
"noBotsYet": "You haven't created any bots yet.",
|
||||
"delete": "Delete Bot",
|
||||
"deleteConfirmation": "This will permanently delete this bot and all its data including task assignments, project memberships, and API tokens. This action cannot be undone.",
|
||||
"disable": "Disable Bot",
|
||||
"enable": "Enable Bot",
|
||||
"disabled": "Disabled",
|
||||
"tokens": "API Tokens",
|
||||
"createToken": "Create Token",
|
||||
"tokenCreated": "Token created. Copy it now — you won't be able to see it again.",
|
||||
"noTokens": "No API tokens yet."
|
||||
}
|
||||
```
|
||||
|
||||
Add under `user`:
|
||||
```json
|
||||
"bot": {
|
||||
"badge": "Bot"
|
||||
}
|
||||
```
|
||||
|
||||
---
|
||||
|
||||
## Phase 4: Testing
|
||||
|
||||
### Step 4.1: Backend Test Fixtures
|
||||
|
||||
**File:** `pkg/db/fixtures/users.yml` (or appropriate fixture file)
|
||||
|
||||
Add bot user fixture entries for testing.
|
||||
|
||||
### Step 4.2: Backend Tests
|
||||
|
||||
**New file:** `pkg/models/bot_users_test.go`
|
||||
|
||||
Test cases:
|
||||
- Create bot successfully
|
||||
- Create bot when feature disabled → error
|
||||
- Create bot with invalid username (no `bot-` prefix) → error
|
||||
- Create bot as a bot → error
|
||||
- List bots returns only owned bots
|
||||
- Update bot name
|
||||
- Delete bot (verify cascading: task assignees, tokens, project memberships removed)
|
||||
- Disable bot → verify API token auth fails
|
||||
- Enable bot → verify API token auth works again
|
||||
- Create token for bot
|
||||
- Create token for bot not owned → error
|
||||
- Login as bot → error
|
||||
- CalDAV auth as bot → rejected
|
||||
|
||||
### Step 4.3: Frontend Tests
|
||||
|
||||
Unit tests for bot service and model. The settings page can be covered by E2E tests if needed.
|
||||
|
||||
---
|
||||
|
||||
## Implementation Order
|
||||
|
||||
1. **Phase 1** (Steps 1.1–1.8): Backend foundation — can be tested independently
|
||||
2. **Phase 2** (Steps 2.1–2.4): API endpoints — testable with curl/API tests
|
||||
3. **Phase 3** (Steps 3.1–3.7): Frontend — requires backend running
|
||||
4. **Phase 4** (Steps 4.1–4.3): Tests — run alongside implementation
|
||||
|
||||
Within Phase 1, the steps are sequential (each depends on prior). Phase 2 depends on Phase 1. Phase 3 depends on Phase 2. Phase 4 can be written alongside Phases 1-3.
|
||||
|
||||
---
|
||||
|
||||
## Files Summary
|
||||
|
||||
### New Files (10)
|
||||
| File | Purpose |
|
||||
|------|---------|
|
||||
| `pkg/models/bot_users.go` | BotOwner + BotUser model, CRUD logic |
|
||||
| `pkg/models/bot_users_permissions.go` | Permission checks |
|
||||
| `pkg/models/bot_users_test.go` | Backend tests |
|
||||
| `pkg/routes/api/v1/bots.go` | Bot token endpoint handlers |
|
||||
| `pkg/migration/<timestamp>.go` | Database migration |
|
||||
| `frontend/src/views/user/settings/Bots.vue` | Bot management UI |
|
||||
| `frontend/src/services/botUser.ts` | Bot CRUD service |
|
||||
| `frontend/src/services/botToken.ts` | Bot token service |
|
||||
| `frontend/src/modelTypes/IBotUser.ts` | Bot TypeScript interface |
|
||||
| `frontend/src/models/botUser.ts` | Bot model class |
|
||||
|
||||
### Modified Files (15)
|
||||
| File | Change |
|
||||
|------|--------|
|
||||
| `pkg/config/config.go` | Add `ServiceEnableBotUsers` key + default |
|
||||
| `config-raw.json` | Add `enablebotusers` schema entry |
|
||||
| `pkg/user/user.go` | Add `IsBot` field; update `ShouldNotify()` |
|
||||
| `pkg/user/user_create.go` | Add `CreateBotUser()`; reserve `bot-` prefix in `checkIfUserIsValid()` |
|
||||
| `pkg/user/error.go` | Add 5 new error types (codes 1030-1034) |
|
||||
| `pkg/models/models.go` | Add `&BotOwner{}` to `GetTables()` |
|
||||
| `pkg/models/api_tokens.go` | Branch `OwnerID` assignment in `Create()` |
|
||||
| `pkg/routes/routes.go` | Register bot API routes |
|
||||
| `pkg/routes/api/v1/info.go` | Expose `BotUsersEnabled` in `/info` |
|
||||
| `pkg/routes/api/v1/login.go` | Block bot login |
|
||||
| `pkg/routes/api_tokens.go` | Add user status check (security fix) |
|
||||
| `pkg/routes/caldav/auth.go` | Block bot CalDAV auth |
|
||||
| `frontend/src/stores/config.ts` | Add `botUsersEnabled` to state |
|
||||
| `frontend/src/modelTypes/IUser.ts` | Add `isBot` field |
|
||||
| `frontend/src/models/user.ts` | Add `isBot` default |
|
||||
| `frontend/src/components/misc/User.vue` | Add bot badge |
|
||||
| `frontend/src/views/user/Settings.vue` | Add bots nav item |
|
||||
| `frontend/src/router/index.ts` | Add bots route |
|
||||
| `frontend/src/i18n/lang/en.json` | Add translation strings |
|
||||
|
|
@ -0,0 +1,259 @@
|
|||
# Bot Users - Gap Analysis & Implementation Resolutions
|
||||
|
||||
## Context
|
||||
|
||||
The [design plan](https://github.com/go-vikunja/vikunja/blob/claude/explore-openclaw-integration-KQEzg/plans/design-bot-users.md) for bot users is solid overall but has gaps that need resolution before implementation. This document identifies each gap and provides a concrete resolution based on codebase analysis and design decisions.
|
||||
|
||||
---
|
||||
|
||||
## Gap 1: Empty Email Uniqueness Conflict (Critical)
|
||||
|
||||
**Problem:** `checkIfUserExists()` (`pkg/user/user_create.go:155-194`) checks email uniqueness for local users. Multiple bots with `Email=""` would all conflict via `getUser(s, &User{Email: "", Issuer: "local"}, false)`.
|
||||
|
||||
**Resolution:** `CreateBotUser()` must:
|
||||
- Only check username uniqueness (reuse `GetUserByUsername`)
|
||||
- Skip `checkIfUserIsValid()` entirely (requires email + password)
|
||||
- Skip the email existence check
|
||||
|
||||
---
|
||||
|
||||
## Gap 2: Bot Token Creation - Minimal Branching (Critical)
|
||||
|
||||
**Problem:** `APIToken.Create()` (`pkg/models/api_tokens.go:86-110`) hardcodes `t.OwnerID = a.GetID()`.
|
||||
|
||||
**Resolution:** Add a `CreateForUser()` method or modify `Create()` to accept an optional owner override. Something like: if `t.OwnerID` is already set (non-zero) when `Create()` is called, preserve it instead of overwriting. The bot token handler would pre-set `t.OwnerID = bot.ID` before calling `Create()`. This is minimal branching - just one `if` check:
|
||||
|
||||
```go
|
||||
if t.OwnerID == 0 {
|
||||
t.OwnerID = a.GetID()
|
||||
}
|
||||
```
|
||||
|
||||
The bot endpoint handler verifies ownership of the bot, then calls `Create()` with `OwnerID` pre-set.
|
||||
|
||||
For `ReadAll()` and `Delete()` on bot tokens, the handler would similarly need to filter by bot's OwnerID rather than `a.GetID()`. Custom handlers for the bot token endpoints can query with the bot's ID directly.
|
||||
|
||||
---
|
||||
|
||||
## Gap 3: Error Codes (Required)
|
||||
|
||||
**Problem:** All error codes are TBD.
|
||||
|
||||
**Resolution:** Existing user error codes go up to ~1021 (`ErrAccountIsNotLocal`). Assign:
|
||||
- `ErrAccountIsBot` → 1022
|
||||
- `ErrBotUsersDisabled` → 1023
|
||||
- `ErrBotNotOwned` → 1024
|
||||
- `ErrCannotMakeBotOwner` → 1025
|
||||
|
||||
Verify the next available code by checking `pkg/user/error.go` and `pkg/models/error.go` at implementation time.
|
||||
|
||||
---
|
||||
|
||||
## Gap 4: CalDAV Bot Blocking Location (Required)
|
||||
|
||||
**Problem:** Plan doesn't specify exact location in `BasicAuth()` (`pkg/routes/caldav/auth.go:31-68`).
|
||||
|
||||
**Resolution:** Add check before `c.Set("userBasicAuth", u)` at line 63-64. Both the token path and password path converge there:
|
||||
|
||||
```go
|
||||
if u != nil && u.IsBot {
|
||||
log.Warningf("CalDAV basic auth rejected for bot user %d", u.ID)
|
||||
return false, nil
|
||||
}
|
||||
```
|
||||
|
||||
---
|
||||
|
||||
## Gap 5: BotOwner Table Registration (Required)
|
||||
|
||||
**Problem:** `BotOwner` not mentioned for registration in `pkg/models/models.go`.
|
||||
|
||||
**Resolution:** Add `&BotOwner{}` to the `GetTables()` return slice.
|
||||
|
||||
---
|
||||
|
||||
## Gap 6: Registration Config Independence (Clarification)
|
||||
|
||||
**Problem:** Unclear if `service.enableregistration = false` blocks bot creation.
|
||||
|
||||
**Resolution:** Non-issue. Bot creation uses `PUT /user/bots` which is gated only by `service.enablebotusers`. The registration endpoint (`POST /register`) is completely separate. No changes needed.
|
||||
|
||||
---
|
||||
|
||||
## Gap 7: LDAP Auth Blocking (Required)
|
||||
|
||||
**Problem:** Login handler tries LDAP first. Need bot check before LDAP attempt.
|
||||
|
||||
**Resolution:** In `pkg/routes/api/v1/login.go`, after resolving the user by username but before LDAP/local auth:
|
||||
```go
|
||||
if user.IsBot {
|
||||
return ErrAccountIsBot{UserID: user.ID}
|
||||
}
|
||||
```
|
||||
|
||||
Note: Need to check exact flow - the current login may not resolve the user before attempting auth. If LDAP auth creates/matches users by username, add a post-LDAP check too.
|
||||
|
||||
---
|
||||
|
||||
## Gap 8: Admin Capabilities → No Admin Panel Exists
|
||||
|
||||
**Problem:** Plan discusses admin management of bots.
|
||||
|
||||
**Resolution:** There is no admin panel in Vikunja. Remove all admin-related discussion from the plan. Bot management is owner-only.
|
||||
|
||||
---
|
||||
|
||||
## Gap 9: Frontend Route & Navigation (Required)
|
||||
|
||||
**Problem:** Missing router config and Settings.vue navigation details.
|
||||
|
||||
**Resolution:**
|
||||
|
||||
**Router** (`frontend/src/router/index.ts`): Add as child of `user.settings`:
|
||||
```ts
|
||||
{
|
||||
path: '/user/settings/bots',
|
||||
name: 'user.settings.bots',
|
||||
component: () => import('@/views/user/settings/Bots.vue'),
|
||||
}
|
||||
```
|
||||
|
||||
**Settings.vue** (`frontend/src/views/user/Settings.vue`): Add to `navigationItems`:
|
||||
```ts
|
||||
{
|
||||
title: t('user.settings.bots.title'),
|
||||
routeName: 'user.settings.bots',
|
||||
condition: botUsersEnabled.value,
|
||||
}
|
||||
```
|
||||
|
||||
Add `const botUsersEnabled = computed(() => configStore.botUsersEnabled)` alongside other feature flags.
|
||||
|
||||
---
|
||||
|
||||
## Gap 10: Frontend Config Store (Required)
|
||||
|
||||
**Problem:** Missing config store changes.
|
||||
|
||||
**Resolution:** In `frontend/src/stores/config.ts`:
|
||||
- Add `botUsersEnabled: false` to the state
|
||||
- Map from API response's `bot_users_enabled` field
|
||||
|
||||
---
|
||||
|
||||
## Gap 11: Bot Deletion - Follow User Deletion Pattern (Critical)
|
||||
|
||||
**Problem:** Plan says "unassign bot from all tasks" but deletion semantics were unclear.
|
||||
|
||||
**Resolution (per user decision):** Two operations:
|
||||
|
||||
### Delete (hard delete, like users)
|
||||
Reuse the existing `DeleteUser()` function in `pkg/models/user_delete.go:132-183`. It already handles:
|
||||
- Deleting task assignees, subscriptions, team members, saved filters, reactions, favorites, API tokens
|
||||
- Reassigning/deleting owned projects
|
||||
- Deleting the user row
|
||||
|
||||
For bots, skip the `AccountDeletedNotification` (bots don't get notifications). Consider adding a bot-specific check in `DeleteUser()` or a wrapper function.
|
||||
|
||||
### Disable (soft disable)
|
||||
Set `Status = StatusDisabled`. Bot tokens still exist but the API token middleware resolves the user and checks... actually need to verify: does the API token auth check user status? If not, disabling a bot might not revoke API access.
|
||||
|
||||
**Action item:** Check if `pkg/routes/api_tokens.go` checks user status after resolving the token owner. If not, add a check.
|
||||
|
||||
### Frontend
|
||||
- Delete button with confirmation dialog (make it clear this is permanent)
|
||||
- Disable/Enable toggle button (reversible)
|
||||
|
||||
---
|
||||
|
||||
## Gap 12: Initial Project → Non-Issue
|
||||
|
||||
Initial project creation happens in `pkg/routes/api/v1/user_register.go:84-89`, not in `CreateUser()`. Bot creation uses a separate endpoint, so this is automatically skipped.
|
||||
|
||||
---
|
||||
|
||||
## Gap 13: is_bot in User Search Responses → Automatic
|
||||
|
||||
Adding `IsBot` to the `User` struct with `json:"is_bot"` includes it in all API responses. No additional changes needed.
|
||||
|
||||
---
|
||||
|
||||
## Gap 14: Bot Username Convention (Design Decision)
|
||||
|
||||
**Resolution (per user):** Require `bot-` prefix for usernames. Display name (the `name` field) is free-form.
|
||||
|
||||
Add validation in bot creation:
|
||||
```go
|
||||
if !strings.HasPrefix(bot.Username, "bot-") {
|
||||
return ErrBotUsernameMustHavePrefix{}
|
||||
}
|
||||
```
|
||||
|
||||
Also reserve the `bot-` prefix for non-bot users (add check in regular `checkIfUserIsValid()`).
|
||||
|
||||
---
|
||||
|
||||
## Gap 15: API Token Auth - User Status Check (Confirmed Critical)
|
||||
|
||||
**Problem:** When a bot is disabled (`StatusDisabled`), its API tokens still work. Verified in `pkg/routes/api_tokens.go:76-103` - `checkAPITokenAndPutItInContext()` fetches the user at line 94 but never checks `Status`. A disabled bot's tokens remain functional.
|
||||
|
||||
**Resolution:** Add status check after line 97 in `checkAPITokenAndPutItInContext()`:
|
||||
```go
|
||||
if u.Status == user.StatusDisabled {
|
||||
log.Debugf("[auth] Tried authenticating with token %d but user %d is disabled", token.ID, u.ID)
|
||||
return echo.NewHTTPError(http.StatusUnauthorized, "Unauthorized")
|
||||
}
|
||||
```
|
||||
|
||||
This benefits both bots and regular disabled users - it's a general security improvement.
|
||||
|
||||
---
|
||||
|
||||
## Gap 16: OpenID Connect → Non-Issue by Design
|
||||
|
||||
OIDC matches users by Subject+Issuer, which bots don't have. No blocking needed.
|
||||
|
||||
---
|
||||
|
||||
## Summary of Resolutions
|
||||
|
||||
| Gap | Status | Key Change |
|
||||
|-----|--------|-----------|
|
||||
| 1. Email uniqueness | Resolved | Skip email check in `CreateBotUser()` |
|
||||
| 2. Token creation | Resolved | Branch in `Create()`: preserve pre-set OwnerID |
|
||||
| 3. Error codes | Resolved | 1022-1025 range |
|
||||
| 4. CalDAV blocking | Resolved | Check before `c.Set("userBasicAuth")` |
|
||||
| 5. Table registration | Resolved | Add to `GetTables()` |
|
||||
| 6. Registration independence | Non-issue | Separate endpoints |
|
||||
| 7. LDAP blocking | Resolved | Check before auth attempt |
|
||||
| 8. Admin capabilities | Removed | No admin panel exists |
|
||||
| 9. Frontend routes | Resolved | Router + Settings.vue details specified |
|
||||
| 10. Config store | Resolved | Add `botUsersEnabled` to config store |
|
||||
| 11. Bot deletion | Resolved | Hard delete via `DeleteUser()`, plus disable toggle |
|
||||
| 12. Initial project | Non-issue | Separate code path |
|
||||
| 13. User search | Automatic | `json:"is_bot"` covers it |
|
||||
| 14. Username prefix | Resolved | Require `bot-` prefix, free display name |
|
||||
| 15. Token auth status | Confirmed gap | Add status check in `checkAPITokenAndPutItInContext()` |
|
||||
| 16. OIDC | Non-issue | By design |
|
||||
|
||||
---
|
||||
|
||||
## Files to Verify During Implementation
|
||||
|
||||
- `pkg/routes/api_tokens.go` - **Confirmed: no user status check** - must add one (line 94-97)
|
||||
- `pkg/user/error.go` - Confirm next available error code before assigning 1022+
|
||||
- `pkg/routes/api/v1/login.go` - Exact location for bot check vs LDAP flow
|
||||
- `frontend/src/stores/config.ts` - Exact state structure for adding `botUsersEnabled`
|
||||
- `frontend/src/router/index.ts` - Exact child route pattern under `user.settings`
|
||||
|
||||
---
|
||||
|
||||
## Verification Plan
|
||||
|
||||
1. **Backend**: `mage test:filter` for bot-related tests
|
||||
2. **Login blocking**: Attempt password login as bot → expect `ErrAccountIsBot`
|
||||
3. **CalDAV blocking**: Attempt CalDAV auth as bot → expect rejection
|
||||
4. **Token flow**: Create bot → create token for bot → use token to call API as bot
|
||||
5. **Disable flow**: Disable bot → verify its tokens stop working
|
||||
6. **Delete flow**: Delete bot → verify all related data is cleaned up
|
||||
7. **Frontend**: Bot badge shows in User.vue, settings page CRUD works, config gate works
|
||||
Loading…
Reference in New Issue