Repository navigation
Lock down registration-device endpoints and reject activation without an OTP - #1771
Conversation
TimePlanningRegistrationDeviceController had no [Authorize], so an anonymous POST to api/time-planning-pn/registration-device/index returned every tablet's live OTP and the customer number, and the create/update/delete/request-otp actions were open as well. Add [Authorize] on the controller and keep only Activate anonymous, since flutter-time calls it to pair a tablet before it has a token. The Angular admin page already sends the user's JWT. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
There was a problem hiding this comment.
🟢 Approval recommended
No active review issues are reported for the change protecting device-management endpoints while preserving anonymous tablet pairing.
No open issues 🎉
What changed in this PR
Secures registration-device management endpoints by requiring authentication, while preserving anonymous tablet pairing.
Changes:
- Adds controller-level
[Authorize]to protect device listing, creation, updates, deletion, and OTP requests. - Adds
[AllowAnonymous]only toActivate, allowing tablets to pair before obtaining a token.
| File | Description |
|---|---|
eFormAPI/Plugins/TimePlanning.Pn/TimePlanning.Pn/Controllers/TimePlanningRegistrationDeviceController.cs |
Requires login for management endpoints while keeping activation anonymous. |
Review effort: Balanced
Activate is anonymous and matched x.OtpCode == model.OtCode. A form post without otCode binds OtCode to null, which EF turns into OtpCode IS NULL, and activation itself clears OtpCode. So a request carrying only the customer number matched an already-paired tablet, reissued its credential and returned it, logging the real tablet out. Reject an empty OtCode up front and only match devices that are waiting for pairing (OtpEnabled). Adds RegistrationDeviceActivateTests and lists it in both CI shard filters. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
renemadsen
changed the title
|
Added a second commit. |
| [SetUp] | ||
| public async Task SetUp() | ||
| { | ||
| await base.Setup(); |
There was a problem hiding this comment.
Correct that the base [SetUp] then runs twice. This class follows the existing convention, though: 47 test classes in this project call await base.Setup() from their own SetUp. Changing only this one would make it inconsistent. The double initialisation applies to the whole suite, so it should be cleaned up across all classes in a separate change, not in this security fix.
Problem
TimePlanningRegistrationDeviceControllerhad no[Authorize], and the host has no global auth filter. Anyone could:POST api/time-planning-pn/registration-device/indexwithout logging in and get every tablet's live OTP and the customer number (confirmed on a local dev server);With an OTP and the customer number,
activatehands out a valid tablet token.Change
[Authorize]on the controller.[AllowAnonymous]toActivateonly. flutter-time calls it (lib/communication/data_helper.dart:38) to pair a tablet before it has a token.Compatibility
ApiBaseService, which sends the user's JWT. The page is guarded byAuthGuardonly, so the controller uses plain[Authorize]rather than an admin role.ActivateDevicegoes straight to the service, so the MVC attributes don't affect it.Follow-ups, not in this PR: B2 (anonymous
settings/registration-siteswith no token), and OTP hardening (B4). See kanban card #5506.🤖 Generated with Claude Code